From ce77c0a8c102861f808df7a9e44c637daa5ecc8c Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 24 Jul 2026 16:22:53 +0200 Subject: [PATCH 01/13] feat: accept track in CLI cut command --- cmd/chisel/cmd_cut.go | 8 +- cmd/chisel/cmd_find.go | 5 + cmd/chisel/cmd_find_test.go | 10 ++ cmd/chisel/cmd_info.go | 6 + cmd/chisel/cmd_info_test.go | 5 + internal/setup/setup.go | 96 +++++++++++++- internal/setup/setup_test.go | 236 ++++++++++++++++++++++++++++++--- internal/slicer/slicer_test.go | 6 +- 8 files changed, 344 insertions(+), 28 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 35c81a79a..0cf6360e8 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -49,13 +49,13 @@ func (cmd *cmdCut) Execute(args []string) error { return ErrExtraArgs } - sliceKeys := make([]setup.SliceKey, len(cmd.Positional.SliceRefs)) + sliceRefs := make([]setup.SliceRef, len(cmd.Positional.SliceRefs)) for i, sliceRef := range cmd.Positional.SliceRefs { - sliceKey, err := setup.ParseSliceKey(sliceRef) + ref, err := setup.ParseSliceRef(sliceRef) if err != nil { return err } - sliceKeys[i] = sliceKey + sliceRefs[i] = ref } release, err := obtainRelease(cmd.Release) @@ -73,7 +73,7 @@ func (cmd *cmdCut) Execute(args []string) error { } } - selection, err := setup.Select(release, sliceKeys, cmd.Arch) + selection, err := setup.Select(release, sliceRefs, cmd.Arch) if err != nil { return err } diff --git a/cmd/chisel/cmd_find.go b/cmd/chisel/cmd_find.go index 1a208d4bd..ae7920ee6 100644 --- a/cmd/chisel/cmd_find.go +++ b/cmd/chisel/cmd_find.go @@ -89,6 +89,11 @@ func match(slice *setup.Slice, query string) bool { // findSlices returns slices from the provided release that match all of the // query strings (AND). func findSlices(release *setup.Release, query []string) (slices []*setup.Slice, err error) { + for _, term := range query { + if strings.Contains(term, "@") { + return nil, fmt.Errorf("track suffix is unsupported for find: %q", term) + } + } slices = []*setup.Slice{} for _, pkg := range release.Packages { for _, slice := range pkg.Slices { diff --git a/cmd/chisel/cmd_find_test.go b/cmd/chisel/cmd_find_test.go index a2a68c2a8..8f51491ae 100644 --- a/cmd/chisel/cmd_find_test.go +++ b/cmd/chisel/cmd_find_test.go @@ -14,6 +14,7 @@ type findTest struct { release *setup.Release query []string result []*setup.Slice + err string } func makeSamplePackage(pkg string, slices []string) *setup.Package { @@ -123,6 +124,11 @@ var findTests = []findTest{{ release: sampleRelease, query: []string{"python", "slice"}, result: []*setup.Slice{}, +}, { + summary: "Track suffix is unsupported for find", + release: sampleRelease, + query: []string{"python3.10_bins@3.0"}, + err: `track suffix is unsupported for find: "python3.10_bins@3.0"`, }} func (s *ChiselSuite) TestFindSlices(c *C) { @@ -131,6 +137,10 @@ func (s *ChiselSuite) TestFindSlices(c *C) { for _, query := range testutil.Permutations(test.query) { slices, err := chisel.FindSlices(test.release, query) + if test.err != "" { + c.Assert(err, ErrorMatches, test.err) + continue + } c.Assert(err, IsNil) c.Assert(slices, DeepEquals, test.result) } diff --git a/cmd/chisel/cmd_info.go b/cmd/chisel/cmd_info.go index 67ede89a8..5561977c3 100644 --- a/cmd/chisel/cmd_info.go +++ b/cmd/chisel/cmd_info.go @@ -50,6 +50,12 @@ func (cmd *infoCmd) Execute(args []string) error { return err } + for _, query := range cmd.Positional.Queries { + if strings.Contains(query, "@") { + return fmt.Errorf("track suffix is unsupported for info: %q", query) + } + } + packages, notFound := selectPackageSlices(release, cmd.Positional.Queries) for i, pkg := range packages { diff --git a/cmd/chisel/cmd_info_test.go b/cmd/chisel/cmd_info_test.go index fc4624874..22bf7ef7c 100644 --- a/cmd/chisel/cmd_info_test.go +++ b/cmd/chisel/cmd_info_test.go @@ -140,6 +140,11 @@ var infoTests = []infoTest{{ input: infoRelease, query: []string{"foo_bar_foo", "a_b", "7_c", "a_b c", "a_b x_y"}, err: `no slice definitions found for: "foo_bar_foo", "a_b", "7_c", "a_b c", "a_b x_y"`, +}, { + summary: "Track suffix is unsupported for info", + input: infoRelease, + query: []string{"mypkg1_myslice1@3.0"}, + err: `track suffix is unsupported for info: "mypkg1_myslice1@3.0"`, }} var infoRelease = map[string]string{ diff --git a/internal/setup/setup.go b/internal/setup/setup.go index 9d7e4a7bf..c6dd686bb 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -145,6 +145,37 @@ func ParseSliceKey(sliceKey string) (SliceKey, error) { return apacheutil.ParseSliceKey(sliceKey) } +// SliceRef is a slice reference with an optional track suffix for store +// packages. +type SliceRef struct { + Key SliceKey + Track string +} + +// ParseSliceRef parses a "pkg_slice[@track]" reference. The track, if +// present, must be non-empty and must not contain '/'. +func ParseSliceRef(ref string) (SliceRef, error) { + keyPart, track, ok := strings.Cut(ref, "@") + if !ok { + key, err := ParseSliceKey(ref) + if err != nil { + return SliceRef{}, err + } + return SliceRef{Key: key}, nil + } + key, err := ParseSliceKey(keyPart) + if err != nil { + return SliceRef{}, err + } + if track == "" { + return SliceRef{}, fmt.Errorf("invalid slice reference %q: missing track", ref) + } + if strings.Contains(track, "/") { + return SliceRef{}, fmt.Errorf("invalid slice reference %q: track must not contain /", ref) + } + return SliceRef{Key: key, Track: track}, nil +} + func (s *Slice) String() string { return s.Package + "_" + s.Name } // Selection holds the required configuration to create a Build for a selection @@ -154,6 +185,8 @@ func (s *Slice) String() string { return s.Package + "_" + s.Name } type Selection struct { Release *Release Slices []*Slice + // Tracks holds the resolved track per store package name. + Tracks map[string]string } // Prefers uses the prefer relationships and returns a map from each path to @@ -493,7 +526,7 @@ func stripBase(baseDir, path string) string { return strings.TrimPrefix(path, baseDir+string(filepath.Separator)) } -func Select(release *Release, slices []SliceKey, arch string) (*Selection, error) { +func Select(release *Release, refs []SliceRef, arch string) (*Selection, error) { logf("Selecting slices...") var err error @@ -506,10 +539,21 @@ func Select(release *Release, slices []SliceKey, arch string) (*Selection, error return nil, err } + // Resolve the track per store package from the references. + tracks, err := resolveTracks(release, refs) + if err != nil { + return nil, err + } + selection := &Selection{ Release: release, + Tracks: tracks, } + slices := make([]SliceKey, len(refs)) + for i, ref := range refs { + slices[i] = ref.Key + } sorted, err := order(release.Packages, slices, arch) if err != nil { return nil, err @@ -519,6 +563,22 @@ func Select(release *Release, slices []SliceKey, arch string) (*Selection, error selection.Slices[i] = release.Packages[key.Package].Slices[key.Slice] } + // Fill in the default-track for store packages without an explicit track. + // Done after ordering so essentials-included packages get their track too. + for _, slice := range selection.Slices { + pkg := release.Packages[slice.Package] + if pkg.Store == "" { + continue + } + if _, ok := selection.Tracks[pkg.Name]; ok { + continue + } + if selection.Tracks == nil { + selection.Tracks = make(map[string]string) + } + selection.Tracks[pkg.Name] = pkg.DefaultTrack + } + for _, new := range selection.Slices { for newPath, newInfo := range new.Contents { // An invalid "generate" value should only throw an error if that @@ -550,6 +610,40 @@ func Select(release *Release, slices []SliceKey, arch string) (*Selection, error return selection, nil } +// resolveTracks collects the explicit track per store package from the +// references. It errors if a track is set on a non-store package or if two +// references to the same package specify different tracks. Default-track +// fallback is applied later by Select, once the full selection is known. +func resolveTracks(release *Release, refs []SliceRef) (map[string]string, error) { + var tracks map[string]string + for _, ref := range refs { + pkg, ok := release.Packages[ref.Key.Package] + if !ok { + // Package existence is reported later by order(). + continue + } + if pkg.Store == "" { + if ref.Track != "" { + return nil, fmt.Errorf("slice %s has track but package %q is not in a store", + ref.Key, pkg.Name) + } + continue + } + if ref.Track == "" { + continue + } + if existing, ok := tracks[pkg.Name]; ok && existing != ref.Track { + return nil, fmt.Errorf("slices of package %q have conflicting tracks %q and %q", + pkg.Name, existing, ref.Track) + } + if tracks == nil { + tracks = make(map[string]string) + } + tracks[pkg.Name] = ref.Track + } + return tracks, nil +} + const ( preferSource = 1 preferTarget = 2 diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 3568b39a6..4425a1c3b 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -26,7 +26,7 @@ type setupTest struct { release *setup.Release relerror string prefers map[string]string - selslices []setup.SliceKey + selrefs []setup.SliceRef selection *setup.Selection selerror string } @@ -426,7 +426,7 @@ var setupTests = []setupTest{{ myslice2: {essential: [mypkg1_myslice1]} `, }, - selslices: []setup.SliceKey{{"mypkg1", "myslice1"}}, + selrefs: []setup.SliceRef{{Key: setup.SliceKey{"mypkg1", "myslice1"}}}, selection: &setup.Selection{ Slices: []*setup.Slice{{ Package: "mypkg1", @@ -449,7 +449,7 @@ var setupTests = []setupTest{{ myslice2: {essential: [mypkg1_myslice1]} `, }, - selslices: []setup.SliceKey{{"mypkg2", "myslice2"}}, + selrefs: []setup.SliceRef{{Key: setup.SliceKey{"mypkg2", "myslice2"}}}, selection: &setup.Selection{ Slices: []*setup.Slice{{ Package: "mypkg1", @@ -488,7 +488,11 @@ var setupTests = []setupTest{{ /path3: {symlink: /link} `, }, - selslices: []setup.SliceKey{{"mypkg1", "myslice1"}, {"mypkg1", "myslice2"}, {"mypkg2", "myslice1"}}, + selrefs: []setup.SliceRef{ + {Key: setup.SliceKey{"mypkg1", "myslice1"}}, + {Key: setup.SliceKey{"mypkg1", "myslice2"}}, + {Key: setup.SliceKey{"mypkg2", "myslice1"}}, + }, }, { summary: "Conflicting paths across slices", input: map[string]string{ @@ -1756,7 +1760,7 @@ var setupTests = []setupTest{{ EndOfLife: time.Date(2100, time.January, 1, 0, 0, 0, 0, time.UTC), }, }, - selslices: []setup.SliceKey{{"mypkg", "myslice"}}, + selrefs: []setup.SliceRef{{Key: setup.SliceKey{"mypkg", "myslice"}}}, selection: &setup.Selection{ Slices: []*setup.Slice{{ Package: "mypkg", @@ -1810,8 +1814,8 @@ var setupTests = []setupTest{{ EndOfLife: time.Date(2100, time.January, 1, 0, 0, 0, 0, time.UTC), }, }, - selslices: []setup.SliceKey{{"mypkg", "myslice"}}, - selerror: `slice mypkg_myslice has invalid 'generate' for path /dir/\*\*: "foo"`, + selrefs: []setup.SliceRef{{Key: setup.SliceKey{"mypkg", "myslice"}}}, + selerror: `slice mypkg_myslice has invalid 'generate' for path /dir/\*\*: "foo"`, }, { summary: "Paths with generate: manifest must have trailing /**", input: map[string]string{ @@ -2430,11 +2434,11 @@ var setupTests = []setupTest{{ relerror: "slice mypkg1_myslice1 cannot 'prefer' its own package for path /file", }, { summary: "Path conflicts with 'prefer'", - selslices: []setup.SliceKey{ - {"mypkg1", "myslice1"}, - {"mypkg1", "myslice2"}, - {"mypkg2", "myslice1"}, - {"mypkg3", "myslice1"}, + selrefs: []setup.SliceRef{ + {Key: setup.SliceKey{"mypkg1", "myslice1"}}, + {Key: setup.SliceKey{"mypkg1", "myslice2"}}, + {Key: setup.SliceKey{"mypkg2", "myslice1"}}, + {Key: setup.SliceKey{"mypkg3", "myslice1"}}, }, input: map[string]string{ "slices/mydir/mypkg1.yaml": ` @@ -2540,10 +2544,10 @@ var setupTests = []setupTest{{ }, }, { summary: "Path conflicts with 'prefer' depends on selection", - selslices: []setup.SliceKey{ - {"mypkg1", "myslice1"}, - {"mypkg1", "myslice2"}, - {"mypkg2", "myslice1"}, + selrefs: []setup.SliceRef{ + {Key: setup.SliceKey{"mypkg1", "myslice1"}}, + {Key: setup.SliceKey{"mypkg1", "myslice2"}}, + {Key: setup.SliceKey{"mypkg2", "myslice1"}}, }, input: map[string]string{ "slices/mydir/mypkg1.yaml": ` @@ -4374,8 +4378,8 @@ var setupTests = []setupTest{{ }, }, }, { - summary: "Store unknown kind", - selslices: []setup.SliceKey{{Package: "bin-mypkg", Slice: "myslice"}}, + summary: "Store unknown kind", + selrefs: []setup.SliceRef{{Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}}}, input: map[string]string{ "chisel.yaml": ` format: v3 @@ -4409,6 +4413,135 @@ var setupTests = []setupTest{{ `, }, selerror: `slice bin-mypkg_myslice refers to store "bin" with unknown kind "unknown"`, +}, { + summary: "Track on bin slice uses default-track when omitted", + selrefs: []setup.SliceRef{{Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}}}, + input: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlWithStores, + "bin-slices/mypkg.yaml": ` + package: mypkg + store: bin + default-track: "3.0" + slices: + myslice: + contents: + /dir/file: {} + `, + }, + selection: &setup.Selection{ + Slices: []*setup.Slice{{ + Package: "bin-mypkg", + Name: "myslice", + Contents: map[string]setup.PathInfo{ + "/dir/file": {Kind: setup.CopyPath}, + }, + }}, + Tracks: map[string]string{"bin-mypkg": "3.0"}, + }, +}, { + summary: "Track on bin slice is set from CLI reference", + selrefs: []setup.SliceRef{{ + Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}, + Track: "2.0", + }}, + input: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlWithStores, + "bin-slices/mypkg.yaml": ` + package: mypkg + store: bin + default-track: "3.0" + slices: + myslice: + contents: + /dir/file: {} + `, + }, + selection: &setup.Selection{ + Slices: []*setup.Slice{{ + Package: "bin-mypkg", + Name: "myslice", + Contents: map[string]setup.PathInfo{ + "/dir/file": {Kind: setup.CopyPath}, + }, + }}, + Tracks: map[string]string{"bin-mypkg": "2.0"}, + }, +}, { + summary: "Same track on two slices of same bin package is allowed", + selrefs: []setup.SliceRef{ + {Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}, Track: "2.0"}, + {Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice2"}, Track: "2.0"}, + }, + input: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlWithStores, + "bin-slices/mypkg.yaml": ` + package: mypkg + store: bin + default-track: "3.0" + slices: + myslice: + contents: + /dir/file1: {} + myslice2: + contents: + /dir/file2: {} + `, + }, + selection: &setup.Selection{ + Slices: []*setup.Slice{{ + Package: "bin-mypkg", + Name: "myslice", + Contents: map[string]setup.PathInfo{ + "/dir/file1": {Kind: setup.CopyPath}, + }, + }, { + Package: "bin-mypkg", + Name: "myslice2", + Contents: map[string]setup.PathInfo{ + "/dir/file2": {Kind: setup.CopyPath}, + }, + }}, + Tracks: map[string]string{"bin-mypkg": "2.0"}, + }, +}, { + summary: "Conflicting tracks on two slices of same bin package fails", + selrefs: []setup.SliceRef{ + {Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}, Track: "2.0"}, + {Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice2"}, Track: "3.0"}, + }, + input: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlWithStores, + "bin-slices/mypkg.yaml": ` + package: mypkg + store: bin + default-track: "3.0" + slices: + myslice: + contents: + /dir/file1: {} + myslice2: + contents: + /dir/file2: {} + `, + }, + selerror: `slices of package "bin-mypkg" have conflicting tracks "2.0" and "3.0"`, +}, { + summary: "Track on a non-store (deb) package fails", + selrefs: []setup.SliceRef{{ + Key: setup.SliceKey{Package: "mypkg", Slice: "myslice"}, + Track: "2.0", + }}, + input: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYaml, + "slices/mypkg.yaml": ` + package: mypkg + slices: + myslice: + contents: + /dir/file: {} + `, + }, + selerror: `slice mypkg_myslice has track but package "mypkg" is not in a store`, }} func (s *S) TestParseRelease(c *C) { @@ -4540,8 +4673,8 @@ func runParseReleaseTests(c *C, tests []setupTest) { c.Assert(release, DeepEquals, test.release) } - if test.selslices != nil { - selection, err := setup.Select(release, test.selslices, "amd64") + if test.selrefs != nil { + selection, err := setup.Select(release, test.selrefs, "amd64") if test.selerror != "" { c.Assert(err, ErrorMatches, test.selerror) continue @@ -4907,8 +5040,8 @@ func (s *S) TestSelectEmptyArch(c *C) { release, err := setup.ReadRelease(dir) c.Assert(err, IsNil) - selslice := []setup.SliceKey{{"mypkg", "myslice"}} - selection, err := setup.Select(release, selslice, "") + refs := []setup.SliceRef{{Key: setup.SliceKey{"mypkg", "myslice"}}} + selection, err := setup.Select(release, refs, "") c.Assert(err, IsNil) var sliceNames []string @@ -4919,6 +5052,65 @@ func (s *S) TestSelectEmptyArch(c *C) { c.Assert(sliceNames, DeepEquals, expected) } +var parseSliceRefTests = []struct { + input string + expected setup.SliceRef + err string +}{{ + input: "foo_bar", + expected: setup.SliceRef{Key: setup.SliceKey{Package: "foo", Slice: "bar"}}, +}, { + input: "foo_bar@3.0", + expected: setup.SliceRef{ + Key: setup.SliceKey{Package: "foo", Slice: "bar"}, + Track: "3.0", + }, +}, { + input: "foo_bar@latest", + expected: setup.SliceRef{ + Key: setup.SliceKey{Package: "foo", Slice: "bar"}, + Track: "latest", + }, +}, { + input: "foo-pkg_dashed-slice@3.0", + expected: setup.SliceRef{ + Key: setup.SliceKey{Package: "foo-pkg", Slice: "dashed-slice"}, + Track: "3.0", + }, +}, { + // Split on the first '@'; the track may itself contain '@'. + input: "foo_bar@3.0@x", + expected: setup.SliceRef{ + Key: setup.SliceKey{Package: "foo", Slice: "bar"}, + Track: "3.0@x", + }, +}, { + input: "foo_bar@", + err: `invalid slice reference "foo_bar@": missing track`, +}, { + input: "foo_bar@3.0/stable", + err: `invalid slice reference "foo_bar@3.0/stable": track must not contain /`, +}, { + // Identity part is still validated by ParseSliceKey. + input: "foo_ba@3.0", + err: `invalid slice reference: "foo_ba"`, +}, { + input: "foo_bar_baz@3.0", + err: `invalid slice reference: "foo_bar_baz"`, +}} + +func (s *S) TestParseSliceRef(c *C) { + for _, test := range parseSliceRefTests { + ref, err := setup.ParseSliceRef(test.input) + if test.err != "" { + c.Assert(err, ErrorMatches, test.err) + continue + } + c.Assert(err, IsNil) + c.Assert(ref, DeepEquals, test.expected) + } +} + // oldEssentialToV3 converts the essentials in v1 and v2, both 'essential', and // 'v3-essential' to the shape expected by the v3 format. // skip is set to true when an accurate translation of the test is not diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index d6ef9ca0d..df9d25d82 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -2094,7 +2094,11 @@ func runSlicerTests(s *S, c *C, tests []slicerTest) { Slice: "manifest", }) - selection, err := setup.Select(release, testSlices, test.arch) + refs := make([]setup.SliceRef, len(testSlices)) + for i, key := range testSlices { + refs[i] = setup.SliceRef{Key: key} + } + selection, err := setup.Select(release, refs, test.arch) c.Assert(err, IsNil) archives := map[string]archive.Archive{} From 8349863f8d29a81c6df6473cd9ab662e821cb5df Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 27 Jul 2026 09:32:47 +0200 Subject: [PATCH 02/13] style: improve consistency --- internal/setup/setup.go | 13 +++---------- internal/setup/setup_test.go | 3 +++ 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/internal/setup/setup.go b/internal/setup/setup.go index c6dd686bb..d0ee794ca 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -573,9 +573,6 @@ func Select(release *Release, refs []SliceRef, arch string) (*Selection, error) if _, ok := selection.Tracks[pkg.Name]; ok { continue } - if selection.Tracks == nil { - selection.Tracks = make(map[string]string) - } selection.Tracks[pkg.Name] = pkg.DefaultTrack } @@ -612,14 +609,13 @@ func Select(release *Release, refs []SliceRef, arch string) (*Selection, error) // resolveTracks collects the explicit track per store package from the // references. It errors if a track is set on a non-store package or if two -// references to the same package specify different tracks. Default-track -// fallback is applied later by Select, once the full selection is known. +// references to the same package specify different tracks. func resolveTracks(release *Release, refs []SliceRef) (map[string]string, error) { - var tracks map[string]string + tracks := make(map[string]string) for _, ref := range refs { pkg, ok := release.Packages[ref.Key.Package] if !ok { - // Package existence is reported later by order(). + // Nothing to validate; the package is unknown. continue } if pkg.Store == "" { @@ -636,9 +632,6 @@ func resolveTracks(release *Release, refs []SliceRef) (map[string]string, error) return nil, fmt.Errorf("slices of package %q have conflicting tracks %q and %q", pkg.Name, existing, ref.Track) } - if tracks == nil { - tracks = make(map[string]string) - } tracks[pkg.Name] = ref.Track } return tracks, nil diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 4425a1c3b..706a98007 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -432,6 +432,7 @@ var setupTests = []setupTest{{ Package: "mypkg1", Name: "myslice1", }}, + Tracks: map[string]string{}, }, }, { summary: "Selection with dependencies", @@ -461,6 +462,7 @@ var setupTests = []setupTest{{ {"mypkg1", "myslice1"}: {}, }, }}, + Tracks: map[string]string{}, }, }, { summary: "Selection with matching paths don't conflict", @@ -1769,6 +1771,7 @@ var setupTests = []setupTest{{ "/dir/**": {Kind: "generate", Generate: "manifest"}, }, }}, + Tracks: map[string]string{}, }, }, { summary: "Can specify generate with bogus value but cannot select those slices", From c97af92f92c937d7ff7c5e811556f6ce5dfbab25 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 27 Jul 2026 11:23:24 +0200 Subject: [PATCH 03/13] feat: handle channels instead of tracks --- cmd/chisel/cmd_find.go | 2 +- cmd/chisel/cmd_find_test.go | 4 +- cmd/chisel/cmd_info.go | 2 +- cmd/chisel/cmd_info_test.go | 4 +- internal/setup/setup.go | 98 ++++++++++++++++++++------------ internal/setup/setup_test.go | 105 +++++++++++++++++++++++------------ 6 files changed, 140 insertions(+), 75 deletions(-) diff --git a/cmd/chisel/cmd_find.go b/cmd/chisel/cmd_find.go index ae7920ee6..d7b9dfdb1 100644 --- a/cmd/chisel/cmd_find.go +++ b/cmd/chisel/cmd_find.go @@ -91,7 +91,7 @@ func match(slice *setup.Slice, query string) bool { func findSlices(release *setup.Release, query []string) (slices []*setup.Slice, err error) { for _, term := range query { if strings.Contains(term, "@") { - return nil, fmt.Errorf("track suffix is unsupported for find: %q", term) + return nil, fmt.Errorf("channel is unsupported for find: %q", term) } } slices = []*setup.Slice{} diff --git a/cmd/chisel/cmd_find_test.go b/cmd/chisel/cmd_find_test.go index 8f51491ae..0d2c117b2 100644 --- a/cmd/chisel/cmd_find_test.go +++ b/cmd/chisel/cmd_find_test.go @@ -125,10 +125,10 @@ var findTests = []findTest{{ query: []string{"python", "slice"}, result: []*setup.Slice{}, }, { - summary: "Track suffix is unsupported for find", + summary: "Channel is unsupported for find", release: sampleRelease, query: []string{"python3.10_bins@3.0"}, - err: `track suffix is unsupported for find: "python3.10_bins@3.0"`, + err: `channel is unsupported for find: "python3.10_bins@3.0"`, }} func (s *ChiselSuite) TestFindSlices(c *C) { diff --git a/cmd/chisel/cmd_info.go b/cmd/chisel/cmd_info.go index 5561977c3..f8f0015c5 100644 --- a/cmd/chisel/cmd_info.go +++ b/cmd/chisel/cmd_info.go @@ -52,7 +52,7 @@ func (cmd *infoCmd) Execute(args []string) error { for _, query := range cmd.Positional.Queries { if strings.Contains(query, "@") { - return fmt.Errorf("track suffix is unsupported for info: %q", query) + return fmt.Errorf("channel is unsupported for info: %q", query) } } diff --git a/cmd/chisel/cmd_info_test.go b/cmd/chisel/cmd_info_test.go index 22bf7ef7c..8c3d13f45 100644 --- a/cmd/chisel/cmd_info_test.go +++ b/cmd/chisel/cmd_info_test.go @@ -141,10 +141,10 @@ var infoTests = []infoTest{{ query: []string{"foo_bar_foo", "a_b", "7_c", "a_b c", "a_b x_y"}, err: `no slice definitions found for: "foo_bar_foo", "a_b", "7_c", "a_b c", "a_b x_y"`, }, { - summary: "Track suffix is unsupported for info", + summary: "Channel is unsupported for info", input: infoRelease, query: []string{"mypkg1_myslice1@3.0"}, - err: `track suffix is unsupported for info: "mypkg1_myslice1@3.0"`, + err: `channel is unsupported for info: "mypkg1_myslice1@3.0"`, }} var infoRelease = map[string]string{ diff --git a/internal/setup/setup.go b/internal/setup/setup.go index d0ee794ca..a0dbaaa77 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -8,6 +8,7 @@ import ( "slices" "strings" "time" + "unicode" "golang.org/x/crypto/openpgp/packet" @@ -145,17 +146,23 @@ func ParseSliceKey(sliceKey string) (SliceKey, error) { return apacheutil.ParseSliceKey(sliceKey) } -// SliceRef is a slice reference with an optional track suffix for store -// packages. +// DefaultRisk is used when a channel does not specify a risk. +const DefaultRisk = "stable" + +// SliceRef is a slice reference with an optional channel for store packages. +// The channel always holds a risk, that is at least "/". type SliceRef struct { - Key SliceKey - Track string + Key SliceKey + Channel string } -// ParseSliceRef parses a "pkg_slice[@track]" reference. The track, if -// present, must be non-empty and must not contain '/'. +// ParseSliceRef parses a "pkg_slice[@channel]" reference. The channel is +// either a track, in which case the default risk is used, or a +// "/" value. Validation is intentionally loose so that longer +// channel forms, such as "//", are accepted without +// changes here. func ParseSliceRef(ref string) (SliceRef, error) { - keyPart, track, ok := strings.Cut(ref, "@") + keyPart, channel, ok := strings.Cut(ref, "@") if !ok { key, err := ParseSliceKey(ref) if err != nil { @@ -167,13 +174,32 @@ func ParseSliceRef(ref string) (SliceRef, error) { if err != nil { return SliceRef{}, err } - if track == "" { - return SliceRef{}, fmt.Errorf("invalid slice reference %q: missing track", ref) + channel, err = parseChannel(channel) + if err != nil { + return SliceRef{}, fmt.Errorf("invalid slice reference %q: %s", ref, err) + } + return SliceRef{Key: key, Channel: channel}, nil +} + +// parseChannel validates a channel and returns it with the default risk +// appended if it holds a track alone. Any number of segments is accepted so +// that longer forms are not rejected here. +func parseChannel(channel string) (string, error) { + if channel == "" { + return "", fmt.Errorf("missing channel") + } + if strings.ContainsFunc(channel, unicode.IsSpace) { + return "", fmt.Errorf("channel must not contain spaces") + } + segments := strings.Split(channel, "/") + if slices.Contains(segments, "") { + return "", fmt.Errorf("channel must be or /") } - if strings.Contains(track, "/") { - return SliceRef{}, fmt.Errorf("invalid slice reference %q: track must not contain /", ref) + if len(segments) == 1 { + // A track alone, the risk is implicit. + return channel + "/" + DefaultRisk, nil } - return SliceRef{Key: key, Track: track}, nil + return channel, nil } func (s *Slice) String() string { return s.Package + "_" + s.Name } @@ -185,8 +211,8 @@ func (s *Slice) String() string { return s.Package + "_" + s.Name } type Selection struct { Release *Release Slices []*Slice - // Tracks holds the resolved track per store package name. - Tracks map[string]string + // Channels holds the resolved channel per store package name. + Channels map[string]string } // Prefers uses the prefer relationships and returns a map from each path to @@ -539,15 +565,15 @@ func Select(release *Release, refs []SliceRef, arch string) (*Selection, error) return nil, err } - // Resolve the track per store package from the references. - tracks, err := resolveTracks(release, refs) + // Resolve the channel per store package from the references. + channels, err := resolveChannels(release, refs) if err != nil { return nil, err } selection := &Selection{ - Release: release, - Tracks: tracks, + Release: release, + Channels: channels, } slices := make([]SliceKey, len(refs)) @@ -563,17 +589,19 @@ func Select(release *Release, refs []SliceRef, arch string) (*Selection, error) selection.Slices[i] = release.Packages[key.Package].Slices[key.Slice] } - // Fill in the default-track for store packages without an explicit track. - // Done after ordering so essentials-included packages get their track too. + // Derive the channel from 'default-track' for store packages without an + // explicit one. Note the release only defines a track, the risk is + // implicit. Done after ordering so essentials-included packages get their + // channel too. for _, slice := range selection.Slices { pkg := release.Packages[slice.Package] if pkg.Store == "" { continue } - if _, ok := selection.Tracks[pkg.Name]; ok { + if _, ok := selection.Channels[pkg.Name]; ok { continue } - selection.Tracks[pkg.Name] = pkg.DefaultTrack + selection.Channels[pkg.Name] = pkg.DefaultTrack + "/" + DefaultRisk } for _, new := range selection.Slices { @@ -607,11 +635,11 @@ func Select(release *Release, refs []SliceRef, arch string) (*Selection, error) return selection, nil } -// resolveTracks collects the explicit track per store package from the -// references. It errors if a track is set on a non-store package or if two -// references to the same package specify different tracks. -func resolveTracks(release *Release, refs []SliceRef) (map[string]string, error) { - tracks := make(map[string]string) +// resolveChannels collects the explicit channel per store package from the +// references. It errors if a channel is set on a non-store package or if two +// references to the same package specify different channels. +func resolveChannels(release *Release, refs []SliceRef) (map[string]string, error) { + channels := make(map[string]string) for _, ref := range refs { pkg, ok := release.Packages[ref.Key.Package] if !ok { @@ -619,22 +647,22 @@ func resolveTracks(release *Release, refs []SliceRef) (map[string]string, error) continue } if pkg.Store == "" { - if ref.Track != "" { - return nil, fmt.Errorf("slice %s has track but package %q is not in a store", + if ref.Channel != "" { + return nil, fmt.Errorf("slice %s has channel but package %q is not in a store", ref.Key, pkg.Name) } continue } - if ref.Track == "" { + if ref.Channel == "" { continue } - if existing, ok := tracks[pkg.Name]; ok && existing != ref.Track { - return nil, fmt.Errorf("slices of package %q have conflicting tracks %q and %q", - pkg.Name, existing, ref.Track) + if existing, ok := channels[pkg.Name]; ok && existing != ref.Channel { + return nil, fmt.Errorf("slices of package %q have conflicting channels %q and %q", + pkg.Name, existing, ref.Channel) } - tracks[pkg.Name] = ref.Track + channels[pkg.Name] = ref.Channel } - return tracks, nil + return channels, nil } const ( diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 706a98007..6d3f687f4 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -432,7 +432,7 @@ var setupTests = []setupTest{{ Package: "mypkg1", Name: "myslice1", }}, - Tracks: map[string]string{}, + Channels: map[string]string{}, }, }, { summary: "Selection with dependencies", @@ -462,7 +462,7 @@ var setupTests = []setupTest{{ {"mypkg1", "myslice1"}: {}, }, }}, - Tracks: map[string]string{}, + Channels: map[string]string{}, }, }, { summary: "Selection with matching paths don't conflict", @@ -1771,7 +1771,7 @@ var setupTests = []setupTest{{ "/dir/**": {Kind: "generate", Generate: "manifest"}, }, }}, - Tracks: map[string]string{}, + Channels: map[string]string{}, }, }, { summary: "Can specify generate with bogus value but cannot select those slices", @@ -4417,7 +4417,7 @@ var setupTests = []setupTest{{ }, selerror: `slice bin-mypkg_myslice refers to store "bin" with unknown kind "unknown"`, }, { - summary: "Track on bin slice uses default-track when omitted", + summary: "Channel on bin slice is derived from default-track when omitted", selrefs: []setup.SliceRef{{Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}}}, input: map[string]string{ "chisel.yaml": testutil.DefaultChiselYamlWithStores, @@ -4439,13 +4439,13 @@ var setupTests = []setupTest{{ "/dir/file": {Kind: setup.CopyPath}, }, }}, - Tracks: map[string]string{"bin-mypkg": "3.0"}, + Channels: map[string]string{"bin-mypkg": "3.0/stable"}, }, }, { - summary: "Track on bin slice is set from CLI reference", + summary: "Channel on bin slice is set from the reference", selrefs: []setup.SliceRef{{ - Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}, - Track: "2.0", + Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}, + Channel: "2.0/edge", }}, input: map[string]string{ "chisel.yaml": testutil.DefaultChiselYamlWithStores, @@ -4467,13 +4467,13 @@ var setupTests = []setupTest{{ "/dir/file": {Kind: setup.CopyPath}, }, }}, - Tracks: map[string]string{"bin-mypkg": "2.0"}, + Channels: map[string]string{"bin-mypkg": "2.0/edge"}, }, }, { - summary: "Same track on two slices of same bin package is allowed", + summary: "Same channel on two slices of same bin package is allowed", selrefs: []setup.SliceRef{ - {Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}, Track: "2.0"}, - {Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice2"}, Track: "2.0"}, + {Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}, Channel: "2.0/stable"}, + {Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice2"}, Channel: "2.0/stable"}, }, input: map[string]string{ "chisel.yaml": testutil.DefaultChiselYamlWithStores, @@ -4504,13 +4504,13 @@ var setupTests = []setupTest{{ "/dir/file2": {Kind: setup.CopyPath}, }, }}, - Tracks: map[string]string{"bin-mypkg": "2.0"}, + Channels: map[string]string{"bin-mypkg": "2.0/stable"}, }, }, { - summary: "Conflicting tracks on two slices of same bin package fails", + summary: "Conflicting channels on two slices of same bin package fails", selrefs: []setup.SliceRef{ - {Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}, Track: "2.0"}, - {Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice2"}, Track: "3.0"}, + {Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}, Channel: "2.0/stable"}, + {Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice2"}, Channel: "2.0/edge"}, }, input: map[string]string{ "chisel.yaml": testutil.DefaultChiselYamlWithStores, @@ -4527,12 +4527,12 @@ var setupTests = []setupTest{{ /dir/file2: {} `, }, - selerror: `slices of package "bin-mypkg" have conflicting tracks "2.0" and "3.0"`, + selerror: `slices of package "bin-mypkg" have conflicting channels "2.0/stable" and "2.0/edge"`, }, { - summary: "Track on a non-store (deb) package fails", + summary: "Channel on a non-store (deb) package fails", selrefs: []setup.SliceRef{{ - Key: setup.SliceKey{Package: "mypkg", Slice: "myslice"}, - Track: "2.0", + Key: setup.SliceKey{Package: "mypkg", Slice: "myslice"}, + Channel: "2.0/stable", }}, input: map[string]string{ "chisel.yaml": testutil.DefaultChiselYaml, @@ -4544,7 +4544,7 @@ var setupTests = []setupTest{{ /dir/file: {} `, }, - selerror: `slice mypkg_myslice has track but package "mypkg" is not in a store`, + selerror: `slice mypkg_myslice has channel but package "mypkg" is not in a store`, }} func (s *S) TestParseRelease(c *C) { @@ -5063,36 +5063,73 @@ var parseSliceRefTests = []struct { input: "foo_bar", expected: setup.SliceRef{Key: setup.SliceKey{Package: "foo", Slice: "bar"}}, }, { + // A track alone gets the default risk. input: "foo_bar@3.0", expected: setup.SliceRef{ - Key: setup.SliceKey{Package: "foo", Slice: "bar"}, - Track: "3.0", + Key: setup.SliceKey{Package: "foo", Slice: "bar"}, + Channel: "3.0/stable", }, }, { input: "foo_bar@latest", expected: setup.SliceRef{ - Key: setup.SliceKey{Package: "foo", Slice: "bar"}, - Track: "latest", + Key: setup.SliceKey{Package: "foo", Slice: "bar"}, + Channel: "latest/stable", }, }, { - input: "foo-pkg_dashed-slice@3.0", + input: "foo_bar@3.0/edge", expected: setup.SliceRef{ - Key: setup.SliceKey{Package: "foo-pkg", Slice: "dashed-slice"}, - Track: "3.0", + Key: setup.SliceKey{Package: "foo", Slice: "bar"}, + Channel: "3.0/edge", }, }, { - // Split on the first '@'; the track may itself contain '@'. + // An explicit default risk is kept as is. + input: "foo_bar@3.0/stable", + expected: setup.SliceRef{ + Key: setup.SliceKey{Package: "foo", Slice: "bar"}, + Channel: "3.0/stable", + }, +}, { + // Validation is loose, unknown risks are accepted. + input: "foo_bar@3.0/whatever", + expected: setup.SliceRef{ + Key: setup.SliceKey{Package: "foo", Slice: "bar"}, + Channel: "3.0/whatever", + }, +}, { + input: "foo-pkg_dashed-slice@3.0/beta", + expected: setup.SliceRef{ + Key: setup.SliceKey{Package: "foo-pkg", Slice: "dashed-slice"}, + Channel: "3.0/beta", + }, +}, { + // Split on the first '@'; the channel may itself contain '@'. input: "foo_bar@3.0@x", expected: setup.SliceRef{ - Key: setup.SliceKey{Package: "foo", Slice: "bar"}, - Track: "3.0@x", + Key: setup.SliceKey{Package: "foo", Slice: "bar"}, + Channel: "3.0@x/stable", + }, +}, { + // Longer forms, such as a branch, are not rejected. + input: "foo_bar@3.0/stable/mybranch", + expected: setup.SliceRef{ + Key: setup.SliceKey{Package: "foo", Slice: "bar"}, + Channel: "3.0/stable/mybranch", }, }, { input: "foo_bar@", - err: `invalid slice reference "foo_bar@": missing track`, + err: `invalid slice reference "foo_bar@": missing channel`, }, { - input: "foo_bar@3.0/stable", - err: `invalid slice reference "foo_bar@3.0/stable": track must not contain /`, + input: "foo_bar@/stable", + err: `invalid slice reference "foo_bar@/stable": channel must be or /`, +}, { + input: "foo_bar@3.0/", + err: `invalid slice reference "foo_bar@3.0/": channel must be or /`, +}, { + input: "foo_bar@3.0//stable", + err: `invalid slice reference "foo_bar@3.0//stable": channel must be or /`, +}, { + input: "foo_bar@3.0 stable", + err: `invalid slice reference "foo_bar@3.0 stable": channel must not contain spaces`, }, { // Identity part is still validated by ParseSliceKey. input: "foo_ba@3.0", From 263ad784e01aa1d2c255b6f2c91268af8572f3c8 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 27 Jul 2026 14:20:09 +0200 Subject: [PATCH 04/13] docs: document @channel syntax in cut's help --- cmd/chisel/cmd_cut.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 0cf6360e8..41c799b64 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -20,6 +20,14 @@ to create a new filesystem tree in the root location. By default it fetches the slices for the same Ubuntu version as the current host, unless the --release flag is used. + +Slices are named _. For packages coming from a store, a +channel may be appended to select which one to fetch, as in +mybin_myslice@2.0/edge. A channel with no risk, such as @2.0, uses the +stable risk. + +Slices of the same package must all agree on the channel. When it is +omitted, the default track of the package is used. ` var cutDescs = map[string]string{ From 6c8efed4d0afc91b6b4d1f56b25e42265deabaed Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 28 Jul 2026 08:32:45 +0200 Subject: [PATCH 05/13] fix: prepare channels map before ordering This map will be needed when implementing the channel filter. --- internal/setup/setup.go | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/internal/setup/setup.go b/internal/setup/setup.go index a0dbaaa77..96c95c6bd 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -570,10 +570,22 @@ func Select(release *Release, refs []SliceRef, arch string) (*Selection, error) if err != nil { return nil, err } + // Derive the channel from 'default-track' for the store packages without an + // explicit one. Note the release only defines a track, the risk is + // implicit. This is done for every known package, before ordering, because + // ordering itself depends on the channel of the packages it traverses. + for _, pkg := range release.Packages { + if pkg.Store == "" { + continue + } + if _, ok := channels[pkg.Name]; ok { + continue + } + channels[pkg.Name] = pkg.DefaultTrack + "/" + DefaultRisk + } selection := &Selection{ Release: release, - Channels: channels, } slices := make([]SliceKey, len(refs)) @@ -589,19 +601,12 @@ func Select(release *Release, refs []SliceRef, arch string) (*Selection, error) selection.Slices[i] = release.Packages[key.Package].Slices[key.Slice] } - // Derive the channel from 'default-track' for store packages without an - // explicit one. Note the release only defines a track, the risk is - // implicit. Done after ordering so essentials-included packages get their - // channel too. + // Only report the channels of the selected packages. + selection.Channels = make(map[string]string) for _, slice := range selection.Slices { - pkg := release.Packages[slice.Package] - if pkg.Store == "" { - continue - } - if _, ok := selection.Channels[pkg.Name]; ok { - continue + if channel, ok := channels[slice.Package]; ok { + selection.Channels[slice.Package] = channel } - selection.Channels[pkg.Name] = pkg.DefaultTrack + "/" + DefaultRisk } for _, new := range selection.Slices { From d593eea27a59208f4dbe3e80ef51cbe75b0d9e82 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 28 Jul 2026 09:09:15 +0200 Subject: [PATCH 06/13] style: minor typo --- internal/setup/setup.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/setup/setup.go b/internal/setup/setup.go index 96c95c6bd..6df6f1c06 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -585,7 +585,7 @@ func Select(release *Release, refs []SliceRef, arch string) (*Selection, error) } selection := &Selection{ - Release: release, + Release: release, } slices := make([]SliceKey, len(refs)) From ca0cec4dc03784a63ed905dc1e557c49fe0342de Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 28 Jul 2026 13:30:15 +0200 Subject: [PATCH 07/13] style: improve naming --- internal/setup/setup.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/setup/setup.go b/internal/setup/setup.go index 6df6f1c06..2c34b67ad 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -174,17 +174,17 @@ func ParseSliceRef(ref string) (SliceRef, error) { if err != nil { return SliceRef{}, err } - channel, err = parseChannel(channel) + channel, err = validateChannel(channel) if err != nil { return SliceRef{}, fmt.Errorf("invalid slice reference %q: %s", ref, err) } return SliceRef{Key: key, Channel: channel}, nil } -// parseChannel validates a channel and returns it with the default risk -// appended if it holds a track alone. Any number of segments is accepted so -// that longer forms are not rejected here. -func parseChannel(channel string) (string, error) { +// validateChannel returns the channel with the default risk appended if it +// holds a track alone. Validation is intentionally loose, any number of +// segments is accepted so that longer forms are not rejected here. +func validateChannel(channel string) (string, error) { if channel == "" { return "", fmt.Errorf("missing channel") } From dc984c4f18be487c359fe67731d2f7b4eb0d5461 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 30 Jul 2026 14:05:44 +0200 Subject: [PATCH 08/13] fix: apply review --- cmd/chisel/cmd_find.go | 2 +- cmd/chisel/cmd_find_test.go | 4 +- cmd/chisel/cmd_info.go | 2 +- cmd/chisel/cmd_info_test.go | 4 +- internal/setup/setup.go | 18 ++++---- internal/setup/setup_test.go | 84 +++++++++++++++++----------------- internal/slicer/slicer_test.go | 2 +- 7 files changed, 58 insertions(+), 58 deletions(-) diff --git a/cmd/chisel/cmd_find.go b/cmd/chisel/cmd_find.go index d7b9dfdb1..08d64b254 100644 --- a/cmd/chisel/cmd_find.go +++ b/cmd/chisel/cmd_find.go @@ -91,7 +91,7 @@ func match(slice *setup.Slice, query string) bool { func findSlices(release *setup.Release, query []string) (slices []*setup.Slice, err error) { for _, term := range query { if strings.Contains(term, "@") { - return nil, fmt.Errorf("channel is unsupported for find: %q", term) + return nil, fmt.Errorf("invalid slice reference %q: slices are not specific to a channel", term) } } slices = []*setup.Slice{} diff --git a/cmd/chisel/cmd_find_test.go b/cmd/chisel/cmd_find_test.go index 0d2c117b2..1d0c6d405 100644 --- a/cmd/chisel/cmd_find_test.go +++ b/cmd/chisel/cmd_find_test.go @@ -125,10 +125,10 @@ var findTests = []findTest{{ query: []string{"python", "slice"}, result: []*setup.Slice{}, }, { - summary: "Channel is unsupported for find", + summary: "Slices are not specific to a channel", release: sampleRelease, query: []string{"python3.10_bins@3.0"}, - err: `channel is unsupported for find: "python3.10_bins@3.0"`, + err: `invalid slice reference "python3.10_bins@3.0": slices are not specific to a channel`, }} func (s *ChiselSuite) TestFindSlices(c *C) { diff --git a/cmd/chisel/cmd_info.go b/cmd/chisel/cmd_info.go index f8f0015c5..1e518d577 100644 --- a/cmd/chisel/cmd_info.go +++ b/cmd/chisel/cmd_info.go @@ -52,7 +52,7 @@ func (cmd *infoCmd) Execute(args []string) error { for _, query := range cmd.Positional.Queries { if strings.Contains(query, "@") { - return fmt.Errorf("channel is unsupported for info: %q", query) + return fmt.Errorf("invalid slice reference %q: slices are not specific to a channel", query) } } diff --git a/cmd/chisel/cmd_info_test.go b/cmd/chisel/cmd_info_test.go index 8c3d13f45..d338049b6 100644 --- a/cmd/chisel/cmd_info_test.go +++ b/cmd/chisel/cmd_info_test.go @@ -141,10 +141,10 @@ var infoTests = []infoTest{{ query: []string{"foo_bar_foo", "a_b", "7_c", "a_b c", "a_b x_y"}, err: `no slice definitions found for: "foo_bar_foo", "a_b", "7_c", "a_b c", "a_b x_y"`, }, { - summary: "Channel is unsupported for info", + summary: "Slices are not specific to a channel", input: infoRelease, query: []string{"mypkg1_myslice1@3.0"}, - err: `channel is unsupported for info: "mypkg1_myslice1@3.0"`, + err: `invalid slice reference "mypkg1_myslice1@3.0": slices are not specific to a channel`, }} var infoRelease = map[string]string{ diff --git a/internal/setup/setup.go b/internal/setup/setup.go index 2c34b67ad..0de33eb86 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -152,8 +152,8 @@ const DefaultRisk = "stable" // SliceRef is a slice reference with an optional channel for store packages. // The channel always holds a risk, that is at least "/". type SliceRef struct { - Key SliceKey - Channel string + SliceKey SliceKey + Channel string } // ParseSliceRef parses a "pkg_slice[@channel]" reference. The channel is @@ -164,13 +164,13 @@ type SliceRef struct { func ParseSliceRef(ref string) (SliceRef, error) { keyPart, channel, ok := strings.Cut(ref, "@") if !ok { - key, err := ParseSliceKey(ref) + sliceKey, err := ParseSliceKey(ref) if err != nil { return SliceRef{}, err } - return SliceRef{Key: key}, nil + return SliceRef{SliceKey: sliceKey}, nil } - key, err := ParseSliceKey(keyPart) + sliceKey, err := ParseSliceKey(keyPart) if err != nil { return SliceRef{}, err } @@ -178,7 +178,7 @@ func ParseSliceRef(ref string) (SliceRef, error) { if err != nil { return SliceRef{}, fmt.Errorf("invalid slice reference %q: %s", ref, err) } - return SliceRef{Key: key, Channel: channel}, nil + return SliceRef{SliceKey: sliceKey, Channel: channel}, nil } // validateChannel returns the channel with the default risk appended if it @@ -590,7 +590,7 @@ func Select(release *Release, refs []SliceRef, arch string) (*Selection, error) slices := make([]SliceKey, len(refs)) for i, ref := range refs { - slices[i] = ref.Key + slices[i] = ref.SliceKey } sorted, err := order(release.Packages, slices, arch) if err != nil { @@ -646,7 +646,7 @@ func Select(release *Release, refs []SliceRef, arch string) (*Selection, error) func resolveChannels(release *Release, refs []SliceRef) (map[string]string, error) { channels := make(map[string]string) for _, ref := range refs { - pkg, ok := release.Packages[ref.Key.Package] + pkg, ok := release.Packages[ref.SliceKey.Package] if !ok { // Nothing to validate; the package is unknown. continue @@ -654,7 +654,7 @@ func resolveChannels(release *Release, refs []SliceRef) (map[string]string, erro if pkg.Store == "" { if ref.Channel != "" { return nil, fmt.Errorf("slice %s has channel but package %q is not in a store", - ref.Key, pkg.Name) + ref.SliceKey, pkg.Name) } continue } diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 6d3f687f4..64fb1f431 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -426,7 +426,7 @@ var setupTests = []setupTest{{ myslice2: {essential: [mypkg1_myslice1]} `, }, - selrefs: []setup.SliceRef{{Key: setup.SliceKey{"mypkg1", "myslice1"}}}, + selrefs: []setup.SliceRef{{SliceKey: setup.SliceKey{"mypkg1", "myslice1"}}}, selection: &setup.Selection{ Slices: []*setup.Slice{{ Package: "mypkg1", @@ -450,7 +450,7 @@ var setupTests = []setupTest{{ myslice2: {essential: [mypkg1_myslice1]} `, }, - selrefs: []setup.SliceRef{{Key: setup.SliceKey{"mypkg2", "myslice2"}}}, + selrefs: []setup.SliceRef{{SliceKey: setup.SliceKey{"mypkg2", "myslice2"}}}, selection: &setup.Selection{ Slices: []*setup.Slice{{ Package: "mypkg1", @@ -491,9 +491,9 @@ var setupTests = []setupTest{{ `, }, selrefs: []setup.SliceRef{ - {Key: setup.SliceKey{"mypkg1", "myslice1"}}, - {Key: setup.SliceKey{"mypkg1", "myslice2"}}, - {Key: setup.SliceKey{"mypkg2", "myslice1"}}, + {SliceKey: setup.SliceKey{"mypkg1", "myslice1"}}, + {SliceKey: setup.SliceKey{"mypkg1", "myslice2"}}, + {SliceKey: setup.SliceKey{"mypkg2", "myslice1"}}, }, }, { summary: "Conflicting paths across slices", @@ -1762,7 +1762,7 @@ var setupTests = []setupTest{{ EndOfLife: time.Date(2100, time.January, 1, 0, 0, 0, 0, time.UTC), }, }, - selrefs: []setup.SliceRef{{Key: setup.SliceKey{"mypkg", "myslice"}}}, + selrefs: []setup.SliceRef{{SliceKey: setup.SliceKey{"mypkg", "myslice"}}}, selection: &setup.Selection{ Slices: []*setup.Slice{{ Package: "mypkg", @@ -1817,7 +1817,7 @@ var setupTests = []setupTest{{ EndOfLife: time.Date(2100, time.January, 1, 0, 0, 0, 0, time.UTC), }, }, - selrefs: []setup.SliceRef{{Key: setup.SliceKey{"mypkg", "myslice"}}}, + selrefs: []setup.SliceRef{{SliceKey: setup.SliceKey{"mypkg", "myslice"}}}, selerror: `slice mypkg_myslice has invalid 'generate' for path /dir/\*\*: "foo"`, }, { summary: "Paths with generate: manifest must have trailing /**", @@ -2438,10 +2438,10 @@ var setupTests = []setupTest{{ }, { summary: "Path conflicts with 'prefer'", selrefs: []setup.SliceRef{ - {Key: setup.SliceKey{"mypkg1", "myslice1"}}, - {Key: setup.SliceKey{"mypkg1", "myslice2"}}, - {Key: setup.SliceKey{"mypkg2", "myslice1"}}, - {Key: setup.SliceKey{"mypkg3", "myslice1"}}, + {SliceKey: setup.SliceKey{"mypkg1", "myslice1"}}, + {SliceKey: setup.SliceKey{"mypkg1", "myslice2"}}, + {SliceKey: setup.SliceKey{"mypkg2", "myslice1"}}, + {SliceKey: setup.SliceKey{"mypkg3", "myslice1"}}, }, input: map[string]string{ "slices/mydir/mypkg1.yaml": ` @@ -2548,9 +2548,9 @@ var setupTests = []setupTest{{ }, { summary: "Path conflicts with 'prefer' depends on selection", selrefs: []setup.SliceRef{ - {Key: setup.SliceKey{"mypkg1", "myslice1"}}, - {Key: setup.SliceKey{"mypkg1", "myslice2"}}, - {Key: setup.SliceKey{"mypkg2", "myslice1"}}, + {SliceKey: setup.SliceKey{"mypkg1", "myslice1"}}, + {SliceKey: setup.SliceKey{"mypkg1", "myslice2"}}, + {SliceKey: setup.SliceKey{"mypkg2", "myslice1"}}, }, input: map[string]string{ "slices/mydir/mypkg1.yaml": ` @@ -4382,7 +4382,7 @@ var setupTests = []setupTest{{ }, }, { summary: "Store unknown kind", - selrefs: []setup.SliceRef{{Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}}}, + selrefs: []setup.SliceRef{{SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}}}, input: map[string]string{ "chisel.yaml": ` format: v3 @@ -4418,7 +4418,7 @@ var setupTests = []setupTest{{ selerror: `slice bin-mypkg_myslice refers to store "bin" with unknown kind "unknown"`, }, { summary: "Channel on bin slice is derived from default-track when omitted", - selrefs: []setup.SliceRef{{Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}}}, + selrefs: []setup.SliceRef{{SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}}}, input: map[string]string{ "chisel.yaml": testutil.DefaultChiselYamlWithStores, "bin-slices/mypkg.yaml": ` @@ -4444,8 +4444,8 @@ var setupTests = []setupTest{{ }, { summary: "Channel on bin slice is set from the reference", selrefs: []setup.SliceRef{{ - Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}, - Channel: "2.0/edge", + SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}, + Channel: "2.0/edge", }}, input: map[string]string{ "chisel.yaml": testutil.DefaultChiselYamlWithStores, @@ -4472,8 +4472,8 @@ var setupTests = []setupTest{{ }, { summary: "Same channel on two slices of same bin package is allowed", selrefs: []setup.SliceRef{ - {Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}, Channel: "2.0/stable"}, - {Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice2"}, Channel: "2.0/stable"}, + {SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}, Channel: "2.0/stable"}, + {SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice2"}, Channel: "2.0/stable"}, }, input: map[string]string{ "chisel.yaml": testutil.DefaultChiselYamlWithStores, @@ -4509,8 +4509,8 @@ var setupTests = []setupTest{{ }, { summary: "Conflicting channels on two slices of same bin package fails", selrefs: []setup.SliceRef{ - {Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}, Channel: "2.0/stable"}, - {Key: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice2"}, Channel: "2.0/edge"}, + {SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}, Channel: "2.0/stable"}, + {SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice2"}, Channel: "2.0/edge"}, }, input: map[string]string{ "chisel.yaml": testutil.DefaultChiselYamlWithStores, @@ -4531,8 +4531,8 @@ var setupTests = []setupTest{{ }, { summary: "Channel on a non-store (deb) package fails", selrefs: []setup.SliceRef{{ - Key: setup.SliceKey{Package: "mypkg", Slice: "myslice"}, - Channel: "2.0/stable", + SliceKey: setup.SliceKey{Package: "mypkg", Slice: "myslice"}, + Channel: "2.0/stable", }}, input: map[string]string{ "chisel.yaml": testutil.DefaultChiselYaml, @@ -5043,7 +5043,7 @@ func (s *S) TestSelectEmptyArch(c *C) { release, err := setup.ReadRelease(dir) c.Assert(err, IsNil) - refs := []setup.SliceRef{{Key: setup.SliceKey{"mypkg", "myslice"}}} + refs := []setup.SliceRef{{SliceKey: setup.SliceKey{"mypkg", "myslice"}}} selection, err := setup.Select(release, refs, "") c.Assert(err, IsNil) @@ -5061,59 +5061,59 @@ var parseSliceRefTests = []struct { err string }{{ input: "foo_bar", - expected: setup.SliceRef{Key: setup.SliceKey{Package: "foo", Slice: "bar"}}, + expected: setup.SliceRef{SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}}, }, { // A track alone gets the default risk. input: "foo_bar@3.0", expected: setup.SliceRef{ - Key: setup.SliceKey{Package: "foo", Slice: "bar"}, - Channel: "3.0/stable", + SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}, + Channel: "3.0/stable", }, }, { input: "foo_bar@latest", expected: setup.SliceRef{ - Key: setup.SliceKey{Package: "foo", Slice: "bar"}, - Channel: "latest/stable", + SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}, + Channel: "latest/stable", }, }, { input: "foo_bar@3.0/edge", expected: setup.SliceRef{ - Key: setup.SliceKey{Package: "foo", Slice: "bar"}, - Channel: "3.0/edge", + SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}, + Channel: "3.0/edge", }, }, { // An explicit default risk is kept as is. input: "foo_bar@3.0/stable", expected: setup.SliceRef{ - Key: setup.SliceKey{Package: "foo", Slice: "bar"}, - Channel: "3.0/stable", + SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}, + Channel: "3.0/stable", }, }, { // Validation is loose, unknown risks are accepted. input: "foo_bar@3.0/whatever", expected: setup.SliceRef{ - Key: setup.SliceKey{Package: "foo", Slice: "bar"}, - Channel: "3.0/whatever", + SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}, + Channel: "3.0/whatever", }, }, { input: "foo-pkg_dashed-slice@3.0/beta", expected: setup.SliceRef{ - Key: setup.SliceKey{Package: "foo-pkg", Slice: "dashed-slice"}, - Channel: "3.0/beta", + SliceKey: setup.SliceKey{Package: "foo-pkg", Slice: "dashed-slice"}, + Channel: "3.0/beta", }, }, { // Split on the first '@'; the channel may itself contain '@'. input: "foo_bar@3.0@x", expected: setup.SliceRef{ - Key: setup.SliceKey{Package: "foo", Slice: "bar"}, - Channel: "3.0@x/stable", + SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}, + Channel: "3.0@x/stable", }, }, { // Longer forms, such as a branch, are not rejected. input: "foo_bar@3.0/stable/mybranch", expected: setup.SliceRef{ - Key: setup.SliceKey{Package: "foo", Slice: "bar"}, - Channel: "3.0/stable/mybranch", + SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}, + Channel: "3.0/stable/mybranch", }, }, { input: "foo_bar@", diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index df9d25d82..a2a6b80ba 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -2096,7 +2096,7 @@ func runSlicerTests(s *S, c *C, tests []slicerTest) { refs := make([]setup.SliceRef, len(testSlices)) for i, key := range testSlices { - refs[i] = setup.SliceRef{Key: key} + refs[i] = setup.SliceRef{SliceKey: key} } selection, err := setup.Select(release, refs, test.arch) c.Assert(err, IsNil) From 09dc0d3c4de0284d0316542a7aa145202d6c233c Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 30 Jul 2026 15:14:17 +0200 Subject: [PATCH 09/13] fix: apply review --- internal/setup/channel.go | 57 +++++++++++++++++++++++ internal/setup/setup.go | 90 +++++++++++++++--------------------- internal/setup/setup_test.go | 55 ++++++++++++---------- 3 files changed, 124 insertions(+), 78 deletions(-) create mode 100644 internal/setup/channel.go diff --git a/internal/setup/channel.go b/internal/setup/channel.go new file mode 100644 index 000000000..881ef93dd --- /dev/null +++ b/internal/setup/channel.go @@ -0,0 +1,57 @@ +package setup + +import ( + "errors" + "strings" + "unicode" +) + +// Channel is a store channel, as in "/[/]". +type Channel struct { + Track string + Risk string + Branch string +} + +func (c Channel) String() string { + if c.Track == "" { + return "" + } + channel := c.Track + if c.Risk != "" { + channel += "/" + c.Risk + } + if c.Branch != "" { + channel += "/" + c.Branch + } + return channel +} + +// parseChannel parses a "[/[/]]" channel, as written. +// Validation is intentionally loose, the track, the risk and the branch are +// only checked for their presence so that their values are not rejected here. +func parseChannel(channel string) (Channel, error) { + if channel == "" { + return Channel{}, errors.New("missing channel") + } + if strings.ContainsFunc(channel, unicode.IsSpace) { + return Channel{}, errors.New("channel must not contain spaces") + } + segments := strings.Split(channel, "/") + if len(segments) > 3 { + return Channel{}, errors.New("channel must be [/[/]]") + } + for _, segment := range segments { + if segment == "" { + return Channel{}, errors.New("channel must be [/[/]]") + } + } + parsed := Channel{Track: segments[0]} + if len(segments) > 1 { + parsed.Risk = segments[1] + } + if len(segments) > 2 { + parsed.Branch = segments[2] + } + return parsed, nil +} diff --git a/internal/setup/setup.go b/internal/setup/setup.go index 0de33eb86..e20d5a11a 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -8,7 +8,6 @@ import ( "slices" "strings" "time" - "unicode" "golang.org/x/crypto/openpgp/packet" @@ -146,21 +145,19 @@ func ParseSliceKey(sliceKey string) (SliceKey, error) { return apacheutil.ParseSliceKey(sliceKey) } -// DefaultRisk is used when a channel does not specify a risk. +// DefaultRisk is used when a slice reference does not specify a risk. const DefaultRisk = "stable" // SliceRef is a slice reference with an optional channel for store packages. -// The channel always holds a risk, that is at least "/". +// The channel always holds a risk, the default one is used when the reference +// does not specify it. type SliceRef struct { SliceKey SliceKey - Channel string + Channel Channel } -// ParseSliceRef parses a "pkg_slice[@channel]" reference. The channel is -// either a track, in which case the default risk is used, or a -// "/" value. Validation is intentionally loose so that longer -// channel forms, such as "//", are accepted without -// changes here. +// ParseSliceRef parses a "pkg_slice[@channel]" reference. See parseChannel +// for the accepted channel forms. func ParseSliceRef(ref string) (SliceRef, error) { keyPart, channel, ok := strings.Cut(ref, "@") if !ok { @@ -174,32 +171,14 @@ func ParseSliceRef(ref string) (SliceRef, error) { if err != nil { return SliceRef{}, err } - channel, err = validateChannel(channel) + parsed, err := parseChannel(channel) if err != nil { return SliceRef{}, fmt.Errorf("invalid slice reference %q: %s", ref, err) } - return SliceRef{SliceKey: sliceKey, Channel: channel}, nil -} - -// validateChannel returns the channel with the default risk appended if it -// holds a track alone. Validation is intentionally loose, any number of -// segments is accepted so that longer forms are not rejected here. -func validateChannel(channel string) (string, error) { - if channel == "" { - return "", fmt.Errorf("missing channel") - } - if strings.ContainsFunc(channel, unicode.IsSpace) { - return "", fmt.Errorf("channel must not contain spaces") + if parsed.Risk == "" { + parsed.Risk = DefaultRisk } - segments := strings.Split(channel, "/") - if slices.Contains(segments, "") { - return "", fmt.Errorf("channel must be or /") - } - if len(segments) == 1 { - // A track alone, the risk is implicit. - return channel + "/" + DefaultRisk, nil - } - return channel, nil + return SliceRef{SliceKey: sliceKey, Channel: parsed}, nil } func (s *Slice) String() string { return s.Package + "_" + s.Name } @@ -212,7 +191,7 @@ type Selection struct { Release *Release Slices []*Slice // Channels holds the resolved channel per store package name. - Channels map[string]string + Channels map[string]Channel } // Prefers uses the prefer relationships and returns a map from each path to @@ -565,24 +544,13 @@ func Select(release *Release, refs []SliceRef, arch string) (*Selection, error) return nil, err } - // Resolve the channel per store package from the references. + // Resolve the channel of every store package, whether it is selected or + // not, and before ordering, because ordering depends on the channel of the + // packages it traverses. channels, err := resolveChannels(release, refs) if err != nil { return nil, err } - // Derive the channel from 'default-track' for the store packages without an - // explicit one. Note the release only defines a track, the risk is - // implicit. This is done for every known package, before ordering, because - // ordering itself depends on the channel of the packages it traverses. - for _, pkg := range release.Packages { - if pkg.Store == "" { - continue - } - if _, ok := channels[pkg.Name]; ok { - continue - } - channels[pkg.Name] = pkg.DefaultTrack + "/" + DefaultRisk - } selection := &Selection{ Release: release, @@ -602,7 +570,7 @@ func Select(release *Release, refs []SliceRef, arch string) (*Selection, error) } // Only report the channels of the selected packages. - selection.Channels = make(map[string]string) + selection.Channels = make(map[string]Channel) for _, slice := range selection.Slices { if channel, ok := channels[slice.Package]; ok { selection.Channels[slice.Package] = channel @@ -640,11 +608,15 @@ func Select(release *Release, refs []SliceRef, arch string) (*Selection, error) return selection, nil } -// resolveChannels collects the explicit channel per store package from the -// references. It errors if a channel is set on a non-store package or if two -// references to the same package specify different channels. -func resolveChannels(release *Release, refs []SliceRef) (map[string]string, error) { - channels := make(map[string]string) +// resolveChannels returns the channel of every store package of the release, +// taken from the references and falling back to the package 'default-track' +// with the default risk. Note the release only defines a track, the risk is +// implicit. +// +// It errors if a channel is set on a non-store package or if two references to +// the same package specify different channels. +func resolveChannels(release *Release, refs []SliceRef) (map[string]Channel, error) { + channels := make(map[string]Channel) for _, ref := range refs { pkg, ok := release.Packages[ref.SliceKey.Package] if !ok { @@ -652,13 +624,13 @@ func resolveChannels(release *Release, refs []SliceRef) (map[string]string, erro continue } if pkg.Store == "" { - if ref.Channel != "" { + if ref.Channel != (Channel{}) { return nil, fmt.Errorf("slice %s has channel but package %q is not in a store", ref.SliceKey, pkg.Name) } continue } - if ref.Channel == "" { + if ref.Channel == (Channel{}) { continue } if existing, ok := channels[pkg.Name]; ok && existing != ref.Channel { @@ -667,6 +639,16 @@ func resolveChannels(release *Release, refs []SliceRef) (map[string]string, erro } channels[pkg.Name] = ref.Channel } + for _, pkg := range release.Packages { + if pkg.Store == "" { + continue + } + if _, ok := channels[pkg.Name]; ok { + // The references take precedence over the 'default-track'. + continue + } + channels[pkg.Name] = Channel{Track: pkg.DefaultTrack, Risk: DefaultRisk} + } return channels, nil } diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 64fb1f431..0b8ea0fa1 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -432,7 +432,7 @@ var setupTests = []setupTest{{ Package: "mypkg1", Name: "myslice1", }}, - Channels: map[string]string{}, + Channels: map[string]setup.Channel{}, }, }, { summary: "Selection with dependencies", @@ -462,7 +462,7 @@ var setupTests = []setupTest{{ {"mypkg1", "myslice1"}: {}, }, }}, - Channels: map[string]string{}, + Channels: map[string]setup.Channel{}, }, }, { summary: "Selection with matching paths don't conflict", @@ -1771,7 +1771,7 @@ var setupTests = []setupTest{{ "/dir/**": {Kind: "generate", Generate: "manifest"}, }, }}, - Channels: map[string]string{}, + Channels: map[string]setup.Channel{}, }, }, { summary: "Can specify generate with bogus value but cannot select those slices", @@ -4439,13 +4439,13 @@ var setupTests = []setupTest{{ "/dir/file": {Kind: setup.CopyPath}, }, }}, - Channels: map[string]string{"bin-mypkg": "3.0/stable"}, + Channels: map[string]setup.Channel{"bin-mypkg": {Track: "3.0", Risk: "stable"}}, }, }, { summary: "Channel on bin slice is set from the reference", selrefs: []setup.SliceRef{{ SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}, - Channel: "2.0/edge", + Channel: setup.Channel{Track: "2.0", Risk: "edge"}, }}, input: map[string]string{ "chisel.yaml": testutil.DefaultChiselYamlWithStores, @@ -4467,13 +4467,13 @@ var setupTests = []setupTest{{ "/dir/file": {Kind: setup.CopyPath}, }, }}, - Channels: map[string]string{"bin-mypkg": "2.0/edge"}, + Channels: map[string]setup.Channel{"bin-mypkg": {Track: "2.0", Risk: "edge"}}, }, }, { summary: "Same channel on two slices of same bin package is allowed", selrefs: []setup.SliceRef{ - {SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}, Channel: "2.0/stable"}, - {SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice2"}, Channel: "2.0/stable"}, + {SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}, Channel: setup.Channel{Track: "2.0", Risk: "stable"}}, + {SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice2"}, Channel: setup.Channel{Track: "2.0", Risk: "stable"}}, }, input: map[string]string{ "chisel.yaml": testutil.DefaultChiselYamlWithStores, @@ -4504,13 +4504,13 @@ var setupTests = []setupTest{{ "/dir/file2": {Kind: setup.CopyPath}, }, }}, - Channels: map[string]string{"bin-mypkg": "2.0/stable"}, + Channels: map[string]setup.Channel{"bin-mypkg": {Track: "2.0", Risk: "stable"}}, }, }, { summary: "Conflicting channels on two slices of same bin package fails", selrefs: []setup.SliceRef{ - {SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}, Channel: "2.0/stable"}, - {SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice2"}, Channel: "2.0/edge"}, + {SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}, Channel: setup.Channel{Track: "2.0", Risk: "stable"}}, + {SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice2"}, Channel: setup.Channel{Track: "2.0", Risk: "edge"}}, }, input: map[string]string{ "chisel.yaml": testutil.DefaultChiselYamlWithStores, @@ -4532,7 +4532,7 @@ var setupTests = []setupTest{{ summary: "Channel on a non-store (deb) package fails", selrefs: []setup.SliceRef{{ SliceKey: setup.SliceKey{Package: "mypkg", Slice: "myslice"}, - Channel: "2.0/stable", + Channel: setup.Channel{Track: "2.0", Risk: "stable"}, }}, input: map[string]string{ "chisel.yaml": testutil.DefaultChiselYaml, @@ -5067,66 +5067,73 @@ var parseSliceRefTests = []struct { input: "foo_bar@3.0", expected: setup.SliceRef{ SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}, - Channel: "3.0/stable", + Channel: setup.Channel{Track: "3.0", Risk: "stable"}, }, }, { input: "foo_bar@latest", expected: setup.SliceRef{ SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}, - Channel: "latest/stable", + Channel: setup.Channel{Track: "latest", Risk: "stable"}, }, }, { input: "foo_bar@3.0/edge", expected: setup.SliceRef{ SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}, - Channel: "3.0/edge", + Channel: setup.Channel{Track: "3.0", Risk: "edge"}, }, }, { // An explicit default risk is kept as is. input: "foo_bar@3.0/stable", expected: setup.SliceRef{ SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}, - Channel: "3.0/stable", + Channel: setup.Channel{Track: "3.0", Risk: "stable"}, }, }, { // Validation is loose, unknown risks are accepted. input: "foo_bar@3.0/whatever", expected: setup.SliceRef{ SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}, - Channel: "3.0/whatever", + Channel: setup.Channel{Track: "3.0", Risk: "whatever"}, }, }, { input: "foo-pkg_dashed-slice@3.0/beta", expected: setup.SliceRef{ SliceKey: setup.SliceKey{Package: "foo-pkg", Slice: "dashed-slice"}, - Channel: "3.0/beta", + Channel: setup.Channel{Track: "3.0", Risk: "beta"}, }, }, { // Split on the first '@'; the channel may itself contain '@'. input: "foo_bar@3.0@x", expected: setup.SliceRef{ SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}, - Channel: "3.0@x/stable", + Channel: setup.Channel{Track: "3.0@x", Risk: "stable"}, }, }, { - // Longer forms, such as a branch, are not rejected. + // A branch is accepted, although not advertised yet. input: "foo_bar@3.0/stable/mybranch", expected: setup.SliceRef{ SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}, - Channel: "3.0/stable/mybranch", + Channel: setup.Channel{Track: "3.0", Risk: "stable", Branch: "mybranch"}, }, }, { input: "foo_bar@", err: `invalid slice reference "foo_bar@": missing channel`, }, { input: "foo_bar@/stable", - err: `invalid slice reference "foo_bar@/stable": channel must be or /`, + err: `invalid slice reference "foo_bar@/stable": channel must be \[/\[/\]\]`, }, { input: "foo_bar@3.0/", - err: `invalid slice reference "foo_bar@3.0/": channel must be or /`, + err: `invalid slice reference "foo_bar@3.0/": channel must be \[/\[/\]\]`, }, { input: "foo_bar@3.0//stable", - err: `invalid slice reference "foo_bar@3.0//stable": channel must be or /`, + err: `invalid slice reference "foo_bar@3.0//stable": channel must be \[/\[/\]\]`, +}, { + input: "foo_bar@3.0/stable/", + err: `invalid slice reference "foo_bar@3.0/stable/": channel must be \[/\[/\]\]`, +}, { + // A branch must not contain a /, hence no more than three segments. + input: "foo_bar@3.0/stable/mybranch/extra", + err: `invalid slice reference "foo_bar@3.0/stable/mybranch/extra": channel must be \[/\[/\]\]`, }, { input: "foo_bar@3.0 stable", err: `invalid slice reference "foo_bar@3.0 stable": channel must not contain spaces`, From fd5571200021030919d17f858850963eb1998d6f Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 30 Jul 2026 15:54:41 +0200 Subject: [PATCH 10/13] test: add missing test for corner case --- internal/setup/setup_test.go | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 0b8ea0fa1..8a6dcbf75 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -4441,6 +4441,40 @@ var setupTests = []setupTest{{ }}, Channels: map[string]setup.Channel{"bin-mypkg": {Track: "3.0", Risk: "stable"}}, }, +}, { + summary: "Channels of unselected bin packages are not reported", + selrefs: []setup.SliceRef{{SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}}}, + input: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlWithStores, + "bin-slices/mypkg.yaml": ` + package: mypkg + store: bin + default-track: "3.0" + slices: + myslice: + contents: + /dir/file: {} + `, + "bin-slices/otherpkg.yaml": ` + package: otherpkg + store: bin + default-track: "9.9" + slices: + myslice: + contents: + /other/file: {} + `, + }, + selection: &setup.Selection{ + Slices: []*setup.Slice{{ + Package: "bin-mypkg", + Name: "myslice", + Contents: map[string]setup.PathInfo{ + "/dir/file": {Kind: setup.CopyPath}, + }, + }}, + Channels: map[string]setup.Channel{"bin-mypkg": {Track: "3.0", Risk: "stable"}}, + }, }, { summary: "Channel on bin slice is set from the reference", selrefs: []setup.SliceRef{{ From 9ce962fc0e6c6f8809b2ff3e37763def1e729676 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 31 Jul 2026 10:46:12 +0200 Subject: [PATCH 11/13] fix: rework cmd cut long help --- cmd/chisel/cmd_cut.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 41c799b64..17588344f 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -22,12 +22,13 @@ By default it fetches the slices for the same Ubuntu version as the current host, unless the --release flag is used. Slices are named _. For packages coming from a store, a -channel may be appended to select which one to fetch, as in -mybin_myslice@2.0/edge. A channel with no risk, such as @2.0, uses the -stable risk. +channel can be appended to select which one to fetch, as in +mybin_myslice@2.0/edge. A channel is a / value. -Slices of the same package must all agree on the channel. When it is -omitted, the default track of the package is used. +The risk defaults to stable, so @2.0 and @2.0/stable are equivalent. When +no channel is given at all, the default track of the package is used, +again with the stable risk. Slices of the same package must all agree on +the channel. ` var cutDescs = map[string]string{ From bd089ce1264419ddb97d9dbf1de75b25ae3fe28f Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 31 Jul 2026 11:05:23 +0200 Subject: [PATCH 12/13] fix: refine String on Channel and add tests --- internal/setup/channel.go | 7 ++--- internal/setup/channel_test.go | 53 ++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 internal/setup/channel_test.go diff --git a/internal/setup/channel.go b/internal/setup/channel.go index 881ef93dd..47218f7e8 100644 --- a/internal/setup/channel.go +++ b/internal/setup/channel.go @@ -14,13 +14,10 @@ type Channel struct { } func (c Channel) String() string { - if c.Track == "" { + if c == (Channel{}) { return "" } - channel := c.Track - if c.Risk != "" { - channel += "/" + c.Risk - } + channel := c.Track + "/" + c.Risk if c.Branch != "" { channel += "/" + c.Branch } diff --git a/internal/setup/channel_test.go b/internal/setup/channel_test.go new file mode 100644 index 000000000..a3fc8b007 --- /dev/null +++ b/internal/setup/channel_test.go @@ -0,0 +1,53 @@ +package setup_test + +import ( + . "gopkg.in/check.v1" + + "github.com/canonical/chisel/internal/setup" +) + +var channelStringTests = []struct { + summary string + channel setup.Channel + expected string +}{{ + summary: "An unset channel renders as empty", + channel: setup.Channel{}, + expected: "", +}, { + summary: "A track and a risk", + channel: setup.Channel{Track: "3.0", Risk: "stable"}, + expected: "3.0/stable", +}, { + summary: "A branch is appended", + channel: setup.Channel{Track: "3.0", Risk: "edge", Branch: "mybranch"}, + expected: "3.0/edge/mybranch", +}, { + // A channel is never built without a risk, but rendering the risk as + // optional would turn the branch into one, that is a different channel. + summary: "A missing risk is not skipped over", + channel: setup.Channel{Track: "3.0", Branch: "mybranch"}, + expected: "3.0//mybranch", +}, { + summary: "A missing track is visible", + channel: setup.Channel{Risk: "edge"}, + expected: "/edge", +}} + +func (s *S) TestChannelString(c *C) { + for _, test := range channelStringTests { + c.Logf("Summary: %s", test.summary) + c.Assert(test.channel.String(), Equals, test.expected) + } +} + +// TestChannelStringRoundTrip ensures a parsed channel renders back to the +// value it was parsed from, once the implicit risk is set as ParseSliceRef +// does. +func (s *S) TestChannelStringRoundTrip(c *C) { + for _, channel := range []string{"3.0/stable", "3.0/edge", "3.0/stable/mybranch"} { + ref, err := setup.ParseSliceRef("mypkg_myslice@" + channel) + c.Assert(err, IsNil) + c.Assert(ref.Channel.String(), Equals, channel) + } +} From 9e512286d720fe06f933648d77cf55c954b30981 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 31 Jul 2026 11:07:52 +0200 Subject: [PATCH 13/13] tests: remove superfluous test --- internal/setup/channel_test.go | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/internal/setup/channel_test.go b/internal/setup/channel_test.go index a3fc8b007..3aa175e1f 100644 --- a/internal/setup/channel_test.go +++ b/internal/setup/channel_test.go @@ -40,14 +40,3 @@ func (s *S) TestChannelString(c *C) { c.Assert(test.channel.String(), Equals, test.expected) } } - -// TestChannelStringRoundTrip ensures a parsed channel renders back to the -// value it was parsed from, once the implicit risk is set as ParseSliceRef -// does. -func (s *S) TestChannelStringRoundTrip(c *C) { - for _, channel := range []string{"3.0/stable", "3.0/edge", "3.0/stable/mybranch"} { - ref, err := setup.ParseSliceRef("mypkg_myslice@" + channel) - c.Assert(err, IsNil) - c.Assert(ref.Channel.String(), Equals, channel) - } -}