From 670a56d5ad1e67e63f90ac9d26dd668819c43b03 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 3 Sep 2026 16:23:54 +0200 Subject: [PATCH 1/7] feat: channel patterns for slice definitions --- internal/setup/channel.go | 191 +++++++++++++++++++++++++++++++ internal/setup/channel_test.go | 203 +++++++++++++++++++++++++++++++++ 2 files changed, 394 insertions(+) create mode 100644 internal/setup/channel.go create mode 100644 internal/setup/channel_test.go diff --git a/internal/setup/channel.go b/internal/setup/channel.go new file mode 100644 index 00000000..60d642e9 --- /dev/null +++ b/internal/setup/channel.go @@ -0,0 +1,191 @@ +package setup + +import ( + "errors" + "fmt" + "slices" + "strings" + "unicode" +) + +// The "channel" field of a slice definition holds patterns selecting which +// concrete "/" channels an entry applies to. The track is a +// literal and only the risk part accepts operators: +// +// * - Any risk of that track +// ! - Any risk of that track but that one +// [,...] - Only those risks of that track +// +// Patterns are kept as written and interpreted on each match, as done for +// globs in the strdist package. They are validated when the release is read so +// that a malformed value is reported early, and rendered back verbatim. + +// 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 +} + +// The form a channel pattern must take, as reported to the user. +const channelPatternForm = "/" + +// splitChannel splits a channel or a channel pattern on "/", checking what the +// two have in common: no spaces and no empty segment. How many segments are +// expected and what each one means is left to the caller, as that is where the +// two differ. form is the shape reported on error. +func splitChannel(value, form string) ([]string, error) { + if strings.ContainsFunc(value, unicode.IsSpace) { + return nil, errors.New("must not contain spaces") + } + segments := strings.Split(value, "/") + if slices.Contains(segments, "") { + return nil, fmt.Errorf("must be %s", form) + } + return segments, nil +} + +// knownRisks holds every risk a channel may hold, from the most to the least +// stable. The set is defined by the store and does not depend on its content, +// hence risks are validated as architectures are. +var knownRisks = []string{"stable", "candidate", "beta", "edge"} + +// validateRisk validates a single risk of a channel or of a channel pattern. +func validateRisk(risk string) error { + if !slices.Contains(knownRisks, risk) { + return fmt.Errorf("unknown risk %q, must be one of %s", risk, strings.Join(knownRisks, ", ")) + } + return nil +} + +// validateChannelPatterns validates the values of a "channel" field. A track +// may appear at most once across the values so that the resulting set of +// channels is unambiguous. +func validateChannelPatterns(patterns []string) error { + seen := make(map[string]bool, len(patterns)) + for _, pattern := range patterns { + track, err := validateChannelPattern(pattern) + if err != nil { + return fmt.Errorf("%q: %s", pattern, err) + } + if seen[track] { + return fmt.Errorf("track %q is repeated", track) + } + seen[track] = true + } + return nil +} + +// validateChannelPattern validates a single pattern and returns its track. A +// pattern holds no branch, hence exactly one track and one risk part. +func validateChannelPattern(pattern string) (track string, err error) { + segments, err := splitChannel(pattern, channelPatternForm) + if err != nil { + return "", err + } + if len(segments) != 2 { + return "", fmt.Errorf("must be %s", channelPatternForm) + } + track, riskPart := segments[0], segments[1] + if strings.ContainsAny(track, "*!,") { + return "", errors.New("only the risk accepts '*', '!' and ','") + } + if riskPart != "*" && strings.Contains(riskPart, "*") { + // Checked before the risk part takes one of the forms below, as a + // wildcard is not allowed within any of them. + return "", errors.New("'*' must be the whole risk") + } + + // The risk part takes one of the three forms of the grammar. + switch { + case riskPart == "*": + // Every risk of the track, nothing more to validate. + case strings.HasPrefix(riskPart, "!"): + // Every risk of the track but the excluded one. + except := strings.TrimPrefix(riskPart, "!") + if strings.Contains(except, ",") { + return "", errors.New("'!' cannot be combined with other risks") + } + if except == "" { + return "", fmt.Errorf("must be %s", channelPatternForm) + } + if err := validateRisk(except); err != nil { + return "", err + } + default: + // Only the listed risks of the track. + risks := strings.Split(riskPart, ",") + for i, risk := range risks { + if risk == "" { + return "", fmt.Errorf("must be %s", channelPatternForm) + } + if strings.Contains(risk, "!") { + return "", errors.New("'!' must prefix the whole risk") + } + if slices.Contains(risks[:i], risk) { + return "", fmt.Errorf("risk %q is repeated", risk) + } + if err := validateRisk(risk); err != nil { + return "", err + } + } + } + return track, nil +} + +// MatchChannelPatterns reports whether the concrete "/" channel +// matches any of the patterns. An empty list matches every channel, which means +// the entry is not channel specific. +// +// A branch, as in "//", is ignored. Branches are ephemeral +// and thus never part of a pattern, so an entry applies to every branch of the +// risk it matches. +func MatchChannelPatterns(patterns []string, channel Channel) bool { + if len(patterns) == 0 { + return true + } + if channel.Track == "" || channel.Risk == "" { + // A channel without a risk is not a channel. Never match it, rather + // than treat the missing risk as one that differs from an excluded one. + return false + } + for _, pattern := range patterns { + if matchChannel(pattern, channel.Track, channel.Risk) { + return true + } + } + return false +} + +// matchChannel reports whether the pattern matches the track and the risk of a +// concrete channel. Note that the exclusion form is scoped to its own track, so +// "1.0/!stable" does not match any risk of the "2.0" track. +// +// The pattern is expected to be valid, as ensured when the release is read. +func matchChannel(pattern, track, risk string) bool { + patternTrack, riskPart, _ := strings.Cut(pattern, "/") + if track != patternTrack { + return false + } + if riskPart == "*" { + return true + } + if except, ok := strings.CutPrefix(riskPart, "!"); ok { + return risk != except + } + return slices.Contains(strings.Split(riskPart, ","), risk) +} diff --git a/internal/setup/channel_test.go b/internal/setup/channel_test.go new file mode 100644 index 00000000..3a3d52b9 --- /dev/null +++ b/internal/setup/channel_test.go @@ -0,0 +1,203 @@ +package setup_test + +import ( + . "gopkg.in/check.v1" + + "github.com/canonical/chisel/internal/setup" +) + +// 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 +// share the "must be /" message while entering through different +// phases, hence none is redundant. +var channelPatternTests = []struct { + summary string + values []string + err string + // match maps a concrete channel to whether the patterns match it. + match map[setup.Channel]bool +}{{ + summary: "No pattern matches every channel", + values: nil, + match: map[setup.Channel]bool{ + {"3.0", "stable", ""}: true, + {"2.0", "edge", ""}: true, + }, +}, { + summary: "Precise channel", + values: []string{"0.3/stable"}, + match: map[setup.Channel]bool{ + {"0.3", "stable", ""}: true, + {"0.3", "edge", ""}: false, + {"0.2", "stable", ""}: false, + // Branches are ephemeral, hence never part of a pattern. The entry + // applies to every branch of the risk it matches. + {"0.3", "stable", "mybranch"}: true, + {"0.3", "edge", "mybranch"}: false, + }, +}, { + summary: "All risks of a track", + values: []string{"0.3/*"}, + match: map[setup.Channel]bool{ + {"0.3", "stable", ""}: true, + {"0.3", "edge", ""}: true, + {"0.2", "stable", ""}: false, + }, +}, { + summary: "Excluded risk", + values: []string{"0.2/!stable"}, + match: map[setup.Channel]bool{ + {"0.2", "stable", ""}: false, + {"0.2", "edge", ""}: true, + {"0.2", "beta", ""}: true, + // The exclusion is scoped to its own track, it never means "any + // other track". + {"0.3", "edge", ""}: false, + // The branch is ignored, it must not be taken for part of the risk. + {"0.2", "stable", "mybranch"}: false, + {"0.2", "edge", "mybranch"}: true, + }, +}, { + summary: "Channel without a risk never matches", + values: []string{"0.3/*"}, + match: map[setup.Channel]bool{ + // A channel without a risk is not a channel, it must not match just + // because the missing risk differs from an excluded one. + {Track: "0.3"}: false, + {}: false, + }, +}, { + summary: "Several risks", + values: []string{"0.2/beta,edge"}, + match: map[setup.Channel]bool{ + {"0.2", "beta", ""}: true, + {"0.2", "edge", ""}: true, + {"0.2", "stable", ""}: false, + }, +}, { + summary: "Union of several tracks", + values: []string{"0.2/!stable", "0.3/*"}, + match: map[setup.Channel]bool{ + {"0.2", "edge", ""}: true, + {"0.2", "stable", ""}: false, + {"0.3", "stable", ""}: true, + }, +}, { + summary: "Every known risk", + values: []string{"0.3/stable,candidate,beta,edge"}, + match: map[setup.Channel]bool{ + {"0.3", "stable", ""}: true, + {"0.3", "candidate", ""}: true, + {"0.3", "beta", ""}: true, + {"0.3", "edge", ""}: true, + }, +}, { + // Splitting the pattern, in common with a concrete channel. + summary: "Spaces", + values: []string{"0.3/not stable"}, + err: `"0.3/not stable": must not contain spaces`, +}, { + summary: "Missing risk", + values: []string{"0.3"}, + err: `"0.3": must be /`, +}, { + summary: "Empty risk", + values: []string{"0.3/"}, + err: `"0.3/": must be /`, +}, { + summary: "Empty track", + values: []string{"/stable"}, + err: `"/stable": must be /`, +}, { + // A pattern never holds a branch, it applies to every branch of the risks + // it matches. + summary: "Pattern holding a branch", + values: []string{"0.3/stable/mybranch"}, + err: `"0.3/stable/mybranch": must be /`, +}, { + // The track is a literal, the operators belong to the risk. + summary: "Wildcard track", + values: []string{"*/stable"}, + err: `"\*/stable": only the risk accepts '\*', '!' and ','`, +}, { + summary: "Partial wildcard in track", + values: []string{"0.3-*/stable"}, + err: `"0.3-\*/stable": only the risk accepts '\*', '!' and ','`, +}, { + // A wildcard is the whole risk part or nothing, it is never a glob. + summary: "Wildcard is not a glob", + values: []string{"0.3/e*"}, + err: `"0.3/e\*": '\*' must be the whole risk`, +}, { + // 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`, +}, { + summary: "Exclusion combined with other risks", + values: []string{"0.3/!stable,edge"}, + err: `"0.3/!stable,edge": '!' cannot be combined with other risks`, +}, { + summary: "Empty excluded risk", + values: []string{"0.3/!"}, + err: `"0.3/!": must be /`, +}, { + // 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`, +}, { + 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`, +}, { + summary: "Risks are case sensitive", + values: []string{"0.3/Stable"}, + err: `"0.3/Stable": unknown risk "Stable", must be one of stable, candidate, beta, edge`, +}, { + summary: "Exclusion not prefixing the risk part", + values: []string{"0.3/edge,!stable"}, + err: `"0.3/edge,!stable": '!' must prefix the whole risk`, +}, { + summary: "Repeated risk", + values: []string{"0.3/edge,edge"}, + err: `"0.3/edge,edge": risk "edge" is repeated`, +}, { + // A trailing or leading comma leaves an empty risk in the list, which the + // split above cannot catch as the risk part is not an empty segment. + summary: "Trailing comma in a list", + values: []string{"0.3/edge,"}, + err: `"0.3/edge,": must be /`, +}, { + summary: "Leading comma in a list", + values: []string{"0.3/,edge"}, + err: `"0.3/,edge": must be /`, +}, { + // Across the patterns of one "channel" field. + summary: "Repeated track", + values: []string{"0.3/*", "0.3/edge"}, + err: `track "0.3" is repeated`, +}, { + summary: "Repeated track with identical values", + values: []string{"0.3/edge", "0.3/edge"}, + err: `track "0.3" is repeated`, +}} + +func (s *S) TestChannelPatterns(c *C) { + for _, test := range channelPatternTests { + c.Logf("Summary: %s", test.summary) + + err := setup.ValidateChannelPatterns(test.values) + if test.err != "" { + c.Assert(err, ErrorMatches, test.err) + continue + } + c.Assert(err, IsNil) + + for channel, expected := range test.match { + c.Assert(setup.MatchChannelPatterns(test.values, channel), Equals, expected, + Commentf("channel %q", channel.String())) + } + } +} From a3e63b365ae6795e695b2a8ead62d4430a29cbab Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 3 Sep 2026 16:24:32 +0200 Subject: [PATCH 2/7] feat: resolve store package channels from default-track --- internal/setup/export_test.go | 2 + internal/setup/setup.go | 57 +++- internal/setup/setup_test.go | 546 +++++++++++++++++++++++++++++++++- 3 files changed, 587 insertions(+), 18 deletions(-) diff --git a/internal/setup/export_test.go b/internal/setup/export_test.go index 35231e50..01403b4e 100644 --- a/internal/setup/export_test.go +++ b/internal/setup/export_test.go @@ -1,3 +1,5 @@ package setup type YAMLPath = yamlPath + +var ValidateChannelPatterns = validateChannelPatterns diff --git a/internal/setup/setup.go b/internal/setup/setup.go index 9d7e4a7b..3b1db4f7 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -80,7 +80,8 @@ type Slice struct { } type EssentialInfo struct { - Arch []string + Arch []string + Channel []string } type SliceScripts struct { @@ -98,7 +99,7 @@ const ( GeneratePath PathKind = "generate" // TODO Maybe in the future, for binary support. - //Base64Path PathKind = "base64" + // Base64Path PathKind = "base64" ) type PathUntil string @@ -123,6 +124,7 @@ type PathInfo struct { Mutable bool Until PathUntil Arch []string + Channel []string Generate GenerateKind Prefer string } @@ -145,6 +147,10 @@ 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. +const DefaultRisk = "stable" + func (s *Slice) String() string { return s.Package + "_" + s.Name } // Selection holds the required configuration to create a Build for a selection @@ -154,6 +160,8 @@ func (s *Slice) String() string { return s.Package + "_" + s.Name } type Selection struct { Release *Release Slices []*Slice + // Channels holds the resolved channel per store package name. + Channels map[string]Channel } // Prefers uses the prefer relationships and returns a map from each path to @@ -321,8 +329,9 @@ func (r *Release) validate() error { // same as an essential with all archs, i.e. Chisel does not use arch to // partition the dependency set. If we were to use arch, we would allow // combinations of dependencies which are overly complex and brittle, that - // is why it is better to be more strict here. - _, err = order(r.Packages, keys, "") + // is why it is better to be more strict here. The same reasoning applies to + // channels, hence the nil map below. + _, err = order(r.Packages, keys, "", nil) if err != nil { return err } @@ -356,9 +365,9 @@ func (r *Release) validate() error { // return an error if there are cycles. // // If arch is supplied, essential(s) not specific to that arch are not -// considered. -func order(pkgs map[string]*Package, keys []SliceKey, arch string) ([]SliceKey, error) { - +// considered. Likewise, if channels holds the channel of the package holding +// the essential, essential(s) not specific to that channel are not considered. +func order(pkgs map[string]*Package, keys []SliceKey, arch string, channels map[string]Channel) ([]SliceKey, error) { // Preprocess the list to improve error messages. for _, key := range keys { if pkg, ok := pkgs[key.Package]; !ok { @@ -387,6 +396,11 @@ func order(pkgs map[string]*Package, keys []SliceKey, arch string) ([]SliceKey, if len(info.Arch) > 0 && arch != "" && !slices.Contains(info.Arch, arch) { continue } + // The channel of the package holding the essential decides, the + // channel of the required package is irrelevant here. + if channel, ok := channels[pkg.Name]; ok && !MatchChannelPatterns(info.Channel, channel) { + continue + } fqreq := req.String() if reqpkg, ok := pkgs[req.Package]; !ok || reqpkg.Slices[req.Slice] == nil { return nil, fmt.Errorf("%s requires %s, but slice is missing", fqslice, fqreq) @@ -506,11 +520,16 @@ func Select(release *Release, slices []SliceKey, arch string) (*Selection, error return nil, err } + // 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 := resolveChannels(release) + selection := &Selection{ Release: release, } - sorted, err := order(release.Packages, slices, arch) + sorted, err := order(release.Packages, slices, arch, channels) if err != nil { return nil, err } @@ -519,6 +538,14 @@ func Select(release *Release, slices []SliceKey, arch string) (*Selection, error selection.Slices[i] = release.Packages[key.Package].Slices[key.Slice] } + // Only report the channels of the selected packages. + selection.Channels = make(map[string]Channel) + for _, slice := range selection.Slices { + if channel, ok := channels[slice.Package]; ok { + selection.Channels[slice.Package] = channel + } + } + for _, new := range selection.Slices { for newPath, newInfo := range new.Contents { // An invalid "generate" value should only throw an error if that @@ -550,6 +577,20 @@ func Select(release *Release, slices []SliceKey, arch string) (*Selection, error return selection, nil } +// resolveChannels 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 resolveChannels(release *Release) map[string]Channel { + channels := make(map[string]Channel) + for _, pkg := range release.Packages { + if pkg.Store == "" { + continue + } + channels[pkg.Name] = Channel{Track: pkg.DefaultTrack, Risk: DefaultRisk} + } + return channels +} + const ( preferSource = 1 preferTarget = 2 diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 3568b39a..d0912b84 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -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"}, }, }, @@ -432,6 +432,7 @@ var setupTests = []setupTest{{ Package: "mypkg1", Name: "myslice1", }}, + Channels: map[string]setup.Channel{}, }, }, { summary: "Selection with dependencies", @@ -461,6 +462,7 @@ var setupTests = []setupTest{{ {"mypkg1", "myslice1"}: {}, }, }}, + Channels: map[string]setup.Channel{}, }, }, { summary: "Selection with matching paths don't conflict", @@ -488,7 +490,11 @@ var setupTests = []setupTest{{ /path3: {symlink: /link} `, }, - selslices: []setup.SliceKey{{"mypkg1", "myslice1"}, {"mypkg1", "myslice2"}, {"mypkg2", "myslice1"}}, + selslices: []setup.SliceKey{ + {"mypkg1", "myslice1"}, + {"mypkg1", "myslice2"}, + {"mypkg2", "myslice1"}, + }, }, { summary: "Conflicting paths across slices", input: map[string]string{ @@ -1765,6 +1771,7 @@ var setupTests = []setupTest{{ "/dir/**": {Kind: "generate", Generate: "manifest"}, }, }}, + Channels: map[string]setup.Channel{}, }, }, { summary: "Can specify generate with bogus value but cannot select those slices", @@ -4409,6 +4416,485 @@ 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"}}, + 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}, + }, + }}, + 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"}}, + 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 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{ + "chisel.yaml": testutil.DefaultChiselYamlWithStores, + "bin-slices/mypkg.yaml": ` + package: mypkg + store: bin + default-track: "0.3" + essential: + bin-mypkg_shared: {channel: ["0.3/*"]} + slices: + myslice: + essential: + bin-mypkg_extra: {channel: ["0.2/!stable"]} + shared: + extra: + `, + }, + 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", + Essential: map[setup.SliceKey]setup.EssentialInfo{ + {Package: "bin-mypkg", Slice: "shared"}: { + Channel: []string{"0.3/*"}, + }, + {Package: "bin-mypkg", Slice: "extra"}: { + Channel: []string{"0.2/!stable"}, + }, + }, + }, + "shared": { + Package: "bin-mypkg", + Name: "shared", + }, + "extra": { + Package: "bin-mypkg", + Name: "extra", + Essential: map[setup.SliceKey]setup.EssentialInfo{ + {Package: "bin-mypkg", Slice: "shared"}: { + Channel: []string{"0.3/*"}, + }, + }, + }, + }, + }, + }, + }, +}, { + summary: "Channel is unsupported before format v3", + input: map[string]string{ + "chisel.yaml": strings.ReplaceAll(testutil.DefaultChiselYaml, "format: v1", "format: v2"), + "slices/mypkg.yaml": ` + package: mypkg + slices: + myslice: + contents: + /dir/file: {channel: ["0.3/stable"]} + `, + }, + relerror: `cannot parse package "mypkg": 'channel' is unsupported before format v3`, +}, { + summary: "Channel is unsupported before format v3 on a v3-essential", + input: map[string]string{ + "chisel.yaml": strings.ReplaceAll(testutil.DefaultChiselYaml, "format: v1", "format: v2"), + "slices/mypkg.yaml": ` + package: mypkg + slices: + myslice: + v3-essential: + mypkg_other: {channel: ["0.3/stable"]} + other: + `, + }, + relerror: `cannot parse package "mypkg": 'channel' is unsupported before format v3`, +}, { + summary: "Channel on a path of a non-store package", + input: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlWithStores, + "slices/mypkg.yaml": ` + package: mypkg + slices: + myslice: + contents: + /dir/file: {channel: ["0.3/stable"]} + `, + }, + relerror: `slice mypkg_myslice has 'channel' for path /dir/file but package is not in a store`, +}, { + summary: "Channel on an essential of a non-store package", + input: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlWithStores, + "slices/mypkg.yaml": ` + package: mypkg + slices: + myslice: + essential: + mypkg_other: {channel: ["0.3/stable"]} + other: + `, + }, + relerror: `slice mypkg_myslice has 'channel' for essential mypkg_other but package is not in a store`, +}, { + summary: "Invalid channel on a path", + input: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlWithStores, + "bin-slices/mypkg.yaml": ` + package: mypkg + store: bin + default-track: "0.3" + slices: + myslice: + contents: + /dir/file: {channel: ["0.3"]} + `, + }, + relerror: `slice bin-mypkg_myslice has invalid 'channel' for path /dir/file: "0.3": must be /`, +}, { + summary: "Invalid channel on an essential", + input: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlWithStores, + "bin-slices/mypkg.yaml": ` + package: mypkg + store: bin + default-track: "0.3" + slices: + myslice: + essential: + bin-mypkg_other: {channel: ["*/stable"]} + other: + `, + }, + relerror: `slice bin-mypkg_myslice has invalid 'channel' for essential bin-mypkg_other: "\*/stable": only the risk accepts '\*', '!' and ','`, +}, { + summary: "Channel is accepted on generate paths, as 'arch' is", + input: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlWithStores, + "bin-slices/mypkg.yaml": ` + package: mypkg + store: bin + default-track: "0.3" + slices: + myslice: + contents: + /dir/**: {generate: manifest, 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/**": { + Kind: setup.GeneratePath, + Generate: setup.GenerateManifest, + Channel: []string{"0.3/*"}, + }, + }, + }, + }, + }, + }, + }, +}, { + summary: "Channel-disjoint paths of different packages still conflict", + input: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlWithStores, + "bin-slices/mypkg1.yaml": ` + package: mypkg1 + store: bin + default-track: "0.3" + slices: + myslice: + contents: + /dir/file: {channel: ["0.3/stable"]} + `, + "bin-slices/mypkg2.yaml": ` + package: mypkg2 + store: bin + default-track: "0.3" + slices: + myslice: + contents: + /dir/file: {channel: ["0.2/edge"]} + `, + }, + relerror: `slices bin-mypkg1_myslice and bin-mypkg2_myslice conflict on /dir/file`, +}, { + // 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"}}, + input: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlWithStores, + "bin-slices/mypkg.yaml": ` + package: mypkg + store: bin + default-track: "0.3" + slices: + myslice: + essential: + bin-mypkg_other: {channel: ["0.3/*"]} + other: + contents: + /dir/other: {} + `, + }, + selection: &setup.Selection{ + Slices: []*setup.Slice{{ + Package: "bin-mypkg", + Name: "other", + Contents: map[string]setup.PathInfo{ + "/dir/other": {Kind: setup.CopyPath}, + }, + }, { + Package: "bin-mypkg", + Name: "myslice", + Essential: map[setup.SliceKey]setup.EssentialInfo{ + {Package: "bin-mypkg", Slice: "other"}: { + Channel: []string{"0.3/*"}, + }, + }, + }}, + Channels: map[string]setup.Channel{ + "bin-mypkg": {Track: "0.3", Risk: "stable"}, + }, + }, +}, { + summary: "Essential gated by a non-matching channel is skipped", + selslices: []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: "0.3" + slices: + myslice: + essential: + bin-mypkg_other: {channel: ["0.2/*"]} + other: + contents: + /dir/other: {} + `, + }, + selection: &setup.Selection{ + Slices: []*setup.Slice{{ + Package: "bin-mypkg", + Name: "myslice", + Essential: map[setup.SliceKey]setup.EssentialInfo{ + {Package: "bin-mypkg", Slice: "other"}: { + Channel: []string{"0.2/*"}, + }, + }, + }}, + Channels: map[string]setup.Channel{ + "bin-mypkg": {Track: "0.3", Risk: "stable"}, + }, + }, +}, { + summary: "Channel-gated essential loops are still detected", + input: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlWithStores, + "bin-slices/mypkg.yaml": ` + package: mypkg + store: bin + default-track: "0.3" + slices: + myslice: + essential: + bin-mypkg_other: {channel: ["0.2/*"]} + other: + essential: + bin-mypkg_myslice: {channel: ["0.3/*"]} + `, + }, + relerror: `essential loop detected: bin-mypkg_myslice, bin-mypkg_other`, }} func (s *S) TestParseRelease(c *C) { @@ -4513,9 +4999,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 @@ -4580,16 +5066,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 @@ -4606,7 +5092,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 @@ -4803,6 +5289,46 @@ func (s *S) TestPackageYAMLFormat(c *C) { /usr/bin/mypkg: {} `, }, + }, { + summary: "All channel forms", + input: map[string]string{ + "chisel.yaml": strings.ReplaceAll(testutil.DefaultChiselYamlWithStores, "format: v3", "format: v4"), + "slices/mypkg.yaml": ` + package: mypkg + store: bin + default-track: "0.3" + slices: + myslice: + essential: + bin-mypkg_other: {channel: 0.3/*} + contents: + /dir/excluded: {channel: 0.2/!stable} + /dir/listed: {channel: '0.2/beta,edge'} + /dir/precise: {channel: 0.3/stable} + /dir/union: {channel: [0.2/*, 0.3/edge]} + other: {} + `, + }, + expected: map[string]string{ + "chisel.yaml": strings.ReplaceAll(testutil.DefaultChiselYamlWithStores, "format: v3", "format: v4"), + // A single value collapses back to a scalar, as with 'arch'. Values + // holding a comma must be quoted to survive the flow style. + "slices/mypkg.yaml": ` + package: mypkg + store: bin + default-track: "0.3" + slices: + myslice: + essential: + bin-mypkg_other: {channel: 0.3/*} + contents: + /dir/excluded: {channel: 0.2/!stable} + /dir/listed: {channel: '0.2/beta,edge'} + /dir/precise: {channel: 0.3/stable} + /dir/union: {channel: [0.2/*, 0.3/edge]} + other: {} + `, + }, }} for _, test := range tests { @@ -4815,9 +5341,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 From 36d10f621b7ebdf3386fbb9757f27a6c2e518ece Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 3 Sep 2026 16:25:16 +0200 Subject: [PATCH 3/7] feat: channel field in slice definition files --- internal/setup/yaml.go | 105 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 101 insertions(+), 4 deletions(-) diff --git a/internal/setup/yaml.go b/internal/setup/yaml.go index f4b1b123..ffb98e12 100644 --- a/internal/setup/yaml.go +++ b/internal/setup/yaml.go @@ -74,6 +74,34 @@ type yamlPackage struct { V3Essential map[string]*yamlEssential `yaml:"v3-essential,omitempty"` } +// hasChannel reports whether any path or essential of the package uses the +// 'channel' field. Every flavour of essential is considered, including +// 'v3-essential', as they all end up parsed the same way. +func (yp *yamlPackage) hasChannel() bool { + essentialsHaveChannel := func(essentials map[string]*yamlEssential) bool { + for _, essential := range essentials { + if essential != nil && len(essential.Channel.List) > 0 { + return true + } + } + return false + } + if essentialsHaveChannel(yp.Essential.Values) || essentialsHaveChannel(yp.V3Essential) { + return true + } + for _, slice := range yp.Slices { + if essentialsHaveChannel(slice.Essential.Values) || essentialsHaveChannel(slice.V3Essential) { + return true + } + for _, path := range slice.Contents { + if path != nil && len(path.Channel.List) > 0 { + return true + } + } + } + return false +} + type essentialStyle int const ( @@ -139,6 +167,7 @@ type yamlPath struct { Mutable bool `yaml:"mutable,omitempty"` Until PathUntil `yaml:"until,omitempty"` Arch yamlArch `yaml:"arch,omitempty"` + Channel yamlChannel `yaml:"channel,omitempty"` Generate GenerateKind `yaml:"generate,omitempty"` Prefer string `yaml:"prefer,omitempty"` } @@ -197,6 +226,33 @@ func (ya yamlArch) MarshalYAML() (any, error) { var _ yaml.Marshaler = yamlArch{} +type yamlChannel struct { + List []string +} + +func (yc *yamlChannel) UnmarshalYAML(value *yaml.Node) error { + var s string + var l []string + if value.Decode(&s) == nil { + yc.List = []string{s} + } else if value.Decode(&l) == nil { + yc.List = l + } else { + return fmt.Errorf("cannot decode channel") + } + // Validate channel correctness later for a better error message. + return nil +} + +func (yc yamlChannel) MarshalYAML() (any, error) { + if len(yc.List) == 1 { + return yc.List[0], nil + } + return yc.List, nil +} + +var _ yaml.Marshaler = yamlChannel{} + type yamlMode uint func (ym yamlMode) MarshalYAML() (any, error) { @@ -232,7 +288,8 @@ type yamlPubKey struct { } type yamlEssential struct { - Arch yamlArch `yaml:"arch,omitempty"` + Arch yamlArch `yaml:"arch,omitempty"` + Channel yamlChannel `yaml:"channel,omitempty"` } func (ye *yamlEssential) MarshalYAML() (any, error) { @@ -519,6 +576,9 @@ func parsePackage(release *Release, pkgName, pkgPath string, data []byte) (*Pack } if release.Format == "v1" || release.Format == "v2" { + if yamlPkg.hasChannel() { + return nil, fmt.Errorf("cannot parse package %q: 'channel' is unsupported before format v3", pkg.Name) + } if yamlPkg.Essential.style != unsetEssential && yamlPkg.Essential.style != listEssential { return nil, fmt.Errorf("cannot parse package %q: essential expects a list", pkg.Name) } @@ -588,6 +648,7 @@ func parsePackage(release *Release, pkgName, pkgPath string, data []byte) (*Pack var mutable bool var until PathUntil var arch []string + var channel []string var generate GenerateKind var prefer string if yamlPath != nil && yamlPath.Generate != "" { @@ -649,6 +710,15 @@ func parsePackage(release *Release, pkgName, pkgPath string, data []byte) (*Pack return nil, fmt.Errorf("slice %s_%s has invalid 'arch' for path %s: %q", pkg.Name, sliceName, contPath, s) } } + if len(yamlPath.Channel.List) > 0 { + if pkg.Store == "" { + return nil, fmt.Errorf("slice %s_%s has 'channel' for path %s but package is not in a store", pkg.Name, sliceName, contPath) + } + if err := validateChannelPatterns(yamlPath.Channel.List); err != nil { + return nil, fmt.Errorf("slice %s_%s has invalid 'channel' for path %s: %s", pkg.Name, sliceName, contPath, err) + } + channel = yamlPath.Channel.List + } } if prefer == pkg.Name { return nil, fmt.Errorf("slice %s_%s cannot 'prefer' its own package for path %s", pkg.Name, sliceName, contPath) @@ -673,6 +743,7 @@ func parsePackage(release *Release, pkgName, pkgPath string, data []byte) (*Pack Mutable: mutable, Until: until, Arch: arch, + Channel: channel, Generate: generate, Prefer: prefer, } @@ -708,6 +779,7 @@ func pathInfoToYAML(pi *PathInfo) (*yamlPath, error) { Mutable: pi.Mutable, Until: pi.Until, Arch: yamlArch{List: pi.Arch}, + Channel: yamlChannel{List: pi.Channel}, Generate: pi.Generate, Prefer: pi.Prefer, } @@ -739,7 +811,10 @@ func sliceToYAML(s *Slice) (*yamlSlice, error) { }, } for key, info := range s.Essential { - slice.Essential.Values[key.String()] = &yamlEssential{Arch: yamlArch{info.Arch}} + slice.Essential.Values[key.String()] = &yamlEssential{ + Arch: yamlArch{info.Arch}, + Channel: yamlChannel{List: info.Channel}, + } } for path, info := range s.Contents { yamlPath, err := pathInfoToYAML(&info) @@ -862,6 +937,20 @@ var defaultMaintenance = map[string]Maintenance{ // processes them to check they are valid and not duplicated and, if // successful, adds them to slice. func parseEssentials(yamlPkg *yamlPackage, yamlSlice *yamlSlice, pkgPath string, slice *Slice) error { + // validateChannels validates the 'channel' field of an essential entry. The + // patterns apply to the channel of the package holding the essential. + validateChannels := func(refName string, essentialInfo *yamlEssential) ([]string, error) { + if essentialInfo == nil || len(essentialInfo.Channel.List) == 0 { + return nil, nil + } + if yamlPkg.Store == "" { + return nil, fmt.Errorf("slice %s has 'channel' for essential %s but package is not in a store", slice, refName) + } + if err := validateChannelPatterns(essentialInfo.Channel.List); err != nil { + return nil, fmt.Errorf("slice %s has invalid 'channel' for essential %s: %s", slice, refName, err) + } + return essentialInfo.Channel.List, nil + } addPackageEssential := func(refName string, essentialInfo *yamlEssential) error { sliceKey, err := ParseSliceKey(refName) if err != nil { @@ -881,7 +970,11 @@ func parseEssentials(yamlPkg *yamlPackage, yamlSlice *yamlSlice, pkgPath string, if essentialInfo != nil { archList = essentialInfo.Arch.List } - slice.Essential[sliceKey] = EssentialInfo{Arch: archList} + channel, err := validateChannels(refName, essentialInfo) + if err != nil { + return err + } + slice.Essential[sliceKey] = EssentialInfo{Arch: archList, Channel: channel} return nil } addSliceEssential := func(refName string, essentialInfo *yamlEssential) error { @@ -902,7 +995,11 @@ func parseEssentials(yamlPkg *yamlPackage, yamlSlice *yamlSlice, pkgPath string, if essentialInfo != nil { archList = essentialInfo.Arch.List } - slice.Essential[sliceKey] = EssentialInfo{Arch: archList} + channel, err := validateChannels(refName, essentialInfo) + if err != nil { + return err + } + slice.Essential[sliceKey] = EssentialInfo{Arch: archList, Channel: channel} return nil } From e795da7117f30ef6ecc2dacbb52e276a07644387 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 3 Sep 2026 16:25:52 +0200 Subject: [PATCH 4/7] feat: filter slice contents by channel --- internal/slicer/slicer.go | 8 ++ internal/slicer/slicer_test.go | 180 +++++++++++++++++---------------- 2 files changed, 102 insertions(+), 86 deletions(-) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index ce791162..e3ccb0ab 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -109,6 +109,7 @@ func Run(options *RunOptions) error { extract[slice.Package] = extractPackage } arch := pkgArchive[slice.Package].Options().Arch + channel := options.Selection.Channels[slice.Package] for targetPath, pathInfo := range slice.Contents { if targetPath == "" { continue @@ -116,6 +117,9 @@ func Run(options *RunOptions) error { if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, arch) { continue } + if !setup.MatchChannelPatterns(pathInfo.Channel, channel) { + continue + } if preferredPkg, ok := prefers[targetPath]; ok && preferredPkg.Name != slice.Package { continue } @@ -271,10 +275,14 @@ func Run(options *RunOptions) error { relPaths := map[string][]*setup.Slice{} for _, slice := range options.Selection.Slices { arch := pkgArchive[slice.Package].Options().Arch + channel := options.Selection.Channels[slice.Package] for relPath, pathInfo := range slice.Contents { if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, arch) { continue } + if !setup.MatchChannelPatterns(pathInfo.Channel, channel) { + continue + } if pathInfo.Kind == setup.CopyPath || pathInfo.Kind == setup.GlobPath || pathInfo.Kind == setup.GeneratePath { continue diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index d6ef9ca0..a21ac6ca 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -22,9 +22,7 @@ import ( "github.com/canonical/chisel/public/manifest" ) -var ( - testKey = testutil.PGPKeys["key1"] -) +var testKey = testutil.PGPKeys["key1"] type slicerTest struct { summary string @@ -46,7 +44,7 @@ var packageEntries = map[string][]testutil.TarEntry{ {Header: tar.Header{Name: "./usr/"}}, {Header: tar.Header{Name: "./usr/lib/"}}, {Header: tar.Header{Name: "./usr/lib/x86_64-linux-gnu/"}}, - {Header: tar.Header{Name: "./usr/lib/x86_64-linux-gnu/libssl.so.3", Mode: 00755}}, + {Header: tar.Header{Name: "./usr/lib/x86_64-linux-gnu/libssl.so.3", Mode: 0o0755}}, {Header: tar.Header{Name: "./usr/share/"}}, {Header: tar.Header{Name: "./usr/share/doc/"}}, {Header: tar.Header{Name: "./usr/share/doc/copyright-symlink-libssl3/"}}, @@ -59,7 +57,7 @@ var packageEntries = map[string][]testutil.TarEntry{ {Header: tar.Header{Name: "./etc/ssl/openssl.cnf"}}, {Header: tar.Header{Name: "./usr/"}}, {Header: tar.Header{Name: "./usr/bin/"}}, - {Header: tar.Header{Name: "./usr/bin/openssl", Mode: 00755}}, + {Header: tar.Header{Name: "./usr/bin/openssl", Mode: 0o0755}}, {Header: tar.Header{Name: "./usr/share/"}}, {Header: tar.Header{Name: "./usr/share/doc/"}}, {Header: tar.Header{Name: "./usr/share/doc/copyright-symlink-openssl/"}}, @@ -69,11 +67,11 @@ var packageEntries = map[string][]testutil.TarEntry{ var testPackageCopyrightEntries = []testutil.TarEntry{ // Hardcoded copyright paths. - testutil.Dir(0755, "./usr/"), - testutil.Dir(0755, "./usr/share/"), - testutil.Dir(0755, "./usr/share/doc/"), - testutil.Dir(0755, "./usr/share/doc/test-package/"), - testutil.Reg(0644, "./usr/share/doc/test-package/copyright", "copyright"), + testutil.Dir(0o755, "./usr/"), + testutil.Dir(0o755, "./usr/share/"), + testutil.Dir(0o755, "./usr/share/doc/"), + testutil.Dir(0o755, "./usr/share/doc/test-package/"), + testutil.Reg(0o644, "./usr/share/doc/test-package/copyright", "copyright"), } var slicerTests = []slicerTest{{ @@ -275,7 +273,8 @@ var slicerTests = []slicerTest{{ summary: "Install two packages", slices: []setup.SliceKey{ {"test-package", "myslice"}, - {"other-package", "myslice"}}, + {"other-package", "myslice"}, + }, pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.PackageData["test-package"], @@ -319,23 +318,24 @@ var slicerTests = []slicerTest{{ slices: []setup.SliceKey{ {"a-implicit-parent", "myslice"}, {"b-explicit-dir", "myslice"}, - {"c-implicit-parent", "myslice"}}, + {"c-implicit-parent", "myslice"}, + }, pkgs: []*testutil.TestPackage{{ Name: "a-implicit-parent", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0755, "./dir/"), - testutil.Reg(0644, "./dir/file-1", "random"), + testutil.Dir(0o755, "./dir/"), + testutil.Reg(0o644, "./dir/file-1", "random"), }), }, { Name: "b-explicit-dir", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(01777, "./dir/"), + testutil.Dir(0o1777, "./dir/"), }), }, { Name: "c-implicit-parent", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0766, "./dir/"), - testutil.Reg(0644, "./dir/file-2", "random"), + testutil.Dir(0o766, "./dir/"), + testutil.Reg(0o644, "./dir/file-2", "random"), }), }}, release: map[string]string{ @@ -377,7 +377,8 @@ var slicerTests = []slicerTest{{ summary: "Valid same file in two slices in different packages", slices: []setup.SliceKey{ {"test-package", "myslice"}, - {"other-package", "myslice"}}, + {"other-package", "myslice"}, + }, pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.PackageData["test-package"], @@ -787,7 +788,7 @@ var slicerTests = []slicerTest{{ Version: "v1", Arch: "a1", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0644, "./file", "from foo"), + testutil.Reg(0o644, "./file", "from foo"), }), Archives: []string{"foo"}, }, { @@ -796,7 +797,7 @@ var slicerTests = []slicerTest{{ Version: "v2", Arch: "a2", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0644, "./file", "from bar"), + testutil.Reg(0o644, "./file", "from bar"), }), Archives: []string{"bar"}, }, { @@ -805,7 +806,7 @@ var slicerTests = []slicerTest{{ Version: "v3", Arch: "a3", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0644, "./other-file", "from bar"), + testutil.Reg(0o644, "./other-file", "from bar"), }), Archives: []string{"bar"}, }}, @@ -871,7 +872,7 @@ var slicerTests = []slicerTest{{ Version: "v1", Arch: "a1", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0644, "./file", "from foo"), + testutil.Reg(0o644, "./file", "from foo"), }), Archives: []string{"foo"}, }, { @@ -880,7 +881,7 @@ var slicerTests = []slicerTest{{ Version: "v2", Arch: "a2", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0644, "./file", "from bar"), + testutil.Reg(0o644, "./file", "from bar"), }), Archives: []string{"bar"}, }}, @@ -936,7 +937,7 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0644, "./file", "from foo"), + testutil.Reg(0o644, "./file", "from foo"), }), Archives: []string{"foo"}, }}, @@ -1019,7 +1020,7 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0644, "./file", "from foo"), + testutil.Reg(0o644, "./file", "from foo"), }), Archives: []string{"foo"}, }}, @@ -1059,7 +1060,7 @@ var slicerTests = []slicerTest{{ Version: "v1", Arch: "a1", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0644, "./file", "from foo"), + testutil.Reg(0o644, "./file", "from foo"), }), Archives: []string{"foo"}, }}, @@ -1436,12 +1437,12 @@ var slicerTests = []slicerTest{{ // relative path. Since TrimLeft takes in a cutset instead of a // prefix, the desired relative path was not produced. // See https://github.com/canonical/chisel/pull/145. - testutil.Dir(0755, "./foo-bar/"), + testutil.Dir(0o755, "./foo-bar/"), }), }}, hackopt: func(c *C, opts *slicer.RunOptions) { opts.TargetDir = filepath.Join(filepath.Clean(opts.TargetDir), "foo") - err := os.Mkdir(opts.TargetDir, 0755) + err := os.Mkdir(opts.TargetDir, 0o755) c.Assert(err, IsNil) }, release: map[string]string{ @@ -1507,13 +1508,14 @@ var slicerTests = []slicerTest{{ summary: "Valid hard link in two slices in the same package", slices: []setup.SliceKey{ {"test-package", "slice1"}, - {"test-package", "slice2"}}, + {"test-package", "slice2"}, + }, pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0755, "./"), - testutil.Reg(0644, "./file", "foo"), - testutil.Hrd(0644, "./hardlink", "./file"), + testutil.Dir(0o755, "./"), + testutil.Reg(0o644, "./file", "foo"), + testutil.Hrd(0o644, "./hardlink", "./file"), }), }}, release: map[string]string{ @@ -1540,14 +1542,15 @@ var slicerTests = []slicerTest{{ }, { summary: "Hard link entries can be extracted without extracting the regular file", slices: []setup.SliceKey{ - {"test-package", "myslice"}}, + {"test-package", "myslice"}, + }, pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0755, "./"), - testutil.Reg(0644, "./file", "foo"), - testutil.Hrd(0644, "./hardlink1", "./file"), - testutil.Hrd(0644, "./hardlink2", "./file"), + testutil.Dir(0o755, "./"), + testutil.Reg(0o644, "./file", "foo"), + testutil.Hrd(0o644, "./hardlink1", "./file"), + testutil.Hrd(0o644, "./hardlink2", "./file"), }), }}, release: map[string]string{ @@ -1570,15 +1573,16 @@ var slicerTests = []slicerTest{{ }, { summary: "Hard link identifier for different groups", slices: []setup.SliceKey{ - {"test-package", "myslice"}}, + {"test-package", "myslice"}, + }, pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0755, "./"), - testutil.Reg(0644, "./file1", "text for file1"), - testutil.Reg(0644, "./file2", "text for file2"), - testutil.Hrd(0644, "./hardlink1", "./file1"), - testutil.Hrd(0644, "./hardlink2", "./file2"), + testutil.Dir(0o755, "./"), + testutil.Reg(0o644, "./file1", "text for file1"), + testutil.Reg(0o644, "./file2", "text for file2"), + testutil.Hrd(0o644, "./hardlink1", "./file1"), + testutil.Hrd(0o644, "./hardlink2", "./file2"), }), }}, release: map[string]string{ @@ -1605,13 +1609,14 @@ var slicerTests = []slicerTest{{ }, { summary: "Single hard link entry can be extracted without regular file and no hard links are created", slices: []setup.SliceKey{ - {"test-package", "myslice"}}, + {"test-package", "myslice"}, + }, pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0755, "./"), - testutil.Reg(0644, "./file", "foo"), - testutil.Hrd(0644, "./hardlink", "./file"), + testutil.Dir(0o755, "./"), + testutil.Reg(0o644, "./file", "foo"), + testutil.Hrd(0o644, "./hardlink", "./file"), }), }}, release: map[string]string{ @@ -1632,15 +1637,16 @@ var slicerTests = []slicerTest{{ }, { summary: "Hard link to symlink does not follow symlink", slices: []setup.SliceKey{ - {"test-package", "myslice"}}, + {"test-package", "myslice"}, + }, pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0755, "./"), - testutil.Dir(0755, "./"), - testutil.Reg(0644, "./file", "foo"), - testutil.Lnk(0644, "./symlink", "./file"), - testutil.Hrd(0644, "./hardlink", "./symlink"), + testutil.Dir(0o755, "./"), + testutil.Dir(0o755, "./"), + testutil.Reg(0o644, "./file", "foo"), + testutil.Lnk(0o644, "./symlink", "./file"), + testutil.Hrd(0o644, "./hardlink", "./symlink"), }), }}, release: map[string]string{ @@ -1670,16 +1676,16 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "test-package1", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0755, "./"), - testutil.Reg(0644, "./file1", "foo"), - testutil.Hrd(0644, "./hardlink1", "./file1"), + testutil.Dir(0o755, "./"), + testutil.Reg(0o644, "./file1", "foo"), + testutil.Hrd(0o644, "./hardlink1", "./file1"), }), }, { Name: "test-package2", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0755, "./"), - testutil.Reg(0644, "./file2", "foo"), - testutil.Hrd(0644, "./hardlink2", "./file2"), + testutil.Dir(0o755, "./"), + testutil.Reg(0o644, "./file2", "foo"), + testutil.Hrd(0o644, "./hardlink2", "./file2"), }), }}, release: map[string]string{ @@ -1715,13 +1721,14 @@ var slicerTests = []slicerTest{{ }, { summary: "Mutations for hard links are forbidden", slices: []setup.SliceKey{ - {"test-package", "myslice"}}, + {"test-package", "myslice"}, + }, pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0755, "./"), - testutil.Reg(0644, "./file", "foo"), - testutil.Hrd(0644, "./hardlink", "./file"), + testutil.Dir(0o755, "./"), + testutil.Reg(0o644, "./file", "foo"), + testutil.Hrd(0o644, "./hardlink", "./file"), }), }}, release: map[string]string{ @@ -1740,13 +1747,14 @@ var slicerTests = []slicerTest{{ }, { summary: "Hard links can be marked as mutable, but not mutated", slices: []setup.SliceKey{ - {"test-package", "myslice"}}, + {"test-package", "myslice"}, + }, pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0755, "./"), - testutil.Reg(0644, "./file", "foo"), - testutil.Hrd(0644, "./hardlink", "./file"), + testutil.Dir(0o755, "./"), + testutil.Reg(0o644, "./file", "foo"), + testutil.Hrd(0o644, "./hardlink", "./file"), }), }}, release: map[string]string{ @@ -1773,8 +1781,8 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0755, "./"), - testutil.Hrd(0644, "./hardlink", "/etc/group"), + testutil.Dir(0o755, "./"), + testutil.Hrd(0o644, "./hardlink", "/etc/group"), }), }}, release: map[string]string{ @@ -1793,8 +1801,8 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0755, "./"), - testutil.Reg(0644, "./../file", "hijacking system file"), + testutil.Dir(0o755, "./"), + testutil.Reg(0o644, "./../file", "hijacking system file"), }), }}, release: map[string]string{ @@ -1816,19 +1824,19 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "test-package1", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0755, "./"), - testutil.Reg(0644, "./file", "foo"), + testutil.Dir(0o755, "./"), + testutil.Reg(0o644, "./file", "foo"), }), }, { Name: "test-package2", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0755, "./"), - testutil.Reg(0644, "./file", "bar"), + testutil.Dir(0o755, "./"), + testutil.Reg(0o644, "./file", "bar"), }), }, { Name: "test-package3", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0755, "./"), + testutil.Dir(0o755, "./"), }), }}, release: map[string]string{ @@ -1877,18 +1885,18 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "test-package1", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0755, "./"), + testutil.Dir(0o755, "./"), // Note that both implicit parents have different permissions. - testutil.Dir(0766, "./parent/"), - testutil.Reg(0644, "./parent/foo", "whatever"), + testutil.Dir(0o766, "./parent/"), + testutil.Reg(0o644, "./parent/foo", "whatever"), }), }, { Name: "test-package2", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0755, "./"), + testutil.Dir(0o755, "./"), // And here. - testutil.Dir(0755, "./parent/"), - testutil.Reg(0644, "./parent/bar", "whatever"), + testutil.Dir(0o755, "./parent/"), + testutil.Reg(0o644, "./parent/bar", "whatever"), }), }}, release: map[string]string{ @@ -2065,9 +2073,9 @@ func runSlicerTests(s *S, c *C, tests []slicerTest) { releaseDir := c.MkDir() for path, data := range test.release { fpath := filepath.Join(releaseDir, 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) } @@ -2245,9 +2253,9 @@ func readManifest(c *C, targetDir, manifestPath string) *manifest.Manifest { // in the manifest itself. s, err := os.Stat(path.Join(targetDir, manifestPath)) c.Assert(err, IsNil) - c.Assert(s.Mode(), Equals, fs.FileMode(0644)) + c.Assert(s.Mode(), Equals, fs.FileMode(0o644)) err = mfest.IteratePaths(manifestPath, func(p *manifest.Path) error { - c.Assert(p.Mode, Equals, fmt.Sprintf("%#o", fs.FileMode(0644))) + c.Assert(p.Mode, Equals, fmt.Sprintf("%#o", fs.FileMode(0o644))) return nil }) c.Assert(err, IsNil) From 28b36c3928bea433209e27858fce92804f78b0c7 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 4 Sep 2026 09:32:18 +0200 Subject: [PATCH 5/7] fix: refine --- internal/setup/setup_test.go | 34 +++++-- internal/setup/yaml.go | 33 ++++--- internal/slicer/slicer.go | 13 +++ internal/slicer/slicer_test.go | 171 +++++++++++++++++++-------------- 4 files changed, 152 insertions(+), 99 deletions(-) diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index d0912b84..7092d8b1 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -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: 0o755, Mutable: true}, + "/file/path5": {Kind: "copy", Mode: 0755, Mutable: true}, "/file/path6/": {Kind: "dir"}, }, }, @@ -4673,7 +4673,7 @@ var setupTests = []setupTest{{ /dir/file: {channel: ["0.3/stable"]} `, }, - relerror: `slice mypkg_myslice has 'channel' for path /dir/file but package is not in a store`, + relerror: `slice mypkg_myslice has invalid 'channel' for path /dir/file: 'channel' requires 'store'`, }, { summary: "Channel on an essential of a non-store package", input: map[string]string{ @@ -4687,7 +4687,21 @@ var setupTests = []setupTest{{ other: `, }, - relerror: `slice mypkg_myslice has 'channel' for essential mypkg_other but package is not in a store`, + relerror: `slice mypkg_myslice has invalid 'channel' for essential mypkg_other: 'channel' requires 'store'`, +}, { + summary: "Channel on a package-level essential of a non-store package", + input: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlWithStores, + "slices/mypkg.yaml": ` + package: mypkg + essential: + mypkg_other: {channel: ["0.3/stable"]} + slices: + myslice: + other: + `, + }, + relerror: `package "mypkg" has invalid 'channel' for essential mypkg_other: 'channel' requires 'store'`, }, { summary: "Invalid channel on a path", input: map[string]string{ @@ -4999,9 +5013,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), 0o755) + err := os.MkdirAll(filepath.Dir(fpath), 0755) c.Assert(err, IsNil) - err = os.WriteFile(fpath, testutil.Reindent(data), 0o644) + err = os.WriteFile(fpath, testutil.Reindent(data), 0644) c.Assert(err, IsNil) } // Ensure the "slices" directory always exists, even if no slice @@ -5066,16 +5080,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), 0o644) + err := os.WriteFile(fpath, testutil.Reindent(data), 0644) 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), 0o755) + err = os.MkdirAll(filepath.Dir(fpath), 0755) c.Assert(err, IsNil) pkgData, err := yaml.Marshal(pkg) c.Assert(err, IsNil) - err = os.WriteFile(fpath, testutil.Reindent(string(pkgData)), 0o644) + err = os.WriteFile(fpath, testutil.Reindent(string(pkgData)), 0644) c.Assert(err, IsNil) } // Ensure the "slices" directory always exists, even if no slice @@ -5341,9 +5355,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), 0o755) + err := os.MkdirAll(filepath.Dir(fpath), 0755) c.Assert(err, IsNil) - err = os.WriteFile(fpath, testutil.Reindent(data), 0o644) + err = os.WriteFile(fpath, testutil.Reindent(data), 0644) c.Assert(err, IsNil) } // Ensure the "slices" directory always exists, even if no slice diff --git a/internal/setup/yaml.go b/internal/setup/yaml.go index ffb98e12..3b97409d 100644 --- a/internal/setup/yaml.go +++ b/internal/setup/yaml.go @@ -78,7 +78,7 @@ type yamlPackage struct { // 'channel' field. Every flavour of essential is considered, including // 'v3-essential', as they all end up parsed the same way. func (yp *yamlPackage) hasChannel() bool { - essentialsHaveChannel := func(essentials map[string]*yamlEssential) bool { + haveChannel := func(essentials map[string]*yamlEssential) bool { for _, essential := range essentials { if essential != nil && len(essential.Channel.List) > 0 { return true @@ -86,11 +86,11 @@ func (yp *yamlPackage) hasChannel() bool { } return false } - if essentialsHaveChannel(yp.Essential.Values) || essentialsHaveChannel(yp.V3Essential) { + if haveChannel(yp.Essential.Values) || haveChannel(yp.V3Essential) { return true } for _, slice := range yp.Slices { - if essentialsHaveChannel(slice.Essential.Values) || essentialsHaveChannel(slice.V3Essential) { + if haveChannel(slice.Essential.Values) || haveChannel(slice.V3Essential) { return true } for _, path := range slice.Contents { @@ -155,8 +155,10 @@ func (es yamlEssentialListMap) MarshalYAML() (any, error) { return es.Values, nil } -var _ yaml.Marshaler = yamlEssentialListMap{} -var _ yaml.Unmarshaler = (*yamlEssentialListMap)(nil) +var ( + _ yaml.Marshaler = yamlEssentialListMap{} + _ yaml.Unmarshaler = (*yamlEssentialListMap)(nil) +) type yamlPath struct { Dir bool `yaml:"make,omitempty"` @@ -712,7 +714,7 @@ func parsePackage(release *Release, pkgName, pkgPath string, data []byte) (*Pack } if len(yamlPath.Channel.List) > 0 { if pkg.Store == "" { - return nil, fmt.Errorf("slice %s_%s has 'channel' for path %s but package is not in a store", pkg.Name, sliceName, contPath) + return nil, fmt.Errorf("slice %s_%s has invalid 'channel' for path %s: 'channel' requires 'store'", pkg.Name, sliceName, contPath) } if err := validateChannelPatterns(yamlPath.Channel.List); err != nil { return nil, fmt.Errorf("slice %s_%s has invalid 'channel' for path %s: %s", pkg.Name, sliceName, contPath, err) @@ -937,17 +939,18 @@ var defaultMaintenance = map[string]Maintenance{ // processes them to check they are valid and not duplicated and, if // successful, adds them to slice. func parseEssentials(yamlPkg *yamlPackage, yamlSlice *yamlSlice, pkgPath string, slice *Slice) error { - // validateChannels validates the 'channel' field of an essential entry. The - // patterns apply to the channel of the package holding the essential. - validateChannels := func(refName string, essentialInfo *yamlEssential) ([]string, error) { + // validateChannels validates the 'channel' field of an essential entry. + // The patterns apply to the channel of the package holding the essential, + // not to the one of the required slice. + validateChannels := func(essentialInfo *yamlEssential) ([]string, error) { if essentialInfo == nil || len(essentialInfo.Channel.List) == 0 { return nil, nil } if yamlPkg.Store == "" { - return nil, fmt.Errorf("slice %s has 'channel' for essential %s but package is not in a store", slice, refName) + return nil, errors.New("'channel' requires 'store'") } if err := validateChannelPatterns(essentialInfo.Channel.List); err != nil { - return nil, fmt.Errorf("slice %s has invalid 'channel' for essential %s: %s", slice, refName, err) + return nil, err } return essentialInfo.Channel.List, nil } @@ -970,9 +973,9 @@ func parseEssentials(yamlPkg *yamlPackage, yamlSlice *yamlSlice, pkgPath string, if essentialInfo != nil { archList = essentialInfo.Arch.List } - channel, err := validateChannels(refName, essentialInfo) + channel, err := validateChannels(essentialInfo) if err != nil { - return err + return fmt.Errorf("package %q has invalid 'channel' for essential %s: %s", yamlPkg.RealName, refName, err) } slice.Essential[sliceKey] = EssentialInfo{Arch: archList, Channel: channel} return nil @@ -995,9 +998,9 @@ func parseEssentials(yamlPkg *yamlPackage, yamlSlice *yamlSlice, pkgPath string, if essentialInfo != nil { archList = essentialInfo.Arch.List } - channel, err := validateChannels(refName, essentialInfo) + channel, err := validateChannels(essentialInfo) if err != nil { - return err + return fmt.Errorf("slice %s has invalid 'channel' for essential %s: %s", slice, refName, err) } slice.Essential[sliceKey] = EssentialInfo{Arch: archList, Channel: channel} return nil diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index e3ccb0ab..3ffb272f 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -90,6 +90,19 @@ func Run(options *RunOptions) error { targetDir = filepath.Join(dir, targetDir) } + // The channel of every selected store package must be known, as the + // channel-specific entries silently apply to nothing without it, which + // would cut content out of the build without any error. + for _, slice := range options.Selection.Slices { + pkg := options.Selection.Release.Packages[slice.Package] + if pkg.Store == "" { + continue + } + if _, ok := options.Selection.Channels[slice.Package]; !ok { + return fmt.Errorf("internal error: slice %s has no channel", slice) + } + } + pkgArchive, err := selectPkgArchives(options.Archives, options.Selection) if err != nil { return err diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index a21ac6ca..c690c716 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -22,7 +22,9 @@ import ( "github.com/canonical/chisel/public/manifest" ) -var testKey = testutil.PGPKeys["key1"] +var ( + testKey = testutil.PGPKeys["key1"] +) type slicerTest struct { summary string @@ -44,7 +46,7 @@ var packageEntries = map[string][]testutil.TarEntry{ {Header: tar.Header{Name: "./usr/"}}, {Header: tar.Header{Name: "./usr/lib/"}}, {Header: tar.Header{Name: "./usr/lib/x86_64-linux-gnu/"}}, - {Header: tar.Header{Name: "./usr/lib/x86_64-linux-gnu/libssl.so.3", Mode: 0o0755}}, + {Header: tar.Header{Name: "./usr/lib/x86_64-linux-gnu/libssl.so.3", Mode: 00755}}, {Header: tar.Header{Name: "./usr/share/"}}, {Header: tar.Header{Name: "./usr/share/doc/"}}, {Header: tar.Header{Name: "./usr/share/doc/copyright-symlink-libssl3/"}}, @@ -57,7 +59,7 @@ var packageEntries = map[string][]testutil.TarEntry{ {Header: tar.Header{Name: "./etc/ssl/openssl.cnf"}}, {Header: tar.Header{Name: "./usr/"}}, {Header: tar.Header{Name: "./usr/bin/"}}, - {Header: tar.Header{Name: "./usr/bin/openssl", Mode: 0o0755}}, + {Header: tar.Header{Name: "./usr/bin/openssl", Mode: 00755}}, {Header: tar.Header{Name: "./usr/share/"}}, {Header: tar.Header{Name: "./usr/share/doc/"}}, {Header: tar.Header{Name: "./usr/share/doc/copyright-symlink-openssl/"}}, @@ -67,11 +69,11 @@ var packageEntries = map[string][]testutil.TarEntry{ var testPackageCopyrightEntries = []testutil.TarEntry{ // Hardcoded copyright paths. - testutil.Dir(0o755, "./usr/"), - testutil.Dir(0o755, "./usr/share/"), - testutil.Dir(0o755, "./usr/share/doc/"), - testutil.Dir(0o755, "./usr/share/doc/test-package/"), - testutil.Reg(0o644, "./usr/share/doc/test-package/copyright", "copyright"), + testutil.Dir(0755, "./usr/"), + testutil.Dir(0755, "./usr/share/"), + testutil.Dir(0755, "./usr/share/doc/"), + testutil.Dir(0755, "./usr/share/doc/test-package/"), + testutil.Reg(0644, "./usr/share/doc/test-package/copyright", "copyright"), } var slicerTests = []slicerTest{{ @@ -323,19 +325,19 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "a-implicit-parent", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0o755, "./dir/"), - testutil.Reg(0o644, "./dir/file-1", "random"), + testutil.Dir(0755, "./dir/"), + testutil.Reg(0644, "./dir/file-1", "random"), }), }, { Name: "b-explicit-dir", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0o1777, "./dir/"), + testutil.Dir(01777, "./dir/"), }), }, { Name: "c-implicit-parent", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0o766, "./dir/"), - testutil.Reg(0o644, "./dir/file-2", "random"), + testutil.Dir(0766, "./dir/"), + testutil.Reg(0644, "./dir/file-2", "random"), }), }}, release: map[string]string{ @@ -788,7 +790,7 @@ var slicerTests = []slicerTest{{ Version: "v1", Arch: "a1", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0o644, "./file", "from foo"), + testutil.Reg(0644, "./file", "from foo"), }), Archives: []string{"foo"}, }, { @@ -797,7 +799,7 @@ var slicerTests = []slicerTest{{ Version: "v2", Arch: "a2", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0o644, "./file", "from bar"), + testutil.Reg(0644, "./file", "from bar"), }), Archives: []string{"bar"}, }, { @@ -806,7 +808,7 @@ var slicerTests = []slicerTest{{ Version: "v3", Arch: "a3", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0o644, "./other-file", "from bar"), + testutil.Reg(0644, "./other-file", "from bar"), }), Archives: []string{"bar"}, }}, @@ -872,7 +874,7 @@ var slicerTests = []slicerTest{{ Version: "v1", Arch: "a1", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0o644, "./file", "from foo"), + testutil.Reg(0644, "./file", "from foo"), }), Archives: []string{"foo"}, }, { @@ -881,7 +883,7 @@ var slicerTests = []slicerTest{{ Version: "v2", Arch: "a2", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0o644, "./file", "from bar"), + testutil.Reg(0644, "./file", "from bar"), }), Archives: []string{"bar"}, }}, @@ -937,7 +939,7 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0o644, "./file", "from foo"), + testutil.Reg(0644, "./file", "from foo"), }), Archives: []string{"foo"}, }}, @@ -1020,7 +1022,7 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0o644, "./file", "from foo"), + testutil.Reg(0644, "./file", "from foo"), }), Archives: []string{"foo"}, }}, @@ -1060,7 +1062,7 @@ var slicerTests = []slicerTest{{ Version: "v1", Arch: "a1", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0o644, "./file", "from foo"), + testutil.Reg(0644, "./file", "from foo"), }), Archives: []string{"foo"}, }}, @@ -1437,12 +1439,12 @@ var slicerTests = []slicerTest{{ // relative path. Since TrimLeft takes in a cutset instead of a // prefix, the desired relative path was not produced. // See https://github.com/canonical/chisel/pull/145. - testutil.Dir(0o755, "./foo-bar/"), + testutil.Dir(0755, "./foo-bar/"), }), }}, hackopt: func(c *C, opts *slicer.RunOptions) { opts.TargetDir = filepath.Join(filepath.Clean(opts.TargetDir), "foo") - err := os.Mkdir(opts.TargetDir, 0o755) + err := os.Mkdir(opts.TargetDir, 0755) c.Assert(err, IsNil) }, release: map[string]string{ @@ -1513,9 +1515,9 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0o755, "./"), - testutil.Reg(0o644, "./file", "foo"), - testutil.Hrd(0o644, "./hardlink", "./file"), + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file", "foo"), + testutil.Hrd(0644, "./hardlink", "./file"), }), }}, release: map[string]string{ @@ -1547,10 +1549,10 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0o755, "./"), - testutil.Reg(0o644, "./file", "foo"), - testutil.Hrd(0o644, "./hardlink1", "./file"), - testutil.Hrd(0o644, "./hardlink2", "./file"), + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file", "foo"), + testutil.Hrd(0644, "./hardlink1", "./file"), + testutil.Hrd(0644, "./hardlink2", "./file"), }), }}, release: map[string]string{ @@ -1578,11 +1580,11 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0o755, "./"), - testutil.Reg(0o644, "./file1", "text for file1"), - testutil.Reg(0o644, "./file2", "text for file2"), - testutil.Hrd(0o644, "./hardlink1", "./file1"), - testutil.Hrd(0o644, "./hardlink2", "./file2"), + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file1", "text for file1"), + testutil.Reg(0644, "./file2", "text for file2"), + testutil.Hrd(0644, "./hardlink1", "./file1"), + testutil.Hrd(0644, "./hardlink2", "./file2"), }), }}, release: map[string]string{ @@ -1614,9 +1616,9 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0o755, "./"), - testutil.Reg(0o644, "./file", "foo"), - testutil.Hrd(0o644, "./hardlink", "./file"), + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file", "foo"), + testutil.Hrd(0644, "./hardlink", "./file"), }), }}, release: map[string]string{ @@ -1642,11 +1644,11 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0o755, "./"), - testutil.Dir(0o755, "./"), - testutil.Reg(0o644, "./file", "foo"), - testutil.Lnk(0o644, "./symlink", "./file"), - testutil.Hrd(0o644, "./hardlink", "./symlink"), + testutil.Dir(0755, "./"), + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file", "foo"), + testutil.Lnk(0644, "./symlink", "./file"), + testutil.Hrd(0644, "./hardlink", "./symlink"), }), }}, release: map[string]string{ @@ -1676,16 +1678,16 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "test-package1", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0o755, "./"), - testutil.Reg(0o644, "./file1", "foo"), - testutil.Hrd(0o644, "./hardlink1", "./file1"), + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file1", "foo"), + testutil.Hrd(0644, "./hardlink1", "./file1"), }), }, { Name: "test-package2", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0o755, "./"), - testutil.Reg(0o644, "./file2", "foo"), - testutil.Hrd(0o644, "./hardlink2", "./file2"), + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file2", "foo"), + testutil.Hrd(0644, "./hardlink2", "./file2"), }), }}, release: map[string]string{ @@ -1726,9 +1728,9 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0o755, "./"), - testutil.Reg(0o644, "./file", "foo"), - testutil.Hrd(0o644, "./hardlink", "./file"), + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file", "foo"), + testutil.Hrd(0644, "./hardlink", "./file"), }), }}, release: map[string]string{ @@ -1752,9 +1754,9 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0o755, "./"), - testutil.Reg(0o644, "./file", "foo"), - testutil.Hrd(0o644, "./hardlink", "./file"), + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file", "foo"), + testutil.Hrd(0644, "./hardlink", "./file"), }), }}, release: map[string]string{ @@ -1781,8 +1783,8 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0o755, "./"), - testutil.Hrd(0o644, "./hardlink", "/etc/group"), + testutil.Dir(0755, "./"), + testutil.Hrd(0644, "./hardlink", "/etc/group"), }), }}, release: map[string]string{ @@ -1801,8 +1803,8 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0o755, "./"), - testutil.Reg(0o644, "./../file", "hijacking system file"), + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./../file", "hijacking system file"), }), }}, release: map[string]string{ @@ -1824,19 +1826,19 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "test-package1", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0o755, "./"), - testutil.Reg(0o644, "./file", "foo"), + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file", "foo"), }), }, { Name: "test-package2", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0o755, "./"), - testutil.Reg(0o644, "./file", "bar"), + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file", "bar"), }), }, { Name: "test-package3", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0o755, "./"), + testutil.Dir(0755, "./"), }), }}, release: map[string]string{ @@ -1885,18 +1887,18 @@ var slicerTests = []slicerTest{{ pkgs: []*testutil.TestPackage{{ Name: "test-package1", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0o755, "./"), + testutil.Dir(0755, "./"), // Note that both implicit parents have different permissions. - testutil.Dir(0o766, "./parent/"), - testutil.Reg(0o644, "./parent/foo", "whatever"), + testutil.Dir(0766, "./parent/"), + testutil.Reg(0644, "./parent/foo", "whatever"), }), }, { Name: "test-package2", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0o755, "./"), + testutil.Dir(0755, "./"), // And here. - testutil.Dir(0o755, "./parent/"), - testutil.Reg(0o644, "./parent/bar", "whatever"), + testutil.Dir(0755, "./parent/"), + testutil.Reg(0644, "./parent/bar", "whatever"), }), }}, release: map[string]string{ @@ -2002,6 +2004,27 @@ var slicerTests = []slicerTest{{ `, }, error: `cannot fetch package "bin-curl" from store "bin": not implemented`, +}, { + summary: "Store package without a resolved channel", + slices: []setup.SliceKey{{"bin-curl", "bin"}}, + release: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlWithStores, + "slices/curl.yaml": ` + package: curl + store: bin + default-track: "0.2" + slices: + bin: + contents: + /usr/bin/curl: + `, + }, + // A selection built by Select always holds the channel of its store + // packages, so the guard is exercised by dropping it here. + hackopt: func(c *C, opts *slicer.RunOptions) { + opts.Selection.Channels = nil + }, + error: `internal error: slice bin-curl_bin has no channel`, }} func (s *S) TestRun(c *C) { @@ -2073,9 +2096,9 @@ func runSlicerTests(s *S, c *C, tests []slicerTest) { releaseDir := c.MkDir() for path, data := range test.release { fpath := filepath.Join(releaseDir, path) - err := os.MkdirAll(filepath.Dir(fpath), 0o755) + err := os.MkdirAll(filepath.Dir(fpath), 0755) c.Assert(err, IsNil) - err = os.WriteFile(fpath, testutil.Reindent(data), 0o644) + err = os.WriteFile(fpath, testutil.Reindent(data), 0644) c.Assert(err, IsNil) } @@ -2253,9 +2276,9 @@ func readManifest(c *C, targetDir, manifestPath string) *manifest.Manifest { // in the manifest itself. s, err := os.Stat(path.Join(targetDir, manifestPath)) c.Assert(err, IsNil) - c.Assert(s.Mode(), Equals, fs.FileMode(0o644)) + c.Assert(s.Mode(), Equals, fs.FileMode(0644)) err = mfest.IteratePaths(manifestPath, func(p *manifest.Path) error { - c.Assert(p.Mode, Equals, fmt.Sprintf("%#o", fs.FileMode(0o644))) + c.Assert(p.Mode, Equals, fmt.Sprintf("%#o", fs.FileMode(0644))) return nil }) c.Assert(err, IsNil) From 8427cfa5c4202e5f92cd816ded498c165839a082 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 4 Sep 2026 09:38:08 +0200 Subject: [PATCH 6/7] fix: refine --- internal/setup/setup_test.go | 8 ++------ internal/slicer/slicer_test.go | 33 +++++++++++---------------------- 2 files changed, 13 insertions(+), 28 deletions(-) diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 7092d8b1..02328fb6 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -490,11 +490,7 @@ var setupTests = []setupTest{{ /path3: {symlink: /link} `, }, - selslices: []setup.SliceKey{ - {"mypkg1", "myslice1"}, - {"mypkg1", "myslice2"}, - {"mypkg2", "myslice1"}, - }, + selslices: []setup.SliceKey{{"mypkg1", "myslice1"},{"mypkg1", "myslice2"},{"mypkg2", "myslice1"}}, }, { summary: "Conflicting paths across slices", input: map[string]string{ @@ -5106,7 +5102,7 @@ func (s *S) TestPackageMarshalYAML(c *C) { } func (s *S) TestPackageYAMLFormat(c *C) { - tests := []struct { + var tests = []struct { summary string input map[string]string expected map[string]string diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index c690c716..c05d7751 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -275,8 +275,7 @@ var slicerTests = []slicerTest{{ summary: "Install two packages", slices: []setup.SliceKey{ {"test-package", "myslice"}, - {"other-package", "myslice"}, - }, + {"other-package", "myslice"}}, pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.PackageData["test-package"], @@ -320,8 +319,7 @@ var slicerTests = []slicerTest{{ slices: []setup.SliceKey{ {"a-implicit-parent", "myslice"}, {"b-explicit-dir", "myslice"}, - {"c-implicit-parent", "myslice"}, - }, + {"c-implicit-parent", "myslice"}}, pkgs: []*testutil.TestPackage{{ Name: "a-implicit-parent", Data: testutil.MustMakeDeb([]testutil.TarEntry{ @@ -379,8 +377,7 @@ var slicerTests = []slicerTest{{ summary: "Valid same file in two slices in different packages", slices: []setup.SliceKey{ {"test-package", "myslice"}, - {"other-package", "myslice"}, - }, + {"other-package", "myslice"}}, pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.PackageData["test-package"], @@ -1510,8 +1507,7 @@ var slicerTests = []slicerTest{{ summary: "Valid hard link in two slices in the same package", slices: []setup.SliceKey{ {"test-package", "slice1"}, - {"test-package", "slice2"}, - }, + {"test-package", "slice2"}}, pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ @@ -1544,8 +1540,7 @@ var slicerTests = []slicerTest{{ }, { summary: "Hard link entries can be extracted without extracting the regular file", slices: []setup.SliceKey{ - {"test-package", "myslice"}, - }, + {"test-package", "myslice"}}, pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ @@ -1575,8 +1570,7 @@ var slicerTests = []slicerTest{{ }, { summary: "Hard link identifier for different groups", slices: []setup.SliceKey{ - {"test-package", "myslice"}, - }, + {"test-package", "myslice"}}, pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ @@ -1611,8 +1605,7 @@ var slicerTests = []slicerTest{{ }, { summary: "Single hard link entry can be extracted without regular file and no hard links are created", slices: []setup.SliceKey{ - {"test-package", "myslice"}, - }, + {"test-package", "myslice"}}, pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ @@ -1639,8 +1632,7 @@ var slicerTests = []slicerTest{{ }, { summary: "Hard link to symlink does not follow symlink", slices: []setup.SliceKey{ - {"test-package", "myslice"}, - }, + {"test-package", "myslice"}}, pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ @@ -1673,8 +1665,7 @@ var slicerTests = []slicerTest{{ summary: "Hard link identifiers are unique across packages", slices: []setup.SliceKey{ {"test-package1", "myslice"}, - {"test-package2", "myslice"}, - }, + {"test-package2", "myslice"}}, pkgs: []*testutil.TestPackage{{ Name: "test-package1", Data: testutil.MustMakeDeb([]testutil.TarEntry{ @@ -1723,8 +1714,7 @@ var slicerTests = []slicerTest{{ }, { summary: "Mutations for hard links are forbidden", slices: []setup.SliceKey{ - {"test-package", "myslice"}, - }, + {"test-package", "myslice"}}, pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ @@ -1749,8 +1739,7 @@ var slicerTests = []slicerTest{{ }, { summary: "Hard links can be marked as mutable, but not mutated", slices: []setup.SliceKey{ - {"test-package", "myslice"}, - }, + {"test-package", "myslice"}}, pkgs: []*testutil.TestPackage{{ Name: "test-package", Data: testutil.MustMakeDeb([]testutil.TarEntry{ From 3f2ac44677386c9a111b1dae91964a80dc61d3e3 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 4 Sep 2026 09:55:55 +0200 Subject: [PATCH 7/7] fix: refine --- internal/setup/setup.go | 12 ++++++------ internal/setup/setup_test.go | 2 +- internal/slicer/slicer_test.go | 3 ++- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/internal/setup/setup.go b/internal/setup/setup.go index 3b1db4f7..cf2dcb80 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -99,7 +99,7 @@ const ( GeneratePath PathKind = "generate" // TODO Maybe in the future, for binary support. - // Base64Path PathKind = "base64" + //Base64Path PathKind = "base64" ) type PathUntil string @@ -160,7 +160,7 @@ func (s *Slice) String() string { return s.Package + "_" + s.Name } type Selection struct { Release *Release Slices []*Slice - // Channels holds the resolved channel per store package name. + // Channels holds the selected channel per store package name. Channels map[string]Channel } @@ -520,10 +520,10 @@ func Select(release *Release, slices []SliceKey, arch string) (*Selection, error return nil, err } - // Resolve the channel of every store package, whether it is selected or + // 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 := resolveChannels(release) + channels := selectChannels(release) selection := &Selection{ Release: release, @@ -577,10 +577,10 @@ func Select(release *Release, slices []SliceKey, arch string) (*Selection, error return selection, nil } -// resolveChannels returns the channel of every store package of the release, +// 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 resolveChannels(release *Release) map[string]Channel { +func selectChannels(release *Release) map[string]Channel { channels := make(map[string]Channel) for _, pkg := range release.Packages { if pkg.Store == "" { diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 02328fb6..5bd51c20 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -490,7 +490,7 @@ var setupTests = []setupTest{{ /path3: {symlink: /link} `, }, - selslices: []setup.SliceKey{{"mypkg1", "myslice1"},{"mypkg1", "myslice2"},{"mypkg2", "myslice1"}}, + selslices: []setup.SliceKey{{"mypkg1", "myslice1"}, {"mypkg1", "myslice2"}, {"mypkg2", "myslice1"}}, }, { summary: "Conflicting paths across slices", input: map[string]string{ diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index c05d7751..96b919f1 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -1665,7 +1665,8 @@ var slicerTests = []slicerTest{{ summary: "Hard link identifiers are unique across packages", slices: []setup.SliceKey{ {"test-package1", "myslice"}, - {"test-package2", "myslice"}}, + {"test-package2", "myslice"}, + }, pkgs: []*testutil.TestPackage{{ Name: "test-package1", Data: testutil.MustMakeDeb([]testutil.TarEntry{