From 02a8ba06572d636bf8eb3e36028efe388ecbf44e Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 18 Sep 2026 13:26:15 +0200 Subject: [PATCH 1/4] feat: support channel in CLI --- cmd/chisel/cmd_cut.go | 17 +- 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/channel.go | 29 ++++ internal/setup/channel_test.go | 35 +++++ internal/setup/setup.go | 80 +++++++++- internal/setup/setup_test.go | 275 +++++++++++++++++++++++++++------ internal/slicer/slicer_test.go | 6 +- 10 files changed, 411 insertions(+), 57 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 35c81a79a..17588344f 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -20,6 +20,15 @@ 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 can be appended to select which one to fetch, as in +mybin_myslice@2.0/edge. A channel is a / value. + +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{ @@ -49,13 +58,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 +82,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..08d64b254 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("invalid slice reference %q: slices are not specific to a channel", 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..1d0c6d405 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: "Slices are not specific to a channel", + release: sampleRelease, + query: []string{"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) { @@ -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..1e518d577 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("invalid slice reference %q: slices are not specific to a channel", 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..d338049b6 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: "Slices are not specific to a channel", + input: infoRelease, + query: []string{"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/channel.go b/internal/setup/channel.go index bb363f3ec..6b7f6e2ae 100644 --- a/internal/setup/channel.go +++ b/internal/setup/channel.go @@ -41,6 +41,35 @@ func (c Channel) String() string { return channel } +// parseChannel parses a "[/[/]]" channel. +// 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 +} + // The form a channel pattern must take, as reported to the user. const channelPatternForm = "/" diff --git a/internal/setup/channel_test.go b/internal/setup/channel_test.go index 3a3d52b93..58b7bc668 100644 --- a/internal/setup/channel_test.go +++ b/internal/setup/channel_test.go @@ -6,6 +6,41 @@ import ( "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) + } +} + // channelPatternTests covers validating and matching the patterns of a // "channel" field. The valid patterns come first, then the invalid ones, // grouped after the validation phase they exercise. Note several of the latter diff --git a/internal/setup/setup.go b/internal/setup/setup.go index cf2dcb80e..ca79e7cd5 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -147,10 +147,42 @@ func ParseSliceKey(sliceKey string) (SliceKey, error) { return apacheutil.ParseSliceKey(sliceKey) } -// DefaultRisk is used when a channel is resolved from a track alone, as done -// for the 'default-track' of a store package. +// 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, the default one is used when the reference +// does not specify it. +type SliceRef struct { + SliceKey SliceKey + Channel Channel +} + +// 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 { + sliceKey, err := ParseSliceKey(ref) + if err != nil { + return SliceRef{}, err + } + return SliceRef{SliceKey: sliceKey}, nil + } + sliceKey, err := ParseSliceKey(keyPart) + if err != nil { + return SliceRef{}, err + } + parsed, err := parseChannel(channel) + if err != nil { + return SliceRef{}, fmt.Errorf("invalid slice reference %q: %s", ref, err) + } + if parsed.Risk == "" { + parsed.Risk = DefaultRisk + } + return SliceRef{SliceKey: sliceKey, Channel: parsed}, nil +} + func (s *Slice) String() string { return s.Package + "_" + s.Name } // Selection holds the required configuration to create a Build for a selection @@ -507,7 +539,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 @@ -523,12 +555,19 @@ func Select(release *Release, slices []SliceKey, arch string) (*Selection, error // Select 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 := selectChannels(release) + channels, err := selectChannels(release, refs) + if err != nil { + return nil, err + } selection := &Selection{ Release: release, } + slices := make([]SliceKey, len(refs)) + for i, ref := range refs { + slices[i] = ref.SliceKey + } sorted, err := order(release.Packages, slices, arch, channels) if err != nil { return nil, err @@ -580,15 +619,44 @@ func Select(release *Release, slices []SliceKey, arch string) (*Selection, error // selectChannels returns the channel of every store package of the release, // derived from its 'default-track' with the default risk. Note the release // only defines a track, the risk is implicit. -func selectChannels(release *Release) map[string]Channel { +// +// It errors if a channel is set on a non-store package or if two references to +// the same package specify different channels. +func selectChannels(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 { + // Nothing to validate; the package is unknown. + continue + } + if pkg.Store == "" { + 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 == (Channel{}) { + continue + } + 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) + } + 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 + return channels, nil } const ( diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 5bd51c201..e42f20ace 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 } @@ -152,7 +152,7 @@ var setupTests = []setupTest{{ "/file/path2": {Kind: "copy", Info: "/other/path"}, "/file/path3": {Kind: "symlink", Info: "/other/path"}, "/file/path4": {Kind: "text", Info: "content", Until: "mutate"}, - "/file/path5": {Kind: "copy", Mode: 0755, Mutable: true}, + "/file/path5": {Kind: "copy", Mode: 0o755, Mutable: true}, "/file/path6/": {Kind: "dir"}, }, }, @@ -426,7 +426,7 @@ var setupTests = []setupTest{{ myslice2: {essential: [mypkg1_myslice1]} `, }, - selslices: []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]} `, }, - selslices: []setup.SliceKey{{"mypkg2", "myslice2"}}, + selrefs: []setup.SliceRef{{SliceKey: setup.SliceKey{"mypkg2", "myslice2"}}}, selection: &setup.Selection{ Slices: []*setup.Slice{{ Package: "mypkg1", @@ -490,7 +490,11 @@ var setupTests = []setupTest{{ /path3: {symlink: /link} `, }, - selslices: []setup.SliceKey{{"mypkg1", "myslice1"}, {"mypkg1", "myslice2"}, {"mypkg2", "myslice1"}}, + selrefs: []setup.SliceRef{ + {SliceKey: setup.SliceKey{"mypkg1", "myslice1"}}, + {SliceKey: setup.SliceKey{"mypkg1", "myslice2"}}, + {SliceKey: setup.SliceKey{"mypkg2", "myslice1"}}, + }, }, { summary: "Conflicting paths across slices", input: map[string]string{ @@ -1758,7 +1762,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{{SliceKey: setup.SliceKey{"mypkg", "myslice"}}}, selection: &setup.Selection{ Slices: []*setup.Slice{{ Package: "mypkg", @@ -1813,8 +1817,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{{SliceKey: 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{ @@ -2433,11 +2437,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{ + {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": ` @@ -2543,10 +2547,10 @@ var setupTests = []setupTest{{ }, }, { summary: "Path conflicts with 'prefer' depends on selection", - selslices: []setup.SliceKey{ - {"mypkg1", "myslice1"}, - {"mypkg1", "myslice2"}, - {"mypkg2", "myslice1"}, + selrefs: []setup.SliceRef{ + {SliceKey: setup.SliceKey{"mypkg1", "myslice1"}}, + {SliceKey: setup.SliceKey{"mypkg1", "myslice2"}}, + {SliceKey: setup.SliceKey{"mypkg2", "myslice1"}}, }, input: map[string]string{ "slices/mydir/mypkg1.yaml": ` @@ -4377,8 +4381,8 @@ var setupTests = []setupTest{{ }, }, }, { - summary: "Store unknown kind", - selslices: []setup.SliceKey{{Package: "bin-mypkg", Slice: "myslice"}}, + summary: "Store unknown kind", + selrefs: []setup.SliceRef{{SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}}}, input: map[string]string{ "chisel.yaml": ` format: v3 @@ -4413,8 +4417,8 @@ 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", - selslices: []setup.SliceKey{{Package: "bin-mypkg", Slice: "myslice"}}, + summary: "Channel on bin slice is derived from default-track when omitted", + selrefs: []setup.SliceRef{{SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}}}, input: map[string]string{ "chisel.yaml": testutil.DefaultChiselYamlWithStores, "bin-slices/mypkg.yaml": ` @@ -4438,8 +4442,8 @@ var setupTests = []setupTest{{ Channels: map[string]setup.Channel{"bin-mypkg": {Track: "3.0", Risk: "stable"}}, }, }, { - summary: "Channels of unselected bin packages are not reported", - selslices: []setup.SliceKey{{Package: "bin-mypkg", Slice: "myslice"}}, + 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": ` @@ -4472,21 +4476,21 @@ var setupTests = []setupTest{{ Channels: map[string]setup.Channel{"bin-mypkg": {Track: "3.0", Risk: "stable"}}, }, }, { - summary: "Channel on paths is parsed correctly", + summary: "Channel on bin slice is set from the reference", + selrefs: []setup.SliceRef{{ + SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}, + Channel: setup.Channel{Track: "2.0", Risk: "edge"}, + }}, input: map[string]string{ "chisel.yaml": testutil.DefaultChiselYamlWithStores, "bin-slices/mypkg.yaml": ` package: mypkg store: bin - default-track: "0.3" + default-track: "3.0" slices: myslice: contents: - /dir/excluded: {channel: ["0.2/!stable"], arch: amd64} - /dir/listed: {channel: ["0.2/beta,edge"]} - /dir/scalar: {channel: 0.3/stable} - /dir/union: {channel: ["0.2/*", "0.3/edge"]} - /dir/wildcard*: {channel: ["0.3/*"]} + /dir/file: {} `, }, release: &setup.Release{ @@ -4818,8 +4822,8 @@ var setupTests = []setupTest{{ // The channel of a store package is resolved from its 'default-track' with // the default risk, so a pattern gates the essential against that channel // alone. Selecting another channel is not possible yet. - summary: "Essential gated by a matching channel is selected", - selslices: []setup.SliceKey{{Package: "bin-mypkg", Slice: "myslice"}}, + summary: "Essential gated by a matching channel is selected", + selrefs: []setup.SliceRef{{SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}}}, input: map[string]string{ "chisel.yaml": testutil.DefaultChiselYamlWithStores, "bin-slices/mypkg.yaml": ` @@ -4856,8 +4860,45 @@ var setupTests = []setupTest{{ }, }, }, { - summary: "Essential gated by a non-matching channel is skipped", - selslices: []setup.SliceKey{{Package: "bin-mypkg", Slice: "myslice"}}, + summary: "Same channel on two slices of same bin package is allowed", + selrefs: []setup.SliceRef{ + {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, + "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}, + }, + }}, + Channels: map[string]setup.Channel{"bin-mypkg": {Track: "2.0", Risk: "stable"}}, + }, +}, { + summary: "Essential gated by a non-matching channel is skipped", + selrefs: []setup.SliceRef{{SliceKey: setup.SliceKey{Package: "bin-mypkg", Slice: "myslice"}}}, input: map[string]string{ "chisel.yaml": testutil.DefaultChiselYamlWithStores, "bin-slices/mypkg.yaml": ` @@ -4905,6 +4946,45 @@ var setupTests = []setupTest{{ `, }, relerror: `essential loop detected: bin-mypkg_myslice, bin-mypkg_other`, +}, { + summary: "Conflicting channels on two slices of same bin package fails", + selrefs: []setup.SliceRef{ + {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, + "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 channels "2.0/stable" and "2.0/edge"`, +}, { + summary: "Channel on a non-store (deb) package fails", + selrefs: []setup.SliceRef{{ + SliceKey: setup.SliceKey{Package: "mypkg", Slice: "myslice"}, + Channel: setup.Channel{Track: "2.0", Risk: "stable"}, + }}, + input: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYaml, + "slices/mypkg.yaml": ` + package: mypkg + slices: + myslice: + contents: + /dir/file: {} + `, + }, + selerror: `slice mypkg_myslice has channel but package "mypkg" is not in a store`, }} func (s *S) TestParseRelease(c *C) { @@ -5009,9 +5089,9 @@ func runParseReleaseTests(c *C, tests []setupTest) { dir := c.MkDir() for path, data := range test.input { fpath := filepath.Join(dir, path) - err := os.MkdirAll(filepath.Dir(fpath), 0755) + err := os.MkdirAll(filepath.Dir(fpath), 0o755) c.Assert(err, IsNil) - err = os.WriteFile(fpath, testutil.Reindent(data), 0644) + err = os.WriteFile(fpath, testutil.Reindent(data), 0o644) c.Assert(err, IsNil) } // Ensure the "slices" directory always exists, even if no slice @@ -5036,8 +5116,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 @@ -5076,16 +5156,16 @@ func (s *S) TestPackageMarshalYAML(c *C) { dir := c.MkDir() // Write chisel.yaml. fpath := filepath.Join(dir, "chisel.yaml") - err := os.WriteFile(fpath, testutil.Reindent(data), 0644) + err := os.WriteFile(fpath, testutil.Reindent(data), 0o644) c.Assert(err, IsNil) // Write the packages YAML. for _, pkg := range test.release.Packages { fpath = filepath.Join(dir, pkg.Path) - err = os.MkdirAll(filepath.Dir(fpath), 0755) + err = os.MkdirAll(filepath.Dir(fpath), 0o755) c.Assert(err, IsNil) pkgData, err := yaml.Marshal(pkg) c.Assert(err, IsNil) - err = os.WriteFile(fpath, testutil.Reindent(string(pkgData)), 0644) + err = os.WriteFile(fpath, testutil.Reindent(string(pkgData)), 0o644) c.Assert(err, IsNil) } // Ensure the "slices" directory always exists, even if no slice @@ -5102,7 +5182,7 @@ func (s *S) TestPackageMarshalYAML(c *C) { } func (s *S) TestPackageYAMLFormat(c *C) { - var tests = []struct { + tests := []struct { summary string input map[string]string expected map[string]string @@ -5351,9 +5431,9 @@ func (s *S) TestPackageYAMLFormat(c *C) { dir := c.MkDir() for path, data := range test.input { fpath := filepath.Join(dir, path) - err := os.MkdirAll(filepath.Dir(fpath), 0755) + err := os.MkdirAll(filepath.Dir(fpath), 0o755) c.Assert(err, IsNil) - err = os.WriteFile(fpath, testutil.Reindent(data), 0644) + err = os.WriteFile(fpath, testutil.Reindent(data), 0o644) c.Assert(err, IsNil) } // Ensure the "slices" directory always exists, even if no slice @@ -5443,8 +5523,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{{SliceKey: setup.SliceKey{"mypkg", "myslice"}}} + selection, err := setup.Select(release, refs, "") c.Assert(err, IsNil) var sliceNames []string @@ -5455,6 +5535,109 @@ 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{SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}}, +}, { + // A track alone gets the default risk. + input: "foo_bar@3.0", + expected: setup.SliceRef{ + SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}, + Channel: setup.Channel{Track: "3.0", Risk: "stable"}, + }, +}, { + input: "foo_bar@latest", + expected: setup.SliceRef{ + SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}, + Channel: setup.Channel{Track: "latest", Risk: "stable"}, + }, +}, { + input: "foo_bar@3.0/edge", + expected: setup.SliceRef{ + SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}, + 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: 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: 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: 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: setup.Channel{Track: "3.0@x", Risk: "stable"}, + }, +}, { + // 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: 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 \[/\[/\]\]`, +}, { + input: "foo_bar@3.0/", + 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 \[/\[/\]\]`, +}, { + 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`, +}, { + // 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 96b919f16..f2b092463 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -2115,7 +2115,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{SliceKey: key} + } + selection, err := setup.Select(release, refs, test.arch) c.Assert(err, IsNil) archives := map[string]archive.Archive{} From e366e44cc96070ab841b58e4463833ef2392c0be Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 18 Sep 2026 13:45:15 +0200 Subject: [PATCH 2/4] tests: cleaning --- internal/setup/setup_test.go | 68 +++++------------------------------- 1 file changed, 8 insertions(+), 60 deletions(-) diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index e42f20ace..f7766c941 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -4493,67 +4493,15 @@ var setupTests = []setupTest{{ /dir/file: {} `, }, - release: &setup.Release{ - Format: "v3", - Archives: map[string]*setup.Archive{ - "ubuntu": { - Name: "ubuntu", - Version: "22.04", - Suites: []string{"jammy"}, - Components: []string{"main", "universe"}, - PubKeys: []*packet.PublicKey{testKey.PubKey}, - Maintained: true, - }, - }, - Stores: map[string]*setup.Store{ - "bin": { - Name: "bin", - Kind: "bin", - Version: "26.10", - DefaultPrefix: "bin-", - }, - }, - Maintenance: &setup.Maintenance{ - Standard: time.Date(2025, time.January, 1, 0, 0, 0, 0, time.UTC), - EndOfLife: time.Date(2100, time.January, 1, 0, 0, 0, 0, time.UTC), - }, - Packages: map[string]*setup.Package{ - "bin-mypkg": { - RealName: "mypkg", - Name: "bin-mypkg", - Path: "bin-slices/mypkg.yaml", - Store: "bin", - DefaultTrack: "0.3", - Slices: map[string]*setup.Slice{ - "myslice": { - Package: "bin-mypkg", - Name: "myslice", - Contents: map[string]setup.PathInfo{ - "/dir/excluded": { - Kind: setup.CopyPath, Arch: []string{"amd64"}, - Channel: []string{"0.2/!stable"}, - }, - "/dir/listed": { - Kind: setup.CopyPath, - Channel: []string{"0.2/beta,edge"}, - }, - "/dir/scalar": { - Kind: setup.CopyPath, - Channel: []string{"0.3/stable"}, - }, - "/dir/union": { - Kind: setup.CopyPath, - Channel: []string{"0.2/*", "0.3/edge"}, - }, - "/dir/wildcard*": { - Kind: setup.GlobPath, - Channel: []string{"0.3/*"}, - }, - }, - }, - }, + 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: "2.0", Risk: "edge"}}, }, }, { summary: "Channel on essentials is parsed correctly", From 57178815c158cf75d709c52bc257745ca452ca03 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 18 Sep 2026 14:32:31 +0200 Subject: [PATCH 3/4] tests: re-add tests dropped by mistake --- internal/setup/setup_test.go | 80 ++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index f7766c941..255f8cb3a 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -4503,6 +4503,86 @@ var setupTests = []setupTest{{ }}, Channels: map[string]setup.Channel{"bin-mypkg": {Track: "2.0", Risk: "edge"}}, }, +}, { + summary: "Channel on paths is parsed correctly", + input: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlWithStores, + "bin-slices/mypkg.yaml": ` + package: mypkg + store: bin + default-track: "0.3" + slices: + myslice: + contents: + /dir/excluded: {channel: ["0.2/!stable"], arch: amd64} + /dir/listed: {channel: ["0.2/beta,edge"]} + /dir/scalar: {channel: 0.3/stable} + /dir/union: {channel: ["0.2/*", "0.3/edge"]} + /dir/wildcard*: {channel: ["0.3/*"]} + `, + }, + release: &setup.Release{ + Format: "v3", + Archives: map[string]*setup.Archive{ + "ubuntu": { + Name: "ubuntu", + Version: "22.04", + Suites: []string{"jammy"}, + Components: []string{"main", "universe"}, + PubKeys: []*packet.PublicKey{testKey.PubKey}, + Maintained: true, + }, + }, + Stores: map[string]*setup.Store{ + "bin": { + Name: "bin", + Kind: "bin", + Version: "26.10", + DefaultPrefix: "bin-", + }, + }, + Maintenance: &setup.Maintenance{ + Standard: time.Date(2025, time.January, 1, 0, 0, 0, 0, time.UTC), + EndOfLife: time.Date(2100, time.January, 1, 0, 0, 0, 0, time.UTC), + }, + Packages: map[string]*setup.Package{ + "bin-mypkg": { + RealName: "mypkg", + Name: "bin-mypkg", + Path: "bin-slices/mypkg.yaml", + Store: "bin", + DefaultTrack: "0.3", + Slices: map[string]*setup.Slice{ + "myslice": { + Package: "bin-mypkg", + Name: "myslice", + Contents: map[string]setup.PathInfo{ + "/dir/excluded": { + Kind: setup.CopyPath, Arch: []string{"amd64"}, + Channel: []string{"0.2/!stable"}, + }, + "/dir/listed": { + Kind: setup.CopyPath, + Channel: []string{"0.2/beta,edge"}, + }, + "/dir/scalar": { + Kind: setup.CopyPath, + Channel: []string{"0.3/stable"}, + }, + "/dir/union": { + Kind: setup.CopyPath, + Channel: []string{"0.2/*", "0.3/edge"}, + }, + "/dir/wildcard*": { + Kind: setup.GlobPath, + Channel: []string{"0.3/*"}, + }, + }, + }, + }, + }, + }, + }, }, { summary: "Channel on essentials is parsed correctly", input: map[string]string{ From a554e3fdd08a01d7b7a7224013feda1381fb044a Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 22 Sep 2026 09:27:46 +0200 Subject: [PATCH 4/4] feat: rework channel parsing Heavily inspired from snapd implementation. --- internal/setup/channel.go | 63 ++++++++++++++++++++++++---------- internal/setup/channel_test.go | 12 +++---- internal/setup/setup.go | 3 +- internal/setup/setup_test.go | 27 ++++++++++----- 4 files changed, 71 insertions(+), 34 deletions(-) diff --git a/internal/setup/channel.go b/internal/setup/channel.go index 6b7f6e2ae..c2e984de7 100644 --- a/internal/setup/channel.go +++ b/internal/setup/channel.go @@ -41,33 +41,60 @@ func (c Channel) String() string { return channel } -// parseChannel parses a "[/[/]]" channel. -// 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 == "" { +var channelRisks = []string{"stable", "candidate", "beta", "edge"} + +// parseChannel parses a "[/[/]]" string, representing +// a store channel. +func parseChannel(s string) (Channel, error) { + if s == "" { return Channel{}, errors.New("missing channel") } - if strings.ContainsFunc(channel, unicode.IsSpace) { + if strings.ContainsFunc(s, 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 [/[/]]") + p := strings.Split(s, "/") + var risk, track, branch *string + switch len(p) { + default: + return Channel{}, fmt.Errorf("channel must be [/[/]]: %s", s) + case 3: + track, risk, branch = &p[0], &p[1], &p[2] + case 2: + if slices.Contains(channelRisks, p[0]) { + risk, branch = &p[0], &p[1] + } else { + track, risk = &p[0], &p[1] + } + case 1: + if slices.Contains(channelRisks, p[0]) { + risk = &p[0] + } else { + track = &p[0] + } } - for _, segment := range segments { - if segment == "" { - return Channel{}, errors.New("channel must be [/[/]]") + + ch := Channel{} + + if risk != nil { + if !slices.Contains(channelRisks, *risk) { + return Channel{}, fmt.Errorf("invalid risk in channel name: %s", s) } + ch.Risk = *risk } - parsed := Channel{Track: segments[0]} - if len(segments) > 1 { - parsed.Risk = segments[1] + if track != nil { + if *track == "" { + return Channel{}, fmt.Errorf("invalid track in channel name: %s", s) + } + ch.Track = *track } - if len(segments) > 2 { - parsed.Branch = segments[2] + if branch != nil { + if *branch == "" { + return Channel{}, fmt.Errorf("invalid branch in channel name: %s", s) + } + ch.Branch = *branch } - return parsed, nil + + return ch, nil } // The form a channel pattern must take, as reported to the user. diff --git a/internal/setup/channel_test.go b/internal/setup/channel_test.go index 58b7bc668..e25f29191 100644 --- a/internal/setup/channel_test.go +++ b/internal/setup/channel_test.go @@ -167,8 +167,8 @@ var channelPatternTests = []struct { }, { // The "!" form. summary: "Unknown excluded risk", - values: []string{"0.3/!whatever"}, - err: `"0.3/!whatever": unknown risk "whatever", must be one of stable, candidate, beta, edge`, + values: []string{"0.3/!invalid"}, + err: `"0.3/!invalid": unknown risk "invalid", must be one of stable, candidate, beta, edge`, }, { summary: "Exclusion combined with other risks", values: []string{"0.3/!stable,edge"}, @@ -180,12 +180,12 @@ var channelPatternTests = []struct { }, { // The "[,]" form. summary: "Unknown risk", - values: []string{"0.3/whatever"}, - err: `"0.3/whatever": unknown risk "whatever", must be one of stable, candidate, beta, edge`, + values: []string{"0.3/invalid"}, + err: `"0.3/invalid": unknown risk "invalid", must be one of stable, candidate, beta, edge`, }, { summary: "Unknown risk in a list", - values: []string{"0.3/edge,whatever"}, - err: `"0.3/edge,whatever": unknown risk "whatever", must be one of stable, candidate, beta, edge`, + values: []string{"0.3/edge,invalid"}, + err: `"0.3/edge,invalid": unknown risk "invalid", must be one of stable, candidate, beta, edge`, }, { summary: "Risks are case sensitive", values: []string{"0.3/Stable"}, diff --git a/internal/setup/setup.go b/internal/setup/setup.go index ca79e7cd5..cc56f26a2 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -158,8 +158,7 @@ type SliceRef struct { Channel Channel } -// ParseSliceRef parses a "pkg_slice[@channel]" reference. See parseChannel -// for the accepted channel forms. +// ParseSliceRef parses a "pkg_slice[@channel]" reference. func ParseSliceRef(ref string) (SliceRef, error) { keyPart, channel, ok := strings.Cut(ref, "@") if !ok { diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 255f8cb3a..f3cf3e66d 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -5597,11 +5597,18 @@ var parseSliceRefTests = []struct { Channel: setup.Channel{Track: "3.0", Risk: "stable"}, }, }, { - // Validation is loose, unknown risks are accepted. - input: "foo_bar@3.0/whatever", + // A risk alone is accepted, with no track. + input: "foo_bar@edge", expected: setup.SliceRef{ SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}, - Channel: setup.Channel{Track: "3.0", Risk: "whatever"}, + Channel: setup.Channel{Risk: "edge"}, + }, +}, { + // A risk and a branch are accepted, with no track. + input: "foo_bar@beta/mybranch", + expected: setup.SliceRef{ + SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}, + Channel: setup.Channel{Risk: "beta", Branch: "mybranch"}, }, }, { input: "foo-pkg_dashed-slice@3.0/beta", @@ -5623,25 +5630,29 @@ var parseSliceRefTests = []struct { SliceKey: setup.SliceKey{Package: "foo", Slice: "bar"}, Channel: setup.Channel{Track: "3.0", Risk: "stable", Branch: "mybranch"}, }, +}, { + // Risks are validated, unknown ones are rejected. + input: "foo_bar@3.0/invalid", + err: `invalid slice reference "foo_bar@3.0/invalid": invalid risk in channel name: 3.0/invalid`, }, { 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 \[/\[/\]\]`, + err: `invalid slice reference "foo_bar@/stable": invalid track in channel name: /stable`, }, { input: "foo_bar@3.0/", - err: `invalid slice reference "foo_bar@3.0/": channel must be \[/\[/\]\]`, + err: `invalid slice reference "foo_bar@3.0/": invalid risk in channel name: 3.0/`, }, { input: "foo_bar@3.0//stable", - err: `invalid slice reference "foo_bar@3.0//stable": channel must be \[/\[/\]\]`, + err: `invalid slice reference "foo_bar@3.0//stable": invalid risk in channel name: 3.0//stable`, }, { input: "foo_bar@3.0/stable/", - err: `invalid slice reference "foo_bar@3.0/stable/": channel must be \[/\[/\]\]`, + err: `invalid slice reference "foo_bar@3.0/stable/": invalid branch in channel name: 3.0/stable/`, }, { // 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 \[/\[/\]\]`, + err: `invalid slice reference "foo_bar@3.0/stable/mybranch/extra": channel must be \[/\[/\]\]: 3.0/stable/mybranch/extra`, }, { input: "foo_bar@3.0 stable", err: `invalid slice reference "foo_bar@3.0 stable": channel must not contain spaces`,