Merge changes from topic "import_make_staging_dir" into main

* changes: Add include_make_built_files Don't use zip files when creating filesystems
2024-03-15 17:34:26 +00:00
parent d10d54af71 4a2a7c98f6
commit b9ae285b55
5 changed files with 173 additions and 71 deletions
--- a/apex/apex_test.go
+++ b/apex/apex_test.go
@@ -11022,7 +11022,7 @@ func TestFileSystemShouldSkipApexLibraries(t *testing.T) {
 		}
 	`)

-	inputs := result.ModuleForTests("myfilesystem", "android_common").Output("deps.zip").Implicits
+	inputs := result.ModuleForTests("myfilesystem", "android_common").Output("myfilesystem.img").Implicits
 	android.AssertStringListDoesNotContain(t, "filesystem should not have libbar",
 		inputs.Strings(),
 		"out/soong/.intermediates/libbar/android_arm64_armv8-a_shared/libbar.so")
--- a/filesystem/filesystem.go
+++ b/filesystem/filesystem.go
@@ -19,6 +19,7 @@ import (
 	"fmt"
 	"io"
 	"path/filepath"
+	"slices"
 	"strings"

 	"android/soong/android"
@@ -109,6 +110,12 @@ type filesystemProperties struct {

 	// Mount point for this image. Default is "/"
 	Mount_point *string
+
+	// If set to the name of a partition ("system", "vendor", etc), this filesystem module
+	// will also include the contents of the make-built staging directories. If any soong
+	// modules would be installed to the same location as a make module, they will overwrite
+	// the make version.
+	Include_make_built_files string
 }

 // android_filesystem packages a set of modules and their transitive dependencies into a filesystem
@@ -183,13 +190,9 @@ func (f *filesystem) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 	ctx.InstallFile(f.installDir, f.installFileName(), f.output)
 }

-// root zip will contain extra files/dirs that are not from the `deps` property.
-func (f *filesystem) buildRootZip(ctx android.ModuleContext) android.OutputPath {
-	rootDir := android.PathForModuleGen(ctx, "root").OutputPath
-	builder := android.NewRuleBuilder(pctx, ctx)
-	builder.Command().Text("rm -rf").Text(rootDir.String())
-	builder.Command().Text("mkdir -p").Text(rootDir.String())
-
+// Copy extra files/dirs that are not from the `deps` property to `rootDir`, checking for conflicts with files
+// already in `rootDir`.
+func (f *filesystem) buildNonDepsFiles(ctx android.ModuleContext, builder *android.RuleBuilder, rootDir android.OutputPath) {
 	// create dirs and symlinks
 	for _, dir := range f.properties.Dirs {
 		// OutputPath.Join verifies dir
@@ -212,65 +215,43 @@ func (f *filesystem) buildRootZip(ctx android.ModuleContext) android.OutputPath

 		// OutputPath.Join verifies name. don't need to verify target.
 		dst := rootDir.Join(ctx, name)
-
+		builder.Command().Textf("(! [ -e %s -o -L %s ] || (echo \"%s already exists from an earlier stage of the build\" && exit 1))", dst, dst, dst)
 		builder.Command().Text("mkdir -p").Text(filepath.Dir(dst.String()))
 		builder.Command().Text("ln -sf").Text(proptools.ShellEscape(target)).Text(dst.String())
 	}

 	// create extra files if there's any
-	rootForExtraFiles := android.PathForModuleGen(ctx, "root-extra").OutputPath
-	var extraFiles android.OutputPaths
 	if f.buildExtraFiles != nil {
-		extraFiles = f.buildExtraFiles(ctx, rootForExtraFiles)
+		rootForExtraFiles := android.PathForModuleGen(ctx, "root-extra").OutputPath
+		extraFiles := f.buildExtraFiles(ctx, rootForExtraFiles)
 		for _, f := range extraFiles {
-			rel, _ := filepath.Rel(rootForExtraFiles.String(), f.String())
-			if strings.HasPrefix(rel, "..") {
-				panic(fmt.Errorf("%q is not under %q\n", f, rootForExtraFiles))
+			rel, err := filepath.Rel(rootForExtraFiles.String(), f.String())
+			if err != nil || strings.HasPrefix(rel, "..") {
+				ctx.ModuleErrorf("can't make %q relative to %q", f, rootForExtraFiles)
 			}
 		}
-	}
-
-	// Zip them all
-	zipOut := android.PathForModuleGen(ctx, "root.zip").OutputPath
-	zipCommand := builder.Command().BuiltTool("soong_zip")
-	zipCommand.FlagWithOutput("-o ", zipOut).
-		FlagWithArg("-C ", rootDir.String()).
-		Flag("-L 0"). // no compression because this will be unzipped soon
-		FlagWithArg("-D ", rootDir.String()).
-		Flag("-d") // include empty directories
-	if len(extraFiles) > 0 {
-		zipCommand.FlagWithArg("-C ", rootForExtraFiles.String())
-		for _, f := range extraFiles {
-			zipCommand.FlagWithInput("-f ", f)
+		if len(extraFiles) > 0 {
+			builder.Command().BuiltTool("merge_directories").
+				Implicits(extraFiles.Paths()).
+				Text(rootDir.String()).
+				Text(rootForExtraFiles.String())
 		}
 	}
-
-	builder.Command().Text("rm -rf").Text(rootDir.String())
-
-	builder.Build("zip_root", fmt.Sprintf("zipping root contents for %s", ctx.ModuleName()))
-	return zipOut
 }

 func (f *filesystem) buildImageUsingBuildImage(ctx android.ModuleContext) android.OutputPath {
-	depsZipFile := android.PathForModuleOut(ctx, "deps.zip").OutputPath
-	f.entries = f.CopyDepsToZip(ctx, f.gatherFilteredPackagingSpecs(ctx), depsZipFile)
-
-	builder := android.NewRuleBuilder(pctx, ctx)
-	depsBase := proptools.StringDefault(f.properties.Base_dir, ".")
-	rebasedDepsZip := android.PathForModuleOut(ctx, "rebased_deps.zip").OutputPath
-	builder.Command().
-		BuiltTool("zip2zip").
-		FlagWithInput("-i ", depsZipFile).
-		FlagWithOutput("-o ", rebasedDepsZip).
-		Text("**/*:" + proptools.ShellEscape(depsBase)) // zip2zip verifies depsBase
-
 	rootDir := android.PathForModuleOut(ctx, "root").OutputPath
-	rootZip := f.buildRootZip(ctx)
-	builder.Command().
-		BuiltTool("zipsync").
-		FlagWithArg("-d ", rootDir.String()). // zipsync wipes this. No need to clear.
-		Input(rootZip).
-		Input(rebasedDepsZip)
+	rebasedDir := rootDir
+	if f.properties.Base_dir != nil {
+		rebasedDir = rootDir.Join(ctx, *f.properties.Base_dir)
+	}
+	builder := android.NewRuleBuilder(pctx, ctx)
+	// Wipe the root dir to get rid of leftover files from prior builds
+	builder.Command().Textf("rm -rf %s && mkdir -p %s", rootDir, rootDir)
+	f.entries = f.CopySpecsToDir(ctx, builder, f.gatherFilteredPackagingSpecs(ctx), rebasedDir)
+
+	f.buildNonDepsFiles(ctx, builder, rootDir)
+	f.addMakeBuiltFiles(ctx, builder, rootDir)

 	// run host_init_verifier
 	// Ideally we should have a concept of pluggable linters that verify the generated image.
@@ -388,25 +369,21 @@ func (f *filesystem) buildCpioImage(ctx android.ModuleContext, compressed bool)
 		ctx.PropertyErrorf("file_contexts", "file_contexts is not supported for compressed cpio image.")
 	}

-	depsZipFile := android.PathForModuleOut(ctx, "deps.zip").OutputPath
-	f.entries = f.CopyDepsToZip(ctx, f.gatherFilteredPackagingSpecs(ctx), depsZipFile)
-
-	builder := android.NewRuleBuilder(pctx, ctx)
-	depsBase := proptools.StringDefault(f.properties.Base_dir, ".")
-	rebasedDepsZip := android.PathForModuleOut(ctx, "rebased_deps.zip").OutputPath
-	builder.Command().
-		BuiltTool("zip2zip").
-		FlagWithInput("-i ", depsZipFile).
-		FlagWithOutput("-o ", rebasedDepsZip).
-		Text("**/*:" + proptools.ShellEscape(depsBase)) // zip2zip verifies depsBase
+	if f.properties.Include_make_built_files != "" {
+		ctx.PropertyErrorf("include_make_built_files", "include_make_built_files is not supported for compressed cpio image.")
+	}

 	rootDir := android.PathForModuleOut(ctx, "root").OutputPath
-	rootZip := f.buildRootZip(ctx)
-	builder.Command().
-		BuiltTool("zipsync").
-		FlagWithArg("-d ", rootDir.String()). // zipsync wipes this. No need to clear.
-		Input(rootZip).
-		Input(rebasedDepsZip)
+	rebasedDir := rootDir
+	if f.properties.Base_dir != nil {
+		rebasedDir = rootDir.Join(ctx, *f.properties.Base_dir)
+	}
+	builder := android.NewRuleBuilder(pctx, ctx)
+	// Wipe the root dir to get rid of leftover files from prior builds
+	builder.Command().Textf("rm -rf %s && mkdir -p %s", rootDir, rootDir)
+	f.entries = f.CopySpecsToDir(ctx, builder, f.gatherFilteredPackagingSpecs(ctx), rebasedDir)
+
+	f.buildNonDepsFiles(ctx, builder, rootDir)

 	output := android.PathForModuleOut(ctx, f.installFileName()).OutputPath
 	cmd := builder.Command().
@@ -429,6 +406,41 @@ func (f *filesystem) buildCpioImage(ctx android.ModuleContext, compressed bool)
 	return output
 }

+var validPartitions = []string{
+	"system",
+	"userdata",
+	"cache",
+	"system_other",
+	"vendor",
+	"product",
+	"system_ext",
+	"odm",
+	"vendor_dlkm",
+	"odm_dlkm",
+	"system_dlkm",
+}
+
+func (f *filesystem) addMakeBuiltFiles(ctx android.ModuleContext, builder *android.RuleBuilder, rootDir android.Path) {
+	partition := f.properties.Include_make_built_files
+	if partition == "" {
+		return
+	}
+	if !slices.Contains(validPartitions, partition) {
+		ctx.PropertyErrorf("include_make_built_files", "Expected one of %#v, found %q", validPartitions, partition)
+		return
+	}
+	stampFile := fmt.Sprintf("target/product/%s/obj/PACKAGING/%s_intermediates/staging_dir.stamp", ctx.Config().DeviceName(), partition)
+	fileListFile := fmt.Sprintf("target/product/%s/obj/PACKAGING/%s_intermediates/file_list.txt", ctx.Config().DeviceName(), partition)
+	stagingDir := fmt.Sprintf("target/product/%s/%s", ctx.Config().DeviceName(), partition)
+
+	builder.Command().BuiltTool("merge_directories").
+		Implicit(android.PathForArbitraryOutput(ctx, stampFile)).
+		Text("--ignore-duplicates").
+		FlagWithInput("--file-list", android.PathForArbitraryOutput(ctx, fileListFile)).
+		Text(rootDir.String()).
+		Text(android.PathForArbitraryOutput(ctx, stagingDir).String())
+}
+
 var _ android.AndroidMkEntriesProvider = (*filesystem)(nil)

 // Implements android.AndroidMkEntriesProvider
--- a/filesystem/filesystem_test.go
+++ b/filesystem/filesystem_test.go
@@ -16,6 +16,7 @@ package filesystem

 import (
 	"os"
+	"path/filepath"
 	"testing"

 	"android/soong/android"
@@ -93,6 +94,22 @@ func TestFileSystemDeps(t *testing.T) {
 	}
 }

+func TestIncludeMakeBuiltFiles(t *testing.T) {
+	result := fixture.RunTestWithBp(t, `
+		android_filesystem {
+			name: "myfilesystem",
+			include_make_built_files: "system",
+		}
+	`)
+
+	output := result.ModuleForTests("myfilesystem", "android_common").Output("myfilesystem.img")
+
+	stampFile := filepath.Join(result.Config.OutDir(), "target/product/test_device/obj/PACKAGING/system_intermediates/staging_dir.stamp")
+	fileListFile := filepath.Join(result.Config.OutDir(), "target/product/test_device/obj/PACKAGING/system_intermediates/file_list.txt")
+	android.AssertStringListContains(t, "deps of filesystem must include the staging dir stamp file", output.Implicits.Strings(), stampFile)
+	android.AssertStringListContains(t, "deps of filesystem must include the staging dir file list", output.Implicits.Strings(), fileListFile)
+}
+
 func TestFileSystemFillsLinkerConfigWithStubLibs(t *testing.T) {
 	result := fixture.RunTestWithBp(t, `
 		android_system_image {
@@ -270,7 +287,7 @@ func TestFileSystemShouldInstallCoreVariantIfTargetBuildAppsIsSet(t *testing.T)
 		}
 	`)

-	inputs := result.ModuleForTests("myfilesystem", "android_common").Output("deps.zip").Implicits
+	inputs := result.ModuleForTests("myfilesystem", "android_common").Output("myfilesystem.img").Implicits
 	android.AssertStringListContains(t, "filesystem should have libbar even for unbundled build",
 		inputs.Strings(),
 		"out/soong/.intermediates/libbar/android_arm64_armv8-a_shared/libbar.so")
@@ -314,7 +331,7 @@ func TestFileSystemWithCoverageVariants(t *testing.T) {
 	`)

 	filesystem := result.ModuleForTests("myfilesystem", "android_common_cov")
