From a0804093e7e20b1846fc205e3c1246881c825086 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Wed, 19 Nov 2025 11:32:58 +0100 Subject: [PATCH 01/11] bugfix: cut does not infer the architecture properly (cherry picked from commit 4f7f18dfccfa1fcba3edda3093f4314c9fb37a11) --- cmd/chisel/cmd_cut.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 35c81a79..9da4aa69 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -9,6 +9,7 @@ import ( "github.com/canonical/chisel/internal/archive" "github.com/canonical/chisel/internal/cache" + "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/internal/slicer" ) @@ -57,6 +58,14 @@ func (cmd *cmdCut) Execute(args []string) error { } sliceKeys[i] = sliceKey } + arch := cmd.Arch + if arch == "" { + darch, err := deb.InferArch() + if err != nil { + return err + } + arch = darch + } release, err := obtainRelease(cmd.Release) if err != nil { @@ -73,7 +82,7 @@ func (cmd *cmdCut) Execute(args []string) error { } } - selection, err := setup.Select(release, sliceKeys, cmd.Arch) + selection, err := setup.Select(release, sliceKeys, arch) if err != nil { return err } @@ -83,7 +92,7 @@ func (cmd *cmdCut) Execute(args []string) error { openArchive, err := archive.Open(&archive.Options{ Label: archiveName, Version: archiveInfo.Version, - Arch: cmd.Arch, + Arch: arch, Suites: archiveInfo.Suites, Components: archiveInfo.Components, Pro: archiveInfo.Pro, From 2fb3c3219aeed165c6570e9e388898f20bf87abc Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 20 Nov 2025 17:42:17 +0100 Subject: [PATCH 02/11] add test showcasing behavior Signed-off-by: Paul Mars --- internal/slicer/slicer_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index fe408e6d..f226ae9d 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -1951,6 +1951,25 @@ var slicerTests = []slicerTest{{ manifestPaths: map[string]string{ "/dir/file": "file 0644 cc55e2ec {test-package_installed}", }, +}, { + summary: "Arch specific slice is not installed when requested arch is empty", + slices: []setup.SliceKey{{"test-package", "myslice"}}, + arch: "", + release: map[string]string{ + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + myslice: + v3-essential: + test-package_not-installed: {arch: amd64} + contents: + not-installed: + contents: + /dir/file: + `, + }, + filesystem: map[string]string{}, + manifestPaths: map[string]string{}, }, { summary: "Transitive essential", slices: []setup.SliceKey{{"test-package", "first"}}, From 53fe07522cda805b6b6b0f6153fbae761d78bd15 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 21 Nov 2025 09:11:32 +0100 Subject: [PATCH 03/11] fix: Detect cycles with v3-essential Signed-off-by: Paul Mars --- internal/setup/setup.go | 2 +- internal/setup/setup_test.go | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/internal/setup/setup.go b/internal/setup/setup.go index db3d9518..562258c2 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -369,7 +369,7 @@ func order(pkgs map[string]*Package, keys []SliceKey, arch string) ([]SliceKey, fqslice := slice.String() predecessors := successors[fqslice] for req, info := range slice.Essential { - if len(info.Arch) > 0 && !slices.Contains(info.Arch, arch) { + if len(info.Arch) > 0 && len(arch) > 0 && !slices.Contains(info.Arch, arch) { continue } fqreq := req.String() diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index ac956ce1..de1c9366 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -3548,6 +3548,25 @@ var setupTests = []setupTest{{ `, }, relerror: `package "mypkg" repeats mypkg_myslice2 in essential fields`, +}, { + summary: "Cycles are detected with 'v3-essential'", + input: map[string]string{ + "slices/mydir/mypkg1.yaml": ` + package: mypkg1 + slices: + myslice: + v3-essential: + mypkg2_myslice: {arch: [amd64, i386]} + `, + "slices/mydir/mypkg2.yaml": ` + package: mypkg2 + slices: + myslice: + v3-essential: + mypkg1_myslice: {arch: [amd64, i386]} + `, + }, + relerror: `essential loop detected: mypkg1_myslice, mypkg2_myslice`, }} func (s *S) TestParseRelease(c *C) { From c80bc7bbc7aa7a01c0225c10fe687cf782c8605a Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 21 Nov 2025 09:17:19 +0100 Subject: [PATCH 04/11] fix: arch specific slice installed if no arch provided Signed-off-by: Paul Mars --- internal/slicer/slicer_test.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index f226ae9d..9fb1ce5e 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -1952,7 +1952,7 @@ var slicerTests = []slicerTest{{ "/dir/file": "file 0644 cc55e2ec {test-package_installed}", }, }, { - summary: "Arch specific slice is not installed when requested arch is empty", + summary: "Arch specific slice is installed when requested arch is empty", slices: []setup.SliceKey{{"test-package", "myslice"}}, arch: "", release: map[string]string{ @@ -1961,15 +1961,20 @@ var slicerTests = []slicerTest{{ slices: myslice: v3-essential: - test-package_not-installed: {arch: amd64} + test-package_myslice2: {arch: amd64} contents: - not-installed: + myslice2: contents: /dir/file: `, }, - filesystem: map[string]string{}, - manifestPaths: map[string]string{}, + filesystem: map[string]string{ + "/dir/": "dir 0755", + "/dir/file": "file 0644 cc55e2ec", + }, + manifestPaths: map[string]string{ + "/dir/file": "file 0644 cc55e2ec {test-package_myslice2}", + }, }, { summary: "Transitive essential", slices: []setup.SliceKey{{"test-package", "first"}}, From 4ce160934359a90e856f5031da53acbe4374b1df Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 21 Nov 2025 11:02:53 +0100 Subject: [PATCH 05/11] fix: Select error out if empty arch Signed-off-by: Paul Mars --- internal/setup/setup.go | 4 ++++ internal/setup/setup_test.go | 26 +++++++++++++++++++++++++- internal/slicer/slicer_test.go | 9 +++++++-- 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/internal/setup/setup.go b/internal/setup/setup.go index 562258c2..0a1d037e 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -465,6 +465,10 @@ func stripBase(baseDir, path string) string { func Select(release *Release, slices []SliceKey, arch string) (*Selection, error) { logf("Selecting slices...") + if len(arch) == 0 { + return nil, fmt.Errorf("cannot select slices for an empty architecture") + } + selection := &Selection{ Release: release, } diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index de1c9366..23250684 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -3567,6 +3567,25 @@ var setupTests = []setupTest{{ `, }, relerror: `essential loop detected: mypkg1_myslice, mypkg2_myslice`, +}, { + summary: "Cycles are detected with 'v3-essential' without architecture", + input: map[string]string{ + "slices/mydir/mypkg1.yaml": ` + package: mypkg1 + slices: + myslice: + v3-essential: + mypkg2_myslice: + `, + "slices/mydir/mypkg2.yaml": ` + package: mypkg2 + slices: + myslice: + v3-essential: + mypkg1_myslice: {arch: [amd64, i386]} + `, + }, + relerror: `essential loop detected: mypkg1_myslice, mypkg2_myslice`, }} func (s *S) TestParseRelease(c *C) { @@ -3644,7 +3663,7 @@ func runParseReleaseTests(c *C, tests []setupTest) { } if test.selslices != nil { - selection, err := setup.Select(release, test.selslices, "") + selection, err := setup.Select(release, test.selslices, "amd64") if test.selerror != "" { c.Assert(err, ErrorMatches, test.selerror) continue @@ -3869,3 +3888,8 @@ func (s *S) TestYAMLPathGenerate(c *C) { c.Assert(result, Equals, test.result) } } + +func (s *S) TestSelectEmptyArch(c *C) { + _, err := setup.Select(nil, nil, "") + c.Assert(err, ErrorMatches, "cannot select slices for an empty architecture") +} diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 9fb1ce5e..6d4fd600 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -2102,7 +2102,12 @@ func runSlicerTests(s *S, c *C, tests []slicerTest) { Slice: "manifest", }) - selection, err := setup.Select(release, testSlices, test.arch) + arch := test.arch + if arch == "" { + arch = "amd64" + } + + selection, err := setup.Select(release, testSlices, arch) c.Assert(err, IsNil) archives := map[string]archive.Archive{} @@ -2120,7 +2125,7 @@ func runSlicerTests(s *S, c *C, tests []slicerTest) { Suites: setupArchive.Suites, Components: setupArchive.Components, Pro: setupArchive.Pro, - Arch: test.arch, + Arch: arch, }, Packages: pkgs, } From 7796bfc88e1bf06b0490cb03dd5b9d6eed1a7391 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 21 Nov 2025 11:15:19 +0100 Subject: [PATCH 06/11] style: improve error message and align with code conventions Signed-off-by: Paul Mars --- internal/setup/setup.go | 6 +++--- internal/setup/setup_test.go | 2 +- internal/slicer/slicer_test.go | 24 ------------------------ 3 files changed, 4 insertions(+), 28 deletions(-) diff --git a/internal/setup/setup.go b/internal/setup/setup.go index 0a1d037e..e361f4f3 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -369,7 +369,7 @@ func order(pkgs map[string]*Package, keys []SliceKey, arch string) ([]SliceKey, fqslice := slice.String() predecessors := successors[fqslice] for req, info := range slice.Essential { - if len(info.Arch) > 0 && len(arch) > 0 && !slices.Contains(info.Arch, arch) { + if len(info.Arch) > 0 && arch != "" && !slices.Contains(info.Arch, arch) { continue } fqreq := req.String() @@ -465,8 +465,8 @@ func stripBase(baseDir, path string) string { func Select(release *Release, slices []SliceKey, arch string) (*Selection, error) { logf("Selecting slices...") - if len(arch) == 0 { - return nil, fmt.Errorf("cannot select slices for an empty architecture") + if arch == "" { + return nil, fmt.Errorf(`cannot select slices: "arch" is unset`) } selection := &Selection{ diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 23250684..7d527c1a 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -3891,5 +3891,5 @@ func (s *S) TestYAMLPathGenerate(c *C) { func (s *S) TestSelectEmptyArch(c *C) { _, err := setup.Select(nil, nil, "") - c.Assert(err, ErrorMatches, "cannot select slices for an empty architecture") + c.Assert(err, ErrorMatches, `cannot select slices: "arch" is unset`) } diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 6d4fd600..16809427 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -1951,30 +1951,6 @@ var slicerTests = []slicerTest{{ manifestPaths: map[string]string{ "/dir/file": "file 0644 cc55e2ec {test-package_installed}", }, -}, { - summary: "Arch specific slice is installed when requested arch is empty", - slices: []setup.SliceKey{{"test-package", "myslice"}}, - arch: "", - release: map[string]string{ - "slices/mydir/test-package.yaml": ` - package: test-package - slices: - myslice: - v3-essential: - test-package_myslice2: {arch: amd64} - contents: - myslice2: - contents: - /dir/file: - `, - }, - filesystem: map[string]string{ - "/dir/": "dir 0755", - "/dir/file": "file 0644 cc55e2ec", - }, - manifestPaths: map[string]string{ - "/dir/file": "file 0644 cc55e2ec {test-package_myslice2}", - }, }, { summary: "Transitive essential", slices: []setup.SliceKey{{"test-package", "first"}}, From 842070cea850be1f8b07d0ad0fa1b8f1e91d4206 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 21 Nov 2025 11:21:42 +0100 Subject: [PATCH 07/11] style: improve quoting Signed-off-by: Paul Mars --- internal/setup/setup_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 7d527c1a..07a7ac62 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -3566,7 +3566,7 @@ var setupTests = []setupTest{{ mypkg1_myslice: {arch: [amd64, i386]} `, }, - relerror: `essential loop detected: mypkg1_myslice, mypkg2_myslice`, + relerror: "essential loop detected: mypkg1_myslice, mypkg2_myslice", }, { summary: "Cycles are detected with 'v3-essential' without architecture", input: map[string]string{ @@ -3585,7 +3585,7 @@ var setupTests = []setupTest{{ mypkg1_myslice: {arch: [amd64, i386]} `, }, - relerror: `essential loop detected: mypkg1_myslice, mypkg2_myslice`, + relerror: "essential loop detected: mypkg1_myslice, mypkg2_myslice", }} func (s *S) TestParseRelease(c *C) { From f1ffee31739d99c2f96ae41bc9109bfc094f84ff Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 21 Nov 2025 15:12:15 +0100 Subject: [PATCH 08/11] refactor: simplify setting default on test.arch Signed-off-by: Paul Mars --- internal/slicer/slicer_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 16809427..d42d7fdc 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -2078,12 +2078,11 @@ func runSlicerTests(s *S, c *C, tests []slicerTest) { Slice: "manifest", }) - arch := test.arch - if arch == "" { - arch = "amd64" + if test.arch == "" { + test.arch = "amd64" } - selection, err := setup.Select(release, testSlices, arch) + selection, err := setup.Select(release, testSlices, test.arch) c.Assert(err, IsNil) archives := map[string]archive.Archive{} @@ -2101,7 +2100,7 @@ func runSlicerTests(s *S, c *C, tests []slicerTest) { Suites: setupArchive.Suites, Components: setupArchive.Components, Pro: setupArchive.Pro, - Arch: arch, + Arch: test.arch, }, Packages: pkgs, } From 9b8ace6e7e1fd43ba7e0d1b0722908639d136b33 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 7 Jan 2026 09:22:03 +0100 Subject: [PATCH 09/11] refactor: handle empty arch in Select Let Select handle a received empty arch as archive.Open() does. This respects the existing logic handling it in other places of the code. --- cmd/chisel/cmd_cut.go | 13 ++----------- internal/setup/setup.go | 9 ++++++++- internal/setup/setup_test.go | 7 +------ internal/slicer/slicer_test.go | 4 ---- 4 files changed, 11 insertions(+), 22 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 9da4aa69..35c81a79 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -9,7 +9,6 @@ import ( "github.com/canonical/chisel/internal/archive" "github.com/canonical/chisel/internal/cache" - "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/internal/slicer" ) @@ -58,14 +57,6 @@ func (cmd *cmdCut) Execute(args []string) error { } sliceKeys[i] = sliceKey } - arch := cmd.Arch - if arch == "" { - darch, err := deb.InferArch() - if err != nil { - return err - } - arch = darch - } release, err := obtainRelease(cmd.Release) if err != nil { @@ -82,7 +73,7 @@ func (cmd *cmdCut) Execute(args []string) error { } } - selection, err := setup.Select(release, sliceKeys, arch) + selection, err := setup.Select(release, sliceKeys, cmd.Arch) if err != nil { return err } @@ -92,7 +83,7 @@ func (cmd *cmdCut) Execute(args []string) error { openArchive, err := archive.Open(&archive.Options{ Label: archiveName, Version: archiveInfo.Version, - Arch: arch, + Arch: cmd.Arch, Suites: archiveInfo.Suites, Components: archiveInfo.Components, Pro: archiveInfo.Pro, diff --git a/internal/setup/setup.go b/internal/setup/setup.go index e361f4f3..b59e30f9 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -12,6 +12,7 @@ import ( "golang.org/x/crypto/openpgp/packet" "github.com/canonical/chisel/internal/apacheutil" + "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/strdist" ) @@ -465,8 +466,14 @@ func stripBase(baseDir, path string) string { func Select(release *Release, slices []SliceKey, arch string) (*Selection, error) { logf("Selecting slices...") + var err error if arch == "" { - return nil, fmt.Errorf(`cannot select slices: "arch" is unset`) + arch, err = deb.InferArch() + } else { + err = deb.ValidateArch(arch) + } + if err != nil { + return nil, err } selection := &Selection{ diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 07a7ac62..d40b3ba6 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -3663,7 +3663,7 @@ func runParseReleaseTests(c *C, tests []setupTest) { } if test.selslices != nil { - selection, err := setup.Select(release, test.selslices, "amd64") + selection, err := setup.Select(release, test.selslices, "") if test.selerror != "" { c.Assert(err, ErrorMatches, test.selerror) continue @@ -3888,8 +3888,3 @@ func (s *S) TestYAMLPathGenerate(c *C) { c.Assert(result, Equals, test.result) } } - -func (s *S) TestSelectEmptyArch(c *C) { - _, err := setup.Select(nil, nil, "") - c.Assert(err, ErrorMatches, `cannot select slices: "arch" is unset`) -} diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index d42d7fdc..fe408e6d 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -2078,10 +2078,6 @@ func runSlicerTests(s *S, c *C, tests []slicerTest) { Slice: "manifest", }) - if test.arch == "" { - test.arch = "amd64" - } - selection, err := setup.Select(release, testSlices, test.arch) c.Assert(err, IsNil) From 79e047df823030e4e754f5a0f783a52fda110054 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 7 Jan 2026 09:49:42 +0100 Subject: [PATCH 10/11] test: Invalid arch received by Select --- internal/setup/setup_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index d40b3ba6..25331a4b 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -3888,3 +3888,8 @@ func (s *S) TestYAMLPathGenerate(c *C) { c.Assert(result, Equals, test.result) } } + +func (s *S) TestSelectInvalidArch(c *C) { + _, err := setup.Select(nil, nil, "foo") + c.Assert(err, ErrorMatches, `invalid package architecture: foo`) +} From 2b506e8d6833a71ec1e6c2c3823c847a9e758ddd Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 7 Jan 2026 11:26:24 +0100 Subject: [PATCH 11/11] test: invalid and empty arch for Select Also set a default value for release parsing tests to make them independent of the testing machine architecture. --- internal/setup/setup_test.go | 52 +++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 4 deletions(-) diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 25331a4b..96495800 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -3663,7 +3663,7 @@ func runParseReleaseTests(c *C, tests []setupTest) { } if test.selslices != nil { - selection, err := setup.Select(release, test.selslices, "") + selection, err := setup.Select(release, test.selslices, "amd64") if test.selerror != "" { c.Assert(err, ErrorMatches, test.selerror) continue @@ -3889,7 +3889,51 @@ func (s *S) TestYAMLPathGenerate(c *C) { } } -func (s *S) TestSelectInvalidArch(c *C) { - _, err := setup.Select(nil, nil, "foo") - c.Assert(err, ErrorMatches, `invalid package architecture: foo`) +var selectWithArchTests = []struct { + summary string + arch string + input map[string]string + selerror string +}{{ + summary: "Empty arch", + arch: "", +}, { + summary: "Invalid arch", + arch: "foo", + selerror: "invalid package architecture: foo", +}} + +func (s *S) TestSelectWithArch(c *C) { + input := map[string]string{ + "chisel.yaml": string(testutil.DefaultChiselYaml), + "slices/mydir/mypkg.yaml": ` + package: mypkg + slices: + myslice: + contents: + /dir/file1: {} + `, + } + selslices := []setup.SliceKey{{"mypkg", "myslice"}} + dir := c.MkDir() + for path, data := range input { + fpath := filepath.Join(dir, path) + err := os.MkdirAll(filepath.Dir(fpath), 0o755) + c.Assert(err, IsNil) + err = os.WriteFile(fpath, testutil.Reindent(data), 0o644) + c.Assert(err, IsNil) + } + + release, err := setup.ReadRelease(dir) + c.Assert(err, IsNil) + + for _, test := range selectWithArchTests { + _, err = setup.Select(release, selslices, test.arch) + if test.selerror != "" { + c.Assert(err, ErrorMatches, test.selerror) + continue + } else { + c.Assert(err, IsNil) + } + } }