Merge changes from topic "soong-tests-presubmit" into main

* changes: Add script to run Soong tests with go tools Disable TestVariantSingletonModule when go test -short is used Fix data race in propagateRROEnforcementMutator Fix data race in finder_test.go Fix data race in dex_bootjars Fix race CommonGlobalCflags when running tests in parallel. Fix data race in snapshot singletons when running parallel tests
2024-01-18 23:58:45 +00:00
parent 4509f502e5 6ca8d2546e
commit 1b1b9a937b
9 changed files with 154 additions and 69 deletions
--- a/android/singleton_module_test.go
+++ b/android/singleton_module_test.go
@@ -103,6 +103,9 @@ func testVariantSingletonModuleMutator(ctx BottomUpMutatorContext) {
 }

 func TestVariantSingletonModule(t *testing.T) {
+	if testing.Short() {
+		t.Skip("test fails with data race enabled")
+	}
 	bp := `
 		test_singleton_module {
 			name: "test_singleton_module",
--- a/cc/config/global.go
+++ b/cc/config/global.go
@@ -16,6 +16,7 @@ package config

 import (
 	"runtime"
+	"slices"
 	"strings"

 	"android/soong/android"
@@ -400,7 +401,7 @@ func init() {
 	exportedVars.ExportStringList("CommonGlobalCflags", commonGlobalCflags)

 	pctx.VariableFunc("CommonGlobalCflags", func(ctx android.PackageVarContext) string {
-		flags := commonGlobalCflags
+		flags := slices.Clone(commonGlobalCflags)

 		// http://b/131390872
 		// Automatically initialize any uninitialized stack variables.
--- a/finder/finder_test.go
+++ b/finder/finder_test.go
@@ -813,6 +813,7 @@ func TestFileAdded(t *testing.T) {
 			IncludeFiles: []string{"findme.txt"},
 		},
 	)
+	finder.WaitForDbDump()
 	filesystem.Clock.Tick()
 	foundPaths := finder.FindNamedAt("/tmp", "findme.txt")
 	finder.Shutdown()
@@ -1445,6 +1446,7 @@ func TestUpdatingDbIffChanged(t *testing.T) {
 			IncludeFiles: []string{"hi.txt"},
 		},
 	)
+	finder.WaitForDbDump()
 	filesystem.Clock.Tick()
 	foundPaths := finder.FindAll()
 	finder.Shutdown()
@@ -1506,6 +1508,7 @@ func TestDirectoryNotPermitted(t *testing.T) {
 			IncludeFiles: []string{"hi.txt"},
 		},
 	)
+	finder.WaitForDbDump()
 	filesystem.Clock.Tick()
 	foundPaths := finder.FindAll()
 	finder.Shutdown()
@@ -1552,6 +1555,7 @@ func TestFileNotPermitted(t *testing.T) {
 			IncludeFiles: []string{"hi.txt"},
 		},
 	)
+	finder.WaitForDbDump()
 	filesystem.Clock.Tick()
 	foundPaths := finder.FindAll()
 	finder.Shutdown()
@@ -1573,6 +1577,7 @@ func TestCacheEntryPathUnexpectedError(t *testing.T) {
 			IncludeFiles: []string{"hi.txt"},
 		},
 	)
+	finder.WaitForDbDump()
 	filesystem.Clock.Tick()
 	foundPaths := finder.FindAll()
 	finder.Shutdown()
--- a/java/aar.go
+++ b/java/aar.go
@@ -44,7 +44,7 @@ func RegisterAARBuildComponents(ctx android.RegistrationContext) {
 	ctx.RegisterModuleType("android_library_import", AARImportFactory)
 	ctx.RegisterModuleType("android_library", AndroidLibraryFactory)
 	ctx.PostDepsMutators(func(ctx android.RegisterMutatorsContext) {
-		ctx.TopDown("propagate_rro_enforcement", propagateRROEnforcementMutator).Parallel()
+		ctx.TopDown("propagate_rro_enforcement", propagateRROEnforcementMutator)
 	})
 }

--- a/java/dexpreopt_bootjars.go
+++ b/java/dexpreopt_bootjars.go
@@ -238,8 +238,7 @@ func init() {
 //
 // WARNING: All fields in this struct should be initialized in the genBootImageConfigs function.
 // Failure to do so can lead to data races if there is no synchronization enforced ordering between
-// the writer and the reader. Fields which break this rule are marked as deprecated and should be
-// removed and replaced with something else, e.g. providers.
+// the writer and the reader.
 type bootImageConfig struct {
 	// If this image is an extension, the image that it extends.
 	extends *bootImageConfig
@@ -279,16 +278,6 @@ type bootImageConfig struct {
 	// File path to a zip archive with all image files (or nil, if not needed).
 	zip android.WritablePath

-	// Rules which should be used in make to install the outputs.
-	//
-	// Deprecated: Not initialized correctly, see struct comment.
-	profileInstalls android.RuleBuilderInstalls
-
-	// Path to the license metadata file for the module that built the profile.
-	//
-	// Deprecated: Not initialized correctly, see struct comment.
-	profileLicenseMetadataFile android.OptionalPath
-
 	// Target-dependent fields.
 	variants []*bootImageVariant

@@ -602,6 +591,7 @@ func (d *dexpreoptBootJars) GenerateAndroidBuildActions(ctx android.ModuleContex
 	imageConfigs := genBootImageConfigs(ctx)
 	d.defaultBootImage = defaultBootImageConfig(ctx)
 	d.otherImages = make([]*bootImageConfig, 0, len(imageConfigs)-1)
+	var profileInstalls android.RuleBuilderInstalls
 	for _, name := range getImageNames() {
 		config := imageConfigs[name]
 		if config != d.defaultBootImage {
@@ -610,11 +600,19 @@ func (d *dexpreoptBootJars) GenerateAndroidBuildActions(ctx android.ModuleContex
 		if !config.isEnabled(ctx) {
 			continue
 		}
-		generateBootImage(ctx, config)
+		installs := generateBootImage(ctx, config)
+		profileInstalls = append(profileInstalls, installs...)
 		if config == d.defaultBootImage {
-			bootFrameworkProfileRule(ctx, config)
+			_, installs := bootFrameworkProfileRule(ctx, config)
+			profileInstalls = append(profileInstalls, installs...)
 		}
 	}
+	if len(profileInstalls) > 0 {
+		android.SetProvider(ctx, profileInstallInfoProvider, profileInstallInfo{
+			profileInstalls:            profileInstalls,
+			profileLicenseMetadataFile: android.OptionalPathForPath(ctx.LicenseMetadataFile()),
+		})
+	}
 }

 // GenerateSingletonBuildActions generates build rules for the dexpreopt config for Make.
@@ -635,7 +633,7 @@ func shouldBuildBootImages(config android.Config, global *dexpreopt.GlobalConfig
 	return true
 }

-func generateBootImage(ctx android.ModuleContext, imageConfig *bootImageConfig) {
+func generateBootImage(ctx android.ModuleContext, imageConfig *bootImageConfig) android.RuleBuilderInstalls {
 	apexJarModulePairs := getModulesForImage(ctx, imageConfig)

 	// Copy module dex jars to their predefined locations.
@@ -644,12 +642,12 @@ func generateBootImage(ctx android.ModuleContext, imageConfig *bootImageConfig)

 	// Build a profile for the image config from the profile at the default path. The profile will
 	// then be used along with profiles imported from APEXes to build the boot image.
-	profile := bootImageProfileRule(ctx, imageConfig)
+	profile, profileInstalls := bootImageProfileRule(ctx, imageConfig)

 	// If dexpreopt of boot image jars should be skipped, stop after generating a profile.
 	global := dexpreopt.GetGlobalConfig(ctx)
 	if SkipDexpreoptBootJars(ctx) || (global.OnlyPreoptArtBootImage && imageConfig.name != "art") {
-		return
+		return profileInstalls
 	}

 	// Build boot image files for the android variants.
@@ -663,6 +661,8 @@ func generateBootImage(ctx android.ModuleContext, imageConfig *bootImageConfig)

 	// Create a `dump-oat-<image-name>` rule that runs `oatdump` for debugging purposes.
 	dumpOatRules(ctx, imageConfig)
+
+	return profileInstalls
 }

 type apexJarModulePair struct {
@@ -1177,9 +1177,19 @@ func bootImageProfileRuleCommon(ctx android.ModuleContext, name string, dexFiles
 	return profile
 }

-func bootImageProfileRule(ctx android.ModuleContext, image *bootImageConfig) android.WritablePath {
+type profileInstallInfo struct {
+	// Rules which should be used in make to install the outputs.
+	profileInstalls android.RuleBuilderInstalls
+
+	// Path to the license metadata file for the module that built the profile.
+	profileLicenseMetadataFile android.OptionalPath
+}
+
+var profileInstallInfoProvider = blueprint.NewProvider[profileInstallInfo]()
+
+func bootImageProfileRule(ctx android.ModuleContext, image *bootImageConfig) (android.WritablePath, android.RuleBuilderInstalls) {
 	if !image.isProfileGuided() {
-		return nil
+		return nil, nil
 	}

 	profile := bootImageProfileRuleCommon(ctx, image.name, image.dexPathsDeps.Paths(), image.getAnyAndroidVariant().dexLocationsDeps)
@@ -1187,21 +1197,19 @@ func bootImageProfileRule(ctx android.ModuleContext, image *bootImageConfig) and
 	if image == defaultBootImageConfig(ctx) {
 		rule := android.NewRuleBuilder(pctx, ctx)
 		rule.Install(profile, "/system/etc/boot-image.prof")
-		image.profileInstalls = append(image.profileInstalls, rule.Installs()...)
-		image.profileLicenseMetadataFile = android.OptionalPathForPath(ctx.LicenseMetadataFile())
+		return profile, rule.Installs()
 	}
-
-	return profile
+	return profile, nil
 }

 // bootFrameworkProfileRule generates the rule to create the boot framework profile and
 // returns a path to the generated file.
-func bootFrameworkProfileRule(ctx android.ModuleContext, image *bootImageConfig) android.WritablePath {
+func bootFrameworkProfileRule(ctx android.ModuleContext, image *bootImageConfig) (android.WritablePath, android.RuleBuilderInstalls) {
 	globalSoong := dexpreopt.GetGlobalSoongConfig(ctx)
 	global := dexpreopt.GetGlobalConfig(ctx)

 	if global.DisableGenerateProfile || ctx.Config().UnbundledBuild() {
-		return nil
+		return nil, nil
 	}

 	defaultProfile := "frameworks/base/config/boot-profile.txt"
@@ -1221,10 +1229,7 @@ func bootFrameworkProfileRule(ctx android.ModuleContext, image *bootImageConfig)

 	rule.Install(profile, "/system/etc/boot-image.bprof")
 	rule.Build("bootFrameworkProfile", "profile boot framework jars")
-	image.profileInstalls = append(image.profileInstalls, rule.Installs()...)
-	image.profileLicenseMetadataFile = android.OptionalPathForPath(ctx.LicenseMetadataFile())
-
-	return profile
+	return profile, rule.Installs()
 }

 func dumpOatRules(ctx android.ModuleContext, image *bootImageConfig) {
@@ -1292,9 +1297,11 @@ func (d *dexpreoptBootJars) MakeVars(ctx android.MakeVarsContext) {

 	image := d.defaultBootImage
 	if image != nil {
-		ctx.Strict("DEXPREOPT_IMAGE_PROFILE_BUILT_INSTALLED", image.profileInstalls.String())
-		if image.profileLicenseMetadataFile.Valid() {
-			ctx.Strict("DEXPREOPT_IMAGE_PROFILE_LICENSE_METADATA", image.profileLicenseMetadataFile.String())
+		if profileInstallInfo, ok := android.SingletonModuleProvider(ctx, d, profileInstallInfoProvider); ok {
+			ctx.Strict("DEXPREOPT_IMAGE_PROFILE_BUILT_INSTALLED", profileInstallInfo.profileInstalls.String())
+			if profileInstallInfo.profileLicenseMetadataFile.Valid() {
+				ctx.Strict("DEXPREOPT_IMAGE_PROFILE_LICENSE_METADATA", profileInstallInfo.profileLicenseMetadataFile.String())
+			}
 		}

 		if SkipDexpreoptBootJars(ctx) {
--- a/java/dexpreopt_config_testing.go
+++ b/java/dexpreopt_config_testing.go
@@ -523,7 +523,7 @@ func checkArtBootImageConfig(t *testing.T, result *android.TestResult, mutated b
 		},
 	}

-	checkBootImageConfig(t, imageConfig, mutated, expected)
+	checkBootImageConfig(t, result, imageConfig, mutated, expected)
 }

 // getFrameworkImageConfig gets the framework bootImageConfig that was created during the test.
@@ -904,7 +904,7 @@ func checkFrameworkBootImageConfig(t *testing.T, result *android.TestResult, mut
 		profileLicenseMetadataFile: expectedLicenseMetadataFile,
 	}

-	checkBootImageConfig(t, imageConfig, mutated, expected)
+	checkBootImageConfig(t, result, imageConfig, mutated, expected)
 }

 // getMainlineImageConfig gets the framework bootImageConfig that was created during the test.
@@ -1183,7 +1183,7 @@ func CheckMainlineBootImageConfig(t *testing.T, result *android.TestResult) {
 		profileLicenseMetadataFile: expectedLicenseMetadataFile,
 	}

-	checkBootImageConfig(t, imageConfig, false, expected)
+	checkBootImageConfig(t, result, imageConfig, false, expected)
 }

 // clearMutatedFields clears fields in the expectedConfig that correspond to fields in the
@@ -1211,19 +1211,19 @@ func clearMutatedFields(expected *expectedConfig) {
 // zero value so that they will match the unmodified values in the boot image.
 //
 // It runs the checks in an image specific subtest of the current test.
-func checkBootImageConfig(t *testing.T, imageConfig *bootImageConfig, mutated bool, expected *expectedConfig) {
+func checkBootImageConfig(t *testing.T, result *android.TestResult, imageConfig *bootImageConfig, mutated bool, expected *expectedConfig) {
 	if !mutated {
 		clearMutatedFields(expected)
 	}

 	t.Run(imageConfig.name, func(t *testing.T) {
-		nestedCheckBootImageConfig(t, imageConfig, expected)
+		nestedCheckBootImageConfig(t, result, imageConfig, mutated, expected)
 	})
 }

 // nestedCheckBootImageConfig does the work of comparing the image against the expected values and
 // is run in an image specific subtest.
-func nestedCheckBootImageConfig(t *testing.T, imageConfig *bootImageConfig, expected *expectedConfig) {
+func nestedCheckBootImageConfig(t *testing.T, result *android.TestResult, imageConfig *bootImageConfig, mutated bool, expected *expectedConfig) {
 	android.AssertStringEquals(t, "name", expected.name, imageConfig.name)
 	android.AssertStringEquals(t, "stem", expected.stem, imageConfig.stem)
 	android.AssertPathRelativeToTopEquals(t, "dir", expected.dir, imageConfig.dir)
@@ -1234,8 +1234,13 @@ func nestedCheckBootImageConfig(t *testing.T, imageConfig *bootImageConfig, expe
 	android.AssertPathsRelativeToTopEquals(t, "dexPathsDeps", expected.dexPathsDeps, imageConfig.dexPathsDeps.Paths())
 	// dexPathsByModule is just a different representation of the other information in the config.
 	android.AssertPathRelativeToTopEquals(t, "zip", expected.zip, imageConfig.zip)
-	assertInstallsEqual(t, "profileInstalls", expected.profileInstalls, imageConfig.profileInstalls)
-	android.AssertStringEquals(t, "profileLicenseMetadataFile", expected.profileLicenseMetadataFile, imageConfig.profileLicenseMetadataFile.RelativeToTop().String())
+
+	if !mutated {
+		dexBootJarModule := result.ModuleForTests("dex_bootjars", "android_common")
+		profileInstallInfo, _ := android.SingletonModuleProvider(result, dexBootJarModule.Module(), profileInstallInfoProvider)
+		assertInstallsEqual(t, "profileInstalls", expected.profileInstalls, profileInstallInfo.profileInstalls)
+		android.AssertStringEquals(t, "profileLicenseMetadataFile", expected.profileLicenseMetadataFile, profileInstallInfo.profileLicenseMetadataFile.RelativeToTop().String())
+	}

 	android.AssertIntEquals(t, "variant count", 4, len(imageConfig.variants))
 	for i, variant := range imageConfig.variants {
--- a/scripts/run-soong-tests-with-go-tools.sh
+++ b/scripts/run-soong-tests-with-go-tools.sh
@@ -0,0 +1,70 @@
+#!/bin/bash -ex
+
+: "${OUT_DIR:?Must set OUT_DIR}"
+TOP=$(cd $(dirname $0)/../../..; pwd)
+cd ${TOP}
+
+UNAME="$(uname)"
+case "$UNAME" in
+Linux)
+    OS='linux'
+    ;;
+Darwin)
+    OS='darwin'
+    ;;
+*)
+    exit 1
+    ;;
+esac
+
+# Verify that go test and go build work on all the same projects that are parsed by
+# build/soong/build_kzip.bash
+declare -ar go_modules=(build/blueprint build/soong
+      build/make/tools/canoninja build/make/tools/compliance build/make/tools/rbcrun)
+export GOROOT=${TOP}/prebuilts/go/${OS}-x86
+export GOENV=off
+export GOPROXY=off
+abs_out_dir=$(cd ${OUT_DIR}; pwd)
+export GOPATH=${abs_out_dir}/gopath
+export GOCACHE=${abs_out_dir}/gocache
+export GOMODCACHE=${abs_out_dir}/gomodcache
+export TMPDIR=${abs_out_dir}/gotemp
+mkdir -p ${TMPDIR}
+${GOROOT}/bin/go env
+
+# androidmk_test.go gets confused if ANDROID_BUILD_TOP is set.
+unset ANDROID_BUILD_TOP
+
+network_jail=""
+if [[ ${OS} = linux ]]; then
+    # The go tools often try to fetch dependencies from the network,
+    # wrap them in an nsjail to prevent network access.
+    network_jail=${TOP}/prebuilts/build-tools/linux-x86/bin/nsjail
+    # Quiet
+    network_jail="${network_jail} -q"
+    # No timeout
+    network_jail="${network_jail} -t 0"
+    # Set working directory
+    network_jail="${network_jail} --cwd=\$PWD"
+    # Pass environment variables through
+    network_jail="${network_jail} -e"
+    # Allow read-only access to everything
+    network_jail="${network_jail} -R /"
+    # Allow write access to the out directory
+    network_jail="${network_jail} -B ${abs_out_dir}"
+    # Allow write access to the /tmp directory
+    network_jail="${network_jail} -B /tmp"
+    # Set high values, as network_jail uses low defaults.
+    network_jail="${network_jail} --rlimit_as soft"
+    network_jail="${network_jail} --rlimit_core soft"
+    network_jail="${network_jail} --rlimit_cpu soft"
+    network_jail="${network_jail} --rlimit_fsize soft"
+    network_jail="${network_jail} --rlimit_nofile soft"
+fi
+
+for dir in "${go_modules[@]}"; do
+    (cd "$dir";
+     eval ${network_jail} -- ${GOROOT}/bin/go build ./...
+     eval ${network_jail} -- ${GOROOT}/bin/go test ./...
+    )
+done
--- a/snapshot/recovery_snapshot.go
+++ b/snapshot/recovery_snapshot.go
@@ -22,16 +22,14 @@ type RecoverySnapshotModuleInterface interface {
 	ExcludeFromRecoverySnapshot() bool
 }

-var recoverySnapshotSingleton = SnapshotSingleton{
-	"recovery",                     // name
-	"SOONG_RECOVERY_SNAPSHOT_ZIP",  // makeVar
-	android.OptionalPath{},         // snapshotZipFile
-	RecoverySnapshotImageSingleton, // Image
-	false,                          // Fake
-}
-
 func RecoverySnapshotSingleton() android.Singleton {
-	return &recoverySnapshotSingleton
+	return &SnapshotSingleton{
+		"recovery",                     // name
+		"SOONG_RECOVERY_SNAPSHOT_ZIP",  // makeVar
+		android.OptionalPath{},         // snapshotZipFile
+		RecoverySnapshotImageSingleton, // Image
+		false,                          // Fake
+	}
 }

 // Determine if a dir under source tree is an SoC-owned proprietary directory based
--- a/snapshot/vendor_snapshot.go
+++ b/snapshot/vendor_snapshot.go
@@ -22,28 +22,24 @@ type VendorSnapshotModuleInterface interface {
 	ExcludeFromVendorSnapshot() bool
 }

-var vendorSnapshotSingleton = SnapshotSingleton{
-	"vendor",                     // name
-	"SOONG_VENDOR_SNAPSHOT_ZIP",  // makeVar
-	android.OptionalPath{},       // snapshotZipFile
-	VendorSnapshotImageSingleton, // Image
-	false,                        // Fake
-}
-
-var vendorFakeSnapshotSingleton = SnapshotSingleton{
-	"vendor",                         // name
-	"SOONG_VENDOR_FAKE_SNAPSHOT_ZIP", // makeVar
-	android.OptionalPath{},           // snapshotZipFile
-	VendorSnapshotImageSingleton,     // Image
-	true,                             // Fake
-}
-
 func VendorSnapshotSingleton() android.Singleton {
-	return &vendorSnapshotSingleton
+	return &SnapshotSingleton{
+		"vendor",                     // name
+		"SOONG_VENDOR_SNAPSHOT_ZIP",  // makeVar
+		android.OptionalPath{},       // snapshotZipFile
+		VendorSnapshotImageSingleton, // Image
+		false,                        // Fake
+	}
 }

 func VendorFakeSnapshotSingleton() android.Singleton {
-	return &vendorFakeSnapshotSingleton
+	return &SnapshotSingleton{
+		"vendor",                         // name
+		"SOONG_VENDOR_FAKE_SNAPSHOT_ZIP", // makeVar
+		android.OptionalPath{},           // snapshotZipFile
+		VendorSnapshotImageSingleton,     // Image
+		true,                             // Fake
+	}
 }

 // Determine if a dir under source tree is an SoC-owned proprietary directory based