From 6ad7774e5d3abc4c5ad005dc82396d04d58a33bf Mon Sep 17 00:00:00 2001 From: William Douglas Date: Mon, 6 Jan 2025 10:54:48 -0800 Subject: [PATCH 1/3] Add missing SSE modifier mappings These were unintentionally left out and while unlikely to be used should be included for correctness. Signed-off-by: William Douglas --- swupd/files.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/swupd/files.go b/swupd/files.go index 758a4ae6..4a2c4253 100644 --- a/swupd/files.go +++ b/swupd/files.go @@ -101,6 +101,9 @@ var modifierPrefixes = map[ModifierFlag]string{ Sse2: "", Sse3: "", Sse4: "", + Sse5: "", + Sse6: "", + Sse7: "", Avx2_1: "/V3", Avx2_3: "/V3", Avx2_5: "/V3", From d067a08dce0bc967a39e512c3980e930cc4cdc0c Mon Sep 17 00:00:00 2001 From: William Douglas Date: Mon, 6 Jan 2025 11:19:27 -0800 Subject: [PATCH 2/3] Limit the amount of certain modifiers in packs Systems using certain modifiers are currently not used as frequently so in order to reduce pack size and improve update times skip those modifiers from being considered for packs. Currently the SSE version is skipped when either AVX2 or AVX512 versions exist and APX is always skipped. This change does not apply to 0 packs as it would break installer offline support. Signed-off-by: William Douglas --- swupd/delta.go | 4 ++-- swupd/delta_test.go | 26 +++++++++++++++++++++++--- swupd/files.go | 20 ++++++++++++++++++++ swupd/files_test.go | 37 +++++++++++++++++++++++++++++++++++++ swupd/helpers_test.go | 24 ++++++++++++++++-------- swupd/packs.go | 4 ++++ swupd/packs_test.go | 16 ++++++++++++---- 7 files changed, 114 insertions(+), 17 deletions(-) diff --git a/swupd/delta.go b/swupd/delta.go index 52b753d3..2b476046 100644 --- a/swupd/delta.go +++ b/swupd/delta.go @@ -315,7 +315,7 @@ func findDeltas(c *config, oldManifest, newManifest *Manifest) ([]Delta, error) deltaCount := 0 for _, nf := range newManifest.Files { - if nf.DeltaPeer != nil { + if nf.DeltaPeer != nil && nf.useInPack() { deltaCount++ } } @@ -327,7 +327,7 @@ func findDeltas(c *config, oldManifest, newManifest *Manifest) ([]Delta, error) seen := make(map[string]bool) for _, nf := range newManifest.Files { - if nf.DeltaPeer == nil { + if nf.DeltaPeer == nil || !nf.useInPack() { continue } diff --git a/swupd/delta_test.go b/swupd/delta_test.go index c04112bf..e5070d4e 100644 --- a/swupd/delta_test.go +++ b/swupd/delta_test.go @@ -22,7 +22,7 @@ func TestCreateDeltas(t *testing.T) { mustMkdir(t, filepath.Join(ts.Dir, "www/20/delta")) mustCreateAllDeltas(t, "Manifest.full", ts.Dir, 10, 20) - mustExistDelta(t, ts.Dir, "/bar", 10, 20) + mustExistDelta(t, ts.Dir, "/bar", Sse0, 10, 20) } func TestCreateDeltaTooBig(t *testing.T) { @@ -41,7 +41,7 @@ func TestCreateDeltaTooBig(t *testing.T) { mustMkdir(t, filepath.Join(ts.Dir, "www/20/delta")) tryCreateAllDeltas(t, "Manifest.full", ts.Dir, 10, 20) - mustNotExistDelta(t, ts.Dir, "/foo", 10, 20) + mustNotExistDelta(t, ts.Dir, "/foo", Sse0, 10, 20) } func TestCreateDeltaFULLDL(t *testing.T) { @@ -61,7 +61,7 @@ m,cvnxcpowertw54lsi8ydoprf g,mdbng.c,mvnxb,.mxhstu;lwey5o;sdfjklgx;cnvjnxbasdfh` mustMkdir(t, filepath.Join(ts.Dir, "www/20/delta")) tryCreateAllDeltas(t, "Manifest.full", ts.Dir, 10, 20) - mustNotExistDelta(t, ts.Dir, "/foo", 10, 20) + mustNotExistDelta(t, ts.Dir, "/foo", Sse0, 10, 20) } // Imported from swupd-server/test/functional/no-delta. @@ -146,3 +146,23 @@ func TestNoDeltasForTypeChangesOrDereferencedSymlinks(t *testing.T) { t.Fatalf("found %d files in %s but expected %d", len(fis), ts.path("www/20/delta"), info.DeltaCount) } } + +func TestOnlyUseInPackDeltas(t *testing.T) { + ts := newTestSwupd(t, "deltas") + defer ts.cleanup() + ts.Bundles = []string{"test-bundle1", "test-bundle2"} + ts.addFile(10, "test-bundle1", "/foo", strings.Repeat("foo", 100)) + ts.addFile(10, "test-bundle2", "/bar", strings.Repeat("bar", 100)) + ts.addFile(10, "test-bundle2", "/V3/bar", strings.Repeat("V3bar", 100)) + ts.createManifests(10) + + ts.addFile(20, "test-bundle1", "/foo", strings.Repeat("foo", 100)) + ts.addFile(20, "test-bundle2", "/bar", strings.Repeat("bar", 100)+"testingdelta") + ts.addFile(20, "test-bundle2", "/V3/bar", strings.Repeat("V3bar", 100)+"testingdelta") + ts.createManifests(20) + mustMkdir(t, filepath.Join(ts.Dir, "www/20/delta")) + + mustCreateAllDeltas(t, "Manifest.full", ts.Dir, 10, 20) + mustNotExistDelta(t, ts.Dir, "/bar", Sse1, 10, 20) + mustExistDelta(t, ts.Dir, "/V3/bar", Avx2_1, 10, 20) +} diff --git a/swupd/files.go b/swupd/files.go index 4a2c4253..8124b7ef 100644 --- a/swupd/files.go +++ b/swupd/files.go @@ -459,6 +459,16 @@ func (f *File) findFileNameInSlice(fs []*File) *File { return nil } +func (f *File) findFileNameAndModifierInSlice(fs []*File) *File { + for _, file := range fs { + if file.Name == f.Name && file.Modifier == f.Modifier { + return file + } + } + + return nil +} + func (f *File) isUnsupportedTypeChange() bool { if f.DeltaPeer == nil { // nothing to check, new or deleted file @@ -489,3 +499,13 @@ func (f *File) Present() bool { func (f *File) hasOptPrefix() bool { return strings.HasPrefix(f.Name, "/V3") || strings.HasPrefix(f.Name, "/V4") || strings.HasPrefix(f.Name, "/VA") } + +func (f *File) useInPack() bool { + // At this point SSE files that also have an AVX version + // aren't going to be added to packs, simalarly, APX systems + // aren't popular enough yet to add to packs + var sseWithAvxOptBin = f.Modifier == Sse1 || f.Modifier == Sse2 || f.Modifier == Sse3 + var apx = f.Modifier == Apx4 || f.Modifier == Apx5 || f.Modifier == Apx6 || f.Modifier == Apx7 + var useFile = !apx && !sseWithAvxOptBin + return useFile +} diff --git a/swupd/files_test.go b/swupd/files_test.go index 37c8ff1c..efef3aee 100644 --- a/swupd/files_test.go +++ b/swupd/files_test.go @@ -314,6 +314,43 @@ func TestFindFileNameInSlice(t *testing.T) { } } +func TestFindFileNameAndModifierInSlice(t *testing.T) { + fs := []*File{ + {Name: "1", Modifier: Sse0}, + {Name: "2", Modifier: Avx2_1}, + {Name: "3", Modifier: Apx7}, + } + + testCases := []struct { + name string + modifier ModifierFlag + hasMatch bool + expectedIdx int + }{ + {"1", Sse0, true, 0}, + {"2", Avx2_1, true, 1}, + {"3", Apx7, true, 2}, + {"1", Sse1, false, 9}, + {"2", Avx2_3, false, 9}, + {"3", Apx4, false, 9}, + {"4", Sse0, false, 9}, + {"notpresent", Avx512_2, false, 9}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + f := File{Name: tc.name} + found := f.findFileNameInSlice(fs) + if tc.hasMatch { + if found.Name != fs[tc.expectedIdx].Name { + t.Errorf("findFileNameInSlice returned %v when %v expected", + found.Name, fs[tc.expectedIdx].Name) + } + } + }) + } +} + func TestTypeHasChanged(t *testing.T) { testCases := []struct { file File diff --git a/swupd/helpers_test.go b/swupd/helpers_test.go index f57d9db9..dff53084 100644 --- a/swupd/helpers_test.go +++ b/swupd/helpers_test.go @@ -124,7 +124,7 @@ func mustExist(t *testing.T, name string) { } } -func existDelta(t *testing.T, testDir, filename string, from, to uint32) string { +func existDelta(t *testing.T, testDir, filename string, modifier ModifierFlag, from, to uint32) string { t.Helper() var fromFull *Manifest var toFull *Manifest @@ -136,22 +136,30 @@ func existDelta(t *testing.T, testDir, filename string, from, to uint32) string t.Fatalf("Failed to load to manifest to read hash from: %q", err) } - var fileNeeded = &File{Name: filename} - fromHash := fileNeeded.findFileNameInSlice(fromFull.Files).Hash - toHash := fileNeeded.findFileNameInSlice(toFull.Files).Hash + var fileNeeded = &File{Name: filename, Modifier: modifier} + fromFile := fileNeeded.findFileNameAndModifierInSlice(fromFull.Files) + if fromFile == nil { + t.Fatal("Failed to find file in from manifest") + } + fromHash := fromFile.Hash + toFile := fileNeeded.findFileNameAndModifierInSlice(toFull.Files) + if toFile == nil { + t.Fatal("Failed to find file in to manifest") + } + toHash := toFile.Hash suffix := fmt.Sprintf("%d-%d-%s-%s", from, to, fromHash, toHash) deltafile := filepath.Join(testDir, "www", fmt.Sprintf("%d", to), "delta", suffix) return deltafile } -func mustExistDelta(t *testing.T, testDir, filename string, from, to uint32) { - deltafile := existDelta(t, testDir, filename, from, to) +func mustExistDelta(t *testing.T, testDir, filename string, modifier ModifierFlag, from, to uint32) { + deltafile := existDelta(t, testDir, filename, modifier, from, to) mustExist(t, deltafile) } -func mustNotExistDelta(t *testing.T, testDir, filename string, from, to uint32) { - deltafile := existDelta(t, testDir, filename, from, to) +func mustNotExistDelta(t *testing.T, testDir, filename string, modifier ModifierFlag, from, to uint32) { + deltafile := existDelta(t, testDir, filename, modifier, from, to) mustNotExist(t, deltafile) } diff --git a/swupd/packs.go b/swupd/packs.go index ccc782ee..8661fde3 100644 --- a/swupd/packs.go +++ b/swupd/packs.go @@ -237,6 +237,10 @@ func WritePack(w io.Writer, fromManifest, toManifest *Manifest, outputDir, chroo entry := &info.Entries[i] entry.File = f + if fromVersion != 0 && !f.useInPack() { + entry.Reason = "skipping file type of already included file" + continue + } if f.Version <= fromVersion || fileContentInManifest(f, fromManifest) { entry.Reason = "already in from manifest" continue diff --git a/swupd/packs_test.go b/swupd/packs_test.go index 6cb4a76b..8f953dc9 100644 --- a/swupd/packs_test.go +++ b/swupd/packs_test.go @@ -200,6 +200,7 @@ func TestCreatePackZeroPacks(t *testing.T) { ts.addFile(10, "shells", "/csh", "csh contents") ts.addFile(10, "shells", "/fish", "fish contents") ts.addFile(10, "shells", "/zsh", "zsh contents") + ts.addFile(10, "shells", "/V3/zsh", "zsh V3 contents") ts.createManifests(10) @@ -212,7 +213,7 @@ func TestCreatePackZeroPacks(t *testing.T) { info = ts.createPack("shells", 0, 10, ts.path("image")) mustHaveNoWarnings(t, info) - mustHaveFullfileCount(t, info, 4+emptyFile) + mustHaveFullfileCount(t, info, 5+emptyFile) mustHaveDeltaCount(t, info, 0) mustValidateZeroPack(t, ts.path("www/10/Manifest.shells"), ts.path("www/10/pack-shells-from-0.tar")) @@ -224,6 +225,7 @@ func TestCreatePackZeroPacks(t *testing.T) { ts.addFile(20, "shells", "/csh", "csh contents") ts.addFile(20, "shells", "/fish", "fish contents") ts.addFile(20, "shells", "/zsh", "zsh contents") + ts.addFile(20, "shells", "/V3/zsh", "zsh V3 contents") ts.addFile(20, "shells", "/ksh", "ksh contents") ts.createManifests(20) @@ -252,7 +254,7 @@ func TestCreatePackZeroPacks(t *testing.T) { // Now we have all fullfiles for both versions. info = ts.createPack("shells", 0, 20, "") mustHaveNoWarnings(t, info) - mustHaveFullfileCount(t, info, 5+emptyFile) + mustHaveFullfileCount(t, info, 6+emptyFile) mustHaveDeltaCount(t, info, 0) mustValidateZeroPack(t, ts.path("www/20/Manifest.shells"), ts.path("www/20/pack-shells-from-0.tar")) } @@ -342,6 +344,8 @@ func TestCreatePackWithDelta(t *testing.T) { fs.write("image/10/contents/small2", smallContents) fs.write("image/10/contents/large1", largeContents) fs.write("image/10/contents/large2", largeContents) + fs.write("image/10/contents/opttest", largeContents+"0opt") + fs.write("image/10/contents/V3/opttest", largeContents+"V3opt") mustCreateManifests(t, 10, 0, minVer, format, fs.Dir) // @@ -353,10 +357,14 @@ func TestCreatePackWithDelta(t *testing.T) { fs.write("image/20/contents/small2", smallContents) fs.write("image/20/contents/large1", strings.ToUpper(largeContents[:1])+largeContents[1:]) fs.write("image/20/contents/large2", largeContents[:1]+strings.ToUpper(largeContents[1:])) + fs.write("image/20/contents/opttest", largeContents+"0OPT") + fs.write("image/20/contents/V3/opttest", largeContents+"V3OPT") + mustCreateManifests(t, 20, 10, minVer, format, fs.Dir) info := mustCreatePack(t, "contents", 10, 20, fs.path("www"), fs.path("image")) - mustHaveDeltaCount(t, info, 2) + // 3 not 4 as the non-V3 opttest shouldn't have a delta added + mustHaveDeltaCount(t, info, 3) // // In version 30, make a change to one large files from 20. @@ -372,7 +380,7 @@ func TestCreatePackWithDelta(t *testing.T) { // Pack between 10 and 30 has both deltas. info = mustCreatePack(t, "contents", 10, 30, fs.path("www"), fs.path("image")) - mustHaveDeltaCount(t, info, 2) + mustHaveDeltaCount(t, info, 3) } func TestCreatePackWithIncompleteChrootDir(t *testing.T) { From 1d1ffaafab6e49c5272e17e2007ba0fe8de569b2 Mon Sep 17 00:00:00 2001 From: William Douglas Date: Mon, 6 Jan 2025 13:44:14 -0800 Subject: [PATCH 3/3] Use mixer-ci from ghcr.io Signed-off-by: William Douglas --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 48ec05a8..fd02cd09 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM clearlinux/mixer-ci:latest +FROM ghcr.io/clearlinux/mixer-ci:latest ENV LC_ALL="en_US.UTF-8" WORKDIR /home/clr/mixer-tools COPY --chown=clr:clr . .