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) {