Skip to content

Commit

Permalink
Make package version selection deterministic
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
woky committed Sep 19, 2023
1 parent ec72c47 commit f11c308
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 8 deletions.
22 changes: 14 additions & 8 deletions internal/slicer/slicer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
}
Expand Down
70 changes: 70 additions & 0 deletions internal/slicer/slicer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.MustMakeDeb([]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.MustMakeDeb([]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.MustMakeDeb([]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 = `
Expand Down

0 comments on commit f11c308

Please sign in to comment.