Merge "Add non-fatal mode for verify_uses_libraries check." am: 7eacdbf8a6 am: d078b18f64

Original change: https://android-review.googlesource.com/c/platform/build/soong/+/1592411

MUST ONLY BE SUBMITTED BY AUTOMERGER

Change-Id: Iabb151ce223f538a346140d8cd78cb0f9c5b6234

dexpreopt/config.go

@@ -84,6 +84,15 @@ type GlobalConfig struct {
 	BootFlags         string        // extra flags to pass to dex2oat for the boot image
 	Dex2oatImageXmx   string        // max heap size for dex2oat for the boot image
 	Dex2oatImageXms   string        // initial heap size for dex2oat for the boot image
+
+	// If true, downgrade the compiler filter of dexpreopt to "extract" when verify_uses_libraries
+	// check fails, instead of failing the build. This will disable any AOT-compilation.
+	//
+	// The intended use case for this flag is to have a smoother migration path for the Java
+	// modules that need to add <uses-library> information in their build files. The flag allows to
+	// quickly silence build errors. This flag should be used with caution and only as a temporary
+	// measure, as it masks real errors and affects performance.
+	RelaxUsesLibraryCheck bool
 }
 
 // GlobalSoongConfig contains the global config that is generated from Soong,
@@ -113,9 +122,10 @@ type ModuleConfig struct {
 	ProfileIsTextListing bool
 	ProfileBootListing   android.OptionalPath
 
-	EnforceUsesLibraries bool
+	EnforceUsesLibraries           bool         // turn on build-time verify_uses_libraries check
-	ProvidesUsesLibrary  string // the name of the <uses-library> (usually the same as its module)
+	EnforceUsesLibrariesStatusFile android.Path // a file with verify_uses_libraries errors (if any)
-	ClassLoaderContexts  ClassLoaderContextMap
+	ProvidesUsesLibrary            string       // library name (usually the same as module name)
+	ClassLoaderContexts            ClassLoaderContextMap
 
 	Archs                   []android.ArchType
 	DexPreoptImages         []android.Path
@@ -258,14 +268,15 @@ func ParseModuleConfig(ctx android.PathContext, data []byte) (*ModuleConfig, err
 
 		// Copies of entries in ModuleConfig that are not constructable without extra parameters.  They will be
 		// used to construct the real value manually below.
-		BuildPath                   string
+		BuildPath                      string
-		DexPath                     string
+		DexPath                        string
-		ManifestPath                string
+		ManifestPath                   string
-		ProfileClassListing         string
+		ProfileClassListing            string
-		ClassLoaderContexts         jsonClassLoaderContextMap
+		EnforceUsesLibrariesStatusFile string
-		DexPreoptImages             []string
+		ClassLoaderContexts            jsonClassLoaderContextMap
-		DexPreoptImageLocations     []string
+		DexPreoptImages                []string
-		PreoptBootClassPathDexFiles []string
+		DexPreoptImageLocations        []string
+		PreoptBootClassPathDexFiles    []string
 	}
 
 	config := ModuleJSONConfig{}
@@ -280,6 +291,7 @@ func ParseModuleConfig(ctx android.PathContext, data []byte) (*ModuleConfig, err
 	config.ModuleConfig.DexPath = constructPath(ctx, config.DexPath)
 	config.ModuleConfig.ManifestPath = constructPath(ctx, config.ManifestPath)
 	config.ModuleConfig.ProfileClassListing = android.OptionalPathForPath(constructPath(ctx, config.ProfileClassListing))
+	config.ModuleConfig.EnforceUsesLibrariesStatusFile = constructPath(ctx, config.EnforceUsesLibrariesStatusFile)
 	config.ModuleConfig.ClassLoaderContexts = fromJsonClassLoaderContext(ctx, config.ClassLoaderContexts)
 	config.ModuleConfig.DexPreoptImages = constructPaths(ctx, config.DexPreoptImages)
 	config.ModuleConfig.DexPreoptImageLocations = config.DexPreoptImageLocations

dexpreopt/dexpreopt.go

@@ -279,11 +279,12 @@ func dexpreoptCommand(ctx android.PathContext, globalSoong *GlobalSoongConfig, g
 
 		// Generate command that saves host and target class loader context in shell variables.
 		clc, paths := ComputeClassLoaderContext(module.ClassLoaderContexts)
-		cmd := rule.Command().
+		rule.Command().
-			Text(`eval "$(`).Tool(globalSoong.ConstructContext).
+			Text("if ! test -s ").Input(module.EnforceUsesLibrariesStatusFile).
+			Text(` ; then eval "$(`).Tool(globalSoong.ConstructContext).
 			Text(` --target-sdk-version ${target_sdk_version}`).
-			Text(clc).Implicits(paths)
+			Text(clc).Implicits(paths).
-		cmd.Text(`)"`)
+			Text(`)" ; fi`)
 
 	} else {
 		// Other libraries or APKs for which the exact <uses-library> list is unknown.
@@ -366,7 +367,16 @@ func dexpreoptCommand(ctx android.PathContext, globalSoong *GlobalSoongConfig, g
 		} else {
 			compilerFilter = "quicken"
 		}
-		cmd.FlagWithArg("--compiler-filter=", compilerFilter)
+		if module.EnforceUsesLibraries {
+			// If the verify_uses_libraries check failed (in this case status file contains a
+			// non-empty error message), then use "extract" compiler filter to avoid compiling any
+			// code (it would be rejected on device because of a class loader context mismatch).
+			cmd.Text("--compiler-filter=$(if test -s ").
+				Input(module.EnforceUsesLibrariesStatusFile).
+				Text(" ; then echo extract ; else echo " + compilerFilter + " ; fi)")
+		} else {
+			cmd.FlagWithArg("--compiler-filter=", compilerFilter)
+		}
 	}
 
 	if generateDM {
@@ -542,6 +552,12 @@ func checkSystemServerOrder(ctx android.PathContext, jarIndex int) {
 	}
 }
 
+// Returns path to a file containing the reult of verify_uses_libraries check (empty if the check
+// has succeeded, or an error message if it failed).
+func UsesLibrariesStatusFile(ctx android.ModuleContext) android.WritablePath {
+	return android.PathForModuleOut(ctx, "enforce_uses_libraries.status")
+}
+
 func contains(l []string, s string) bool {
 	for _, e := range l {
 		if e == s {

dexpreopt/dexpreopt_test.go

@@ -43,6 +43,7 @@ func testModuleConfig(ctx android.PathContext, name, partition string) *ModuleCo
 		PreoptFlags:                     nil,
 		ProfileClassListing:             android.OptionalPath{},
 		ProfileIsTextListing:            false,
+		EnforceUsesLibrariesStatusFile:  android.PathForOutput(ctx, fmt.Sprintf("%s/enforce_uses_libraries.status", name)),
 		EnforceUsesLibraries:            false,
 		ClassLoaderContexts:             nil,
 		Archs:                           []android.ArchType{android.Arm},

java/app.go

@@ -1255,13 +1255,19 @@ func (u *usesLibrary) freezeEnforceUsesLibraries() {
 // in the uses_libs and optional_uses_libs properties.  It returns the path to a copy of the manifest.
 func (u *usesLibrary) verifyUsesLibrariesManifest(ctx android.ModuleContext, manifest android.Path) android.Path {
 	outputFile := android.PathForModuleOut(ctx, "manifest_check", "AndroidManifest.xml")
+	statusFile := dexpreopt.UsesLibrariesStatusFile(ctx)
 
 	rule := android.NewRuleBuilder(pctx, ctx)
 	cmd := rule.Command().BuiltTool("manifest_check").
 		Flag("--enforce-uses-libraries").
 		Input(manifest).
+		FlagWithOutput("--enforce-uses-libraries-status ", statusFile).
 		FlagWithOutput("-o ", outputFile)
 
+	if dexpreopt.GetGlobalConfig(ctx).RelaxUsesLibraryCheck {
+		cmd.Flag("--enforce-uses-libraries-relax")
+	}
+
 	for _, lib := range u.usesLibraryProperties.Uses_libs {
 		cmd.FlagWithArg("--uses-library ", lib)
 	}
@@ -1279,6 +1285,7 @@ func (u *usesLibrary) verifyUsesLibrariesManifest(ctx android.ModuleContext, man
 // in the uses_libs and optional_uses_libs properties.  It returns the path to a copy of the APK.
 func (u *usesLibrary) verifyUsesLibrariesAPK(ctx android.ModuleContext, apk android.Path) android.Path {
 	outputFile := android.PathForModuleOut(ctx, "verify_uses_libraries", apk.Base())
+	statusFile := dexpreopt.UsesLibrariesStatusFile(ctx)
 
 	rule := android.NewRuleBuilder(pctx, ctx)
 	aapt := ctx.Config().HostToolPath(ctx, "aapt")
@@ -1286,7 +1293,8 @@ func (u *usesLibrary) verifyUsesLibrariesAPK(ctx android.ModuleContext, apk andr
 		Textf("aapt_binary=%s", aapt.String()).Implicit(aapt).
 		Textf(`uses_library_names="%s"`, strings.Join(u.usesLibraryProperties.Uses_libs, " ")).
 		Textf(`optional_uses_library_names="%s"`, strings.Join(u.usesLibraryProperties.Optional_uses_libs, " ")).
-		Tool(android.PathForSource(ctx, "build/make/core/verify_uses_libraries.sh")).Input(apk)
+		Textf(`relax_check="%b"`, dexpreopt.GetGlobalConfig(ctx).RelaxUsesLibraryCheck).
+		Tool(android.PathForSource(ctx, "build/make/core/verify_uses_libraries.sh")).Input(apk).Output(statusFile)
 	rule.Command().Text("cp -f").Input(apk).Output(outputFile)
 
 	rule.Build("verify_uses_libraries", "verify <uses-library>")

java/dexpreopt.go

@@ -35,6 +35,7 @@ type dexpreopter struct {
 	isPresignedPrebuilt bool
 
 	manifestFile        android.Path
+	statusFile          android.WritablePath
 	enforceUsesLibs     bool
 	classLoaderContexts dexpreopt.ClassLoaderContextMap
 
@@ -226,9 +227,10 @@ func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.Wr
 		ProfileIsTextListing: profileIsTextListing,
 		ProfileBootListing:   profileBootListing,
 
-		EnforceUsesLibraries: d.enforceUsesLibs,
+		EnforceUsesLibrariesStatusFile: dexpreopt.UsesLibrariesStatusFile(ctx),
-		ProvidesUsesLibrary:  providesUsesLib,
+		EnforceUsesLibraries:           d.enforceUsesLibs,
-		ClassLoaderContexts:  d.classLoaderContexts,
+		ProvidesUsesLibrary:            providesUsesLib,
+		ClassLoaderContexts:            d.classLoaderContexts,
 
 		Archs:                   archs,
 		DexPreoptImages:         images,

scripts/manifest_check.py

@@ -48,6 +48,13 @@ def parse_args():
                       dest='enforce_uses_libraries',
                       action='store_true',
                       help='check the uses-library entries known to the build system against the manifest')
+  parser.add_argument('--enforce-uses-libraries-relax',
+                      dest='enforce_uses_libraries_relax',
+                      action='store_true',
+                      help='do not fail immediately, just save the error message to file')
+  parser.add_argument('--enforce-uses-libraries-status',
+                      dest='enforce_uses_libraries_status',
+                      help='output file to store check status (error message)')
   parser.add_argument('--extract-target-sdk-version',
                       dest='extract_target_sdk_version',
                       action='store_true',
@@ -57,7 +64,7 @@ def parse_args():
   return parser.parse_args()
 
 
-def enforce_uses_libraries(doc, uses_libraries, optional_uses_libraries):
+def enforce_uses_libraries(doc, uses_libraries, optional_uses_libraries, relax):
   """Verify that the <uses-library> tags in the manifest match those provided by the build system.
 
   Args:
@@ -80,10 +87,10 @@ def enforce_uses_libraries(doc, uses_libraries, optional_uses_libraries):
       raise ManifestMismatchError('no <application> tag found')
     return
 
-  verify_uses_library(application, uses_libraries, optional_uses_libraries)
+  return verify_uses_library(application, uses_libraries, optional_uses_libraries, relax)
 
 
-def verify_uses_library(application, uses_libraries, optional_uses_libraries):
+def verify_uses_library(application, uses_libraries, optional_uses_libraries, relax):
   """Verify that the uses-library values known to the build system match the manifest.
 
   Args:
@@ -112,8 +119,12 @@ def verify_uses_library(application, uses_libraries, optional_uses_libraries):
                (', '.join(optional_uses_libraries), ', '.join(manifest_optional_uses_libraries)))
 
   if err:
-    raise ManifestMismatchError('\n'.join(err))
+    errmsg = '\n'.join(err)
+    if not relax:
+      raise ManifestMismatchError(errmsg)
+    return errmsg
 
+  return None
 
 def parse_uses_library(application):
   """Extract uses-library tags from the manifest.
@@ -195,9 +206,19 @@ def main():
     doc = minidom.parse(args.input)
 
     if args.enforce_uses_libraries:
-      enforce_uses_libraries(doc,
+      # Check if the <uses-library> lists in the build system agree with those
-                             args.uses_libraries,
+      # in the manifest. Raise an exception on mismatch, unless the script was
-                             args.optional_uses_libraries)
+      # passed a special parameter to suppress exceptions.
+      errmsg = enforce_uses_libraries(doc, args.uses_libraries,
+        args.optional_uses_libraries, args.enforce_uses_libraries_relax)
+
+      # Create a status file that is empty on success, or contains an error
+      # message on failure. When exceptions are suppressed, dexpreopt command
+      # command will check file size to determine if the check has failed.
+      if args.enforce_uses_libraries_status:
+        with open(args.enforce_uses_libraries_status, 'w') as f:
+          if not errmsg == None:
+            f.write("%s\n" % errmsg)
 
     if args.extract_target_sdk_version:
       print(extract_target_sdk_version(doc))

scripts/manifest_check_test.py

@@ -39,7 +39,9 @@ class EnforceUsesLibrariesTest(unittest.TestCase):
   def run_test(self, input_manifest, uses_libraries=None, optional_uses_libraries=None):
     doc = minidom.parseString(input_manifest)
     try:
-      manifest_check.enforce_uses_libraries(doc, uses_libraries, optional_uses_libraries)
+      relax = False
+      manifest_check.enforce_uses_libraries(doc, uses_libraries,
+        optional_uses_libraries, relax)
       return True
     except manifest_check.ManifestMismatchError:
       return False