-	inputs := filesystem.Output("deps.zip").Implicits
+	inputs := filesystem.Output("myfilesystem.img").Implicits
 	android.AssertStringListContains(t, "filesystem should have libfoo(cov)",
 		inputs.Strings(),
 		"out/soong/.intermediates/libfoo/android_arm64_armv8-a_shared_cov/libfoo.so")
--- a/scripts/Android.bp
+++ b/scripts/Android.bp
@@ -292,3 +292,16 @@ sh_binary_host {
    name: "keep-flagged-apis",
    src: "keep-flagged-apis.sh",
 }
+
+python_binary_host {
+    name: "merge_directories",
+    main: "merge_directories.py",
+    srcs: [
+        "merge_directories.py",
+    ],
+    version: {
+        py3: {
+            embedded_launcher: true,
+        },
+    },
+}
--- a/scripts/merge_directories.py
+++ b/scripts/merge_directories.py
@@ -0,0 +1,60 @@
+#!/usr/bin/env python3
+import argparse
+import os
+import shutil
+import sys
+
+def main():
+    parser = argparse.ArgumentParser(
+        description="Given a list of directories, this script will copy the contents of all of "
+        "them into the first directory, erroring out if any duplicate files are found."
+    )
+    parser.add_argument(
+        "--ignore-duplicates",
+        action="store_true",
+        help="Don't error out on duplicate files, just skip them. The file from the earliest "
+        "directory listed on the command line will be the winner."
+    )
+    parser.add_argument(
+        "--file-list",
+        help="Path to a text file containing paths relative to in_dir. Only these paths will be "
+        "copied out of in_dir."
+    )
+    parser.add_argument("out_dir")
+    parser.add_argument("in_dir")
+    args = parser.parse_args()
+
+    if not os.path.isdir(args.out_dir):
+        sys.exit(f"error: {args.out_dir} must be a directory")
+    if not os.path.isdir(args.in_dir):
+        sys.exit(f"error: {args.in_dir} must be a directory")
+
+    file_list = None
+    if args.file_list:
+        with open(file_list_file, "r") as f:
+            file_list = f.read().strip().splitlines()
+
+    in_dir = args.in_dir
+    for root, dirs, files in os.walk(in_dir):
+        rel_root = os.path.relpath(root, in_dir)
+        dst_root = os.path.join(args.out_dir, rel_root)
+        made_parent_dirs = False
+        for f in files:
+            src = os.path.join(root, f)
+            dst = os.path.join(dst_root, f)
+            p = os.path.normpath(os.path.join(rel_root, f))
+            if file_list is not None and p not in file_list:
+                continue
+            if os.path.lexists(dst):
+                if args.ignore_duplicates:
+                    continue
+                sys.exit(f"error: {p} exists in both {args.out_dir} and {in_dir}")
+
+            if not made_parent_dirs:
+                os.makedirs(dst_root, exist_ok=True)
+                made_parent_dirs = True
+
+            shutil.copy2(src, dst, follow_symlinks=False)
+
+if __name__ == "__main__":
+    main()