Improve manifest include subtraction

Instead of giving up on directory subtraction entirely, try to
subtract directories that are in an included manifest *and* do not
have a non-directory file somewhere in their path.

For example:

bundle1:
/dir/file

bundle2: includes bundle1
/dir

In this case /dir would be subtracted from bundle2.

Also add a special case for subtracting directories if they are in
os-core as this reduces a lot of unused content from a bundle's
manifest.

Signed-off-by: William Douglas <[email protected]>

Author: William Douglas

swupd/manifest.go

@@ -22,6 +22,7 @@ import (
 	"os"
 	"path/filepath"
 	"runtime"
+	"slices"
 	"sort"
 	"strconv"
 	"strings"
@@ -757,20 +758,58 @@ func (m *Manifest) addManifestFiles(ui UpdateInfo, c config) error {
 		// Add files to manifest that do not exist in included bundles.
 		chrootDir := filepath.Join(c.imageBase, fmt.Sprint(ui.version), "full")
 		includes := m.GetRecursiveIncludes()
+		usedDirs := make(map[string]struct{})
+		bundleDirs := []string{}
+		bundleFiles := []string{}
 		for f := range m.BundleInfo.Files {
+			fullPath := filepath.Join(chrootDir, f)
+			if fi, err := os.Lstat(fullPath); err == nil {
+				if !fi.IsDir() {
+					usedDirs[filepath.Dir(f)] = struct{}{}
+					bundleFiles = append(bundleFiles, f)
+				} else {
+					bundleDirs = append(bundleDirs, f)
+				}
+			}
+		}
+		for _, f := range bundleFiles {
 			isIncluded := false
 			for _, inc := range includes {
 				// Handle cycles
 				if inc.Name == m.Name {
 					continue
 				}
-				fullPath := filepath.Join(chrootDir, f)
-				if fi, err := os.Lstat(fullPath); err == nil {
-					if !fi.IsDir() {
-						if _, ok := inc.BundleInfo.Files[f]; ok {
-							isIncluded = true
-							break
-						}
+				if _, ok := inc.BundleInfo.Files[f]; ok {
+					isIncluded = true
+					break
+				}
+			}
+			if !isIncluded {
+				if err := m.addFile(f, c, ui.version); err != nil {
+					return err
+				}
+			}
+		}
+		// Directories are sorted into reverse order to allow usedDirs
+		// to populate correctly (/a/b/c will be seen before /a/b which
+		// will allow /a to be set when /a/b is seen)
+		slices.Sort(bundleDirs)
+		slices.Reverse(bundleDirs)
+		for _, f := range bundleDirs {
+			if _, ok := usedDirs[f]; ok {
+				usedDirs[filepath.Dir(f)] = struct{}{}
+			}
+		}
+		for _, f := range bundleDirs {
+			isIncluded := false
+			for _, inc := range includes {
+				if _, ok := inc.BundleInfo.Files[f]; ok {
+					if inc.Name == "os-core" {
+						isIncluded = true
+						break
+					} else if _, ok := usedDirs[f]; !ok {
+						isIncluded = true
+						break
 					}
 				}
 			}

swupd/swupd_test.go

@@ -99,7 +99,7 @@ func TestFullRun(t *testing.T) {
 	mustValidateZeroPack(t, ts.path("www/10/Manifest.test-bundle"), ts.path("www/10/pack-test-bundle-from-0.tar"))
 	mustHaveDeltaCount(t, infoTestBundle, 0)
 	// Empty file (bundle file), "foo".
-	mustHaveFullfileCount(t, infoTestBundle, 3)
+	mustHaveFullfileCount(t, infoTestBundle, 2)
 
 	testBundle := ts.parseManifest(10, "test-bundle")
 	checkIncludes(t, testBundle, "os-core")
@@ -112,7 +112,6 @@ func TestFullRun(t *testing.T) {
 	checkFileInManifest(t, osCore, 10, "/usr/share")
 	checkFileInManifest(t, osCore, 10, "/usr/share/clear")
 	checkFileInManifest(t, osCore, 10, "/usr/share/clear/bundles")
-	checkFileInManifest(t, osCore, 10, "/usr")
 }
 
 // Imported from swupd-server/test/functional/full-run-delta.
@@ -137,7 +136,7 @@ func TestFullRunDelta(t *testing.T) {
 
 	ts.createPack("os-core", 0, 10, ts.path("image"))
 	info := ts.createPack("test-bundle", 0, 10, ts.path("image"))
-	mustHaveFullfileCount(t, info, 5) // largefile, foo and foobarbaz and the test-bundle file.
+	mustHaveFullfileCount(t, info, 4) // largefile, foo and foobarbaz and the test-bundle file.
 
 	mustValidateZeroPack(t, ts.path("www/10/Manifest.os-core"), ts.path("www/10/pack-os-core-from-0.tar"))
 	mustValidateZeroPack(t, ts.path("www/10/Manifest.test-bundle"), ts.path("www/10/pack-test-bundle-from-0.tar"))
@@ -164,7 +163,7 @@ func TestFullRunDelta(t *testing.T) {
 
 	ts.createPack("os-core", 0, 20, ts.path("image"))
 	info = ts.createPack("test-bundle", 0, 20, ts.path("image"))
-	mustHaveFullfileCount(t, info, 3) // largefile and the test-bundle file.
+	mustHaveFullfileCount(t, info, 2) // largefile and the test-bundle file.
 
 	mustValidateZeroPack(t, ts.path("www/20/Manifest.os-core"), ts.path("www/20/pack-os-core-from-0.tar"))
 	mustValidateZeroPack(t, ts.path("www/20/Manifest.test-bundle"), ts.path("www/20/pack-test-bundle-from-0.tar"))
@@ -191,6 +190,82 @@ func TestFullRunDelta(t *testing.T) {
 	// not done by new swupd since it seems the client doesn't take advantage of them.
 }
 
+// Test an import cycle for includes subtraction
+func TestRunBundleCycle(t *testing.T) {
+	ts := newTestSwupd(t, "bundle-cycle-")
+	defer ts.cleanup()
+
+	// Version 10.
+	ts.Bundles = []string{"os-core", "test-bundle1", "test-bundle2"}
+
+	ts.mkdir("image/10/os-core/a")
+	ts.write("image/10/test-bundle1/a/b/t1", "t1")
+	ts.write("image/10/test-bundle2/a/b/t2", "t2")
+	ts.write("image/10/noship/test-bundle1-includes", "test-bundle2")
+	ts.write("image/10/noship/test-bundle2-includes", "test-bundle1")
+
+	ts.createManifestsFromChroots(10)
+	ts.createFullfiles(10)
+
+	ts.createPack("os-core", 0, 10, ts.path("image"))
+	info1 := ts.createPack("test-bundle1", 0, 10, ts.path("image"))
+	info2 := ts.createPack("test-bundle2", 0, 10, ts.path("image"))
+	mustHaveFullfileCount(t, info1, 3) // b dir, t1 file and the test-bundle1 file.
+	mustHaveFullfileCount(t, info2, 3) // b dir, t2 file and the test-bundle2 file.
+
+	mustValidateZeroPack(t, ts.path("www/10/Manifest.os-core"), ts.path("www/10/pack-os-core-from-0.tar"))
+	mustValidateZeroPack(t, ts.path("www/10/Manifest.test-bundle1"), ts.path("www/10/pack-test-bundle1-from-0.tar"))
+	mustValidateZeroPack(t, ts.path("www/10/Manifest.test-bundle2"), ts.path("www/10/pack-test-bundle2-from-0.tar"))
+
+	osCore := ts.parseManifest(10, "os-core")
+	testBundle1 := ts.parseManifest(10, "test-bundle1")
+	testBundle2 := ts.parseManifest(10, "test-bundle2")
+	checkIncludes(t, testBundle1, "os-core", "test-bundle2")
+	checkIncludes(t, testBundle2, "os-core", "test-bundle1")
+	checkFileInManifest(t, osCore, 10, "/a")
+	checkFileInManifest(t, testBundle1, 10, "/a/b")
+	checkFileInManifest(t, testBundle1, 10, "/a/b/t1")
+	checkFileInManifest(t, testBundle2, 10, "/a/b")
+	checkFileInManifest(t, testBundle2, 10, "/a/b/t2")
+}
+
+// Test an import without cycle to see directory subtraction outside os-core
+func TestRunBundleIncludes(t *testing.T) {
+	ts := newTestSwupd(t, "bundle-includes-")
+	defer ts.cleanup()
+
+	// Version 10.
+	ts.Bundles = []string{"os-core", "test-bundle1", "test-bundle2"}
+
+	ts.write("image/10/test-bundle1/a/t1", "t1")
+	ts.write("image/10/test-bundle2/t2", "t2")
+	// this would have subtracted with a cycle too but
+	// keeping this test separate in case directory subtraction
+	// improves in the case of cycles.
+	ts.mkdir("image/10/test-bundle2/a")
+	ts.write("image/10/noship/test-bundle2-includes", "test-bundle1")
+
+	ts.createManifestsFromChroots(10)
+	ts.createFullfiles(10)
+
+	ts.createPack("os-core", 0, 10, ts.path("image"))
+	info1 := ts.createPack("test-bundle1", 0, 10, ts.path("image"))
+	info2 := ts.createPack("test-bundle2", 0, 10, ts.path("image"))
+	mustHaveFullfileCount(t, info1, 3) // a dir, t1 file and the test-bundle1 file.
+	mustHaveFullfileCount(t, info2, 2) // t2 file and the test-bundle2 file.
+
+	mustValidateZeroPack(t, ts.path("www/10/Manifest.os-core"), ts.path("www/10/pack-os-core-from-0.tar"))
+	mustValidateZeroPack(t, ts.path("www/10/Manifest.test-bundle1"), ts.path("www/10/pack-test-bundle1-from-0.tar"))
+	mustValidateZeroPack(t, ts.path("www/10/Manifest.test-bundle2"), ts.path("www/10/pack-test-bundle2-from-0.tar"))
+
+	testBundle1 := ts.parseManifest(10, "test-bundle1")
+	testBundle2 := ts.parseManifest(10, "test-bundle2")
+	checkIncludes(t, testBundle2, "os-core", "test-bundle1")
+	checkFileInManifest(t, testBundle1, 10, "/a")
+	checkFileInManifest(t, testBundle1, 10, "/a/t1")
+	checkFileInManifest(t, testBundle2, 10, "/t2")
+}
+
 func TestAddFilesToBundleInfo(t *testing.T) {
 	ts := newTestSwupd(t, "extra-files")
 	defer ts.cleanup()