Limit pack modifiers

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
 		}
 

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)
+}

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",
@@ -456,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
@@ -486,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
+}

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

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

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

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