From 2daf446f1e9907ab4a990c4ee0a177acb6c7bc06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Virtus?= Date: Wed, 31 May 2023 13:23:19 +0200 Subject: [PATCH] Make package version selection deterministic When two or more archives with highest priority contain a package with the same version, the choice is effectively random, because the iteration order over options.Archives map is not defined. Get rid of this nondeterminism by sorting archives by their priority and then lexicographically by their labels (greater label wins) when the priorities are equal. Since archive labels are used as keys in YAML map, they should be unique and the order should be total. --- internal/slicer/slicer.go | 22 +++++++---- internal/slicer/slicer_test.go | 70 ++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 8 deletions(-) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 70c49e08..e6c8e4b9 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -86,17 +86,23 @@ func Run(options *RunOptions) error { // if any. // // If two or more archives with the same priority contain the - // package with the same version, the first archive from the - // options.Archives iterator is used. In effect, the choice is - // random. + // package with the same version, the archive with + // lexicographically greatest label is used. Since archive + // labels are used as keys in YAML map, they should be unique. orderedArchives := make([]archive.Archive, 0, len(options.Archives)) for _, archive := range options.Archives { orderedArchives = append(orderedArchives, archive) } - sort.Slice(orderedArchives, func(a, b int) bool { - prioA := orderedArchives[a].Options().Priority - prioB := orderedArchives[b].Options().Priority - return prioA > prioB + sort.Slice(orderedArchives, func(i, j int) bool { + a := orderedArchives[i].Options() + b := orderedArchives[j].Options() + if a.Priority == b.Priority { + if a.Label == b.Label { + panic("internal error: archive labels are not unique") + } + return a.Label > b.Label + } + return a.Priority > b.Priority }) // Build information to process the selection. @@ -109,7 +115,7 @@ func Run(options *RunOptions) error { for _, currentArchive := range orderedArchives { if prio := currentArchive.Options().Priority; prio < currentPrio { if selectedVersion != "" { - break + break // already have a package from a higher priority archive } currentPrio = prio } diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 2cb611f3..100e4e20 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -773,6 +773,76 @@ var slicerTests = []slicerTest{{ result: map[string]string{ "/hello": "file 0644 b5621b65", }, +}, { + summary: "Archive with lexicographically greatest label wins on stalemate", + pkgs: map[string]map[string]testPackage{ + "10-official": { + "hello": testPackage{ + info: map[string]string{ + "Version": "1.2", + }, + content: testutil.MakeTestDeb([]testutil.TarEntry{ + DIR(0755, "./"), + REG(0644, "./hello", "Hello, The Official 1.2\n"), + }), + }, + }, + "50-thirdparty": { + "hello": testPackage{ + info: map[string]string{ + "Version": "1.2", + }, + content: testutil.MakeTestDeb([]testutil.TarEntry{ + DIR(0755, "./"), + REG(0644, "./hello", "Hello, The Third-party 1.2\n"), + }), + }, + }, + "99-testing": { + "hello": testPackage{ + info: map[string]string{ + "Version": "1.3", + }, + content: testutil.MakeTestDeb([]testutil.TarEntry{ + DIR(0755, "./"), + REG(0644, "./hello", "Hello, The Testing 1.3\n"), + }), + }, + }, + }, + release: map[string]string{ + "chisel.yaml": ` + format: chisel-v1 + archives: + 10-official: + version: 1 + suites: [main] + components: [main] + priority: 10 + 50-thirdparty: + version: 1 + suites: [main] + components: [main] + priority: 10 + 99-testing: + version: 1 + suites: [main] + components: [main] + `, + "slices/mydir/hello.yaml": ` + package: hello + slices: + all: + contents: + /hello: + `, + }, + slices: []setup.SliceKey{ + {"hello", "all"}, + }, + result: map[string]string{ + "/hello": "file 0644 d538d7b8", // "Hello, The Testing 1.3\n" + }, }} const defaultChiselYaml = `