From c5d397c1f476c3c41d2db98fd740d43b5b7bc9ae Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 9 Jul 2026 14:55:35 +0200 Subject: [PATCH 01/24] feat: prepare interface for store Signed-off-by: Paul Mars --- internal/slicer/slicer.go | 74 +++++++++++++++++++++++++++------- internal/slicer/slicer_test.go | 24 +++++++++++ 2 files changed, 84 insertions(+), 14 deletions(-) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 6f4783b8..a2ab6032 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -41,6 +41,15 @@ type contentChecker struct { knownPaths map[string]pathData } +// pkgSource holds the resolved source for a package: its architecture and a +// fetch function returning the package reader and metadata. The fetch +// function is bound at resolution time, so callers are agnostic to whether +// the package comes from an archive or a store. +type pkgSource struct { + arch string + fetch func() (io.ReadSeekCloser, *archive.PackageInfo, error) +} + func (cc *contentChecker) checkMutable(path string) error { if !cc.knownPaths[path].mutable { return fmt.Errorf("cannot write file which is not mutable: %s", path) @@ -90,7 +99,7 @@ func Run(options *RunOptions) error { targetDir = filepath.Join(dir, targetDir) } - pkgArchive, err := selectPkgArchives(options.Archives, options.Selection) + pkgSources, err := resolvePkgSources(options.Archives, options.Selection) if err != nil { return err } @@ -108,12 +117,12 @@ func Run(options *RunOptions) error { extractPackage = make(map[string][]tarball.ExtractInfo) extract[slice.Package] = extractPackage } - arch := pkgArchive[slice.Package].Options().Arch + src := pkgSources[slice.Package] for targetPath, pathInfo := range slice.Contents { if targetPath == "" { continue } - if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, arch) { + if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, src.arch) { continue } if preferredPkg, ok := prefers[targetPath]; ok && preferredPkg.Name != slice.Package { @@ -153,7 +162,8 @@ func Run(options *RunOptions) error { continue } pkg := options.Selection.Release.Packages[slice.Package] - reader, info, err := pkgArchive[slice.Package].Fetch(pkg.RealName) + src := pkgSources[pkg] + reader, info, err := src.fetch() if err != nil { return err } @@ -270,9 +280,9 @@ func Run(options *RunOptions) error { // them to the appropriate slices. relPaths := map[string][]*setup.Slice{} for _, slice := range options.Selection.Slices { - arch := pkgArchive[slice.Package].Options().Arch + src := pkgSources[slice.Package] for relPath, pathInfo := range slice.Contents { - if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, arch) { + if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, src.arch) { continue } if pathInfo.Kind == setup.CopyPath || pathInfo.Kind == setup.GlobPath || @@ -490,10 +500,13 @@ func createFile(targetDir, relPath string, pathInfo setup.PathInfo) (*fsutil.Ent }) } -// selectPkgArchives selects the highest priority archive containing the package -// unless a particular archive is pinned within the slice definition file. It -// returns a map of archives indexed by package names. -func selectPkgArchives(archives map[string]archive.Archive, selection *setup.Selection) (map[string]archive.Archive, error) { +// resolvePkgSources determines the source for each package in the selection. +// For archive packages it selects the highest priority archive containing the +// package unless a particular archive is pinned within the slice definition +// file. For store packages it records a fetch function that returns an error +// until store support is implemented. It returns a map of pkgSource indexed by +// package names. +func resolvePkgSources(archives map[string]archive.Archive, selection *setup.Selection) (map[string]pkgSource, error) { sortedArchives := make([]*setup.Archive, 0, len(selection.Release.Archives)) for _, archive := range selection.Release.Archives { if archive.Priority < 0 { @@ -507,12 +520,20 @@ func selectPkgArchives(archives map[string]archive.Archive, selection *setup.Sel return b.Priority - a.Priority }) - pkgArchive := make(map[string]archive.Archive) + pkgSources := make(map[string]pkgSource) for _, s := range selection.Slices { - if _, ok := pkgArchive[s.Package]; ok { + if _, ok := pkgSources[s.Package]; ok { continue } pkg := selection.Release.Packages[s.Package] + if pkg.Store != "" { + pkgSources[pkg.Name] = pkgSource{ + fetch: func() (io.ReadSeekCloser, *archive.PackageInfo, error) { + return nil, nil, fmt.Errorf("cannot fetch package %q from store: store packages are not yet supported", pkg.Name) + }, + } + continue + } if pkg.Store != "" { return nil, fmt.Errorf("cannot fetch package %q from store %q: not implemented", pkg.Name, pkg.Store) @@ -538,7 +559,32 @@ func selectPkgArchives(archives map[string]archive.Archive, selection *setup.Sel if chosen == nil { return nil, fmt.Errorf("cannot find package %q in archive(s)", pkg.RealName) } - pkgArchive[pkg.Name] = chosen + chosenArchive := chosen + pkgSources[pkg.Name] = pkgSource{ + arch: chosen.Options().Arch, + fetch: func() (io.ReadSeekCloser, *archive.PackageInfo, error) { + return chosenArchive.Fetch(pkg.RealName) + }, + } } - return pkgArchive, nil + + // Until a store is implemented as a package source there is no proper way to + // determine the architecture for store packages. + // So relying on the fact that all packages in a selection share the same architecture, + // we can borrow it from any archive package that was already resolved. + var arch string + for _, src := range pkgSources { + if src.arch != "" { + arch = src.arch + break + } + } + for name, src := range pkgSources { + if src.arch == "" { + src.arch = arch + pkgSources[name] = src + } + } + + return pkgSources, nil } diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index d6ef9ca0..4e1e39d1 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -1994,6 +1994,30 @@ var slicerTests = []slicerTest{{ `, }, error: `cannot fetch package "bin-curl" from store "bin": not implemented`, +}, { + summary: "Store package fails as it is not yet supported", + slices: []setup.SliceKey{{"test-package", "myslice"}, {"bin-store-pkg", "myslice"}}, + arch: "amd64", + release: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlWithStores, + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + myslice: + contents: + /dir/file: + `, + "slices/mydir/store-pkg.yaml": ` + package: store-pkg + store: bin + default-track: stable + slices: + myslice: + contents: + /dir/store-file: + `, + }, + error: `cannot fetch package "bin-store-pkg" from store: store packages are not yet supported`, }} func (s *S) TestRun(c *C) { From f79521ea21a040baef490ea9987aab08d0d1d795 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 10 Jul 2026 09:59:59 +0200 Subject: [PATCH 02/24] fix: error fetching from the store Signed-off-by: Paul Mars --- internal/slicer/slicer.go | 8 ++------ internal/slicer/slicer_test.go | 20 ++------------------ 2 files changed, 4 insertions(+), 24 deletions(-) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index a2ab6032..83ff84a7 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -162,7 +162,7 @@ func Run(options *RunOptions) error { continue } pkg := options.Selection.Release.Packages[slice.Package] - src := pkgSources[pkg] + src := pkgSources[pkg.Name] reader, info, err := src.fetch() if err != nil { return err @@ -529,16 +529,12 @@ func resolvePkgSources(archives map[string]archive.Archive, selection *setup.Sel if pkg.Store != "" { pkgSources[pkg.Name] = pkgSource{ fetch: func() (io.ReadSeekCloser, *archive.PackageInfo, error) { - return nil, nil, fmt.Errorf("cannot fetch package %q from store: store packages are not yet supported", pkg.Name) + return nil, nil, fmt.Errorf("cannot fetch package %q from store %q: not implemented", pkg.Name, pkg.Store) }, } continue } - if pkg.Store != "" { - return nil, fmt.Errorf("cannot fetch package %q from store %q: not implemented", pkg.Name, pkg.Store) - } - var candidates []*setup.Archive if pkg.Archive == "" { // If the package has not pinned any archive, choose the highest diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 4e1e39d1..af107779 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -1978,23 +1978,7 @@ var slicerTests = []slicerTest{{ manifestPaths: map[string]string{ "/dir/file": "file 0644 cc55e2ec {test-package_third}", }, -}, { - summary: "Store package is not yet implemented", - slices: []setup.SliceKey{{"bin-curl", "bin"}}, - release: map[string]string{ - "chisel.yaml": testutil.DefaultChiselYamlWithStores, - "slices/curl.yaml": ` - package: curl - store: bin - default-track: latest - slices: - bin: - contents: - /usr/bin/curl: - `, - }, - error: `cannot fetch package "bin-curl" from store "bin": not implemented`, -}, { +},{ summary: "Store package fails as it is not yet supported", slices: []setup.SliceKey{{"test-package", "myslice"}, {"bin-store-pkg", "myslice"}}, arch: "amd64", @@ -2017,7 +2001,7 @@ var slicerTests = []slicerTest{{ /dir/store-file: `, }, - error: `cannot fetch package "bin-store-pkg" from store: store packages are not yet supported`, + error: `cannot fetch package "bin-store-pkg" from store "bin": not implemented`, }} func (s *S) TestRun(c *C) { From a2e22f27ee82997cf6305825a60b330b68f2612a Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 10 Jul 2026 10:37:57 +0200 Subject: [PATCH 03/24] fix: cleaning Signed-off-by: Paul Mars --- internal/slicer/slicer.go | 14 ++++++-------- internal/slicer/slicer_test.go | 4 ++-- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 83ff84a7..ab510628 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -117,12 +117,12 @@ func Run(options *RunOptions) error { extractPackage = make(map[string][]tarball.ExtractInfo) extract[slice.Package] = extractPackage } - src := pkgSources[slice.Package] + arch := pkgSources[slice.Package].arch for targetPath, pathInfo := range slice.Contents { if targetPath == "" { continue } - if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, src.arch) { + if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, arch) { continue } if preferredPkg, ok := prefers[targetPath]; ok && preferredPkg.Name != slice.Package { @@ -162,8 +162,7 @@ func Run(options *RunOptions) error { continue } pkg := options.Selection.Release.Packages[slice.Package] - src := pkgSources[pkg.Name] - reader, info, err := src.fetch() + reader, info, err := pkgSources[pkg.Name].fetch() if err != nil { return err } @@ -280,9 +279,9 @@ func Run(options *RunOptions) error { // them to the appropriate slices. relPaths := map[string][]*setup.Slice{} for _, slice := range options.Selection.Slices { - src := pkgSources[slice.Package] + arch := pkgSources[slice.Package].arch for relPath, pathInfo := range slice.Contents { - if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, src.arch) { + if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, arch) { continue } if pathInfo.Kind == setup.CopyPath || pathInfo.Kind == setup.GlobPath || @@ -555,11 +554,10 @@ func resolvePkgSources(archives map[string]archive.Archive, selection *setup.Sel if chosen == nil { return nil, fmt.Errorf("cannot find package %q in archive(s)", pkg.RealName) } - chosenArchive := chosen pkgSources[pkg.Name] = pkgSource{ arch: chosen.Options().Arch, fetch: func() (io.ReadSeekCloser, *archive.PackageInfo, error) { - return chosenArchive.Fetch(pkg.RealName) + return chosen.Fetch(pkg.RealName) }, } } diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index af107779..eaedfa28 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -1978,8 +1978,8 @@ var slicerTests = []slicerTest{{ manifestPaths: map[string]string{ "/dir/file": "file 0644 cc55e2ec {test-package_third}", }, -},{ - summary: "Store package fails as it is not yet supported", +}, { + summary: "Store package fails as not yet implemented", slices: []setup.SliceKey{{"test-package", "myslice"}, {"bin-store-pkg", "myslice"}}, arch: "amd64", release: map[string]string{ From 2eb85d0aecd46e0a7d1fb2d3f8a378f9c4db41eb Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 15 Jul 2026 15:52:47 +0200 Subject: [PATCH 04/24] feat: remove arch setting until store implementation Signed-off-by: Paul Mars --- internal/slicer/slicer.go | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index ab510628..fcc16d67 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -527,6 +527,7 @@ func resolvePkgSources(archives map[string]archive.Archive, selection *setup.Sel pkg := selection.Release.Packages[s.Package] if pkg.Store != "" { pkgSources[pkg.Name] = pkgSource{ + // TODO: set the arch when implementing fetching from the store. fetch: func() (io.ReadSeekCloser, *archive.PackageInfo, error) { return nil, nil, fmt.Errorf("cannot fetch package %q from store %q: not implemented", pkg.Name, pkg.Store) }, @@ -561,24 +562,5 @@ func resolvePkgSources(archives map[string]archive.Archive, selection *setup.Sel }, } } - - // Until a store is implemented as a package source there is no proper way to - // determine the architecture for store packages. - // So relying on the fact that all packages in a selection share the same architecture, - // we can borrow it from any archive package that was already resolved. - var arch string - for _, src := range pkgSources { - if src.arch != "" { - arch = src.arch - break - } - } - for name, src := range pkgSources { - if src.arch == "" { - src.arch = arch - pkgSources[name] = src - } - } - return pkgSources, nil } From 3004a6eff32ba8b8dd5c61177cdf450fc85a024e Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 15 Jul 2026 15:57:02 +0200 Subject: [PATCH 05/24] test: refine Signed-off-by: Paul Mars --- internal/slicer/slicer_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index eaedfa28..954602de 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -1979,7 +1979,7 @@ var slicerTests = []slicerTest{{ "/dir/file": "file 0644 cc55e2ec {test-package_third}", }, }, { - summary: "Store package fails as not yet implemented", + summary: "Store package fetching not yet implemented", slices: []setup.SliceKey{{"test-package", "myslice"}, {"bin-store-pkg", "myslice"}}, arch: "amd64", release: map[string]string{ @@ -1994,7 +1994,7 @@ var slicerTests = []slicerTest{{ "slices/mydir/store-pkg.yaml": ` package: store-pkg store: bin - default-track: stable + default-track: 3.1 slices: myslice: contents: From ed0e070951ca544e1a2744b498b487154429ea16 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 18 Aug 2026 11:54:44 +0200 Subject: [PATCH 06/24] refactor: adopt alternative approach Add a source package with a Source interface to abstract over the archive and store interface. This is needed because fetching from the store requires the architecture, track and risk. --- internal/slicer/slicer.go | 84 +--------- internal/source/source.go | 112 ++++++++++++++ internal/source/source_test.go | 275 +++++++++++++++++++++++++++++++++ internal/source/suite_test.go | 13 ++ internal/testutil/defaults.go | 23 +++ 5 files changed, 428 insertions(+), 79 deletions(-) create mode 100644 internal/source/source.go create mode 100644 internal/source/source_test.go create mode 100644 internal/source/suite_test.go diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index fcc16d67..2f6a18bb 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -20,6 +20,7 @@ import ( "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/scripts" "github.com/canonical/chisel/internal/setup" + "github.com/canonical/chisel/internal/source" "github.com/canonical/chisel/internal/tarball" ) @@ -41,15 +42,6 @@ type contentChecker struct { knownPaths map[string]pathData } -// pkgSource holds the resolved source for a package: its architecture and a -// fetch function returning the package reader and metadata. The fetch -// function is bound at resolution time, so callers are agnostic to whether -// the package comes from an archive or a store. -type pkgSource struct { - arch string - fetch func() (io.ReadSeekCloser, *archive.PackageInfo, error) -} - func (cc *contentChecker) checkMutable(path string) error { if !cc.knownPaths[path].mutable { return fmt.Errorf("cannot write file which is not mutable: %s", path) @@ -99,7 +91,7 @@ func Run(options *RunOptions) error { targetDir = filepath.Join(dir, targetDir) } - pkgSources, err := resolvePkgSources(options.Archives, options.Selection) + pkgSources, err := source.Resolve(options.Archives, options.Selection) if err != nil { return err } @@ -117,7 +109,7 @@ func Run(options *RunOptions) error { extractPackage = make(map[string][]tarball.ExtractInfo) extract[slice.Package] = extractPackage } - arch := pkgSources[slice.Package].arch + arch := pkgSources[slice.Package].Arch() for targetPath, pathInfo := range slice.Contents { if targetPath == "" { continue @@ -162,7 +154,7 @@ func Run(options *RunOptions) error { continue } pkg := options.Selection.Release.Packages[slice.Package] - reader, info, err := pkgSources[pkg.Name].fetch() + reader, info, err := pkgSources[pkg.Name].Fetch() if err != nil { return err } @@ -279,7 +271,7 @@ func Run(options *RunOptions) error { // them to the appropriate slices. relPaths := map[string][]*setup.Slice{} for _, slice := range options.Selection.Slices { - arch := pkgSources[slice.Package].arch + arch := pkgSources[slice.Package].Arch() for relPath, pathInfo := range slice.Contents { if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, arch) { continue @@ -498,69 +490,3 @@ func createFile(targetDir, relPath string, pathInfo setup.PathInfo) (*fsutil.Ent MakeParents: true, }) } - -// resolvePkgSources determines the source for each package in the selection. -// For archive packages it selects the highest priority archive containing the -// package unless a particular archive is pinned within the slice definition -// file. For store packages it records a fetch function that returns an error -// until store support is implemented. It returns a map of pkgSource indexed by -// package names. -func resolvePkgSources(archives map[string]archive.Archive, selection *setup.Selection) (map[string]pkgSource, error) { - sortedArchives := make([]*setup.Archive, 0, len(selection.Release.Archives)) - for _, archive := range selection.Release.Archives { - if archive.Priority < 0 { - // Ignore negative priority archives unless a package specifically - // asks for it with the "archive" field. - continue - } - sortedArchives = append(sortedArchives, archive) - } - slices.SortFunc(sortedArchives, func(a, b *setup.Archive) int { - return b.Priority - a.Priority - }) - - pkgSources := make(map[string]pkgSource) - for _, s := range selection.Slices { - if _, ok := pkgSources[s.Package]; ok { - continue - } - pkg := selection.Release.Packages[s.Package] - if pkg.Store != "" { - pkgSources[pkg.Name] = pkgSource{ - // TODO: set the arch when implementing fetching from the store. - fetch: func() (io.ReadSeekCloser, *archive.PackageInfo, error) { - return nil, nil, fmt.Errorf("cannot fetch package %q from store %q: not implemented", pkg.Name, pkg.Store) - }, - } - continue - } - - var candidates []*setup.Archive - if pkg.Archive == "" { - // If the package has not pinned any archive, choose the highest - // priority archive in which the package exists. - candidates = sortedArchives - } else { - candidates = []*setup.Archive{selection.Release.Archives[pkg.Archive]} - } - - var chosen archive.Archive - for _, archiveInfo := range candidates { - archive := archives[archiveInfo.Name] - if archive != nil && archive.Exists(pkg.RealName) { - chosen = archive - break - } - } - if chosen == nil { - return nil, fmt.Errorf("cannot find package %q in archive(s)", pkg.RealName) - } - pkgSources[pkg.Name] = pkgSource{ - arch: chosen.Options().Arch, - fetch: func() (io.ReadSeekCloser, *archive.PackageInfo, error) { - return chosen.Fetch(pkg.RealName) - }, - } - } - return pkgSources, nil -} diff --git a/internal/source/source.go b/internal/source/source.go new file mode 100644 index 00000000..46df8e4a --- /dev/null +++ b/internal/source/source.go @@ -0,0 +1,112 @@ +package source + +import ( + "fmt" + "io" + "slices" + + "github.com/canonical/chisel/internal/archive" + "github.com/canonical/chisel/internal/setup" +) + +// Source is a resolved package source, abstracting over archives and stores. +type Source interface { + Arch() string + Fetch() (io.ReadSeekCloser, *archive.PackageInfo, error) +} + +// archiveSource adapts an archive.Archive to the Source interface for a +// specific package. +type archiveSource struct { + archive archive.Archive + name string +} + +func (a *archiveSource) Arch() string { + return a.archive.Options().Arch +} + +func (a *archiveSource) Fetch() (io.ReadSeekCloser, *archive.PackageInfo, error) { + return a.archive.Fetch(a.name) +} + +// storeSource adapts a store to the Source interface for a specific package. +type storeSource struct { + arch string + name string + store string + track string + risk string +} + +func (s *storeSource) Arch() string { + return s.arch +} + +func (s *storeSource) Fetch() (io.ReadSeekCloser, *archive.PackageInfo, error) { + return nil, nil, fmt.Errorf("cannot fetch package %q from store %q: not implemented", s.name, s.store) +} + +// Resolve determines the source for each package in the selection. +// For archive packages it selects the highest priority archive containing the +// package unless a particular archive is pinned within the slice definition +// file. For store packages it records a store source. It returns a map of +// Source indexed by package names. +func Resolve(archives map[string]archive.Archive, selection *setup.Selection) (map[string]Source, error) { + sortedArchives := make([]*setup.Archive, 0, len(selection.Release.Archives)) + for _, archive := range selection.Release.Archives { + if archive.Priority < 0 { + // Ignore negative priority archives unless a package specifically + // asks for it with the "archive" field. + continue + } + sortedArchives = append(sortedArchives, archive) + } + slices.SortFunc(sortedArchives, func(a, b *setup.Archive) int { + return b.Priority - a.Priority + }) + + sources := make(map[string]Source) + for _, s := range selection.Slices { + if _, ok := sources[s.Package]; ok { + continue + } + pkg := selection.Release.Packages[s.Package] + if pkg.Store != "" { + sources[pkg.Name] = &storeSource{ + name: pkg.Name, + store: pkg.Store, + // TODO: populate arch, track and risk when implementing + // fetching from the store. + } + continue + } + + var candidates []*setup.Archive + if pkg.Archive == "" { + // If the package has not pinned any archive, choose the highest + // priority archive in which the package exists. + candidates = sortedArchives + } else { + candidates = []*setup.Archive{selection.Release.Archives[pkg.Archive]} + } + + var chosen archive.Archive + for _, archiveInfo := range candidates { + archive := archives[archiveInfo.Name] + if archive != nil && archive.Exists(pkg.RealName) { + chosen = archive + break + } + } + if chosen == nil { + return nil, fmt.Errorf("cannot find package %q in archive(s)", pkg.RealName) + } + sources[pkg.Name] = &archiveSource{ + archive: chosen, + name: pkg.RealName, + } + } + + return sources, nil +} diff --git a/internal/source/source_test.go b/internal/source/source_test.go new file mode 100644 index 00000000..edf3ed92 --- /dev/null +++ b/internal/source/source_test.go @@ -0,0 +1,275 @@ +package source_test + +import ( + "os" + "path/filepath" + "slices" + + . "gopkg.in/check.v1" + + "github.com/canonical/chisel/internal/archive" + "github.com/canonical/chisel/internal/setup" + "github.com/canonical/chisel/internal/source" + "github.com/canonical/chisel/internal/testutil" +) + +var testKey = testutil.PGPKeys["key1"] + +type sourceTest struct { + summary string + arch string + release map[string]string + pkgs []*testutil.TestPackage + slices []setup.SliceKey + archs map[string]string + fetchErrors map[string]string + error string +} + +var sourceTests = []sourceTest{{ + summary: "Highest priority archive is selected", + slices: []setup.SliceKey{{"test-package", "myslice"}}, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Hash: "h1", + Version: "v1", + Arch: "amd64", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Reg(0644, "./file", "from foo"), + }), + Archives: []string{"foo"}, + }}, + arch: "amd64", + release: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlTwoArchives, + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + myslice: + contents: + /file: + `, + }, + archs: map[string]string{"test-package": "amd64"}, +}, { + summary: "Pinned archive bypasses higher priority", + slices: []setup.SliceKey{{"test-package", "myslice"}}, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Hash: "h1", + Version: "v1", + Arch: "amd64", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Reg(0644, "./file", "from foo"), + }), + Archives: []string{"foo"}, + }, { + Name: "test-package", + Hash: "h2", + Version: "v2", + Arch: "amd64", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Reg(0644, "./file", "from bar"), + }), + Archives: []string{"bar"}, + }}, + arch: "amd64", + release: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlTwoArchives, + "slices/mydir/test-package.yaml": ` + package: test-package + archive: bar + slices: + myslice: + contents: + /file: + `, + }, + archs: map[string]string{"test-package": "amd64"}, +}, { + summary: "Pinned archive not available fails", + slices: []setup.SliceKey{{"test-package", "myslice"}}, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Hash: "h1", + Version: "v1", + Arch: "amd64", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Reg(0644, "./file", "from foo"), + }), + Archives: []string{"foo"}, + }}, + arch: "amd64", + release: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlTwoArchives, + "slices/mydir/test-package.yaml": ` + package: test-package + archive: bar + slices: + myslice: + contents: + /file: + `, + }, + error: `cannot find package "test-package" in archive\(s\)`, +}, { + summary: "No archives have the package", + slices: []setup.SliceKey{{"test-package", "myslice"}}, + pkgs: []*testutil.TestPackage{}, + arch: "amd64", + release: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlTwoArchives, + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + myslice: + contents: + /file: + `, + }, + error: `cannot find package "test-package" in archive\(s\)`, +}, { + summary: "Negative priority archives are ignored when not explicitly pinned", + slices: []setup.SliceKey{{"test-package", "myslice"}}, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Reg(0644, "./file", "from foo"), + }), + Archives: []string{"foo"}, + }}, + arch: "amd64", + release: map[string]string{ + "chisel.yaml": ` + format: v1 + maintenance: + standard: 2025-01-01 + end-of-life: 2100-01-01 + archives: + foo: + version: 22.04 + components: [main, universe] + suites: [jammy] + priority: -20 + public-keys: [test-key] + public-keys: + test-key: + id: ` + testKey.ID + ` + armor: |` + "\n" + testutil.PrefixEachLine(testKey.PubKeyArmor, "\t\t\t\t\t\t") + ` + `, + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + myslice: + contents: + /file: + `, + }, + error: `cannot find package "test-package" in archive\(s\)`, +}, { + summary: "Store package fetching not yet implemented", + slices: []setup.SliceKey{{"test-package", "myslice"}, {"bin-store-pkg", "myslice"}}, + arch: "amd64", + release: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlWithStores, + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + myslice: + contents: + /dir/file: + `, + "slices/mydir/store-pkg.yaml": ` + package: store-pkg + store: bin + default-track: 3.1 + slices: + myslice: + contents: + /dir/store-file: + `, + }, + fetchErrors: map[string]string{ + "bin-store-pkg": `cannot fetch package "bin-store-pkg" from store "bin": not implemented`, + }, +}} + +func (s *S) TestResolve(c *C) { + for _, test := range sourceTests { + c.Logf("Summary: %s", test.summary) + + if _, ok := test.release["chisel.yaml"]; !ok { + test.release["chisel.yaml"] = testutil.DefaultChiselYaml + } + if test.pkgs == nil { + test.pkgs = []*testutil.TestPackage{{ + Name: "test-package", + Data: testutil.PackageData["test-package"], + }} + } + for _, pkg := range test.pkgs { + if pkg.Arch == "" { + pkg.Arch = "arch" + } + if pkg.Hash == "" { + pkg.Hash = "hash" + } + if pkg.Version == "" { + pkg.Version = "version" + } + } + + releaseDir := c.MkDir() + for path, data := range test.release { + fpath := filepath.Join(releaseDir, path) + err := os.MkdirAll(filepath.Dir(fpath), 0755) + c.Assert(err, IsNil) + err = os.WriteFile(fpath, testutil.Reindent(data), 0644) + c.Assert(err, IsNil) + } + + release, err := setup.ReadRelease(releaseDir) + c.Assert(err, IsNil) + + selection, err := setup.Select(release, test.slices, test.arch) + c.Assert(err, IsNil) + + archives := map[string]archive.Archive{} + for name, setupArchive := range release.Archives { + pkgs := make(map[string]*testutil.TestPackage) + for _, pkg := range test.pkgs { + if len(pkg.Archives) == 0 || slices.Contains(pkg.Archives, name) { + pkgs[pkg.Name] = pkg + } + } + archive := &testutil.TestArchive{ + Opts: archive.Options{ + Label: setupArchive.Name, + Version: setupArchive.Version, + Suites: setupArchive.Suites, + Components: setupArchive.Components, + Pro: setupArchive.Pro, + Arch: test.arch, + }, + Packages: pkgs, + } + archives[name] = archive + } + + sources, err := source.Resolve(archives, selection) + if test.error != "" { + c.Assert(err, ErrorMatches, test.error) + continue + } + c.Assert(err, IsNil) + + for pkgName, arch := range test.archs { + c.Assert(sources[pkgName].Arch(), Equals, arch) + } + + for pkgName, fetchErr := range test.fetchErrors { + _, _, err := sources[pkgName].Fetch() + c.Assert(err, ErrorMatches, fetchErr) + } + } +} diff --git a/internal/source/suite_test.go b/internal/source/suite_test.go new file mode 100644 index 00000000..f720627f --- /dev/null +++ b/internal/source/suite_test.go @@ -0,0 +1,13 @@ +package source_test + +import ( + "testing" + + . "gopkg.in/check.v1" +) + +func Test(t *testing.T) { TestingT(t) } + +type S struct{} + +var _ = Suite(&S{}) diff --git a/internal/testutil/defaults.go b/internal/testutil/defaults.go index 046d4636..2b4b318c 100644 --- a/internal/testutil/defaults.go +++ b/internal/testutil/defaults.go @@ -27,3 +27,26 @@ var DefaultChiselYamlWithStores = strings.ReplaceAll(DefaultChiselYaml, "format: version: 26.10 default-prefix: "bin-" ` + +var DefaultChiselYamlTwoArchives = ` + format: v1 + maintenance: + standard: 2025-01-01 + end-of-life: 2100-01-01 + archives: + foo: + version: 22.04 + components: [main, universe] + suites: [jammy] + priority: 20 + public-keys: [test-key] + bar: + version: 22.04 + components: [main] + suites: [jammy] + priority: 10 + public-keys: [test-key] + public-keys: + test-key: + id: ` + testKey.ID + ` + armor: |` + "\n" + PrefixEachLine(testKey.PubKeyArmor, "\t\t\t\t\t\t") From 268501e7e436b1ba2a3dd937c423d3c790bc8548 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 19 Aug 2026 08:25:33 +0200 Subject: [PATCH 07/24] style: remove unused track and risk --- internal/source/source.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/source/source.go b/internal/source/source.go index 46df8e4a..217b5db6 100644 --- a/internal/source/source.go +++ b/internal/source/source.go @@ -35,8 +35,6 @@ type storeSource struct { arch string name string store string - track string - risk string } func (s *storeSource) Arch() string { From 107103d0eb9078569dbbf423a6c315709d49fe2f Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 1 Sep 2026 14:35:18 +0200 Subject: [PATCH 08/24] fix: rework fetcher interface Signed-off-by: Paul Mars --- internal/slicer/export_test.go | 3 + .../{source/source.go => slicer/fetch.go} | 60 ++++++++++--------- .../source_test.go => slicer/fetch_test.go} | 42 ++++++------- internal/slicer/slicer.go | 11 ++-- internal/source/suite_test.go | 13 ---- 5 files changed, 62 insertions(+), 67 deletions(-) create mode 100644 internal/slicer/export_test.go rename internal/{source/source.go => slicer/fetch.go} (56%) rename internal/{source/source_test.go => slicer/fetch_test.go} (86%) delete mode 100644 internal/source/suite_test.go diff --git a/internal/slicer/export_test.go b/internal/slicer/export_test.go new file mode 100644 index 00000000..9fee3531 --- /dev/null +++ b/internal/slicer/export_test.go @@ -0,0 +1,3 @@ +package slicer + +var ResolveFetchers = resolveFetchers diff --git a/internal/source/source.go b/internal/slicer/fetch.go similarity index 56% rename from internal/source/source.go rename to internal/slicer/fetch.go index 217b5db6..65a23903 100644 --- a/internal/source/source.go +++ b/internal/slicer/fetch.go @@ -1,4 +1,4 @@ -package source +package slicer import ( "fmt" @@ -9,48 +9,54 @@ import ( "github.com/canonical/chisel/internal/setup" ) -// Source is a resolved package source, abstracting over archives and stores. -type Source interface { +// Fetcher fetches a package from the location selected for it in the +// release. +type Fetcher interface { Arch() string Fetch() (io.ReadSeekCloser, *archive.PackageInfo, error) } -// archiveSource adapts an archive.Archive to the Source interface for a -// specific package. -type archiveSource struct { +var ( + _ Fetcher = (*debFetcher)(nil) + _ Fetcher = (*binFetcher)(nil) +) + +// debFetcher fetches packages from an archive. +type debFetcher struct { archive archive.Archive name string } -func (a *archiveSource) Arch() string { - return a.archive.Options().Arch +func (d *debFetcher) Arch() string { + return d.archive.Options().Arch } -func (a *archiveSource) Fetch() (io.ReadSeekCloser, *archive.PackageInfo, error) { - return a.archive.Fetch(a.name) +func (d *debFetcher) Fetch() (io.ReadSeekCloser, *archive.PackageInfo, error) { + return d.archive.Fetch(d.name) } -// storeSource adapts a store to the Source interface for a specific package. -type storeSource struct { +// binFetcher fetches bin packages from a store. +type binFetcher struct { arch string name string store string } -func (s *storeSource) Arch() string { - return s.arch +func (b *binFetcher) Arch() string { + return b.arch } -func (s *storeSource) Fetch() (io.ReadSeekCloser, *archive.PackageInfo, error) { - return nil, nil, fmt.Errorf("cannot fetch package %q from store %q: not implemented", s.name, s.store) +func (b *binFetcher) Fetch() (io.ReadSeekCloser, *archive.PackageInfo, error) { + return nil, nil, fmt.Errorf("cannot fetch package %q from store %q: not implemented", b.name, b.store) } -// Resolve determines the source for each package in the selection. -// For archive packages it selects the highest priority archive containing the -// package unless a particular archive is pinned within the slice definition -// file. For store packages it records a store source. It returns a map of -// Source indexed by package names. -func Resolve(archives map[string]archive.Archive, selection *setup.Selection) (map[string]Source, error) { +// resolveFetchers determines the fetcher for each package in the selection. +// For packages from an archive it selects the highest priority archive +// containing the package unless a particular archive is pinned within the +// slice definition file. For packages from a store it selects the store +// named in the slice definition file. It returns a map of Fetcher indexed +// by package names. +func resolveFetchers(archives map[string]archive.Archive, selection *setup.Selection) (map[string]Fetcher, error) { sortedArchives := make([]*setup.Archive, 0, len(selection.Release.Archives)) for _, archive := range selection.Release.Archives { if archive.Priority < 0 { @@ -64,14 +70,14 @@ func Resolve(archives map[string]archive.Archive, selection *setup.Selection) (m return b.Priority - a.Priority }) - sources := make(map[string]Source) + fetchers := make(map[string]Fetcher) for _, s := range selection.Slices { - if _, ok := sources[s.Package]; ok { + if _, ok := fetchers[s.Package]; ok { continue } pkg := selection.Release.Packages[s.Package] if pkg.Store != "" { - sources[pkg.Name] = &storeSource{ + fetchers[pkg.Name] = &binFetcher{ name: pkg.Name, store: pkg.Store, // TODO: populate arch, track and risk when implementing @@ -100,11 +106,11 @@ func Resolve(archives map[string]archive.Archive, selection *setup.Selection) (m if chosen == nil { return nil, fmt.Errorf("cannot find package %q in archive(s)", pkg.RealName) } - sources[pkg.Name] = &archiveSource{ + fetchers[pkg.Name] = &debFetcher{ archive: chosen, name: pkg.RealName, } } - return sources, nil + return fetchers, nil } diff --git a/internal/source/source_test.go b/internal/slicer/fetch_test.go similarity index 86% rename from internal/source/source_test.go rename to internal/slicer/fetch_test.go index edf3ed92..eb5832e5 100644 --- a/internal/source/source_test.go +++ b/internal/slicer/fetch_test.go @@ -1,4 +1,4 @@ -package source_test +package slicer_test import ( "os" @@ -9,13 +9,13 @@ import ( "github.com/canonical/chisel/internal/archive" "github.com/canonical/chisel/internal/setup" - "github.com/canonical/chisel/internal/source" + "github.com/canonical/chisel/internal/slicer" "github.com/canonical/chisel/internal/testutil" ) -var testKey = testutil.PGPKeys["key1"] +var fetchTestKey = testutil.PGPKeys["key1"] -type sourceTest struct { +type fetchTest struct { summary string arch string release map[string]string @@ -26,7 +26,7 @@ type sourceTest struct { error string } -var sourceTests = []sourceTest{{ +var fetchTests = []fetchTest{{ summary: "Highest priority archive is selected", slices: []setup.SliceKey{{"test-package", "myslice"}}, pkgs: []*testutil.TestPackage{{ @@ -35,7 +35,7 @@ var sourceTests = []sourceTest{{ Version: "v1", Arch: "amd64", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0644, "./file", "from foo"), + testutil.Reg(0o644, "./file", "from foo"), }), Archives: []string{"foo"}, }}, @@ -60,7 +60,7 @@ var sourceTests = []sourceTest{{ Version: "v1", Arch: "amd64", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0644, "./file", "from foo"), + testutil.Reg(0o644, "./file", "from foo"), }), Archives: []string{"foo"}, }, { @@ -69,7 +69,7 @@ var sourceTests = []sourceTest{{ Version: "v2", Arch: "amd64", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0644, "./file", "from bar"), + testutil.Reg(0o644, "./file", "from bar"), }), Archives: []string{"bar"}, }}, @@ -95,7 +95,7 @@ var sourceTests = []sourceTest{{ Version: "v1", Arch: "amd64", Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0644, "./file", "from foo"), + testutil.Reg(0o644, "./file", "from foo"), }), Archives: []string{"foo"}, }}, @@ -134,7 +134,7 @@ var sourceTests = []sourceTest{{ 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"}, }}, @@ -154,8 +154,8 @@ var sourceTests = []sourceTest{{ public-keys: [test-key] public-keys: test-key: - id: ` + testKey.ID + ` - armor: |` + "\n" + testutil.PrefixEachLine(testKey.PubKeyArmor, "\t\t\t\t\t\t") + ` + id: ` + fetchTestKey.ID + ` + armor: |` + "\n" + testutil.PrefixEachLine(fetchTestKey.PubKeyArmor, "\t\t\t\t\t\t") + ` `, "slices/mydir/test-package.yaml": ` package: test-package @@ -194,8 +194,8 @@ var sourceTests = []sourceTest{{ }, }} -func (s *S) TestResolve(c *C) { - for _, test := range sourceTests { +func (s *S) TestResolveFetchers(c *C) { + for _, test := range fetchTests { c.Logf("Summary: %s", test.summary) if _, ok := test.release["chisel.yaml"]; !ok { @@ -222,9 +222,9 @@ func (s *S) TestResolve(c *C) { 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) } @@ -242,7 +242,7 @@ func (s *S) TestResolve(c *C) { pkgs[pkg.Name] = pkg } } - archive := &testutil.TestArchive{ + arc := &testutil.TestArchive{ Opts: archive.Options{ Label: setupArchive.Name, Version: setupArchive.Version, @@ -253,10 +253,10 @@ func (s *S) TestResolve(c *C) { }, Packages: pkgs, } - archives[name] = archive + archives[name] = arc } - sources, err := source.Resolve(archives, selection) + fetchers, err := slicer.ResolveFetchers(archives, selection) if test.error != "" { c.Assert(err, ErrorMatches, test.error) continue @@ -264,11 +264,11 @@ func (s *S) TestResolve(c *C) { c.Assert(err, IsNil) for pkgName, arch := range test.archs { - c.Assert(sources[pkgName].Arch(), Equals, arch) + c.Assert(fetchers[pkgName].Arch(), Equals, arch) } for pkgName, fetchErr := range test.fetchErrors { - _, _, err := sources[pkgName].Fetch() + _, _, err := fetchers[pkgName].Fetch() c.Assert(err, ErrorMatches, fetchErr) } } diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 2f6a18bb..3223febe 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -20,11 +20,10 @@ import ( "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/scripts" "github.com/canonical/chisel/internal/setup" - "github.com/canonical/chisel/internal/source" "github.com/canonical/chisel/internal/tarball" ) -const manifestMode fs.FileMode = 0644 +const manifestMode fs.FileMode = 0o644 type RunOptions struct { Selection *setup.Selection @@ -91,7 +90,7 @@ func Run(options *RunOptions) error { targetDir = filepath.Join(dir, targetDir) } - pkgSources, err := source.Resolve(options.Archives, options.Selection) + pkgFetchers, err := resolveFetchers(options.Archives, options.Selection) if err != nil { return err } @@ -109,7 +108,7 @@ func Run(options *RunOptions) error { extractPackage = make(map[string][]tarball.ExtractInfo) extract[slice.Package] = extractPackage } - arch := pkgSources[slice.Package].Arch() + arch := pkgFetchers[slice.Package].Arch() for targetPath, pathInfo := range slice.Contents { if targetPath == "" { continue @@ -154,7 +153,7 @@ func Run(options *RunOptions) error { continue } pkg := options.Selection.Release.Packages[slice.Package] - reader, info, err := pkgSources[pkg.Name].Fetch() + reader, info, err := pkgFetchers[pkg.Name].Fetch() if err != nil { return err } @@ -271,7 +270,7 @@ func Run(options *RunOptions) error { // them to the appropriate slices. relPaths := map[string][]*setup.Slice{} for _, slice := range options.Selection.Slices { - arch := pkgSources[slice.Package].Arch() + arch := pkgFetchers[slice.Package].Arch() for relPath, pathInfo := range slice.Contents { if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, arch) { continue diff --git a/internal/source/suite_test.go b/internal/source/suite_test.go deleted file mode 100644 index f720627f..00000000 --- a/internal/source/suite_test.go +++ /dev/null @@ -1,13 +0,0 @@ -package source_test - -import ( - "testing" - - . "gopkg.in/check.v1" -) - -func Test(t *testing.T) { TestingT(t) } - -type S struct{} - -var _ = Suite(&S{}) From 4cfed24bbd74774a30733d40086ce7332dc42c7a Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 1 Sep 2026 14:52:38 +0200 Subject: [PATCH 09/24] refactor: use the manifestutil.PackageInfo interface Signed-off-by: Paul Mars --- internal/slicer/fetch.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/slicer/fetch.go b/internal/slicer/fetch.go index 65a23903..4315e352 100644 --- a/internal/slicer/fetch.go +++ b/internal/slicer/fetch.go @@ -6,6 +6,7 @@ import ( "slices" "github.com/canonical/chisel/internal/archive" + "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/setup" ) @@ -13,7 +14,7 @@ import ( // release. type Fetcher interface { Arch() string - Fetch() (io.ReadSeekCloser, *archive.PackageInfo, error) + Fetch() (io.ReadSeekCloser, manifestutil.PackageInfo, error) } var ( @@ -21,7 +22,7 @@ var ( _ Fetcher = (*binFetcher)(nil) ) -// debFetcher fetches packages from an archive. +// debFetcher fetches deb packages from an archive. type debFetcher struct { archive archive.Archive name string @@ -31,7 +32,7 @@ func (d *debFetcher) Arch() string { return d.archive.Options().Arch } -func (d *debFetcher) Fetch() (io.ReadSeekCloser, *archive.PackageInfo, error) { +func (d *debFetcher) Fetch() (io.ReadSeekCloser, manifestutil.PackageInfo, error) { return d.archive.Fetch(d.name) } @@ -46,7 +47,7 @@ func (b *binFetcher) Arch() string { return b.arch } -func (b *binFetcher) Fetch() (io.ReadSeekCloser, *archive.PackageInfo, error) { +func (b *binFetcher) Fetch() (io.ReadSeekCloser, manifestutil.PackageInfo, error) { return nil, nil, fmt.Errorf("cannot fetch package %q from store %q: not implemented", b.name, b.store) } From 7b124b359c1b529e29a3d75fb695bf3b64cad0e4 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 2 Sep 2026 09:16:34 +0200 Subject: [PATCH 10/24] test: remove redundant tests Signed-off-by: Paul Mars --- internal/slicer/slicer_test.go | 144 --------------------------------- 1 file changed, 144 deletions(-) diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 954602de..9654a5d8 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -930,126 +930,6 @@ var slicerTests = []slicerTest{{ manifestPkgs: map[string]string{ "test-package": "test-package v2 a2 h2", }, -}, { - summary: "Pinned archive does not have the package", - slices: []setup.SliceKey{{"test-package", "myslice"}}, - pkgs: []*testutil.TestPackage{{ - Name: "test-package", - Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0644, "./file", "from foo"), - }), - Archives: []string{"foo"}, - }}, - release: map[string]string{ - "chisel.yaml": ` - format: v1 - maintenance: - standard: 2025-01-01 - end-of-life: 2100-01-01 - archives: - foo: - version: 22.04 - components: [main, universe] - suites: [jammy] - priority: 20 - public-keys: [test-key] - bar: - version: 22.04 - components: [main] - suites: [jammy] - priority: 10 - public-keys: [test-key] - public-keys: - test-key: - id: ` + testKey.ID + ` - armor: |` + "\n" + testutil.PrefixEachLine(testKey.PubKeyArmor, "\t\t\t\t\t\t") + ` - `, - "slices/mydir/test-package.yaml": ` - package: test-package - archive: bar - slices: - myslice: - contents: - /file: - `, - }, - // Although archive "foo" does have the package, since archive "bar" has - // been pinned in the slice definition, no other archives will be checked. - error: `cannot find package "test-package" in archive\(s\)`, -}, { - summary: "No archives have the package", - slices: []setup.SliceKey{{"test-package", "myslice"}}, - pkgs: []*testutil.TestPackage{}, - release: map[string]string{ - "chisel.yaml": ` - format: v1 - maintenance: - standard: 2025-01-01 - end-of-life: 2100-01-01 - archives: - foo: - version: 22.04 - components: [main, universe] - suites: [jammy] - priority: 20 - public-keys: [test-key] - bar: - version: 22.04 - components: [main] - suites: [jammy] - priority: 10 - public-keys: [test-key] - public-keys: - test-key: - id: ` + testKey.ID + ` - armor: |` + "\n" + testutil.PrefixEachLine(testKey.PubKeyArmor, "\t\t\t\t\t\t") + ` - `, - "slices/mydir/test-package.yaml": ` - package: test-package - slices: - myslice: - contents: - /file: - `, - }, - error: `cannot find package "test-package" in archive\(s\)`, -}, { - summary: "Negative priority archives are ignored when not explicitly pinned in package", - slices: []setup.SliceKey{{"test-package", "myslice"}}, - pkgs: []*testutil.TestPackage{{ - Name: "test-package", - Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0644, "./file", "from foo"), - }), - Archives: []string{"foo"}, - }}, - release: map[string]string{ - "chisel.yaml": ` - format: v1 - maintenance: - standard: 2025-01-01 - end-of-life: 2100-01-01 - archives: - foo: - version: 22.04 - components: [main, universe] - suites: [jammy] - priority: -20 - public-keys: [test-key] - public-keys: - test-key: - id: ` + testKey.ID + ` - armor: |` + "\n" + testutil.PrefixEachLine(testKey.PubKeyArmor, "\t\t\t\t\t\t") + ` - `, - "slices/mydir/test-package.yaml": ` - package: test-package - slices: - myslice: - contents: - /file: - `, - }, - error: `cannot find package "test-package" in archive\(s\)`, }, { summary: "Negative priority archive explicitly pinned in package", slices: []setup.SliceKey{{"test-package", "myslice"}}, @@ -1978,30 +1858,6 @@ var slicerTests = []slicerTest{{ manifestPaths: map[string]string{ "/dir/file": "file 0644 cc55e2ec {test-package_third}", }, -}, { - summary: "Store package fetching not yet implemented", - slices: []setup.SliceKey{{"test-package", "myslice"}, {"bin-store-pkg", "myslice"}}, - arch: "amd64", - release: map[string]string{ - "chisel.yaml": testutil.DefaultChiselYamlWithStores, - "slices/mydir/test-package.yaml": ` - package: test-package - slices: - myslice: - contents: - /dir/file: - `, - "slices/mydir/store-pkg.yaml": ` - package: store-pkg - store: bin - default-track: 3.1 - slices: - myslice: - contents: - /dir/store-file: - `, - }, - error: `cannot fetch package "bin-store-pkg" from store "bin": not implemented`, }} func (s *S) TestRun(c *C) { From 076ff37ddf88a5e89828db0b9f60e2b10fb34c83 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 2 Sep 2026 16:04:47 +0200 Subject: [PATCH 11/24] style: renaming and tests simplification --- internal/slicer/export_test.go | 3 - internal/slicer/fetch.go | 4 +- internal/slicer/fetch_test.go | 275 --------------------------------- internal/slicer/slicer.go | 2 +- internal/slicer/slicer_test.go | 100 ++++++++++++ 5 files changed, 103 insertions(+), 281 deletions(-) delete mode 100644 internal/slicer/export_test.go delete mode 100644 internal/slicer/fetch_test.go diff --git a/internal/slicer/export_test.go b/internal/slicer/export_test.go deleted file mode 100644 index 9fee3531..00000000 --- a/internal/slicer/export_test.go +++ /dev/null @@ -1,3 +0,0 @@ -package slicer - -var ResolveFetchers = resolveFetchers diff --git a/internal/slicer/fetch.go b/internal/slicer/fetch.go index 4315e352..4f8598b2 100644 --- a/internal/slicer/fetch.go +++ b/internal/slicer/fetch.go @@ -51,13 +51,13 @@ func (b *binFetcher) Fetch() (io.ReadSeekCloser, manifestutil.PackageInfo, error return nil, nil, fmt.Errorf("cannot fetch package %q from store %q: not implemented", b.name, b.store) } -// resolveFetchers determines the fetcher for each package in the selection. +// selectPkgFetchers determines the fetcher for each package in the selection. // For packages from an archive it selects the highest priority archive // containing the package unless a particular archive is pinned within the // slice definition file. For packages from a store it selects the store // named in the slice definition file. It returns a map of Fetcher indexed // by package names. -func resolveFetchers(archives map[string]archive.Archive, selection *setup.Selection) (map[string]Fetcher, error) { +func selectPkgFetchers(archives map[string]archive.Archive, selection *setup.Selection) (map[string]Fetcher, error) { sortedArchives := make([]*setup.Archive, 0, len(selection.Release.Archives)) for _, archive := range selection.Release.Archives { if archive.Priority < 0 { diff --git a/internal/slicer/fetch_test.go b/internal/slicer/fetch_test.go deleted file mode 100644 index eb5832e5..00000000 --- a/internal/slicer/fetch_test.go +++ /dev/null @@ -1,275 +0,0 @@ -package slicer_test - -import ( - "os" - "path/filepath" - "slices" - - . "gopkg.in/check.v1" - - "github.com/canonical/chisel/internal/archive" - "github.com/canonical/chisel/internal/setup" - "github.com/canonical/chisel/internal/slicer" - "github.com/canonical/chisel/internal/testutil" -) - -var fetchTestKey = testutil.PGPKeys["key1"] - -type fetchTest struct { - summary string - arch string - release map[string]string - pkgs []*testutil.TestPackage - slices []setup.SliceKey - archs map[string]string - fetchErrors map[string]string - error string -} - -var fetchTests = []fetchTest{{ - summary: "Highest priority archive is selected", - slices: []setup.SliceKey{{"test-package", "myslice"}}, - pkgs: []*testutil.TestPackage{{ - Name: "test-package", - Hash: "h1", - Version: "v1", - Arch: "amd64", - Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0o644, "./file", "from foo"), - }), - Archives: []string{"foo"}, - }}, - arch: "amd64", - release: map[string]string{ - "chisel.yaml": testutil.DefaultChiselYamlTwoArchives, - "slices/mydir/test-package.yaml": ` - package: test-package - slices: - myslice: - contents: - /file: - `, - }, - archs: map[string]string{"test-package": "amd64"}, -}, { - summary: "Pinned archive bypasses higher priority", - slices: []setup.SliceKey{{"test-package", "myslice"}}, - pkgs: []*testutil.TestPackage{{ - Name: "test-package", - Hash: "h1", - Version: "v1", - Arch: "amd64", - Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0o644, "./file", "from foo"), - }), - Archives: []string{"foo"}, - }, { - Name: "test-package", - Hash: "h2", - Version: "v2", - Arch: "amd64", - Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0o644, "./file", "from bar"), - }), - Archives: []string{"bar"}, - }}, - arch: "amd64", - release: map[string]string{ - "chisel.yaml": testutil.DefaultChiselYamlTwoArchives, - "slices/mydir/test-package.yaml": ` - package: test-package - archive: bar - slices: - myslice: - contents: - /file: - `, - }, - archs: map[string]string{"test-package": "amd64"}, -}, { - summary: "Pinned archive not available fails", - slices: []setup.SliceKey{{"test-package", "myslice"}}, - pkgs: []*testutil.TestPackage{{ - Name: "test-package", - Hash: "h1", - Version: "v1", - Arch: "amd64", - Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0o644, "./file", "from foo"), - }), - Archives: []string{"foo"}, - }}, - arch: "amd64", - release: map[string]string{ - "chisel.yaml": testutil.DefaultChiselYamlTwoArchives, - "slices/mydir/test-package.yaml": ` - package: test-package - archive: bar - slices: - myslice: - contents: - /file: - `, - }, - error: `cannot find package "test-package" in archive\(s\)`, -}, { - summary: "No archives have the package", - slices: []setup.SliceKey{{"test-package", "myslice"}}, - pkgs: []*testutil.TestPackage{}, - arch: "amd64", - release: map[string]string{ - "chisel.yaml": testutil.DefaultChiselYamlTwoArchives, - "slices/mydir/test-package.yaml": ` - package: test-package - slices: - myslice: - contents: - /file: - `, - }, - error: `cannot find package "test-package" in archive\(s\)`, -}, { - summary: "Negative priority archives are ignored when not explicitly pinned", - slices: []setup.SliceKey{{"test-package", "myslice"}}, - pkgs: []*testutil.TestPackage{{ - Name: "test-package", - Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Reg(0o644, "./file", "from foo"), - }), - Archives: []string{"foo"}, - }}, - arch: "amd64", - release: map[string]string{ - "chisel.yaml": ` - format: v1 - maintenance: - standard: 2025-01-01 - end-of-life: 2100-01-01 - archives: - foo: - version: 22.04 - components: [main, universe] - suites: [jammy] - priority: -20 - public-keys: [test-key] - public-keys: - test-key: - id: ` + fetchTestKey.ID + ` - armor: |` + "\n" + testutil.PrefixEachLine(fetchTestKey.PubKeyArmor, "\t\t\t\t\t\t") + ` - `, - "slices/mydir/test-package.yaml": ` - package: test-package - slices: - myslice: - contents: - /file: - `, - }, - error: `cannot find package "test-package" in archive\(s\)`, -}, { - summary: "Store package fetching not yet implemented", - slices: []setup.SliceKey{{"test-package", "myslice"}, {"bin-store-pkg", "myslice"}}, - arch: "amd64", - release: map[string]string{ - "chisel.yaml": testutil.DefaultChiselYamlWithStores, - "slices/mydir/test-package.yaml": ` - package: test-package - slices: - myslice: - contents: - /dir/file: - `, - "slices/mydir/store-pkg.yaml": ` - package: store-pkg - store: bin - default-track: 3.1 - slices: - myslice: - contents: - /dir/store-file: - `, - }, - fetchErrors: map[string]string{ - "bin-store-pkg": `cannot fetch package "bin-store-pkg" from store "bin": not implemented`, - }, -}} - -func (s *S) TestResolveFetchers(c *C) { - for _, test := range fetchTests { - c.Logf("Summary: %s", test.summary) - - if _, ok := test.release["chisel.yaml"]; !ok { - test.release["chisel.yaml"] = testutil.DefaultChiselYaml - } - if test.pkgs == nil { - test.pkgs = []*testutil.TestPackage{{ - Name: "test-package", - Data: testutil.PackageData["test-package"], - }} - } - for _, pkg := range test.pkgs { - if pkg.Arch == "" { - pkg.Arch = "arch" - } - if pkg.Hash == "" { - pkg.Hash = "hash" - } - if pkg.Version == "" { - pkg.Version = "version" - } - } - - releaseDir := c.MkDir() - for path, data := range test.release { - fpath := filepath.Join(releaseDir, path) - err := os.MkdirAll(filepath.Dir(fpath), 0o755) - c.Assert(err, IsNil) - err = os.WriteFile(fpath, testutil.Reindent(data), 0o644) - c.Assert(err, IsNil) - } - - release, err := setup.ReadRelease(releaseDir) - c.Assert(err, IsNil) - - selection, err := setup.Select(release, test.slices, test.arch) - c.Assert(err, IsNil) - - archives := map[string]archive.Archive{} - for name, setupArchive := range release.Archives { - pkgs := make(map[string]*testutil.TestPackage) - for _, pkg := range test.pkgs { - if len(pkg.Archives) == 0 || slices.Contains(pkg.Archives, name) { - pkgs[pkg.Name] = pkg - } - } - arc := &testutil.TestArchive{ - Opts: archive.Options{ - Label: setupArchive.Name, - Version: setupArchive.Version, - Suites: setupArchive.Suites, - Components: setupArchive.Components, - Pro: setupArchive.Pro, - Arch: test.arch, - }, - Packages: pkgs, - } - archives[name] = arc - } - - fetchers, err := slicer.ResolveFetchers(archives, selection) - if test.error != "" { - c.Assert(err, ErrorMatches, test.error) - continue - } - c.Assert(err, IsNil) - - for pkgName, arch := range test.archs { - c.Assert(fetchers[pkgName].Arch(), Equals, arch) - } - - for pkgName, fetchErr := range test.fetchErrors { - _, _, err := fetchers[pkgName].Fetch() - c.Assert(err, ErrorMatches, fetchErr) - } - } -} diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 54c4a831..e124ce6e 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -90,7 +90,7 @@ func Run(options *RunOptions) error { targetDir = filepath.Join(dir, targetDir) } - pkgFetchers, err := resolveFetchers(options.Archives, options.Selection) + pkgFetchers, err := selectPkgFetchers(options.Archives, options.Selection) if err != nil { return err } diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 9654a5d8..a2690cdd 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -930,6 +930,82 @@ var slicerTests = []slicerTest{{ manifestPkgs: map[string]string{ "test-package": "test-package v2 a2 h2", }, +}, { + summary: "Pinned archive does not have the package", + slices: []setup.SliceKey{{"test-package", "myslice"}}, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Reg(0o644, "./file", "from foo"), + }), + Archives: []string{"foo"}, + }}, + release: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlTwoArchives, + "slices/mydir/test-package.yaml": ` + package: test-package + archive: bar + slices: + myslice: + contents: + /file: + `, + }, + // Although archive "foo" does have the package, since archive "bar" has + // been pinned in the slice definition, no other archives will be checked. + error: `cannot find package "test-package" in archive\(s\)`, +}, { + summary: "No archives have the package", + slices: []setup.SliceKey{{"test-package", "myslice"}}, + pkgs: []*testutil.TestPackage{}, + release: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlTwoArchives, + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + myslice: + contents: + /file: + `, + }, + error: `cannot find package "test-package" in archive\(s\)`, +}, { + summary: "Negative priority archives are ignored when not explicitly pinned in package", + slices: []setup.SliceKey{{"test-package", "myslice"}}, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Reg(0o644, "./file", "from foo"), + }), + Archives: []string{"foo"}, + }}, + release: map[string]string{ + "chisel.yaml": ` + format: v1 + maintenance: + standard: 2025-01-01 + end-of-life: 2100-01-01 + archives: + foo: + version: 22.04 + components: [main, universe] + suites: [jammy] + priority: -20 + public-keys: [test-key] + public-keys: + test-key: + id: ` + testKey.ID + ` + armor: |` + "\n" + testutil.PrefixEachLine(testKey.PubKeyArmor, "\t\t\t\t\t\t") + ` + `, + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + myslice: + contents: + /file: + `, + }, + error: `cannot find package "test-package" in archive\(s\)`, }, { summary: "Negative priority archive explicitly pinned in package", slices: []setup.SliceKey{{"test-package", "myslice"}}, @@ -1858,6 +1934,30 @@ var slicerTests = []slicerTest{{ manifestPaths: map[string]string{ "/dir/file": "file 0644 cc55e2ec {test-package_third}", }, +}, { + summary: "Store package fetching not yet implemented", + slices: []setup.SliceKey{{"test-package", "myslice"}, {"bin-store-pkg", "myslice"}}, + arch: "amd64", + release: map[string]string{ + "chisel.yaml": testutil.DefaultChiselYamlWithStores, + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + myslice: + contents: + /dir/file: + `, + "slices/mydir/store-pkg.yaml": ` + package: store-pkg + store: bin + default-track: 3.1 + slices: + myslice: + contents: + /dir/store-file: + `, + }, + error: `cannot fetch package "bin-store-pkg" from store "bin": not implemented`, }} func (s *S) TestRun(c *C) { From eaddfd939131e675ec561300f2d41cee5feec65d Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 2 Sep 2026 16:10:02 +0200 Subject: [PATCH 12/24] fix: revert unintended changes --- internal/slicer/slicer.go | 2 +- internal/slicer/slicer_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index e124ce6e..7405fcdb 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -23,7 +23,7 @@ import ( "github.com/canonical/chisel/internal/tarball" ) -const manifestMode fs.FileMode = 0o644 +const manifestMode fs.FileMode = 0644 type RunOptions struct { Selection *setup.Selection diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index a2690cdd..751920f9 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -936,7 +936,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"}, }}, @@ -975,7 +975,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"}, }}, From bb39d06e6ffb555c716e9151eab2c694a1ba5a72 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 3 Sep 2026 11:24:21 +0200 Subject: [PATCH 13/24] docs: use package slices file term --- internal/slicer/fetch.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/slicer/fetch.go b/internal/slicer/fetch.go index 4f8598b2..1429ebc7 100644 --- a/internal/slicer/fetch.go +++ b/internal/slicer/fetch.go @@ -54,8 +54,8 @@ func (b *binFetcher) Fetch() (io.ReadSeekCloser, manifestutil.PackageInfo, error // selectPkgFetchers determines the fetcher for each package in the selection. // For packages from an archive it selects the highest priority archive // containing the package unless a particular archive is pinned within the -// slice definition file. For packages from a store it selects the store -// named in the slice definition file. It returns a map of Fetcher indexed +// package slices file. For packages from a store it selects the store +// named in the package slices file. It returns a map of Fetcher indexed // by package names. func selectPkgFetchers(archives map[string]archive.Archive, selection *setup.Selection) (map[string]Fetcher, error) { sortedArchives := make([]*setup.Archive, 0, len(selection.Release.Archives)) From b930f97804220ada22332761bec917531ba26935 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 3 Sep 2026 13:27:46 +0200 Subject: [PATCH 14/24] test: revert unrelated changes --- internal/slicer/slicer_test.go | 48 ++++++++++++++++++++++++++++++++-- internal/testutil/defaults.go | 23 ---------------- 2 files changed, 46 insertions(+), 25 deletions(-) diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 751920f9..954602de 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -941,7 +941,29 @@ var slicerTests = []slicerTest{{ Archives: []string{"foo"}, }}, release: map[string]string{ - "chisel.yaml": testutil.DefaultChiselYamlTwoArchives, + "chisel.yaml": ` + format: v1 + maintenance: + standard: 2025-01-01 + end-of-life: 2100-01-01 + archives: + foo: + version: 22.04 + components: [main, universe] + suites: [jammy] + priority: 20 + public-keys: [test-key] + bar: + version: 22.04 + components: [main] + suites: [jammy] + priority: 10 + public-keys: [test-key] + public-keys: + test-key: + id: ` + testKey.ID + ` + armor: |` + "\n" + testutil.PrefixEachLine(testKey.PubKeyArmor, "\t\t\t\t\t\t") + ` + `, "slices/mydir/test-package.yaml": ` package: test-package archive: bar @@ -959,7 +981,29 @@ var slicerTests = []slicerTest{{ slices: []setup.SliceKey{{"test-package", "myslice"}}, pkgs: []*testutil.TestPackage{}, release: map[string]string{ - "chisel.yaml": testutil.DefaultChiselYamlTwoArchives, + "chisel.yaml": ` + format: v1 + maintenance: + standard: 2025-01-01 + end-of-life: 2100-01-01 + archives: + foo: + version: 22.04 + components: [main, universe] + suites: [jammy] + priority: 20 + public-keys: [test-key] + bar: + version: 22.04 + components: [main] + suites: [jammy] + priority: 10 + public-keys: [test-key] + public-keys: + test-key: + id: ` + testKey.ID + ` + armor: |` + "\n" + testutil.PrefixEachLine(testKey.PubKeyArmor, "\t\t\t\t\t\t") + ` + `, "slices/mydir/test-package.yaml": ` package: test-package slices: diff --git a/internal/testutil/defaults.go b/internal/testutil/defaults.go index 2b4b318c..046d4636 100644 --- a/internal/testutil/defaults.go +++ b/internal/testutil/defaults.go @@ -27,26 +27,3 @@ var DefaultChiselYamlWithStores = strings.ReplaceAll(DefaultChiselYaml, "format: version: 26.10 default-prefix: "bin-" ` - -var DefaultChiselYamlTwoArchives = ` - format: v1 - maintenance: - standard: 2025-01-01 - end-of-life: 2100-01-01 - archives: - foo: - version: 22.04 - components: [main, universe] - suites: [jammy] - priority: 20 - public-keys: [test-key] - bar: - version: 22.04 - components: [main] - suites: [jammy] - priority: 10 - public-keys: [test-key] - public-keys: - test-key: - id: ` + testKey.ID + ` - armor: |` + "\n" + PrefixEachLine(testKey.PubKeyArmor, "\t\t\t\t\t\t") From 8c24bda9adefeed6ba2ea83455f3c7c2bb08ce26 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 16 Sep 2026 08:31:49 +0200 Subject: [PATCH 15/24] docs: clean useless comment --- internal/slicer/fetch.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/slicer/fetch.go b/internal/slicer/fetch.go index 1429ebc7..e18286b9 100644 --- a/internal/slicer/fetch.go +++ b/internal/slicer/fetch.go @@ -10,8 +10,6 @@ import ( "github.com/canonical/chisel/internal/setup" ) -// Fetcher fetches a package from the location selected for it in the -// release. type Fetcher interface { Arch() string Fetch() (io.ReadSeekCloser, manifestutil.PackageInfo, error) From a13c63703d205e37f439b82f18409ece8161ead9 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 1 Oct 2026 09:24:04 +0200 Subject: [PATCH 16/24] fix: apply review comments --- internal/archive/archive.go | 7 - internal/manifestutil/manifestutil.go | 41 +++++- internal/manifestutil/manifestutil_test.go | 161 ++++++++++++++++----- internal/setup/setup.go | 2 + internal/setup/setup_test.go | 4 + internal/slicer/fetch.go | 115 --------------- internal/slicer/remote_package.go | 69 +++++++++ internal/slicer/slicer.go | 84 ++++++++++- internal/slicer/slicer_test.go | 40 ++++- public/manifest/manifest.go | 1 + public/manifest/manifest_test.go | 10 +- 11 files changed, 361 insertions(+), 173 deletions(-) delete mode 100644 internal/slicer/fetch.go create mode 100644 internal/slicer/remote_package.go diff --git a/internal/archive/archive.go b/internal/archive/archive.go index d301494b..1d3f1af6 100644 --- a/internal/archive/archive.go +++ b/internal/archive/archive.go @@ -32,13 +32,6 @@ type PackageInfo struct { SHA256 string } -func (p *PackageInfo) PkgName() string { return p.Name } -func (p *PackageInfo) PkgVersion() string { return p.Version } -func (p *PackageInfo) PkgRevision() int { return 0 } -func (p *PackageInfo) PkgArch() string { return p.Arch } -func (p *PackageInfo) PkgDigestKind() cache.DigestKind { return cache.SHA256 } -func (p *PackageInfo) PkgDigest() string { return p.SHA256 } - type Options struct { Label string Version string diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 3cf4db75..ac2ee517 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -16,16 +16,16 @@ import ( "github.com/canonical/chisel/public/manifest" ) -// PackageInfo describes a package as obtained from its source, abstracting -// over archives, stores, and any other backend. type PackageInfo interface { PkgName() string + PkgRealName() string PkgVersion() string // PkgRevision further identifies the package when the source versions are // not unique on their own. It returns 0 when the source does not use // revisions. PkgRevision() int PkgArch() string + PkgStore() string PkgDigestKind() cache.DigestKind PkgDigest() string } @@ -85,12 +85,14 @@ func Write(options *WriteOptions, writer io.Writer) error { func manifestAddPackages(dbw *jsonwall.DBWriter, infos []PackageInfo) error { for _, info := range infos { + name, alias := packageNames(info) err := dbw.Add(&manifest.Package{ Kind: "package", - Name: info.PkgName(), + Name: name, + Alias: alias, Version: info.PkgVersion(), - Digest: info.PkgDigest(), Arch: info.PkgArch(), + Digest: info.PkgDigest(), }) if err != nil { return err @@ -99,6 +101,17 @@ func manifestAddPackages(dbw *jsonwall.DBWriter, infos []PackageInfo) error { return nil } +func packageNames(info PackageInfo) (name, alias string) { + name = info.PkgRealName() + if name == "" { + name = info.PkgName() + } + if info.PkgName() != name { + alias = info.PkgName() + } + return name, alias +} + func manifestAddSlices(dbw *jsonwall.DBWriter, slices []*setup.Slice) error { for _, slice := range slices { err := dbw.Add(&manifest.Slice{ @@ -169,7 +182,14 @@ func fastValidate(options *WriteOptions) (err error) { if err != nil { return err } - pkgExist[pkg.PkgName()] = true + name, alias := packageNames(pkg) + if alias != "" { + name = alias + } + if pkgExist[name] { + return fmt.Errorf("package name or alias %q is used more than once", name) + } + pkgExist[name] = true } sliceExist := map[string]bool{} for _, slice := range options.Selection { @@ -265,7 +285,7 @@ func validateReportEntry(entry *ReportEntry) (err error) { } func validatePackage(pkg PackageInfo) (err error) { - name := pkg.PkgName() + name, _ := packageNames(pkg) if name == "" { return fmt.Errorf("package name not set") } @@ -298,7 +318,14 @@ func Validate(mfest *manifest.Manifest) (err error) { pkgExist := map[string]bool{} err = mfest.IteratePackages(func(pkg *manifest.Package) error { - pkgExist[pkg.Name] = true + name := pkg.Name + if pkg.Alias != "" { + name = pkg.Alias + } + if pkgExist[name] { + return fmt.Errorf("package name or alias %q is used more than once", name) + } + pkgExist[name] = true return nil }) if err != nil { diff --git a/internal/manifestutil/manifestutil_test.go b/internal/manifestutil/manifestutil_test.go index e7e68752..a264a138 100644 --- a/internal/manifestutil/manifestutil_test.go +++ b/internal/manifestutil/manifestutil_test.go @@ -12,7 +12,7 @@ import ( . "gopkg.in/check.v1" "github.com/canonical/chisel/internal/apachetestutil" - "github.com/canonical/chisel/internal/archive" + "github.com/canonical/chisel/internal/cache" "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/public/manifest" @@ -119,6 +119,26 @@ var slice2 = &setup.Slice{ Name: "slice2", } +type testPackageInfo struct { + Name string + RealName string + Version string + Revision int + Arch string + Store string + DigestKind cache.DigestKind + Digest string +} + +func (p *testPackageInfo) PkgName() string { return p.Name } +func (p *testPackageInfo) PkgRealName() string { return p.RealName } +func (p *testPackageInfo) PkgVersion() string { return p.Version } +func (p *testPackageInfo) PkgRevision() int { return p.Revision } +func (p *testPackageInfo) PkgArch() string { return p.Arch } +func (p *testPackageInfo) PkgStore() string { return p.Store } +func (p *testPackageInfo) PkgDigestKind() cache.DigestKind { return p.DigestKind } +func (p *testPackageInfo) PkgDigest() string { return p.Digest } + var generateManifestTests = []struct { summary string report *manifestutil.Report @@ -149,17 +169,19 @@ var generateManifestTests = []struct { }, }, packageInfo: []manifestutil.PackageInfo{ - &archive.PackageInfo{ - Name: "package1", - Version: "v1", - Arch: "a1", - SHA256: "s1", + &testPackageInfo{ + Name: "package1", + Version: "v1", + Arch: "a1", + DigestKind: cache.SHA256, + Digest: "s1", }, - &archive.PackageInfo{ - Name: "package2", - Version: "v2", - Arch: "a2", - SHA256: "s2", + &testPackageInfo{ + Name: "package2", + Version: "v2", + Arch: "a2", + DigestKind: cache.SHA256, + Digest: "s2", }, }, expected: &apachetestutil.ManifestContents{ @@ -212,6 +234,24 @@ var generateManifestTests = []struct { Path: "/link", }}, }, +}, { + summary: "Package alias matches slice reference", + selection: []*setup.Slice{{Package: "bin-foo", Name: "libs"}}, + report: &manifestutil.Report{ + Entries: map[string]manifestutil.ReportEntry{}, + }, + packageInfo: []manifestutil.PackageInfo{&testPackageInfo{ + Name: "bin-foo", + RealName: "foo", + Version: "2.10", + Arch: "amd64", + DigestKind: cache.SHA256, + Digest: "sum", + }}, + expected: &apachetestutil.ManifestContents{ + Packages: []*manifest.Package{{Kind: "package", Name: "foo", Alias: "bin-foo", Version: "2.10", Arch: "amd64", Digest: "sum"}}, + Slices: []*manifest.Slice{{Kind: "slice", Name: "bin-foo_libs"}}, + }, }, { summary: "Missing slice", report: &manifestutil.Report{ @@ -399,11 +439,12 @@ var generateManifestTests = []struct { }, }, packageInfo: []manifestutil.PackageInfo{ - &archive.PackageInfo{ - Name: "package1", - Version: "v1", - Arch: "a1", - SHA256: "s1", + &testPackageInfo{ + Name: "package1", + Version: "v1", + Arch: "a1", + DigestKind: cache.SHA256, + Digest: "s1", }, }, expected: &apachetestutil.ManifestContents{ @@ -500,43 +541,63 @@ var generateManifestTests = []struct { }, { summary: "Invalid package: missing name", packageInfo: []manifestutil.PackageInfo{ - &archive.PackageInfo{ - Version: "v1", - Arch: "a1", - SHA256: "s1", + &testPackageInfo{ + Version: "v1", + Arch: "a1", + DigestKind: cache.SHA256, + Digest: "s1", }, }, error: `internal error: invalid manifest: package name not set`, }, { summary: "Invalid package: missing version", packageInfo: []manifestutil.PackageInfo{ - &archive.PackageInfo{ - Name: "package-1", - Arch: "a1", - SHA256: "s1", + &testPackageInfo{ + Name: "package-1", + Arch: "a1", + DigestKind: cache.SHA256, + Digest: "s1", }, }, error: `internal error: invalid manifest: package "package-1" missing version`, }, { summary: "Invalid package: missing arch", packageInfo: []manifestutil.PackageInfo{ - &archive.PackageInfo{ - Name: "package-1", - Version: "v1", - SHA256: "s1", + &testPackageInfo{ + Name: "package-1", + Version: "v1", + DigestKind: cache.SHA256, + Digest: "s1", }, }, error: `internal error: invalid manifest: package "package-1" missing arch`, }, { summary: "Invalid package: missing sha256", packageInfo: []manifestutil.PackageInfo{ - &archive.PackageInfo{ + &testPackageInfo{ Name: "package-1", Version: "v1", Arch: "a1", }, }, error: `internal error: invalid manifest: package "package-1" missing sha256`, +}, { + summary: "Duplicate package alias", + packageInfo: []manifestutil.PackageInfo{&testPackageInfo{ + Name: "bin-foo", + RealName: "foo", + Version: "v1", + Arch: "a1", + DigestKind: cache.SHA256, + Digest: "s1", + }, &testPackageInfo{ + Name: "bin-foo", + Version: "v1", + Arch: "a1", + DigestKind: cache.SHA256, + Digest: "s2", + }}, + error: `internal error: invalid manifest: package name or alias "bin-foo" is used more than once`, }} func (s *S) TestGenerateManifests(c *C) { @@ -547,11 +608,12 @@ func (s *S) TestGenerateManifests(c *C) { } if test.packageInfo == nil { test.packageInfo = []manifestutil.PackageInfo{ - &archive.PackageInfo{ - Name: "package1", - Version: "v1", - Arch: "a1", - SHA256: "s1", + &testPackageInfo{ + Name: "package1", + Version: "v1", + Arch: "a1", + DigestKind: cache.SHA256, + Digest: "s1", }, } } @@ -613,6 +675,37 @@ var validateManifestTests = []struct { {"kind":"slice","name":"pkg1_manifest"} `, error: `invalid manifest: slice pkg1_manifest refers to missing package "pkg1"`, +}, { + summary: "Alias resolves slice reference", + input: ` + {"jsonwall":"1.0","schema":"1.0","count":2} + {"kind":"package","name":"foo","alias":"bin-foo","version":"v1","sha256":"hash1","arch":"amd64"} + {"kind":"slice","name":"bin-foo_libs"} + `, +}, { + summary: "Alias is not interchangeable with real name", + input: ` + {"jsonwall":"1.0","schema":"1.0","count":2} + {"kind":"package","name":"foo","alias":"bin-foo","version":"v1","sha256":"hash1","arch":"amd64"} + {"kind":"slice","name":"foo_libs"} + `, + error: `invalid manifest: slice foo_libs refers to missing package "foo"`, +}, { + summary: "Same real name from different providers", + input: ` + {"jsonwall":"1.0","schema":"1.0","count":3} + {"kind":"package","name":"foo","alias":"bin-foo","version":"v2","sha256":"hash2","arch":"amd64"} + {"kind":"package","name":"foo","version":"v1","sha256":"hash1","arch":"amd64"} + {"kind":"slice","name":"bin-foo_libs"} + `, +}, { + summary: "Duplicate alias is ambiguous", + input: ` + {"jsonwall":"1.0","schema":"1.0","count":2} + {"kind":"package","name":"foo","alias":"bin-foo","version":"v1","sha256":"hash1","arch":"amd64"} + {"kind":"package","name":"other","alias":"bin-foo","version":"v2","sha256":"hash2","arch":"amd64"} + `, + error: `invalid manifest: package name or alias "bin-foo" is used more than once`, }, { summary: "Path not found in contents", input: ` diff --git a/internal/setup/setup.go b/internal/setup/setup.go index 9d7e4a7b..b52aab6a 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -154,6 +154,7 @@ func (s *Slice) String() string { return s.Package + "_" + s.Name } type Selection struct { Release *Release Slices []*Slice + Arch string } // Prefers uses the prefer relationships and returns a map from each path to @@ -508,6 +509,7 @@ func Select(release *Release, slices []SliceKey, arch string) (*Selection, error selection := &Selection{ Release: release, + Arch: arch, } sorted, err := order(release.Packages, slices, arch) diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 3568b39a..656f01ae 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -428,6 +428,7 @@ var setupTests = []setupTest{{ }, selslices: []setup.SliceKey{{"mypkg1", "myslice1"}}, selection: &setup.Selection{ + Arch: "amd64", Slices: []*setup.Slice{{ Package: "mypkg1", Name: "myslice1", @@ -451,6 +452,7 @@ var setupTests = []setupTest{{ }, selslices: []setup.SliceKey{{"mypkg2", "myslice2"}}, selection: &setup.Selection{ + Arch: "amd64", Slices: []*setup.Slice{{ Package: "mypkg1", Name: "myslice1", @@ -1758,6 +1760,7 @@ var setupTests = []setupTest{{ }, selslices: []setup.SliceKey{{"mypkg", "myslice"}}, selection: &setup.Selection{ + Arch: "amd64", Slices: []*setup.Slice{{ Package: "mypkg", Name: "myslice", @@ -4910,6 +4913,7 @@ func (s *S) TestSelectEmptyArch(c *C) { selslice := []setup.SliceKey{{"mypkg", "myslice"}} selection, err := setup.Select(release, selslice, "") c.Assert(err, IsNil) + c.Assert(selection.Arch, Equals, arch) var sliceNames []string for _, s := range selection.Slices { diff --git a/internal/slicer/fetch.go b/internal/slicer/fetch.go deleted file mode 100644 index e18286b9..00000000 --- a/internal/slicer/fetch.go +++ /dev/null @@ -1,115 +0,0 @@ -package slicer - -import ( - "fmt" - "io" - "slices" - - "github.com/canonical/chisel/internal/archive" - "github.com/canonical/chisel/internal/manifestutil" - "github.com/canonical/chisel/internal/setup" -) - -type Fetcher interface { - Arch() string - Fetch() (io.ReadSeekCloser, manifestutil.PackageInfo, error) -} - -var ( - _ Fetcher = (*debFetcher)(nil) - _ Fetcher = (*binFetcher)(nil) -) - -// debFetcher fetches deb packages from an archive. -type debFetcher struct { - archive archive.Archive - name string -} - -func (d *debFetcher) Arch() string { - return d.archive.Options().Arch -} - -func (d *debFetcher) Fetch() (io.ReadSeekCloser, manifestutil.PackageInfo, error) { - return d.archive.Fetch(d.name) -} - -// binFetcher fetches bin packages from a store. -type binFetcher struct { - arch string - name string - store string -} - -func (b *binFetcher) Arch() string { - return b.arch -} - -func (b *binFetcher) Fetch() (io.ReadSeekCloser, manifestutil.PackageInfo, error) { - return nil, nil, fmt.Errorf("cannot fetch package %q from store %q: not implemented", b.name, b.store) -} - -// selectPkgFetchers determines the fetcher for each package in the selection. -// For packages from an archive it selects the highest priority archive -// containing the package unless a particular archive is pinned within the -// package slices file. For packages from a store it selects the store -// named in the package slices file. It returns a map of Fetcher indexed -// by package names. -func selectPkgFetchers(archives map[string]archive.Archive, selection *setup.Selection) (map[string]Fetcher, error) { - sortedArchives := make([]*setup.Archive, 0, len(selection.Release.Archives)) - for _, archive := range selection.Release.Archives { - if archive.Priority < 0 { - // Ignore negative priority archives unless a package specifically - // asks for it with the "archive" field. - continue - } - sortedArchives = append(sortedArchives, archive) - } - slices.SortFunc(sortedArchives, func(a, b *setup.Archive) int { - return b.Priority - a.Priority - }) - - fetchers := make(map[string]Fetcher) - for _, s := range selection.Slices { - if _, ok := fetchers[s.Package]; ok { - continue - } - pkg := selection.Release.Packages[s.Package] - if pkg.Store != "" { - fetchers[pkg.Name] = &binFetcher{ - name: pkg.Name, - store: pkg.Store, - // TODO: populate arch, track and risk when implementing - // fetching from the store. - } - continue - } - - var candidates []*setup.Archive - if pkg.Archive == "" { - // If the package has not pinned any archive, choose the highest - // priority archive in which the package exists. - candidates = sortedArchives - } else { - candidates = []*setup.Archive{selection.Release.Archives[pkg.Archive]} - } - - var chosen archive.Archive - for _, archiveInfo := range candidates { - archive := archives[archiveInfo.Name] - if archive != nil && archive.Exists(pkg.RealName) { - chosen = archive - break - } - } - if chosen == nil { - return nil, fmt.Errorf("cannot find package %q in archive(s)", pkg.RealName) - } - fetchers[pkg.Name] = &debFetcher{ - archive: chosen, - name: pkg.RealName, - } - } - - return fetchers, nil -} diff --git a/internal/slicer/remote_package.go b/internal/slicer/remote_package.go new file mode 100644 index 00000000..488cd1f3 --- /dev/null +++ b/internal/slicer/remote_package.go @@ -0,0 +1,69 @@ +package slicer + +import ( + "fmt" + "io" + + "github.com/canonical/chisel/internal/archive" + "github.com/canonical/chisel/internal/cache" +) + +// RemotePackageInfo describes a package as reported by its provider. +type RemotePackageInfo struct { + Name string + RealName string + Version string + Revision int + Arch string + Store string + DigestKind cache.DigestKind + Digest string +} + +func (p *RemotePackageInfo) PkgName() string { return p.Name } +func (p *RemotePackageInfo) PkgRealName() string { return p.RealName } +func (p *RemotePackageInfo) PkgVersion() string { return p.Version } +func (p *RemotePackageInfo) PkgRevision() int { return p.Revision } +func (p *RemotePackageInfo) PkgArch() string { return p.Arch } +func (p *RemotePackageInfo) PkgStore() string { return p.Store } +func (p *RemotePackageInfo) PkgDigestKind() cache.DigestKind { return p.DigestKind } +func (p *RemotePackageInfo) PkgDigest() string { return p.Digest } + +type RemotePackage interface { + Fetch() (io.ReadSeekCloser, *RemotePackageInfo, error) +} + +var ( + _ RemotePackage = (*debPackage)(nil) + _ RemotePackage = (*binPackage)(nil) +) + +type debPackage struct { + archive archive.Archive + name string +} + +func (d *debPackage) Fetch() (io.ReadSeekCloser, *RemotePackageInfo, error) { + reader, info, err := d.archive.Fetch(d.name) + if err != nil { + return nil, nil, err + } + return reader, &RemotePackageInfo{ + Name: info.Name, + RealName: info.Name, + Version: info.Version, + Arch: info.Arch, + DigestKind: cache.SHA256, + Digest: info.SHA256, + }, nil +} + +type binPackage struct { + name string + realName string + store string +} + +func (b *binPackage) Fetch() (io.ReadSeekCloser, *RemotePackageInfo, error) { + return nil, nil, fmt.Errorf("cannot fetch package %q from store %q: not implemented", b.name, b.store) +} diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 7405fcdb..1bc60ccb 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -16,6 +16,7 @@ import ( "github.com/klauspost/compress/zstd" "github.com/canonical/chisel/internal/archive" + "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/scripts" @@ -76,6 +77,13 @@ func (cc *contentChecker) checkKnown(path string) error { } func Run(options *RunOptions) error { + if options.Selection.Arch == "" { + return fmt.Errorf("internal error: selection missing arch") + } + if err := deb.ValidateArch(options.Selection.Arch); err != nil { + return err + } + oldUmask := syscall.Umask(0) defer func() { syscall.Umask(oldUmask) @@ -90,7 +98,7 @@ func Run(options *RunOptions) error { targetDir = filepath.Join(dir, targetDir) } - pkgFetchers, err := selectPkgFetchers(options.Archives, options.Selection) + remotePackages, err := selectRemotePackages(options.Archives, options.Selection) if err != nil { return err } @@ -108,12 +116,11 @@ func Run(options *RunOptions) error { extractPackage = make(map[string][]tarball.ExtractInfo) extract[slice.Package] = extractPackage } - arch := pkgFetchers[slice.Package].Arch() for targetPath, pathInfo := range slice.Contents { if targetPath == "" { continue } - if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, arch) { + if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, options.Selection.Arch) { continue } if preferredPkg, ok := prefers[targetPath]; ok && preferredPkg.Name != slice.Package { @@ -153,7 +160,7 @@ func Run(options *RunOptions) error { continue } pkg := options.Selection.Release.Packages[slice.Package] - reader, info, err := pkgFetchers[pkg.Name].Fetch() + reader, info, err := remotePackages[pkg.Name].Fetch() if err != nil { return err } @@ -270,9 +277,8 @@ func Run(options *RunOptions) error { // them to the appropriate slices. relPaths := map[string][]*setup.Slice{} for _, slice := range options.Selection.Slices { - arch := pkgFetchers[slice.Package].Arch() for relPath, pathInfo := range slice.Contents { - if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, arch) { + if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, options.Selection.Arch) { continue } if pathInfo.Kind == setup.CopyPath || pathInfo.Kind == setup.GlobPath || @@ -489,3 +495,69 @@ func createFile(targetDir, relPath string, pathInfo setup.PathInfo) (*fsutil.Ent MakeParents: true, }) } + +// selectRemotePackages determines the remote package for each package in the selection. +// For packages from an archive it selects the highest priority archive +// containing the package unless a particular archive is pinned within the +// package slices file. For packages from a store it selects the store +// named in the package slices file. It returns a map of RemotePackage indexed +// by package names. +func selectRemotePackages(archives map[string]archive.Archive, selection *setup.Selection) (map[string]RemotePackage, error) { + sortedArchives := make([]*setup.Archive, 0, len(selection.Release.Archives)) + for _, archive := range selection.Release.Archives { + if archive.Priority < 0 { + // Ignore negative priority archives unless a package specifically + // asks for it with the "archive" field. + continue + } + sortedArchives = append(sortedArchives, archive) + } + slices.SortFunc(sortedArchives, func(a, b *setup.Archive) int { + return b.Priority - a.Priority + }) + + remotePackages := make(map[string]RemotePackage) + for _, s := range selection.Slices { + if _, ok := remotePackages[s.Package]; ok { + continue + } + pkg := selection.Release.Packages[s.Package] + if pkg.Store != "" { + remotePackages[pkg.Name] = &binPackage{ + name: pkg.Name, + realName: pkg.RealName, + store: pkg.Store, + // TODO: populate arch, track and risk when implementing + // fetching from the store. + } + continue + } + + var candidates []*setup.Archive + if pkg.Archive == "" { + // If the package has not pinned any archive, choose the highest + // priority archive in which the package exists. + candidates = sortedArchives + } else { + candidates = []*setup.Archive{selection.Release.Archives[pkg.Archive]} + } + + var chosen archive.Archive + for _, archiveInfo := range candidates { + archive := archives[archiveInfo.Name] + if archive != nil && archive.Exists(pkg.RealName) { + chosen = archive + break + } + } + if chosen == nil { + return nil, fmt.Errorf("cannot find package %q in archive(s)", pkg.RealName) + } + remotePackages[pkg.Name] = &debPackage{ + archive: chosen, + name: pkg.RealName, + } + } + + return remotePackages, nil +} diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 954602de..3495e117 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -219,6 +219,11 @@ var slicerTests = []slicerTest{{ summary: "Conditional architecture", arch: "amd64", slices: []setup.SliceKey{{"test-package", "myslice"}}, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Arch: "all", + Data: testutil.PackageData["test-package"], + }}, release: map[string]string{ "slices/mydir/test-package.yaml": ` package: test-package @@ -247,6 +252,9 @@ var slicerTests = []slicerTest{{ "/dir/text-file-1": "file 0644 5b41362b {test-package_myslice}", "/dir/text-file-3": "file 0644 5b41362b {test-package_myslice}", }, + manifestPkgs: map[string]string{ + "test-package": "test-package version all hash", + }, }, { summary: "Copyright is not installed implicitly", slices: []setup.SliceKey{{"test-package", "myslice"}}, @@ -2002,6 +2010,36 @@ var slicerTests = []slicerTest{{ `, }, error: `cannot fetch package "bin-store-pkg" from store "bin": not implemented`, +}, { + summary: "Selection missing architecture", + slices: []setup.SliceKey{{"test-package", "myslice"}}, + release: map[string]string{ + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + myslice: + contents: + `, + }, + hackopt: func(c *C, opts *slicer.RunOptions) { + opts.Selection.Arch = "" + }, + error: `internal error: selection missing arch`, +}, { + summary: "Selection has invalid architecture", + slices: []setup.SliceKey{{"test-package", "myslice"}}, + release: map[string]string{ + "slices/mydir/test-package.yaml": ` + package: test-package + slices: + myslice: + contents: + `, + }, + hackopt: func(c *C, opts *slicer.RunOptions) { + opts.Selection.Arch = "foo" + }, + error: `invalid package architecture: foo`, }} func (s *S) TestRun(c *C) { @@ -2120,7 +2158,7 @@ func runSlicerTests(s *S, c *C, tests []slicerTest) { Suites: setupArchive.Suites, Components: setupArchive.Components, Pro: setupArchive.Pro, - Arch: test.arch, + Arch: selection.Arch, }, Packages: pkgs, } diff --git a/public/manifest/manifest.go b/public/manifest/manifest.go index 1e4809b8..d4b412af 100644 --- a/public/manifest/manifest.go +++ b/public/manifest/manifest.go @@ -14,6 +14,7 @@ const Schema = "1.0" type Package struct { Kind string `json:"kind"` Name string `json:"name,omitempty"` + Alias string `json:"alias,omitempty"` Version string `json:"version,omitempty"` Digest string `json:"sha256,omitempty"` Arch string `json:"arch,omitempty"` diff --git a/public/manifest/manifest_test.go b/public/manifest/manifest_test.go index d710e121..3428bd72 100644 --- a/public/manifest/manifest_test.go +++ b/public/manifest/manifest_test.go @@ -22,7 +22,7 @@ var readManifestTests = []struct { }{{ summary: "All types", input: ` - {"jsonwall":"1.0","schema":"1.0","count":13} + {"jsonwall":"1.0","schema":"1.0","count":15} {"kind":"content","slice":"pkg1_manifest","path":"/manifest/manifest.wall"} {"kind":"content","slice":"pkg1_myslice","path":"/dir/file"} {"kind":"content","slice":"pkg1_myslice","path":"/dir/file2"} @@ -30,6 +30,7 @@ var readManifestTests = []struct { {"kind":"content","slice":"pkg1_myslice","path":"/dir/hardlink"} {"kind":"content","slice":"pkg1_myslice","path":"/dir/link/file"} {"kind":"content","slice":"pkg2_myotherslice","path":"/dir/foo/bar/"} + {"kind":"package","name":"foo","alias":"bin-foo","version":"v1","sha256":"hash1","arch":"amd64"} {"kind":"package","name":"pkg1","version":"v1","sha256":"hash1","arch":"arch1"} {"kind":"package","name":"pkg2","version":"v2","sha256":"hash2","arch":"arch2"} {"kind":"path","path":"/dir/file","mode":"0644","slices":["pkg1_myslice"],"sha256":"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855","final_sha256":"8067926c032c090867013d14fb0eb21ae858344f62ad07086fd32375845c91a6","size":21} @@ -38,6 +39,7 @@ var readManifestTests = []struct { {"kind":"path","path":"/dir/hardlink","mode":"0644","slices":["pkg1_myslice"],"sha256":"b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c","size":3,"inode":1} {"kind":"path","path":"/dir/link/file","mode":"0644","slices":["pkg1_myslice"],"link":"/dir/file"} {"kind":"path","path":"/manifest/manifest.wall","mode":"0644","slices":["pkg1_manifest"]} + {"kind":"slice","name":"bin-foo_libs"} {"kind":"slice","name":"pkg1_manifest"} {"kind":"slice","name":"pkg1_myslice"} {"kind":"slice","name":"pkg2_myotherslice"} @@ -52,10 +54,12 @@ var readManifestTests = []struct { {Kind: "path", Path: "/manifest/manifest.wall", Mode: "0644", Slices: []string{"pkg1_manifest"}, SHA256: "", FinalSHA256: "", Size: 0x0, Link: ""}, }, Packages: []*manifest.Package{ + {Kind: "package", Name: "foo", Alias: "bin-foo", Version: "v1", Digest: "hash1", Arch: "amd64"}, {Kind: "package", Name: "pkg1", Version: "v1", Digest: "hash1", Arch: "arch1"}, {Kind: "package", Name: "pkg2", Version: "v2", Digest: "hash2", Arch: "arch2"}, }, Slices: []*manifest.Slice{ + {Kind: "slice", Name: "bin-foo_libs"}, {Kind: "slice", Name: "pkg1_manifest"}, {Kind: "slice", Name: "pkg1_myslice"}, {Kind: "slice", Name: "pkg2_myotherslice"}, @@ -97,13 +101,13 @@ func (s *S) TestManifestRead(c *C) { tmpDir := c.MkDir() manifestPath := path.Join(tmpDir, "manifest.wall") - w, err := os.OpenFile(manifestPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0644) + w, err := os.OpenFile(manifestPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0o644) c.Assert(err, IsNil) _, err = w.Write([]byte(test.input)) c.Assert(err, IsNil) w.Close() - r, err := os.OpenFile(manifestPath, os.O_RDONLY, 0644) + r, err := os.OpenFile(manifestPath, os.O_RDONLY, 0o644) c.Assert(err, IsNil) defer r.Close() From 36558436db9eee93006dbb5f5d261a8d1821c80f Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 1 Oct 2026 14:12:01 +0200 Subject: [PATCH 17/24] fix: revert unwanted changes --- public/manifest/manifest_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/public/manifest/manifest_test.go b/public/manifest/manifest_test.go index 3428bd72..76dedbc8 100644 --- a/public/manifest/manifest_test.go +++ b/public/manifest/manifest_test.go @@ -101,13 +101,13 @@ func (s *S) TestManifestRead(c *C) { tmpDir := c.MkDir() manifestPath := path.Join(tmpDir, "manifest.wall") - w, err := os.OpenFile(manifestPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0o644) + w, err := os.OpenFile(manifestPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0644) c.Assert(err, IsNil) _, err = w.Write([]byte(test.input)) c.Assert(err, IsNil) w.Close() - r, err := os.OpenFile(manifestPath, os.O_RDONLY, 0o644) + r, err := os.OpenFile(manifestPath, os.O_RDONLY, 0644) c.Assert(err, IsNil) defer r.Close() From a81c9a7dc8adead738e8a66d9f73b903d3c5cadb Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 1 Oct 2026 14:42:11 +0200 Subject: [PATCH 18/24] fix: move out fix for package naming collision --- internal/manifestutil/manifestutil.go | 36 ++---------- internal/manifestutil/manifestutil_test.go | 68 ---------------------- internal/slicer/remote_package.go | 3 - public/manifest/manifest.go | 1 - public/manifest/manifest_test.go | 6 +- 5 files changed, 5 insertions(+), 109 deletions(-) diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index ac2ee517..04306a57 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -18,7 +18,6 @@ import ( type PackageInfo interface { PkgName() string - PkgRealName() string PkgVersion() string // PkgRevision further identifies the package when the source versions are // not unique on their own. It returns 0 when the source does not use @@ -85,11 +84,9 @@ func Write(options *WriteOptions, writer io.Writer) error { func manifestAddPackages(dbw *jsonwall.DBWriter, infos []PackageInfo) error { for _, info := range infos { - name, alias := packageNames(info) err := dbw.Add(&manifest.Package{ Kind: "package", - Name: name, - Alias: alias, + Name: info.PkgName(), Version: info.PkgVersion(), Arch: info.PkgArch(), Digest: info.PkgDigest(), @@ -101,17 +98,6 @@ func manifestAddPackages(dbw *jsonwall.DBWriter, infos []PackageInfo) error { return nil } -func packageNames(info PackageInfo) (name, alias string) { - name = info.PkgRealName() - if name == "" { - name = info.PkgName() - } - if info.PkgName() != name { - alias = info.PkgName() - } - return name, alias -} - func manifestAddSlices(dbw *jsonwall.DBWriter, slices []*setup.Slice) error { for _, slice := range slices { err := dbw.Add(&manifest.Slice{ @@ -182,14 +168,7 @@ func fastValidate(options *WriteOptions) (err error) { if err != nil { return err } - name, alias := packageNames(pkg) - if alias != "" { - name = alias - } - if pkgExist[name] { - return fmt.Errorf("package name or alias %q is used more than once", name) - } - pkgExist[name] = true + pkgExist[pkg.PkgName()] = true } sliceExist := map[string]bool{} for _, slice := range options.Selection { @@ -285,7 +264,7 @@ func validateReportEntry(entry *ReportEntry) (err error) { } func validatePackage(pkg PackageInfo) (err error) { - name, _ := packageNames(pkg) + name := pkg.PkgName() if name == "" { return fmt.Errorf("package name not set") } @@ -318,14 +297,7 @@ func Validate(mfest *manifest.Manifest) (err error) { pkgExist := map[string]bool{} err = mfest.IteratePackages(func(pkg *manifest.Package) error { - name := pkg.Name - if pkg.Alias != "" { - name = pkg.Alias - } - if pkgExist[name] { - return fmt.Errorf("package name or alias %q is used more than once", name) - } - pkgExist[name] = true + pkgExist[pkg.Name] = true return nil }) if err != nil { diff --git a/internal/manifestutil/manifestutil_test.go b/internal/manifestutil/manifestutil_test.go index a264a138..ec9bb339 100644 --- a/internal/manifestutil/manifestutil_test.go +++ b/internal/manifestutil/manifestutil_test.go @@ -121,7 +121,6 @@ var slice2 = &setup.Slice{ type testPackageInfo struct { Name string - RealName string Version string Revision int Arch string @@ -131,7 +130,6 @@ type testPackageInfo struct { } func (p *testPackageInfo) PkgName() string { return p.Name } -func (p *testPackageInfo) PkgRealName() string { return p.RealName } func (p *testPackageInfo) PkgVersion() string { return p.Version } func (p *testPackageInfo) PkgRevision() int { return p.Revision } func (p *testPackageInfo) PkgArch() string { return p.Arch } @@ -234,24 +232,6 @@ var generateManifestTests = []struct { Path: "/link", }}, }, -}, { - summary: "Package alias matches slice reference", - selection: []*setup.Slice{{Package: "bin-foo", Name: "libs"}}, - report: &manifestutil.Report{ - Entries: map[string]manifestutil.ReportEntry{}, - }, - packageInfo: []manifestutil.PackageInfo{&testPackageInfo{ - Name: "bin-foo", - RealName: "foo", - Version: "2.10", - Arch: "amd64", - DigestKind: cache.SHA256, - Digest: "sum", - }}, - expected: &apachetestutil.ManifestContents{ - Packages: []*manifest.Package{{Kind: "package", Name: "foo", Alias: "bin-foo", Version: "2.10", Arch: "amd64", Digest: "sum"}}, - Slices: []*manifest.Slice{{Kind: "slice", Name: "bin-foo_libs"}}, - }, }, { summary: "Missing slice", report: &manifestutil.Report{ @@ -581,23 +561,6 @@ var generateManifestTests = []struct { }, }, error: `internal error: invalid manifest: package "package-1" missing sha256`, -}, { - summary: "Duplicate package alias", - packageInfo: []manifestutil.PackageInfo{&testPackageInfo{ - Name: "bin-foo", - RealName: "foo", - Version: "v1", - Arch: "a1", - DigestKind: cache.SHA256, - Digest: "s1", - }, &testPackageInfo{ - Name: "bin-foo", - Version: "v1", - Arch: "a1", - DigestKind: cache.SHA256, - Digest: "s2", - }}, - error: `internal error: invalid manifest: package name or alias "bin-foo" is used more than once`, }} func (s *S) TestGenerateManifests(c *C) { @@ -675,37 +638,6 @@ var validateManifestTests = []struct { {"kind":"slice","name":"pkg1_manifest"} `, error: `invalid manifest: slice pkg1_manifest refers to missing package "pkg1"`, -}, { - summary: "Alias resolves slice reference", - input: ` - {"jsonwall":"1.0","schema":"1.0","count":2} - {"kind":"package","name":"foo","alias":"bin-foo","version":"v1","sha256":"hash1","arch":"amd64"} - {"kind":"slice","name":"bin-foo_libs"} - `, -}, { - summary: "Alias is not interchangeable with real name", - input: ` - {"jsonwall":"1.0","schema":"1.0","count":2} - {"kind":"package","name":"foo","alias":"bin-foo","version":"v1","sha256":"hash1","arch":"amd64"} - {"kind":"slice","name":"foo_libs"} - `, - error: `invalid manifest: slice foo_libs refers to missing package "foo"`, -}, { - summary: "Same real name from different providers", - input: ` - {"jsonwall":"1.0","schema":"1.0","count":3} - {"kind":"package","name":"foo","alias":"bin-foo","version":"v2","sha256":"hash2","arch":"amd64"} - {"kind":"package","name":"foo","version":"v1","sha256":"hash1","arch":"amd64"} - {"kind":"slice","name":"bin-foo_libs"} - `, -}, { - summary: "Duplicate alias is ambiguous", - input: ` - {"jsonwall":"1.0","schema":"1.0","count":2} - {"kind":"package","name":"foo","alias":"bin-foo","version":"v1","sha256":"hash1","arch":"amd64"} - {"kind":"package","name":"other","alias":"bin-foo","version":"v2","sha256":"hash2","arch":"amd64"} - `, - error: `invalid manifest: package name or alias "bin-foo" is used more than once`, }, { summary: "Path not found in contents", input: ` diff --git a/internal/slicer/remote_package.go b/internal/slicer/remote_package.go index 488cd1f3..77a7e82e 100644 --- a/internal/slicer/remote_package.go +++ b/internal/slicer/remote_package.go @@ -11,7 +11,6 @@ import ( // RemotePackageInfo describes a package as reported by its provider. type RemotePackageInfo struct { Name string - RealName string Version string Revision int Arch string @@ -21,7 +20,6 @@ type RemotePackageInfo struct { } func (p *RemotePackageInfo) PkgName() string { return p.Name } -func (p *RemotePackageInfo) PkgRealName() string { return p.RealName } func (p *RemotePackageInfo) PkgVersion() string { return p.Version } func (p *RemotePackageInfo) PkgRevision() int { return p.Revision } func (p *RemotePackageInfo) PkgArch() string { return p.Arch } @@ -50,7 +48,6 @@ func (d *debPackage) Fetch() (io.ReadSeekCloser, *RemotePackageInfo, error) { } return reader, &RemotePackageInfo{ Name: info.Name, - RealName: info.Name, Version: info.Version, Arch: info.Arch, DigestKind: cache.SHA256, diff --git a/public/manifest/manifest.go b/public/manifest/manifest.go index d4b412af..1e4809b8 100644 --- a/public/manifest/manifest.go +++ b/public/manifest/manifest.go @@ -14,7 +14,6 @@ const Schema = "1.0" type Package struct { Kind string `json:"kind"` Name string `json:"name,omitempty"` - Alias string `json:"alias,omitempty"` Version string `json:"version,omitempty"` Digest string `json:"sha256,omitempty"` Arch string `json:"arch,omitempty"` diff --git a/public/manifest/manifest_test.go b/public/manifest/manifest_test.go index 76dedbc8..d710e121 100644 --- a/public/manifest/manifest_test.go +++ b/public/manifest/manifest_test.go @@ -22,7 +22,7 @@ var readManifestTests = []struct { }{{ summary: "All types", input: ` - {"jsonwall":"1.0","schema":"1.0","count":15} + {"jsonwall":"1.0","schema":"1.0","count":13} {"kind":"content","slice":"pkg1_manifest","path":"/manifest/manifest.wall"} {"kind":"content","slice":"pkg1_myslice","path":"/dir/file"} {"kind":"content","slice":"pkg1_myslice","path":"/dir/file2"} @@ -30,7 +30,6 @@ var readManifestTests = []struct { {"kind":"content","slice":"pkg1_myslice","path":"/dir/hardlink"} {"kind":"content","slice":"pkg1_myslice","path":"/dir/link/file"} {"kind":"content","slice":"pkg2_myotherslice","path":"/dir/foo/bar/"} - {"kind":"package","name":"foo","alias":"bin-foo","version":"v1","sha256":"hash1","arch":"amd64"} {"kind":"package","name":"pkg1","version":"v1","sha256":"hash1","arch":"arch1"} {"kind":"package","name":"pkg2","version":"v2","sha256":"hash2","arch":"arch2"} {"kind":"path","path":"/dir/file","mode":"0644","slices":["pkg1_myslice"],"sha256":"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855","final_sha256":"8067926c032c090867013d14fb0eb21ae858344f62ad07086fd32375845c91a6","size":21} @@ -39,7 +38,6 @@ var readManifestTests = []struct { {"kind":"path","path":"/dir/hardlink","mode":"0644","slices":["pkg1_myslice"],"sha256":"b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c","size":3,"inode":1} {"kind":"path","path":"/dir/link/file","mode":"0644","slices":["pkg1_myslice"],"link":"/dir/file"} {"kind":"path","path":"/manifest/manifest.wall","mode":"0644","slices":["pkg1_manifest"]} - {"kind":"slice","name":"bin-foo_libs"} {"kind":"slice","name":"pkg1_manifest"} {"kind":"slice","name":"pkg1_myslice"} {"kind":"slice","name":"pkg2_myotherslice"} @@ -54,12 +52,10 @@ var readManifestTests = []struct { {Kind: "path", Path: "/manifest/manifest.wall", Mode: "0644", Slices: []string{"pkg1_manifest"}, SHA256: "", FinalSHA256: "", Size: 0x0, Link: ""}, }, Packages: []*manifest.Package{ - {Kind: "package", Name: "foo", Alias: "bin-foo", Version: "v1", Digest: "hash1", Arch: "amd64"}, {Kind: "package", Name: "pkg1", Version: "v1", Digest: "hash1", Arch: "arch1"}, {Kind: "package", Name: "pkg2", Version: "v2", Digest: "hash2", Arch: "arch2"}, }, Slices: []*manifest.Slice{ - {Kind: "slice", Name: "bin-foo_libs"}, {Kind: "slice", Name: "pkg1_manifest"}, {Kind: "slice", Name: "pkg1_myslice"}, {Kind: "slice", Name: "pkg2_myotherslice"}, From b932d177aa881d305332736c222f6db40c4cffc1 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 1 Oct 2026 17:56:01 +0200 Subject: [PATCH 19/24] fix: simplify arch check in Run --- internal/manifestutil/manifestutil.go | 2 +- internal/slicer/slicer.go | 5 +---- internal/slicer/slicer_test.go | 17 +---------------- 3 files changed, 3 insertions(+), 21 deletions(-) diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 04306a57..2959ce8a 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -88,8 +88,8 @@ func manifestAddPackages(dbw *jsonwall.DBWriter, infos []PackageInfo) error { Kind: "package", Name: info.PkgName(), Version: info.PkgVersion(), - Arch: info.PkgArch(), Digest: info.PkgDigest(), + Arch: info.PkgArch(), }) if err != nil { return err diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 1bc60ccb..52cbc41a 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -77,11 +77,8 @@ func (cc *contentChecker) checkKnown(path string) error { } func Run(options *RunOptions) error { - if options.Selection.Arch == "" { - return fmt.Errorf("internal error: selection missing arch") - } if err := deb.ValidateArch(options.Selection.Arch); err != nil { - return err + return fmt.Errorf("internal error: %s", err) } oldUmask := syscall.Umask(0) diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 3495e117..f43f057d 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -2010,21 +2010,6 @@ var slicerTests = []slicerTest{{ `, }, error: `cannot fetch package "bin-store-pkg" from store "bin": not implemented`, -}, { - summary: "Selection missing architecture", - slices: []setup.SliceKey{{"test-package", "myslice"}}, - release: map[string]string{ - "slices/mydir/test-package.yaml": ` - package: test-package - slices: - myslice: - contents: - `, - }, - hackopt: func(c *C, opts *slicer.RunOptions) { - opts.Selection.Arch = "" - }, - error: `internal error: selection missing arch`, }, { summary: "Selection has invalid architecture", slices: []setup.SliceKey{{"test-package", "myslice"}}, @@ -2039,7 +2024,7 @@ var slicerTests = []slicerTest{{ hackopt: func(c *C, opts *slicer.RunOptions) { opts.Selection.Arch = "foo" }, - error: `invalid package architecture: foo`, + error: `internal error: invalid package architecture: foo`, }} func (s *S) TestRun(c *C) { From 1a997eabdb0044738bfea7f1b7b46304a656deab Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 2 Oct 2026 09:54:24 +0200 Subject: [PATCH 20/24] docs: fix comment --- internal/slicer/slicer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 52cbc41a..8865f24a 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -524,7 +524,7 @@ func selectRemotePackages(archives map[string]archive.Archive, selection *setup. name: pkg.Name, realName: pkg.RealName, store: pkg.Store, - // TODO: populate arch, track and risk when implementing + // TODO: add track and risk when implementing // fetching from the store. } continue From aa574b99dbbaebea015831d4a87085cef3bd5973 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 13 Oct 2026 14:37:33 +0200 Subject: [PATCH 21/24] fix: refine abstraction --- internal/setup/setup.go | 2 - internal/setup/setup_test.go | 4 -- .../{remote_package.go => package_remote.go} | 33 +++++++------- internal/slicer/slicer.go | 44 ++++++++----------- internal/slicer/slicer_test.go | 19 +------- 5 files changed, 35 insertions(+), 67 deletions(-) rename internal/slicer/{remote_package.go => package_remote.go} (68%) diff --git a/internal/setup/setup.go b/internal/setup/setup.go index b52aab6a..9d7e4a7b 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -154,7 +154,6 @@ func (s *Slice) String() string { return s.Package + "_" + s.Name } type Selection struct { Release *Release Slices []*Slice - Arch string } // Prefers uses the prefer relationships and returns a map from each path to @@ -509,7 +508,6 @@ func Select(release *Release, slices []SliceKey, arch string) (*Selection, error selection := &Selection{ Release: release, - Arch: arch, } sorted, err := order(release.Packages, slices, arch) diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 656f01ae..3568b39a 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -428,7 +428,6 @@ var setupTests = []setupTest{{ }, selslices: []setup.SliceKey{{"mypkg1", "myslice1"}}, selection: &setup.Selection{ - Arch: "amd64", Slices: []*setup.Slice{{ Package: "mypkg1", Name: "myslice1", @@ -452,7 +451,6 @@ var setupTests = []setupTest{{ }, selslices: []setup.SliceKey{{"mypkg2", "myslice2"}}, selection: &setup.Selection{ - Arch: "amd64", Slices: []*setup.Slice{{ Package: "mypkg1", Name: "myslice1", @@ -1760,7 +1758,6 @@ var setupTests = []setupTest{{ }, selslices: []setup.SliceKey{{"mypkg", "myslice"}}, selection: &setup.Selection{ - Arch: "amd64", Slices: []*setup.Slice{{ Package: "mypkg", Name: "myslice", @@ -4913,7 +4910,6 @@ func (s *S) TestSelectEmptyArch(c *C) { selslice := []setup.SliceKey{{"mypkg", "myslice"}} selection, err := setup.Select(release, selslice, "") c.Assert(err, IsNil) - c.Assert(selection.Arch, Equals, arch) var sliceNames []string for _, s := range selection.Slices { diff --git a/internal/slicer/remote_package.go b/internal/slicer/package_remote.go similarity index 68% rename from internal/slicer/remote_package.go rename to internal/slicer/package_remote.go index 77a7e82e..553d31d9 100644 --- a/internal/slicer/remote_package.go +++ b/internal/slicer/package_remote.go @@ -1,7 +1,6 @@ package slicer import ( - "fmt" "io" "github.com/canonical/chisel/internal/archive" @@ -27,21 +26,29 @@ func (p *RemotePackageInfo) PkgStore() string { return p.Store } func (p *RemotePackageInfo) PkgDigestKind() cache.DigestKind { return p.DigestKind } func (p *RemotePackageInfo) PkgDigest() string { return p.Digest } -type RemotePackage interface { +// PackageRemote represents the remote from which a package is obtained, +// bound to a specific package. +type PackageRemote interface { + // Arch returns the architecture the remote + // serves packages for, which is the target architecture used to request + // the package, not the architecture of the fetched package itself (which + // may be "all", for example). + Arch() string Fetch() (io.ReadSeekCloser, *RemotePackageInfo, error) } -var ( - _ RemotePackage = (*debPackage)(nil) - _ RemotePackage = (*binPackage)(nil) -) +var _ PackageRemote = (*archiveRemote)(nil) -type debPackage struct { +type archiveRemote struct { archive archive.Archive name string } -func (d *debPackage) Fetch() (io.ReadSeekCloser, *RemotePackageInfo, error) { +func (d *archiveRemote) Arch() string { + return d.archive.Options().Arch +} + +func (d *archiveRemote) Fetch() (io.ReadSeekCloser, *RemotePackageInfo, error) { reader, info, err := d.archive.Fetch(d.name) if err != nil { return nil, nil, err @@ -54,13 +61,3 @@ func (d *debPackage) Fetch() (io.ReadSeekCloser, *RemotePackageInfo, error) { Digest: info.SHA256, }, nil } - -type binPackage struct { - name string - realName string - store string -} - -func (b *binPackage) Fetch() (io.ReadSeekCloser, *RemotePackageInfo, error) { - return nil, nil, fmt.Errorf("cannot fetch package %q from store %q: not implemented", b.name, b.store) -} diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 8865f24a..d8563bbd 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -16,7 +16,6 @@ import ( "github.com/klauspost/compress/zstd" "github.com/canonical/chisel/internal/archive" - "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/scripts" @@ -77,10 +76,6 @@ func (cc *contentChecker) checkKnown(path string) error { } func Run(options *RunOptions) error { - if err := deb.ValidateArch(options.Selection.Arch); err != nil { - return fmt.Errorf("internal error: %s", err) - } - oldUmask := syscall.Umask(0) defer func() { syscall.Umask(oldUmask) @@ -95,7 +90,7 @@ func Run(options *RunOptions) error { targetDir = filepath.Join(dir, targetDir) } - remotePackages, err := selectRemotePackages(options.Archives, options.Selection) + packageRemotes, err := selectPackageRemotes(options.Archives, options.Selection) if err != nil { return err } @@ -113,11 +108,12 @@ func Run(options *RunOptions) error { extractPackage = make(map[string][]tarball.ExtractInfo) extract[slice.Package] = extractPackage } + arch := packageRemotes[slice.Package].Arch() for targetPath, pathInfo := range slice.Contents { if targetPath == "" { continue } - if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, options.Selection.Arch) { + if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, arch) { continue } if preferredPkg, ok := prefers[targetPath]; ok && preferredPkg.Name != slice.Package { @@ -157,7 +153,7 @@ func Run(options *RunOptions) error { continue } pkg := options.Selection.Release.Packages[slice.Package] - reader, info, err := remotePackages[pkg.Name].Fetch() + reader, info, err := packageRemotes[pkg.Name].Fetch() if err != nil { return err } @@ -274,8 +270,9 @@ func Run(options *RunOptions) error { // them to the appropriate slices. relPaths := map[string][]*setup.Slice{} for _, slice := range options.Selection.Slices { + arch := packageRemotes[slice.Package].Arch() for relPath, pathInfo := range slice.Contents { - if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, options.Selection.Arch) { + if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, arch) { continue } if pathInfo.Kind == setup.CopyPath || pathInfo.Kind == setup.GlobPath || @@ -493,13 +490,12 @@ func createFile(targetDir, relPath string, pathInfo setup.PathInfo) (*fsutil.Ent }) } -// selectRemotePackages determines the remote package for each package in the selection. +// selectPackageRemotes determines the remote for each package in the selection. // For packages from an archive it selects the highest priority archive // containing the package unless a particular archive is pinned within the -// package slices file. For packages from a store it selects the store -// named in the package slices file. It returns a map of RemotePackage indexed -// by package names. -func selectRemotePackages(archives map[string]archive.Archive, selection *setup.Selection) (map[string]RemotePackage, error) { +// package slices file. Packages from a store are not supported yet. +// It returns a map of PackageRemote indexed by package names. +func selectPackageRemotes(archives map[string]archive.Archive, selection *setup.Selection) (map[string]PackageRemote, error) { sortedArchives := make([]*setup.Archive, 0, len(selection.Release.Archives)) for _, archive := range selection.Release.Archives { if archive.Priority < 0 { @@ -513,21 +509,17 @@ func selectRemotePackages(archives map[string]archive.Archive, selection *setup. return b.Priority - a.Priority }) - remotePackages := make(map[string]RemotePackage) + packageRemotes := make(map[string]PackageRemote) for _, s := range selection.Slices { - if _, ok := remotePackages[s.Package]; ok { + if _, ok := packageRemotes[s.Package]; ok { continue } pkg := selection.Release.Packages[s.Package] if pkg.Store != "" { - remotePackages[pkg.Name] = &binPackage{ - name: pkg.Name, - realName: pkg.RealName, - store: pkg.Store, - // TODO: add track and risk when implementing - // fetching from the store. - } - continue + // TODO: select the remote for the package from the store, which + // requires the arch, track and risk, when fetching from the + // store is implemented. + return nil, fmt.Errorf("cannot fetch package %q from store %q: not implemented", pkg.Name, pkg.Store) } var candidates []*setup.Archive @@ -550,11 +542,11 @@ func selectRemotePackages(archives map[string]archive.Archive, selection *setup. if chosen == nil { return nil, fmt.Errorf("cannot find package %q in archive(s)", pkg.RealName) } - remotePackages[pkg.Name] = &debPackage{ + packageRemotes[pkg.Name] = &archiveRemote{ archive: chosen, name: pkg.RealName, } } - return remotePackages, nil + return packageRemotes, nil } diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index f43f057d..44ad0ccf 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -1988,7 +1988,7 @@ var slicerTests = []slicerTest{{ }, }, { summary: "Store package fetching not yet implemented", - slices: []setup.SliceKey{{"test-package", "myslice"}, {"bin-store-pkg", "myslice"}}, + slices: []setup.SliceKey{{"bin-store-pkg", "myslice"}}, arch: "amd64", release: map[string]string{ "chisel.yaml": testutil.DefaultChiselYamlWithStores, @@ -2010,21 +2010,6 @@ var slicerTests = []slicerTest{{ `, }, error: `cannot fetch package "bin-store-pkg" from store "bin": not implemented`, -}, { - summary: "Selection has invalid architecture", - slices: []setup.SliceKey{{"test-package", "myslice"}}, - release: map[string]string{ - "slices/mydir/test-package.yaml": ` - package: test-package - slices: - myslice: - contents: - `, - }, - hackopt: func(c *C, opts *slicer.RunOptions) { - opts.Selection.Arch = "foo" - }, - error: `internal error: invalid package architecture: foo`, }} func (s *S) TestRun(c *C) { @@ -2143,7 +2128,7 @@ func runSlicerTests(s *S, c *C, tests []slicerTest) { Suites: setupArchive.Suites, Components: setupArchive.Components, Pro: setupArchive.Pro, - Arch: selection.Arch, + Arch: test.arch, }, Packages: pkgs, } From aac4ef18979900c2f84811d135e2af080ddcff88 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 13 Oct 2026 14:55:04 +0200 Subject: [PATCH 22/24] style: align name with project convention --- internal/slicer/slicer.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index d8563bbd..cda8d8b1 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -90,7 +90,7 @@ func Run(options *RunOptions) error { targetDir = filepath.Join(dir, targetDir) } - packageRemotes, err := selectPackageRemotes(options.Archives, options.Selection) + pkgRemotes, err := selectPkgRemotes(options.Archives, options.Selection) if err != nil { return err } @@ -108,7 +108,7 @@ func Run(options *RunOptions) error { extractPackage = make(map[string][]tarball.ExtractInfo) extract[slice.Package] = extractPackage } - arch := packageRemotes[slice.Package].Arch() + arch := pkgRemotes[slice.Package].Arch() for targetPath, pathInfo := range slice.Contents { if targetPath == "" { continue @@ -153,7 +153,7 @@ func Run(options *RunOptions) error { continue } pkg := options.Selection.Release.Packages[slice.Package] - reader, info, err := packageRemotes[pkg.Name].Fetch() + reader, info, err := pkgRemotes[pkg.Name].Fetch() if err != nil { return err } @@ -270,7 +270,7 @@ func Run(options *RunOptions) error { // them to the appropriate slices. relPaths := map[string][]*setup.Slice{} for _, slice := range options.Selection.Slices { - arch := packageRemotes[slice.Package].Arch() + arch := pkgRemotes[slice.Package].Arch() for relPath, pathInfo := range slice.Contents { if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, arch) { continue @@ -490,12 +490,12 @@ func createFile(targetDir, relPath string, pathInfo setup.PathInfo) (*fsutil.Ent }) } -// selectPackageRemotes determines the remote for each package in the selection. +// selectPkgRemotes determines the remote for each package in the selection. // For packages from an archive it selects the highest priority archive // containing the package unless a particular archive is pinned within the // package slices file. Packages from a store are not supported yet. // It returns a map of PackageRemote indexed by package names. -func selectPackageRemotes(archives map[string]archive.Archive, selection *setup.Selection) (map[string]PackageRemote, error) { +func selectPkgRemotes(archives map[string]archive.Archive, selection *setup.Selection) (map[string]PackageRemote, error) { sortedArchives := make([]*setup.Archive, 0, len(selection.Release.Archives)) for _, archive := range selection.Release.Archives { if archive.Priority < 0 { @@ -509,9 +509,9 @@ func selectPackageRemotes(archives map[string]archive.Archive, selection *setup. return b.Priority - a.Priority }) - packageRemotes := make(map[string]PackageRemote) + pkgRemotes := make(map[string]PackageRemote) for _, s := range selection.Slices { - if _, ok := packageRemotes[s.Package]; ok { + if _, ok := pkgRemotes[s.Package]; ok { continue } pkg := selection.Release.Packages[s.Package] @@ -542,11 +542,11 @@ func selectPackageRemotes(archives map[string]archive.Archive, selection *setup. if chosen == nil { return nil, fmt.Errorf("cannot find package %q in archive(s)", pkg.RealName) } - packageRemotes[pkg.Name] = &archiveRemote{ + pkgRemotes[pkg.Name] = &archiveRemote{ archive: chosen, name: pkg.RealName, } } - return packageRemotes, nil + return pkgRemotes, nil } From 8c63924d464d998795314f8fd95bf991de4f651e Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 13 Oct 2026 15:17:50 +0200 Subject: [PATCH 23/24] fix: keep the diff focus on the added abstraction --- internal/slicer/package_remote.go | 11 +++++------ internal/slicer/slicer.go | 2 +- internal/slicer/slicer_test.go | 32 ++++++++----------------------- 3 files changed, 14 insertions(+), 31 deletions(-) diff --git a/internal/slicer/package_remote.go b/internal/slicer/package_remote.go index 553d31d9..0306556f 100644 --- a/internal/slicer/package_remote.go +++ b/internal/slicer/package_remote.go @@ -26,13 +26,12 @@ func (p *RemotePackageInfo) PkgStore() string { return p.Store } func (p *RemotePackageInfo) PkgDigestKind() cache.DigestKind { return p.DigestKind } func (p *RemotePackageInfo) PkgDigest() string { return p.Digest } -// PackageRemote represents the remote from which a package is obtained, -// bound to a specific package. +// PackageRemote represents the remote from which a package is obtained. type PackageRemote interface { - // Arch returns the architecture the remote - // serves packages for, which is the target architecture used to request - // the package, not the architecture of the fetched package itself (which - // may be "all", for example). + // Arch returns the architecture the remote serves packages for, which is + // the target architecture used to request the package, not the + // architecture of the fetched package itself (which may be "all", for + // example). Arch() string Fetch() (io.ReadSeekCloser, *RemotePackageInfo, error) } diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index cda8d8b1..8cfa9925 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -515,6 +515,7 @@ func selectPkgRemotes(archives map[string]archive.Archive, selection *setup.Sele continue } pkg := selection.Release.Packages[s.Package] + if pkg.Store != "" { // TODO: select the remote for the package from the store, which // requires the arch, track and risk, when fetching from the @@ -547,6 +548,5 @@ func selectPkgRemotes(archives map[string]archive.Archive, selection *setup.Sele name: pkg.RealName, } } - return pkgRemotes, nil } diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 44ad0ccf..d6ef9ca0 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -219,11 +219,6 @@ var slicerTests = []slicerTest{{ summary: "Conditional architecture", arch: "amd64", slices: []setup.SliceKey{{"test-package", "myslice"}}, - pkgs: []*testutil.TestPackage{{ - Name: "test-package", - Arch: "all", - Data: testutil.PackageData["test-package"], - }}, release: map[string]string{ "slices/mydir/test-package.yaml": ` package: test-package @@ -252,9 +247,6 @@ var slicerTests = []slicerTest{{ "/dir/text-file-1": "file 0644 5b41362b {test-package_myslice}", "/dir/text-file-3": "file 0644 5b41362b {test-package_myslice}", }, - manifestPkgs: map[string]string{ - "test-package": "test-package version all hash", - }, }, { summary: "Copyright is not installed implicitly", slices: []setup.SliceKey{{"test-package", "myslice"}}, @@ -1987,29 +1979,21 @@ var slicerTests = []slicerTest{{ "/dir/file": "file 0644 cc55e2ec {test-package_third}", }, }, { - summary: "Store package fetching not yet implemented", - slices: []setup.SliceKey{{"bin-store-pkg", "myslice"}}, - arch: "amd64", + summary: "Store package is not yet implemented", + slices: []setup.SliceKey{{"bin-curl", "bin"}}, release: map[string]string{ "chisel.yaml": testutil.DefaultChiselYamlWithStores, - "slices/mydir/test-package.yaml": ` - package: test-package - slices: - myslice: - contents: - /dir/file: - `, - "slices/mydir/store-pkg.yaml": ` - package: store-pkg + "slices/curl.yaml": ` + package: curl store: bin - default-track: 3.1 + default-track: latest slices: - myslice: + bin: contents: - /dir/store-file: + /usr/bin/curl: `, }, - error: `cannot fetch package "bin-store-pkg" from store "bin": not implemented`, + error: `cannot fetch package "bin-curl" from store "bin": not implemented`, }} func (s *S) TestRun(c *C) { From 3cdef0a37e80fb4dd712c938fd8329475ab9f7c7 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 13 Oct 2026 15:44:38 +0200 Subject: [PATCH 24/24] docs: remove useless comment --- internal/slicer/slicer.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 8cfa9925..7a5e8bf9 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -517,9 +517,6 @@ func selectPkgRemotes(archives map[string]archive.Archive, selection *setup.Sele pkg := selection.Release.Packages[s.Package] if pkg.Store != "" { - // TODO: select the remote for the package from the store, which - // requires the arch, track and risk, when fetching from the - // store is implemented. return nil, fmt.Errorf("cannot fetch package %q from store %q: not implemented", pkg.Name, pkg.Store) }