From 64d1e748273c5498419a659b5f4d802c3b6385f3 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Thu, 3 Oct 2024 17:24:53 +0600 Subject: [PATCH 01/11] feat: support multiple archives with "priority" field (#12) This commit adds support for fetching packages from multiple archives. It introduces a new field `archives..priority` which takes in a signed integer and specifies the priority of a certain archive. A package is fetched from the archive with highest priority. It also deprecates the concept of default archive in chisel.yaml. However, the field can still be present and it will be parsed but IGNORED. DEPRECATED: "archives..default" field in chisel.yaml. --------- Co-authored-by: Alberto Carretero --- cmd/chisel/cmd_find_test.go | 2 - cmd/chisel/cmd_info_test.go | 8 - internal/setup/setup.go | 52 ++-- internal/setup/setup_test.go | 238 ++++++++++------- internal/slicer/slicer.go | 64 ++++- internal/slicer/slicer_test.go | 463 +++++++++++++++++++++++++-------- internal/testutil/archive.go | 13 +- 7 files changed, 591 insertions(+), 249 deletions(-) diff --git a/cmd/chisel/cmd_find_test.go b/cmd/chisel/cmd_find_test.go index 7d7c0d7a..a2a68c2a 100644 --- a/cmd/chisel/cmd_find_test.go +++ b/cmd/chisel/cmd_find_test.go @@ -33,8 +33,6 @@ func makeSamplePackage(pkg string, slices []string) *setup.Package { } var sampleRelease = &setup.Release{ - DefaultArchive: "ubuntu", - Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", diff --git a/cmd/chisel/cmd_info_test.go b/cmd/chisel/cmd_info_test.go index c78a477c..9dcfa3d8 100644 --- a/cmd/chisel/cmd_info_test.go +++ b/cmd/chisel/cmd_info_test.go @@ -25,7 +25,6 @@ var infoTests = []infoTest{{ query: []string{"mypkg1_myslice1"}, stdout: ` package: mypkg1 - archive: ubuntu slices: myslice1: contents: @@ -37,7 +36,6 @@ var infoTests = []infoTest{{ query: []string{"mypkg2"}, stdout: ` package: mypkg2 - archive: ubuntu slices: myslice: contents: @@ -49,7 +47,6 @@ var infoTests = []infoTest{{ query: []string{"mypkg1_myslice2", "mypkg1_myslice1"}, stdout: ` package: mypkg1 - archive: ubuntu slices: myslice1: contents: @@ -65,7 +62,6 @@ var infoTests = []infoTest{{ query: []string{"mypkg1_myslice1", "mypkg2", "mypkg1_myslice2"}, stdout: ` package: mypkg1 - archive: ubuntu slices: myslice1: contents: @@ -76,7 +72,6 @@ var infoTests = []infoTest{{ - mypkg2_myslice --- package: mypkg2 - archive: ubuntu slices: myslice: contents: @@ -88,7 +83,6 @@ var infoTests = []infoTest{{ query: []string{"mypkg1_myslice1", "mypkg1"}, stdout: ` package: mypkg1 - archive: ubuntu slices: myslice1: contents: @@ -104,7 +98,6 @@ var infoTests = []infoTest{{ query: []string{"mypkg1_myslice1", "mypkg1_myslice1", "mypkg1_myslice1"}, stdout: ` package: mypkg1 - archive: ubuntu slices: myslice1: contents: @@ -121,7 +114,6 @@ var infoTests = []infoTest{{ query: []string{"foo", "mypkg1_myslice1", "bar_foo"}, stdout: ` package: mypkg1 - archive: ubuntu slices: myslice1: contents: diff --git a/internal/setup/setup.go b/internal/setup/setup.go index e604786d..c9b255d7 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -21,10 +21,9 @@ import ( // Release is a collection of package slices targeting a particular // distribution version. type Release struct { - Path string - Packages map[string]*Package - Archives map[string]*Archive - DefaultArchive string + Path string + Packages map[string]*Package + Archives map[string]*Archive } // Archive is the location from which binary packages are obtained. @@ -33,6 +32,7 @@ type Archive struct { Version string Suites []string Components []string + Priority int PubKeys []*packet.PublicKey } @@ -229,6 +229,28 @@ func (r *Release) validate() error { return err } + // Check for archive priority conflicts. + priorities := make(map[int]*Archive) + for _, archive := range r.Archives { + if old, ok := priorities[archive.Priority]; ok { + if old.Name > archive.Name { + archive, old = old, archive + } + return fmt.Errorf("chisel.yaml: archives %q and %q have the same priority value of %d", old.Name, archive.Name, archive.Priority) + } + priorities[archive.Priority] = archive + } + + // Check that archives pinned in packages are defined. + for _, pkg := range r.Packages { + if pkg.Archive == "" { + continue + } + if _, ok := r.Archives[pkg.Archive]; !ok { + return fmt.Errorf("%s: package refers to undefined archive %q", pkg.Path, pkg.Archive) + } + } + return nil } @@ -355,9 +377,6 @@ func readSlices(release *Release, baseDir, dirName string) error { if err != nil { return err } - if pkg.Archive == "" { - pkg.Archive = release.DefaultArchive - } release.Packages[pkg.Name] = pkg } @@ -372,11 +391,16 @@ type yamlRelease struct { V1PubKeys map[string]yamlPubKey `yaml:"v1-public-keys"` } +const ( + MaxArchivePriority = 1000 + MinArchivePriority = -1000 +) + type yamlArchive struct { Version string `yaml:"version"` Suites []string `yaml:"suites"` Components []string `yaml:"components"` - Default bool `yaml:"default"` + Priority int `yaml:"priority"` PubKeys []string `yaml:"public-keys"` // V1PubKeys is used for compatibility with format "chisel-v1". V1PubKeys []string `yaml:"v1-public-keys"` @@ -547,14 +571,6 @@ func parseRelease(baseDir, filePath string, data []byte) (*Release, error) { if len(details.Components) == 0 { return nil, fmt.Errorf("%s: archive %q missing components field", fileName, archiveName) } - if len(yamlVar.Archives) == 1 { - details.Default = true - } else if details.Default && release.DefaultArchive != "" { - return nil, fmt.Errorf("%s: more than one default archive: %s, %s", fileName, release.DefaultArchive, archiveName) - } - if details.Default { - release.DefaultArchive = archiveName - } if len(details.PubKeys) == 0 { if yamlVar.Format == "chisel-v1" { return nil, fmt.Errorf("%s: archive %q missing v1-public-keys field", fileName, archiveName) @@ -570,11 +586,15 @@ func parseRelease(baseDir, filePath string, data []byte) (*Release, error) { } archiveKeys = append(archiveKeys, key) } + if details.Priority > MaxArchivePriority || details.Priority < MinArchivePriority { + return nil, fmt.Errorf("%s: archive %q has invalid priority value of %d", fileName, archiveName, details.Priority) + } release.Archives[archiveName] = &Archive{ Name: archiveName, Version: details.Version, Suites: details.Suites, Components: details.Components, + Priority: details.Priority, PubKeys: archiveKeys, } } diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 2d146585..ae919dcf 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -73,8 +73,6 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ - DefaultArchive: "ubuntu", - Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -86,10 +84,9 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Archive: "ubuntu", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", - Slices: map[string]*setup.Slice{}, + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", + Slices: map[string]*setup.Slice{}, }, }, }, @@ -117,8 +114,6 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ - DefaultArchive: "ubuntu", - Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -130,9 +125,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Archive: "ubuntu", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice1": { Package: "mypkg", @@ -179,8 +173,6 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ - DefaultArchive: "ubuntu", - Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -192,9 +184,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Archive: "ubuntu", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice1": { Package: "mypkg", @@ -440,8 +431,6 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ - DefaultArchive: "ubuntu", - Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -453,9 +442,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Archive: "ubuntu", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice1": { Package: "mypkg", @@ -655,8 +643,6 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ - DefaultArchive: "ubuntu", - Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -668,9 +654,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Archive: "ubuntu", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg", @@ -695,8 +680,6 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ - DefaultArchive: "ubuntu", - Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -708,9 +691,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Archive: "ubuntu", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg", @@ -736,8 +718,6 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ - DefaultArchive: "ubuntu", - Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -749,9 +729,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Archive: "ubuntu", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg", @@ -766,7 +745,7 @@ var setupTests = []setupTest{{ }, }, }, { - summary: "Multiple archives", + summary: "Multiple archives with priorities", input: map[string]string{ "chisel.yaml": ` format: chisel-v1 @@ -775,12 +754,13 @@ var setupTests = []setupTest{{ version: 22.04 components: [main, universe] suites: [jammy] - default: true + priority: 20 v1-public-keys: [test-key] bar: version: 22.04 components: [universe] suites: [jammy-updates] + priority: -10 v1-public-keys: [test-key] v1-public-keys: test-key: @@ -792,14 +772,13 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ - DefaultArchive: "foo", - Archives: map[string]*setup.Archive{ "foo": { Name: "foo", Version: "22.04", Suites: []string{"jammy"}, Components: []string{"main", "universe"}, + Priority: 20, PubKeys: []*packet.PublicKey{testKey.PubKey}, }, "bar": { @@ -807,18 +786,65 @@ var setupTests = []setupTest{{ Version: "22.04", Suites: []string{"jammy-updates"}, Components: []string{"universe"}, + Priority: -10, PubKeys: []*packet.PublicKey{testKey.PubKey}, }, }, Packages: map[string]*setup.Package{ "mypkg": { - Archive: "foo", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", - Slices: map[string]*setup.Slice{}, + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", + Slices: map[string]*setup.Slice{}, }, }, }, +}, { + summary: "Two archives cannot have same priority", + input: map[string]string{ + "chisel.yaml": ` + format: chisel-v1 + archives: + foo: + version: 22.04 + components: [main, universe] + suites: [jammy] + priority: 20 + v1-public-keys: [test-key] + bar: + version: 22.04 + components: [universe] + suites: [jammy-updates] + priority: 20 + v1-public-keys: [test-key] + v1-public-keys: + test-key: + id: ` + testKey.ID + ` + armor: |` + "\n" + testutil.PrefixEachLine(testKey.PubKeyArmor, "\t\t\t\t\t\t") + ` + `, + "slices/mydir/mypkg.yaml": ` + package: mypkg + `, + }, + relerror: `chisel.yaml: archives "bar" and "foo" have the same priority value of 20`, +}, { + summary: "Invalid archive priority", + input: map[string]string{ + "chisel.yaml": ` + format: chisel-v1 + archives: + foo: + version: 22.04 + components: [main, universe] + suites: [jammy] + priority: 10000 + v1-public-keys: [test-key] + v1-public-keys: + test-key: + id: ` + testKey.ID + ` + armor: |` + "\n" + testutil.PrefixEachLine(testKey.PubKeyArmor, "\t\t\t\t\t\t") + ` + `, + }, + relerror: `chisel.yaml: archive "foo" has invalid priority value of 10000`, }, { summary: "Extra fields in YAML are ignored (necessary for forward compatibility)", input: map[string]string{ @@ -849,8 +875,6 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ - DefaultArchive: "ubuntu", - Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -862,9 +886,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Archive: "ubuntu", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg", @@ -888,12 +911,13 @@ var setupTests = []setupTest{{ components: [main, universe] suites: [jammy] v1-public-keys: [extra-key] - default: true + priority: 20 bar: version: 22.04 components: [universe] suites: [jammy-updates] v1-public-keys: [test-key, extra-key] + priority: 10 v1-public-keys: extra-key: id: ` + extraTestKey.ID + ` @@ -907,14 +931,13 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ - DefaultArchive: "foo", - Archives: map[string]*setup.Archive{ "foo": { Name: "foo", Version: "22.04", Suites: []string{"jammy"}, Components: []string{"main", "universe"}, + Priority: 20, PubKeys: []*packet.PublicKey{extraTestKey.PubKey}, }, "bar": { @@ -922,15 +945,15 @@ var setupTests = []setupTest{{ Version: "22.04", Suites: []string{"jammy-updates"}, Components: []string{"universe"}, + Priority: 10, PubKeys: []*packet.PublicKey{testKey.PubKey, extraTestKey.PubKey}, }, }, Packages: map[string]*setup.Package{ "mypkg": { - Archive: "foo", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", - Slices: map[string]*setup.Slice{}, + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", + Slices: map[string]*setup.Slice{}, }, }, }, @@ -944,7 +967,6 @@ var setupTests = []setupTest{{ version: 22.04 components: [main, universe] suites: [jammy] - default: true `, }, relerror: `chisel.yaml: archive "foo" missing v1-public-keys field`, @@ -959,7 +981,6 @@ var setupTests = []setupTest{{ components: [main, universe] suites: [jammy] v1-public-keys: [extra-key] - default: true `, "slices/mydir/mypkg.yaml": ` package: mypkg @@ -977,7 +998,6 @@ var setupTests = []setupTest{{ components: [main, universe] suites: [jammy] v1-public-keys: [extra-key] - default: true v1-public-keys: extra-key: id: foo @@ -1005,7 +1025,6 @@ var setupTests = []setupTest{{ components: [main, universe] suites: [jammy] v1-public-keys: [extra-key] - default: true v1-public-keys: extra-key: id: ` + extraTestKey.ID + ` @@ -1028,8 +1047,6 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ - DefaultArchive: "ubuntu", - Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -1041,9 +1058,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "jq": { - Archive: "ubuntu", - Name: "jq", - Path: "slices/mydir/jq.yaml", + Name: "jq", + Path: "slices/mydir/jq.yaml", Slices: map[string]*setup.Slice{ "bins": { Package: "jq", @@ -1082,7 +1098,6 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ - DefaultArchive: "ubuntu", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -1094,9 +1109,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Archive: "ubuntu", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "slice1": { Package: "mypkg", @@ -1151,8 +1165,6 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ - DefaultArchive: "ubuntu", - Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -1164,9 +1176,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Archive: "ubuntu", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "slice1": { Package: "mypkg", @@ -1187,9 +1198,8 @@ var setupTests = []setupTest{{ }, }, "myotherpkg": { - Archive: "ubuntu", - Name: "myotherpkg", - Path: "slices/mydir/myotherpkg.yaml", + Name: "myotherpkg", + Path: "slices/mydir/myotherpkg.yaml", Slices: map[string]*setup.Slice{ "slice1": { Package: "myotherpkg", @@ -1311,6 +1321,15 @@ var setupTests = []setupTest{{ `, }, relerror: `slices test-package_myslice1 and test-package_myslice2 conflict on /dir/\*\* and /dir/file`, +}, { + summary: "Pinned archive is not defined", + input: map[string]string{ + "slices/test-package.yaml": ` + package: test-package + archive: non-existing + `, + }, + relerror: `slices/test-package.yaml: package refers to undefined archive "non-existing"`, }, { summary: "Specify generate: manifest", input: map[string]string{ @@ -1323,8 +1342,6 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ - DefaultArchive: "ubuntu", - Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -1336,9 +1353,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Archive: "ubuntu", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg", @@ -1373,8 +1389,6 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ - DefaultArchive: "ubuntu", - Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -1386,9 +1400,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Archive: "ubuntu", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg", @@ -1465,8 +1478,6 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ - DefaultArchive: "ubuntu", - Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -1478,9 +1489,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Archive: "ubuntu", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg", @@ -1492,9 +1502,8 @@ var setupTests = []setupTest{{ }, }, "mypkg2": { - Archive: "ubuntu", - Name: "mypkg2", - Path: "slices/mydir/mypkg2.yaml", + Name: "mypkg2", + Path: "slices/mydir/mypkg2.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg2", @@ -1562,6 +1571,45 @@ var setupTests = []setupTest{{ `, }, relerror: `slice mypkg_myslice path /path/\*\* has invalid generate options`, +}, { + summary: "Default archive is deprecated and ignored", + input: map[string]string{ + "chisel.yaml": ` + format: chisel-v1 + archives: + ubuntu: + default: true + version: 22.04 + components: [main] + suites: [jammy] + v1-public-keys: [test-key] + v1-public-keys: + test-key: + id: ` + testKey.ID + ` + armor: |` + "\n" + testutil.PrefixEachLine(testKey.PubKeyArmor, "\t\t\t\t\t\t") + ` + `, + "slices/mydir/mypkg.yaml": ` + package: mypkg + `, + }, + release: &setup.Release{ + Archives: map[string]*setup.Archive{ + "ubuntu": { + Name: "ubuntu", + Version: "22.04", + Suites: []string{"jammy"}, + Components: []string{"main"}, + PubKeys: []*packet.PublicKey{testKey.PubKey}, + }, + }, + Packages: map[string]*setup.Package{ + "mypkg": { + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", + Slices: map[string]*setup.Slice{}, + }, + }, + }, }} var defaultChiselYaml = ` diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index e76b07af..89d8b377 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -86,21 +86,16 @@ func Run(options *RunOptions) error { targetDir = filepath.Join(dir, targetDir) } + archives, err := selectPkgArchives(options.Archives, options.Selection) + if err != nil { + return err + } + // Build information to process the selection. extract := make(map[string]map[string][]deb.ExtractInfo) - archives := make(map[string]archive.Archive) for _, slice := range options.Selection.Slices { extractPackage := extract[slice.Package] if extractPackage == nil { - archiveName := options.Selection.Release.Packages[slice.Package].Archive - archive := options.Archives[archiveName] - if archive == nil { - return fmt.Errorf("archive %q not defined", archiveName) - } - if !archive.Exists(slice.Package) { - return fmt.Errorf("slice package %q missing from archive", slice.Package) - } - archives[slice.Package] = archive extractPackage = make(map[string][]deb.ExtractInfo) extract[slice.Package] = extractPackage } @@ -469,3 +464,52 @@ func createFile(targetPath string, pathInfo setup.PathInfo) (*fsutil.Entry, erro MakeParents: true, }) } + +// 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) { + 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 + }) + + pkgArchives := make(map[string]archive.Archive) + for _, s := range selection.Slices { + if _, ok := pkgArchives[s.Package]; ok { + continue + } + pkg := selection.Release.Packages[s.Package] + + 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.Name) { + chosen = archive + break + } + } + if chosen == nil { + return nil, fmt.Errorf("cannot find package %q in archive(s)", pkg.Name) + } + pkgArchives[pkg.Name] = chosen + } + return pkgArchives, nil +} diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 8035ef0d..2cbfe1f3 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -7,6 +7,7 @@ import ( "os" "path" "path/filepath" + "slices" "sort" "strings" @@ -28,7 +29,7 @@ type slicerTest struct { summary string arch string release map[string]string - pkgs map[string]testutil.TestPackage + pkgs []*testutil.TestPackage slices []setup.SliceKey hackopt func(c *C, opts *slicer.RunOptions) filesystem map[string]string @@ -247,12 +248,11 @@ var slicerTests = []slicerTest{{ }, { summary: "Copyright is installed", slices: []setup.SliceKey{{"test-package", "myslice"}}, - pkgs: map[string]testutil.TestPackage{ - "test-package": { - // Add the copyright entries to the package. - Data: testutil.MustMakeDeb(append(testutil.TestPackageEntries, testPackageCopyrightEntries...)), - }, - }, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + // Add the copyright entries to the package. + Data: testutil.MustMakeDeb(append(testutil.TestPackageEntries, testPackageCopyrightEntries...)), + }}, release: map[string]string{ "slices/mydir/test-package.yaml": ` package: test-package @@ -280,14 +280,13 @@ var slicerTests = []slicerTest{{ slices: []setup.SliceKey{ {"test-package", "myslice"}, {"other-package", "myslice"}}, - pkgs: map[string]testutil.TestPackage{ - "test-package": { - Data: testutil.PackageData["test-package"], - }, - "other-package": { - Data: testutil.PackageData["other-package"], - }, - }, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Data: testutil.PackageData["test-package"], + }, { + Name: "other-package", + Data: testutil.PackageData["other-package"], + }}, release: map[string]string{ "slices/mydir/test-package.yaml": ` package: test-package @@ -324,19 +323,18 @@ var slicerTests = []slicerTest{{ slices: []setup.SliceKey{ {"implicit-parent", "myslice"}, {"explicit-dir", "myslice"}}, - pkgs: map[string]testutil.TestPackage{ - "implicit-parent": { - Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(0755, "./dir/"), - testutil.Reg(0644, "./dir/file", "random"), - }), - }, - "explicit-dir": { - Data: testutil.MustMakeDeb([]testutil.TarEntry{ - testutil.Dir(01777, "./dir/"), - }), - }, - }, + pkgs: []*testutil.TestPackage{{ + Name: "implicit-parent", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0755, "./dir/"), + testutil.Reg(0644, "./dir/file", "random"), + }), + }, { + Name: "explicit-dir", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(01777, "./dir/"), + }), + }}, release: map[string]string{ "slices/mydir/implicit-parent.yaml": ` package: implicit-parent @@ -366,14 +364,13 @@ var slicerTests = []slicerTest{{ slices: []setup.SliceKey{ {"test-package", "myslice"}, {"other-package", "myslice"}}, - pkgs: map[string]testutil.TestPackage{ - "test-package": { - Data: testutil.PackageData["test-package"], - }, - "other-package": { - Data: testutil.PackageData["other-package"], - }, - }, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Data: testutil.PackageData["test-package"], + }, { + Name: "other-package", + Data: testutil.PackageData["other-package"], + }}, release: map[string]string{ "slices/mydir/test-package.yaml": ` package: test-package @@ -704,14 +701,13 @@ var slicerTests = []slicerTest{{ }, { summary: "Duplicate copyright symlink is ignored", slices: []setup.SliceKey{{"copyright-symlink-openssl", "bins"}}, - pkgs: map[string]testutil.TestPackage{ - "copyright-symlink-openssl": { - Data: testutil.MustMakeDeb(packageEntries["copyright-symlink-openssl"]), - }, - "copyright-symlink-libssl3": { - Data: testutil.MustMakeDeb(packageEntries["copyright-symlink-libssl3"]), - }, - }, + pkgs: []*testutil.TestPackage{{ + Name: "copyright-symlink-openssl", + Data: testutil.MustMakeDeb(packageEntries["copyright-symlink-openssl"]), + }, { + Name: "copyright-symlink-libssl3", + Data: testutil.MustMakeDeb(packageEntries["copyright-symlink-libssl3"]), + }}, release: map[string]string{ "slices/mydir/copyright-symlink-libssl3.yaml": ` package: copyright-symlink-libssl3 @@ -769,8 +765,106 @@ var slicerTests = []slicerTest{{ }, error: `slice test-package_myslice: content is not a file: /x/y`, }, { - summary: "Non-default archive", + summary: "Multiple archives with priority", + slices: []setup.SliceKey{{"test-package", "myslice"}, {"other-package", "myslice"}}, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Hash: "h1", + Version: "v1", + Arch: "a1", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Reg(0644, "./file", "from foo"), + }), + Archives: []string{"foo"}, + }, { + Name: "test-package", + Hash: "h2", + Version: "v2", + Arch: "a2", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Reg(0644, "./file", "from bar"), + }), + Archives: []string{"bar"}, + }, { + Name: "other-package", + Hash: "h3", + Version: "v3", + Arch: "a3", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Reg(0644, "./other-file", "from bar"), + }), + Archives: []string{"bar"}, + }}, + release: map[string]string{ + "chisel.yaml": ` + format: chisel-v1 + archives: + foo: + version: 22.04 + components: [main, universe] + priority: 20 + v1-public-keys: [test-key] + bar: + version: 22.04 + components: [main] + priority: 10 + v1-public-keys: [test-key] + v1-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: + `, + "slices/mydir/other-package.yaml": ` + package: other-package + slices: + myslice: + contents: + /other-file: + `, + }, + filesystem: map[string]string{ + // The notion of "default" is obsolete and highest priority is selected. + "/file": "file 0644 7a3e00f5", + // Fetched from archive "bar" as no other archive has the package. + "/other-file": "file 0644 fa0c9cdb", + }, + manifestPaths: map[string]string{ + "/file": "file 0644 7a3e00f5 {test-package_myslice}", + "/other-file": "file 0644 fa0c9cdb {other-package_myslice}", + }, + manifestPkgs: map[string]string{ + "test-package": "test-package v1 a1 h1", + "other-package": "other-package v3 a3 h3", + }, +}, { + summary: "Pinned archive bypasses higher priority", slices: []setup.SliceKey{{"test-package", "myslice"}}, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Hash: "h1", + Version: "v1", + Arch: "a1", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Reg(0644, "./file", "from foo"), + }), + Archives: []string{"foo"}, + }, { + Name: "test-package", + Hash: "h2", + Version: "v2", + Arch: "a2", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Reg(0644, "./file", "from bar"), + }), + Archives: []string{"bar"}, + }}, release: map[string]string{ "chisel.yaml": ` format: chisel-v1 @@ -778,11 +872,12 @@ var slicerTests = []slicerTest{{ foo: version: 22.04 components: [main, universe] - default: true + priority: 20 v1-public-keys: [test-key] bar: version: 22.04 components: [main] + priority: 10 v1-public-keys: [test-key] v1-public-keys: test-key: @@ -795,19 +890,172 @@ var slicerTests = []slicerTest{{ slices: myslice: contents: - /dir/nested/file: + /file: `, }, hackopt: func(c *C, opts *slicer.RunOptions) { delete(opts.Archives, "foo") }, filesystem: map[string]string{ - "/dir/": "dir 0755", - "/dir/nested/": "dir 0755", - "/dir/nested/file": "file 0644 84237a05", + // test-package fetched from pinned archive "bar". + "/file": "file 0644 fa0c9cdb", + }, + manifestPaths: map[string]string{ + "/file": "file 0644 fa0c9cdb {test-package_myslice}", + }, + 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: chisel-v1 + archives: + foo: + version: 22.04 + components: [main, universe] + priority: 20 + v1-public-keys: [test-key] + bar: + version: 22.04 + components: [main] + priority: 10 + v1-public-keys: [test-key] + v1-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: chisel-v1 + archives: + foo: + version: 22.04 + components: [main, universe] + priority: 20 + v1-public-keys: [test-key] + bar: + version: 22.04 + components: [main] + priority: 10 + v1-public-keys: [test-key] + v1-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: chisel-v1 + archives: + foo: + version: 22.04 + components: [main, universe] + priority: -20 + v1-public-keys: [test-key] + v1-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"}}, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Hash: "h1", + Version: "v1", + Arch: "a1", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Reg(0644, "./file", "from foo"), + }), + Archives: []string{"foo"}, + }}, + release: map[string]string{ + "chisel.yaml": ` + format: chisel-v1 + archives: + foo: + version: 22.04 + components: [main, universe] + priority: -20 + v1-public-keys: [test-key] + v1-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: foo + slices: + myslice: + contents: + /file: + `, + }, + filesystem: map[string]string{ + "/file": "file 0644 7a3e00f5", }, manifestPaths: map[string]string{ - "/dir/nested/file": "file 0644 84237a05 {test-package_myslice}", + "/file": "file 0644 7a3e00f5 {test-package_myslice}", + }, + manifestPkgs: map[string]string{ + "test-package": "test-package v1 a1 h1", }, }, { summary: "Multiple slices of same package", @@ -1068,22 +1316,19 @@ var slicerTests = []slicerTest{{ {"test-package", "myslice"}, {"other-package", "myslice"}, }, - pkgs: map[string]testutil.TestPackage{ - "test-package": { - Name: "test-package", - Hash: "h1", - Version: "v1", - Arch: "a1", - Data: testutil.PackageData["test-package"], - }, - "other-package": { - Name: "other-package", - Hash: "h2", - Version: "v2", - Arch: "a2", - Data: testutil.PackageData["other-package"], - }, - }, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Hash: "h1", + Version: "v1", + Arch: "a1", + Data: testutil.PackageData["test-package"], + }, { + Name: "other-package", + Hash: "h2", + Version: "v2", + Arch: "a2", + Data: testutil.PackageData["other-package"], + }}, release: map[string]string{ "slices/mydir/test-package.yaml": ` package: test-package @@ -1107,22 +1352,19 @@ var slicerTests = []slicerTest{{ slices: []setup.SliceKey{ {"test-package", "myslice"}, }, - pkgs: map[string]testutil.TestPackage{ - "test-package": { - Name: "test-package", - Hash: "h1", - Version: "v1", - Arch: "a1", - Data: testutil.PackageData["test-package"], - }, - "other-package": { - Name: "other-package", - Hash: "h2", - Version: "v2", - Arch: "a2", - Data: testutil.PackageData["other-package"], - }, - }, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Hash: "h1", + Version: "v1", + Arch: "a1", + Data: testutil.PackageData["test-package"], + }, { + Name: "other-package", + Hash: "h2", + Version: "v2", + Arch: "a2", + Data: testutil.PackageData["other-package"], + }}, release: map[string]string{ "slices/mydir/test-package.yaml": ` package: test-package @@ -1143,19 +1385,18 @@ var slicerTests = []slicerTest{{ }, { summary: "Relative paths are properly trimmed during extraction", slices: []setup.SliceKey{{"test-package", "myslice"}}, - pkgs: map[string]testutil.TestPackage{ - "test-package": { - Data: testutil.MustMakeDeb([]testutil.TarEntry{ - // This particular path starting with "/foo" is chosen to test for - // a particular bug; which appeared due to the usage of - // strings.TrimLeft() instead strings.TrimPrefix() to determine a - // relative path. Since TrimLeft takes in a cutset instead of a - // prefix, the desired relative path was not produced. - // See https://github.com/canonical/chisel/pull/145. - testutil.Dir(0755, "./foo-bar/"), - }), - }, - }, + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + // This particular path starting with "/foo" is chosen to test for + // a particular bug; which appeared due to the usage of + // strings.TrimLeft() instead strings.TrimPrefix() to determine a + // relative path. Since TrimLeft takes in a cutset instead of a + // prefix, the desired relative path was not produced. + // See https://github.com/canonical/chisel/pull/145. + testutil.Dir(0755, "./foo-bar/"), + }), + }}, hackopt: func(c *C, opts *slicer.RunOptions) { opts.TargetDir = filepath.Join(filepath.Clean(opts.TargetDir), "foo") err := os.Mkdir(opts.TargetDir, 0755) @@ -1232,25 +1473,17 @@ func (s *S) TestRun(c *C) { func runSlicerTests(c *C, tests []slicerTest) { for _, test := range tests { - for _, slices := range testutil.Permutations(test.slices) { + for _, testSlices := range testutil.Permutations(test.slices) { c.Logf("Summary: %s", test.summary) if _, ok := test.release["chisel.yaml"]; !ok { test.release["chisel.yaml"] = defaultChiselYaml } if test.pkgs == nil { - test.pkgs = map[string]testutil.TestPackage{ - "test-package": { - Data: testutil.PackageData["test-package"], - }, - } - } - for pkgName, pkg := range test.pkgs { - if pkg.Name == "" { - // We need to add the name for the manifest validation. - pkg.Name = pkgName - test.pkgs[pkgName] = pkg - } + test.pkgs = []*testutil.TestPackage{{ + Name: "test-package", + Data: testutil.PackageData["test-package"], + }} } releaseDir := c.MkDir() @@ -1280,16 +1513,22 @@ func runSlicerTests(c *C, tests []slicerTest) { }, Scripts: setup.SliceScripts{}, } - slices = append(slices, setup.SliceKey{ + testSlices = append(testSlices, setup.SliceKey{ Package: manifestPackage, Slice: "manifest", }) - selection, err := setup.Select(release, slices) + selection, err := setup.Select(release, testSlices) 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, @@ -1298,7 +1537,7 @@ func runSlicerTests(c *C, tests []slicerTest) { Components: setupArchive.Components, Arch: test.arch, }, - Packages: test.pkgs, + Packages: pkgs, } archives[name] = archive } diff --git a/internal/testutil/archive.go b/internal/testutil/archive.go index 77b945cc..ae8258c2 100644 --- a/internal/testutil/archive.go +++ b/internal/testutil/archive.go @@ -10,15 +10,16 @@ import ( type TestArchive struct { Opts archive.Options - Packages map[string]TestPackage + Packages map[string]*TestPackage } type TestPackage struct { - Name string - Version string - Hash string - Arch string - Data []byte + Name string + Version string + Hash string + Arch string + Data []byte + Archives []string } func (a *TestArchive) Options() *archive.Options { From a19c876a48e3fdcfb204886fdfd11173a25055c1 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Mon, 14 Oct 2024 16:51:16 +0200 Subject: [PATCH 02/11] more descriptive variable name --- internal/slicer/slicer.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 89d8b377..b45d2065 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -86,7 +86,7 @@ func Run(options *RunOptions) error { targetDir = filepath.Join(dir, targetDir) } - archives, err := selectPkgArchives(options.Archives, options.Selection) + pkgToArchive, err := selectPkgArchives(options.Archives, options.Selection) if err != nil { return err } @@ -99,7 +99,7 @@ func Run(options *RunOptions) error { extractPackage = make(map[string][]deb.ExtractInfo) extract[slice.Package] = extractPackage } - arch := archives[slice.Package].Options().Arch + arch := pkgToArchive[slice.Package].Options().Arch copyrightPath := "/usr/share/doc/" + slice.Package + "/copyright" hasCopyright := false for targetPath, pathInfo := range slice.Contents { @@ -151,7 +151,7 @@ func Run(options *RunOptions) error { if packages[slice.Package] != nil { continue } - reader, info, err := archives[slice.Package].Fetch(slice.Package) + reader, info, err := pkgToArchive[slice.Package].Fetch(slice.Package) if err != nil { return err } @@ -248,7 +248,7 @@ func Run(options *RunOptions) error { // them to the appropriate slices. relPaths := map[string][]*setup.Slice{} for _, slice := range options.Selection.Slices { - arch := archives[slice.Package].Options().Arch + arch := pkgToArchive[slice.Package].Options().Arch for relPath, pathInfo := range slice.Contents { if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, arch) { continue @@ -482,9 +482,9 @@ func selectPkgArchives(archives map[string]archive.Archive, selection *setup.Sel return b.Priority - a.Priority }) - pkgArchives := make(map[string]archive.Archive) + pkgToArchive := make(map[string]archive.Archive) for _, s := range selection.Slices { - if _, ok := pkgArchives[s.Package]; ok { + if _, ok := pkgToArchive[s.Package]; ok { continue } pkg := selection.Release.Packages[s.Package] @@ -509,7 +509,7 @@ 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.Name) } - pkgArchives[pkg.Name] = chosen + pkgToArchive[pkg.Name] = chosen } - return pkgArchives, nil + return pkgToArchive, nil } From bb18527ae836fe79bdc978cbd8f2eb5993e9dcd9 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Mon, 14 Oct 2024 19:25:17 +0200 Subject: [PATCH 03/11] pkgToArchive -> pkgArchive --- internal/slicer/slicer.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index b45d2065..2991e253 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -86,7 +86,7 @@ func Run(options *RunOptions) error { targetDir = filepath.Join(dir, targetDir) } - pkgToArchive, err := selectPkgArchives(options.Archives, options.Selection) + pkgArchive, err := selectPkgArchives(options.Archives, options.Selection) if err != nil { return err } @@ -99,7 +99,7 @@ func Run(options *RunOptions) error { extractPackage = make(map[string][]deb.ExtractInfo) extract[slice.Package] = extractPackage } - arch := pkgToArchive[slice.Package].Options().Arch + arch := pkgArchive[slice.Package].Options().Arch copyrightPath := "/usr/share/doc/" + slice.Package + "/copyright" hasCopyright := false for targetPath, pathInfo := range slice.Contents { @@ -151,7 +151,7 @@ func Run(options *RunOptions) error { if packages[slice.Package] != nil { continue } - reader, info, err := pkgToArchive[slice.Package].Fetch(slice.Package) + reader, info, err := pkgArchive[slice.Package].Fetch(slice.Package) if err != nil { return err } @@ -248,7 +248,7 @@ func Run(options *RunOptions) error { // them to the appropriate slices. relPaths := map[string][]*setup.Slice{} for _, slice := range options.Selection.Slices { - arch := pkgToArchive[slice.Package].Options().Arch + arch := pkgArchive[slice.Package].Options().Arch for relPath, pathInfo := range slice.Contents { if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, arch) { continue @@ -482,9 +482,9 @@ func selectPkgArchives(archives map[string]archive.Archive, selection *setup.Sel return b.Priority - a.Priority }) - pkgToArchive := make(map[string]archive.Archive) + pkgArchive := make(map[string]archive.Archive) for _, s := range selection.Slices { - if _, ok := pkgToArchive[s.Package]; ok { + if _, ok := pkgArchive[s.Package]; ok { continue } pkg := selection.Release.Packages[s.Package] @@ -509,7 +509,7 @@ 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.Name) } - pkgToArchive[pkg.Name] = chosen + pkgArchive[pkg.Name] = chosen } - return pkgToArchive, nil + return pkgArchive, nil } From a8cdd864714de2deee9d9b47b539f0ba769adc68 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Tue, 15 Oct 2024 15:28:07 +0200 Subject: [PATCH 04/11] respect default archive if priorities not being used --- internal/setup/setup.go | 27 ++++++++ internal/setup/setup_test.go | 118 +++++++++++++++++++++++++++++++++-- 2 files changed, 141 insertions(+), 4 deletions(-) diff --git a/internal/setup/setup.go b/internal/setup/setup.go index 39fea30f..7190c9d0 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -399,6 +399,7 @@ type yamlArchive struct { Suites []string `yaml:"suites"` Components []string `yaml:"components"` Priority int `yaml:"priority"` + Default bool `yaml:"default"` PubKeys []string `yaml:"public-keys"` } @@ -537,6 +538,11 @@ func parseRelease(baseDir, filePath string, data []byte) (*Release, error) { pubKeys[keyName] = key } + // For compatibility if there is a default archive set and priorities are + // not being used, we will revert back to the default archive behaviour. + hasDefault := false + hasPriority := false + var defaultArchive string for archiveName, details := range yamlVar.Archives { if details.Version == "" { return nil, fmt.Errorf("%s: archive %q missing version field", fileName, archiveName) @@ -547,6 +553,13 @@ func parseRelease(baseDir, filePath string, data []byte) (*Release, error) { if len(details.Components) == 0 { return nil, fmt.Errorf("%s: archive %q missing components field", fileName, archiveName) } + if details.Default && defaultArchive != "" { + return nil, fmt.Errorf("%s: more than one default archive: %s, %s", fileName, defaultArchive, archiveName) + } + if details.Default { + defaultArchive = archiveName + hasDefault = true + } if len(details.PubKeys) == 0 { return nil, fmt.Errorf("%s: archive %q missing public-keys field", fileName, archiveName) } @@ -561,6 +574,9 @@ func parseRelease(baseDir, filePath string, data []byte) (*Release, error) { if details.Priority > MaxArchivePriority || details.Priority < MinArchivePriority { return nil, fmt.Errorf("%s: archive %q has invalid priority value of %d", fileName, archiveName, details.Priority) } + if details.Priority != 0 { + hasPriority = true + } release.Archives[archiveName] = &Archive{ Name: archiveName, Version: details.Version, @@ -570,6 +586,17 @@ func parseRelease(baseDir, filePath string, data []byte) (*Release, error) { PubKeys: archiveKeys, } } + if hasDefault && !hasPriority { + // For compatibility with the default archive behaviour we will set + // negative priorities to all but the default one, which means all + // others will be ignored unless pinned. + i := -1 + for archiveName, _ := range yamlVar.Archives { + release.Archives[archiveName].Priority = i + i-- + } + release.Archives[defaultArchive].Priority = 0 + } return release, err } diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 26105238..79fc9707 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -1602,17 +1602,27 @@ var setupTests = []setupTest{{ }, relerror: `chisel.yaml: unknown format "chisel-v1"`, }, { - summary: "Default archive is deprecated and ignored", + summary: "Default archive compatibility", input: map[string]string{ "chisel.yaml": ` format: v1 archives: - ubuntu: + default: default: true version: 22.04 components: [main] suites: [jammy] public-keys: [test-key] + other-1: + version: 22.04 + components: [main] + suites: [jammy] + public-keys: [test-key] + other-2: + version: 22.04 + components: [main] + suites: [jammy] + public-keys: [test-key] public-keys: test-key: id: ` + testKey.ID + ` @@ -1624,12 +1634,29 @@ var setupTests = []setupTest{{ }, release: &setup.Release{ Archives: map[string]*setup.Archive{ - "ubuntu": { - Name: "ubuntu", + "default": { + Name: "default", + Version: "22.04", + Suites: []string{"jammy"}, + Components: []string{"main"}, + PubKeys: []*packet.PublicKey{testKey.PubKey}, + Priority: 0, + }, + "other-1": { + Name: "other-1", + Version: "22.04", + Suites: []string{"jammy"}, + Components: []string{"main"}, + PubKeys: []*packet.PublicKey{testKey.PubKey}, + Priority: -2, + }, + "other-2": { + Name: "other-2", Version: "22.04", Suites: []string{"jammy"}, Components: []string{"main"}, PubKeys: []*packet.PublicKey{testKey.PubKey}, + Priority: -3, }, }, Packages: map[string]*setup.Package{ @@ -1640,6 +1667,89 @@ var setupTests = []setupTest{{ }, }, }, +}, { + summary: "Default is ignored", + input: map[string]string{ + "chisel.yaml": ` + format: v1 + archives: + default: + default: true + priority: 10 + version: 22.04 + components: [main] + suites: [jammy] + public-keys: [test-key] + other: + priority: 20 + version: 22.04 + components: [main] + suites: [jammy] + 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/mypkg.yaml": ` + package: mypkg + `, + }, + release: &setup.Release{ + Archives: map[string]*setup.Archive{ + "default": { + Name: "default", + Version: "22.04", + Suites: []string{"jammy"}, + Components: []string{"main"}, + PubKeys: []*packet.PublicKey{testKey.PubKey}, + Priority: 10, + }, + "other": { + Name: "other", + Version: "22.04", + Suites: []string{"jammy"}, + Components: []string{"main"}, + PubKeys: []*packet.PublicKey{testKey.PubKey}, + Priority: 20, + }, + }, + Packages: map[string]*setup.Package{ + "mypkg": { + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", + Slices: map[string]*setup.Slice{}, + }, + }, + }, +}, { + summary: "Multiple default archives", + input: map[string]string{ + "chisel.yaml": ` + format: v1 + archives: + foo: + default: true + version: 22.04 + components: [main] + suites: [jammy] + public-keys: [test-key] + bar: + default: true + version: 22.04 + components: [main, universe] + suites: [jammy] + v1-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/mypkg.yaml": ` + package: mypkg + `, + }, + relerror: `chisel.yaml: more than one default archive: foo, bar`, }} var defaultChiselYaml = ` From 40d08675a75bbca18910b7952c756bec8f4c7c83 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Tue, 15 Oct 2024 15:39:17 +0200 Subject: [PATCH 05/11] simplify for loop, unnecessary variable --- internal/setup/setup.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/setup/setup.go b/internal/setup/setup.go index 7190c9d0..f62c72d1 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -591,7 +591,7 @@ func parseRelease(baseDir, filePath string, data []byte) (*Release, error) { // negative priorities to all but the default one, which means all // others will be ignored unless pinned. i := -1 - for archiveName, _ := range yamlVar.Archives { + for archiveName := range yamlVar.Archives { release.Archives[archiveName].Priority = i i-- } From 7d1e3f0d75f99481c185eae070b2723ef88d41c5 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Wed, 16 Oct 2024 11:27:29 +0200 Subject: [PATCH 06/11] remove unnecessary bool flag --- internal/setup/setup.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/setup/setup.go b/internal/setup/setup.go index f62c72d1..3381a6ed 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -540,7 +540,6 @@ func parseRelease(baseDir, filePath string, data []byte) (*Release, error) { // For compatibility if there is a default archive set and priorities are // not being used, we will revert back to the default archive behaviour. - hasDefault := false hasPriority := false var defaultArchive string for archiveName, details := range yamlVar.Archives { @@ -558,7 +557,6 @@ func parseRelease(baseDir, filePath string, data []byte) (*Release, error) { } if details.Default { defaultArchive = archiveName - hasDefault = true } if len(details.PubKeys) == 0 { return nil, fmt.Errorf("%s: archive %q missing public-keys field", fileName, archiveName) @@ -586,7 +584,7 @@ func parseRelease(baseDir, filePath string, data []byte) (*Release, error) { PubKeys: archiveKeys, } } - if hasDefault && !hasPriority { + if defaultArchive != "" && !hasPriority { // For compatibility with the default archive behaviour we will set // negative priorities to all but the default one, which means all // others will be ignored unless pinned. From 118322602beb0d9f64c1b90c05ae43b9046c69bf Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Wed, 16 Oct 2024 11:46:19 +0200 Subject: [PATCH 07/11] make tests deterministic --- internal/setup/setup.go | 10 +++++++--- internal/setup/setup_test.go | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/setup/setup.go b/internal/setup/setup.go index 3381a6ed..40999bd2 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -588,10 +588,14 @@ func parseRelease(baseDir, filePath string, data []byte) (*Release, error) { // For compatibility with the default archive behaviour we will set // negative priorities to all but the default one, which means all // others will be ignored unless pinned. - i := -1 + var archiveNames []string for archiveName := range yamlVar.Archives { - release.Archives[archiveName].Priority = i - i-- + archiveNames = append(archiveNames, archiveName) + } + // Make it deterministic. + slices.Sort(archiveNames) + for i, archiveName := range archiveNames { + release.Archives[archiveName].Priority = -i - 1 } release.Archives[defaultArchive].Priority = 0 } diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 79fc9707..43ae6faa 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -1739,7 +1739,7 @@ var setupTests = []setupTest{{ version: 22.04 components: [main, universe] suites: [jammy] - v1-public-keys: [test-key] + public-keys: [test-key] public-keys: test-key: id: ` + testKey.ID + ` From bb5f58b516e11577d60ac0b659a6873fd57a79c6 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Wed, 16 Oct 2024 11:48:42 +0200 Subject: [PATCH 08/11] make more tests deterministic --- internal/setup/setup.go | 3 +++ internal/setup/setup_test.go | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/setup/setup.go b/internal/setup/setup.go index 40999bd2..8f25e5da 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -553,6 +553,9 @@ func parseRelease(baseDir, filePath string, data []byte) (*Release, error) { return nil, fmt.Errorf("%s: archive %q missing components field", fileName, archiveName) } if details.Default && defaultArchive != "" { + if archiveName < defaultArchive { + archiveName, defaultArchive = defaultArchive, archiveName + } return nil, fmt.Errorf("%s: more than one default archive: %s, %s", fileName, defaultArchive, archiveName) } if details.Default { diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 43ae6faa..84704921 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -1749,7 +1749,7 @@ var setupTests = []setupTest{{ package: mypkg `, }, - relerror: `chisel.yaml: more than one default archive: foo, bar`, + relerror: `chisel.yaml: more than one default archive: bar, foo`, }} var defaultChiselYaml = ` From 4962b27187613e585c20bd00590b505cdb50c3d4 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Thu, 17 Oct 2024 11:51:57 +0200 Subject: [PATCH 09/11] Trigger CI From 418ea0d85a5d307da5801536b9ab98d1af65fb55 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Fri, 18 Oct 2024 16:36:39 +0200 Subject: [PATCH 10/11] fix: empty priority is no longer valid --- internal/setup/setup.go | 23 ++++++++++++---- internal/setup/setup_test.go | 53 ++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 6 deletions(-) diff --git a/internal/setup/setup.go b/internal/setup/setup.go index 8f25e5da..d3ceaf68 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -398,7 +398,7 @@ type yamlArchive struct { Version string `yaml:"version"` Suites []string `yaml:"suites"` Components []string `yaml:"components"` - Priority int `yaml:"priority"` + Priority *int `yaml:"priority"` Default bool `yaml:"default"` PubKeys []string `yaml:"public-keys"` } @@ -542,6 +542,7 @@ func parseRelease(baseDir, filePath string, data []byte) (*Release, error) { // not being used, we will revert back to the default archive behaviour. hasPriority := false var defaultArchive string + var archiveNoPriority string for archiveName, details := range yamlVar.Archives { if details.Version == "" { return nil, fmt.Errorf("%s: archive %q missing version field", fileName, archiveName) @@ -572,18 +573,28 @@ func parseRelease(baseDir, filePath string, data []byte) (*Release, error) { } archiveKeys = append(archiveKeys, key) } - if details.Priority > MaxArchivePriority || details.Priority < MinArchivePriority { - return nil, fmt.Errorf("%s: archive %q has invalid priority value of %d", fileName, archiveName, details.Priority) - } - if details.Priority != 0 { + priority := 0 + if details.Priority != nil { hasPriority = true + if archiveNoPriority != "" { + return nil, fmt.Errorf("%s: archive %q missing priority", fileName, archiveNoPriority) + } + priority = *details.Priority + if priority > MaxArchivePriority || priority < MinArchivePriority { + return nil, fmt.Errorf("%s: archive %q has invalid priority value of %d", fileName, archiveName, priority) + } + } else { + if hasPriority { + return nil, fmt.Errorf("%s: archive %q missing priority", fileName, archiveName) + } + archiveNoPriority = archiveName } release.Archives[archiveName] = &Archive{ Name: archiveName, Version: details.Version, Suites: details.Suites, Components: details.Components, - Priority: details.Priority, + Priority: priority, PubKeys: archiveKeys, } } diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 84704921..5afebb0b 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -798,6 +798,59 @@ var setupTests = []setupTest{{ }, }, }, +}, { + summary: "Multiple archives inconsistent use of priorities", + input: map[string]string{ + "chisel.yaml": ` + format: v1 + archives: + foo: + version: 22.04 + components: [main, universe] + suites: [jammy] + priority: 20 + public-keys: [test-key] + bar: + version: 22.04 + components: [universe] + suites: [jammy-updates] + 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/mypkg.yaml": ` + package: mypkg + `, + }, + relerror: `chisel.yaml: archive "bar" missing priority`, +}, { + summary: "Multiple archives with no priorities", + input: map[string]string{ + "chisel.yaml": ` + format: v1 + archives: + foo: + version: 22.04 + components: [main, universe] + suites: [jammy] + public-keys: [test-key] + bar: + version: 22.04 + components: [universe] + suites: [jammy-updates] + 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/mypkg.yaml": ` + package: mypkg + `, + }, + relerror: `chisel.yaml: archives "bar" and "foo" have the same priority value of 0`, }, { summary: "Archive with suites unset", input: map[string]string{ From c52173aacd00ecebfdb0fdc41d53b16157e8a56d Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Fri, 18 Oct 2024 17:17:57 +0200 Subject: [PATCH 11/11] fix: priority cannot be 0 --- internal/setup/setup.go | 17 +++++++++-------- internal/setup/setup_test.go | 25 ++++++++++++++++++++++--- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/internal/setup/setup.go b/internal/setup/setup.go index d3ceaf68..00337bb6 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -576,18 +576,15 @@ func parseRelease(baseDir, filePath string, data []byte) (*Release, error) { priority := 0 if details.Priority != nil { hasPriority = true - if archiveNoPriority != "" { - return nil, fmt.Errorf("%s: archive %q missing priority", fileName, archiveNoPriority) - } priority = *details.Priority - if priority > MaxArchivePriority || priority < MinArchivePriority { + if priority > MaxArchivePriority || priority < MinArchivePriority || priority == 0 { return nil, fmt.Errorf("%s: archive %q has invalid priority value of %d", fileName, archiveName, priority) } } else { - if hasPriority { - return nil, fmt.Errorf("%s: archive %q missing priority", fileName, archiveName) + if archiveNoPriority == "" || archiveName < archiveNoPriority { + // Make it deterministic. + archiveNoPriority = archiveName } - archiveNoPriority = archiveName } release.Archives[archiveName] = &Archive{ Name: archiveName, @@ -598,6 +595,10 @@ func parseRelease(baseDir, filePath string, data []byte) (*Release, error) { PubKeys: archiveKeys, } } + if (hasPriority && archiveNoPriority != "") || + (!hasPriority && defaultArchive == "" && len(yamlVar.Archives) > 1) { + return nil, fmt.Errorf("%s: archive %q is missing the priority setting", fileName, archiveNoPriority) + } if defaultArchive != "" && !hasPriority { // For compatibility with the default archive behaviour we will set // negative priorities to all but the default one, which means all @@ -611,7 +612,7 @@ func parseRelease(baseDir, filePath string, data []byte) (*Release, error) { for i, archiveName := range archiveNames { release.Archives[archiveName].Priority = -i - 1 } - release.Archives[defaultArchive].Priority = 0 + release.Archives[defaultArchive].Priority = 1 } return release, err diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 5afebb0b..d8d2f564 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -824,7 +824,7 @@ var setupTests = []setupTest{{ package: mypkg `, }, - relerror: `chisel.yaml: archive "bar" missing priority`, + relerror: `chisel.yaml: archive "bar" is missing the priority setting`, }, { summary: "Multiple archives with no priorities", input: map[string]string{ @@ -850,7 +850,7 @@ var setupTests = []setupTest{{ package: mypkg `, }, - relerror: `chisel.yaml: archives "bar" and "foo" have the same priority value of 0`, + relerror: `chisel.yaml: archive "bar" is missing the priority setting`, }, { summary: "Archive with suites unset", input: map[string]string{ @@ -910,6 +910,25 @@ var setupTests = []setupTest{{ `, }, relerror: `chisel.yaml: archive "foo" has invalid priority value of 10000`, +}, { + summary: "Invalid archive priority of 0", + input: map[string]string{ + "chisel.yaml": ` + format: v1 + archives: + foo: + version: 22.04 + components: [main, universe] + suites: [jammy] + priority: 0 + public-keys: [test-key] + public-keys: + test-key: + id: ` + testKey.ID + ` + armor: |` + "\n" + testutil.PrefixEachLine(testKey.PubKeyArmor, "\t\t\t\t\t\t") + ` + `, + }, + relerror: `chisel.yaml: archive "foo" has invalid priority value of 0`, }, { summary: "Extra fields in YAML are ignored (necessary for forward compatibility)", input: map[string]string{ @@ -1693,7 +1712,7 @@ var setupTests = []setupTest{{ Suites: []string{"jammy"}, Components: []string{"main"}, PubKeys: []*packet.PublicKey{testKey.PubKey}, - Priority: 0, + Priority: 1, }, "other-1": { Name: "other-1",