diff options
| -rw-r--r-- | android/base_module_context.go | 8 | ||||
| -rw-r--r-- | android/config.go | 6 | ||||
| -rw-r--r-- | android/module.go | 77 | ||||
| -rw-r--r-- | android/selects_test.go | 166 | ||||
| -rw-r--r-- | android/variable.go | 33 | ||||
| -rw-r--r-- | cc/cc.go | 27 | ||||
| -rw-r--r-- | cc/cc_test_only_property_test.go | 32 | ||||
| -rw-r--r-- | dexpreopt/class_loader_context.go | 1 | ||||
| -rw-r--r-- | java/aar.go | 21 | ||||
| -rw-r--r-- | java/app.go | 78 | ||||
| -rw-r--r-- | java/app_import.go | 8 | ||||
| -rw-r--r-- | java/app_test.go | 20 | ||||
| -rw-r--r-- | java/base.go | 17 | ||||
| -rw-r--r-- | java/java.go | 26 | ||||
| -rwxr-xr-x | scripts/manifest_check.py | 35 | ||||
| -rwxr-xr-x | scripts/manifest_check_test.py | 17 |
16 files changed, 438 insertions, 134 deletions
diff --git a/android/base_module_context.go b/android/base_module_context.go index c4922f4bc..c5fe58578 100644 --- a/android/base_module_context.go +++ b/android/base_module_context.go @@ -20,7 +20,7 @@ import ( "strings" "github.com/google/blueprint" - "github.com/google/blueprint/parser" + "github.com/google/blueprint/proptools" ) // BaseModuleContext is the same as blueprint.BaseModuleContext except that Config() returns @@ -219,7 +219,7 @@ type BaseModuleContext interface { // EvaluateConfiguration makes ModuleContext a valid proptools.ConfigurableEvaluator, so this context // can be used to evaluate the final value of Configurable properties. - EvaluateConfiguration(parser.SelectType, string, string) (string, bool) + EvaluateConfiguration(condition proptools.ConfigurableCondition, property string) proptools.ConfigurableValue } type baseModuleContext struct { @@ -577,6 +577,6 @@ func (b *baseModuleContext) GetPathString(skipFirst bool) string { return sb.String() } -func (m *baseModuleContext) EvaluateConfiguration(ty parser.SelectType, property, condition string) (string, bool) { - return m.Module().ConfigurableEvaluator(m).EvaluateConfiguration(ty, property, condition) +func (m *baseModuleContext) EvaluateConfiguration(condition proptools.ConfigurableCondition, property string) proptools.ConfigurableValue { + return m.Module().ConfigurableEvaluator(m).EvaluateConfiguration(condition, property) } diff --git a/android/config.go b/android/config.go index 11bd81b89..75d135fc0 100644 --- a/android/config.go +++ b/android/config.go @@ -949,7 +949,11 @@ func (c *config) PlatformPreviewSdkVersion() string { } func (c *config) PlatformMinSupportedTargetSdkVersion() string { - return String(c.productVariables.Platform_min_supported_target_sdk_version) + var val, ok = c.productVariables.BuildFlags["RELEASE_PLATFORM_MIN_SUPPORTED_TARGET_SDK_VERSION"] + if !ok { + return "" + } + return val } func (c *config) PlatformBaseOS() string { diff --git a/android/module.go b/android/module.go index 89c4ddde9..47bc8295e 100644 --- a/android/module.go +++ b/android/module.go @@ -29,7 +29,6 @@ import ( "android/soong/bazel" "github.com/google/blueprint" - "github.com/google/blueprint/parser" "github.com/google/blueprint/proptools" ) @@ -2140,41 +2139,75 @@ func (e configurationEvalutor) PropertyErrorf(property string, fmt string, args e.ctx.OtherModulePropertyErrorf(e.m, property, fmt, args) } -func (e configurationEvalutor) EvaluateConfiguration(ty parser.SelectType, property, condition string) (string, bool) { +func (e configurationEvalutor) EvaluateConfiguration(condition proptools.ConfigurableCondition, property string) proptools.ConfigurableValue { ctx := e.ctx m := e.m - switch ty { - case parser.SelectTypeReleaseVariable: - if v, ok := ctx.Config().productVariables.BuildFlags[condition]; ok { - return v, true + switch condition.FunctionName { + case "release_variable": + if len(condition.Args) != 1 { + ctx.OtherModulePropertyErrorf(m, property, "release_variable requires 1 argument, found %d", len(condition.Args)) + return proptools.ConfigurableValueUndefined() } - return "", false - case parser.SelectTypeProductVariable: + if v, ok := ctx.Config().productVariables.BuildFlags[condition.Args[0]]; ok { + return proptools.ConfigurableValueString(v) + } + return proptools.ConfigurableValueUndefined() + case "product_variable": // TODO(b/323382414): Might add these on a case-by-case basis ctx.OtherModulePropertyErrorf(m, property, "TODO(b/323382414): Product variables are not yet supported in selects") - return "", false - case parser.SelectTypeSoongConfigVariable: - parts := strings.Split(condition, ":") - namespace := parts[0] - variable := parts[1] + return proptools.ConfigurableValueUndefined() + case "soong_config_variable": + if len(condition.Args) != 2 { + ctx.OtherModulePropertyErrorf(m, property, "soong_config_variable requires 2 arguments, found %d", len(condition.Args)) + return proptools.ConfigurableValueUndefined() + } + namespace := condition.Args[0] + variable := condition.Args[1] if n, ok := ctx.Config().productVariables.VendorVars[namespace]; ok { if v, ok := n[variable]; ok { - return v, true + return proptools.ConfigurableValueString(v) } } - return "", false - case parser.SelectTypeVariant: - if condition == "arch" { + return proptools.ConfigurableValueUndefined() + case "variant": + if len(condition.Args) != 1 { + ctx.OtherModulePropertyErrorf(m, property, "variant requires 1 argument, found %d", len(condition.Args)) + return proptools.ConfigurableValueUndefined() + } + if condition.Args[0] == "arch" { if !m.base().ArchReady() { ctx.OtherModulePropertyErrorf(m, property, "A select on arch was attempted before the arch mutator ran") - return "", false + return proptools.ConfigurableValueUndefined() + } + return proptools.ConfigurableValueString(m.base().Arch().ArchType.Name) + } + ctx.OtherModulePropertyErrorf(m, property, "Unknown variant %s", condition.Args[0]) + return proptools.ConfigurableValueUndefined() + case "boolean_var_for_testing": + // We currently don't have any other boolean variables (we should add support for typing + // the soong config variables), so add this fake one for testing the boolean select + // functionality. + if len(condition.Args) != 0 { + ctx.OtherModulePropertyErrorf(m, property, "boolean_var_for_testing requires 0 arguments, found %d", len(condition.Args)) + return proptools.ConfigurableValueUndefined() + } + + if n, ok := ctx.Config().productVariables.VendorVars["boolean_var"]; ok { + if v, ok := n["for_testing"]; ok { + switch v { + case "true": + return proptools.ConfigurableValueBool(true) + case "false": + return proptools.ConfigurableValueBool(false) + default: + ctx.OtherModulePropertyErrorf(m, property, "testing:my_boolean_var can only be true or false, found %q", v) + } } - return m.base().Arch().ArchType.Name, true } - ctx.OtherModulePropertyErrorf(m, property, "Unknown variant %s", condition) - return "", false + return proptools.ConfigurableValueUndefined() default: - panic("Should be unreachable") + ctx.OtherModulePropertyErrorf(m, property, "Unknown select condition %s", condition.FunctionName) + return proptools.ConfigurableValueUndefined() } } diff --git a/android/selects_test.go b/android/selects_test.go index e59b3e65d..1eb137beb 100644 --- a/android/selects_test.go +++ b/android/selects_test.go @@ -327,8 +327,10 @@ func TestSelects(t *testing.T) { my_module_type { name: "foo", my_string: select(soong_config_variable("my_namespace", "my_variable"), { + "foo": "bar", default: unset, }) + select(soong_config_variable("my_namespace", "my_variable2"), { + "baz": "qux", default: unset, }) } @@ -341,6 +343,7 @@ func TestSelects(t *testing.T) { my_module_type { name: "foo", my_string: select(soong_config_variable("my_namespace", "my_variable"), { + "foo": "bar", default: unset, }) + select(soong_config_variable("my_namespace", "my_variable2"), { default: "a", @@ -414,6 +417,169 @@ func TestSelects(t *testing.T) { replacing_string_list: &[]string{"b1"}, }, }, + { + name: "Multi-condition string 1", + bp: ` + my_module_type { + name: "foo", + my_string: select(( + soong_config_variable("my_namespace", "my_variable"), + soong_config_variable("my_namespace", "my_variable2"), + ), { + ("a", "b"): "a+b", + ("a", default): "a+default", + (default, default): "default", + }), + } + `, + vendorVars: map[string]map[string]string{ + "my_namespace": { + "my_variable": "a", + "my_variable2": "b", + }, + }, + provider: selectsTestProvider{ + my_string: proptools.StringPtr("a+b"), + }, + }, + { + name: "Multi-condition string 2", + bp: ` + my_module_type { + name: "foo", + my_string: select(( + soong_config_variable("my_namespace", "my_variable"), + soong_config_variable("my_namespace", "my_variable2"), + ), { + ("a", "b"): "a+b", + ("a", default): "a+default", + (default, default): "default", + }), + } + `, + vendorVars: map[string]map[string]string{ + "my_namespace": { + "my_variable": "a", + "my_variable2": "c", + }, + }, + provider: selectsTestProvider{ + my_string: proptools.StringPtr("a+default"), + }, + }, + { + name: "Multi-condition string 3", + bp: ` + my_module_type { + name: "foo", + my_string: select(( + soong_config_variable("my_namespace", "my_variable"), + soong_config_variable("my_namespace", "my_variable2"), + ), { + ("a", "b"): "a+b", + ("a", default): "a+default", + (default, default): "default", + }), + } + `, + vendorVars: map[string]map[string]string{ + "my_namespace": { + "my_variable": "c", + "my_variable2": "b", + }, + }, + provider: selectsTestProvider{ + my_string: proptools.StringPtr("default"), + }, + }, + { + name: "Select on boolean", + bp: ` + my_module_type { + name: "foo", + my_string: select(boolean_var_for_testing(), { + true: "t", + false: "f", + }), + } + `, + vendorVars: map[string]map[string]string{ + "boolean_var": { + "for_testing": "true", + }, + }, + provider: selectsTestProvider{ + my_string: proptools.StringPtr("t"), + }, + }, + { + name: "Select on boolean false", + bp: ` + my_module_type { + name: "foo", + my_string: select(boolean_var_for_testing(), { + true: "t", + false: "f", + }), + } + `, + vendorVars: map[string]map[string]string{ + "boolean_var": { + "for_testing": "false", + }, + }, + provider: selectsTestProvider{ + my_string: proptools.StringPtr("f"), + }, + }, + { + name: "Select on boolean undefined", + bp: ` + my_module_type { + name: "foo", + my_string: select(boolean_var_for_testing(), { + true: "t", + false: "f", + }), + } + `, + expectedError: "foo", + }, + { + name: "Select on boolean undefined with default", + bp: ` + my_module_type { + name: "foo", + my_string: select(boolean_var_for_testing(), { + true: "t", + false: "f", + default: "default", + }), + } + `, + provider: selectsTestProvider{ + my_string: proptools.StringPtr("default"), + }, + }, + { + name: "Mismatched condition types", + bp: ` + my_module_type { + name: "foo", + my_string: select(boolean_var_for_testing(), { + "true": "t", + "false": "f", + default: "default", + }), + } + `, + vendorVars: map[string]map[string]string{ + "boolean_var": { + "for_testing": "false", + }, + }, + expectedError: "Expected all branches of a select on condition boolean_var_for_testing\\(\\) to have type bool, found string", + }, } for _, tc := range testCases { diff --git a/android/variable.go b/android/variable.go index 5a079db50..0040d836d 100644 --- a/android/variable.go +++ b/android/variable.go @@ -201,23 +201,22 @@ type ProductVariables struct { BuildThumbprintFile *string `json:",omitempty"` DisplayBuildNumber *bool `json:",omitempty"` - Platform_display_version_name *string `json:",omitempty"` - Platform_version_name *string `json:",omitempty"` - Platform_sdk_version *int `json:",omitempty"` - Platform_sdk_codename *string `json:",omitempty"` - Platform_sdk_version_or_codename *string `json:",omitempty"` - Platform_sdk_final *bool `json:",omitempty"` - Platform_sdk_extension_version *int `json:",omitempty"` - Platform_base_sdk_extension_version *int `json:",omitempty"` - Platform_version_active_codenames []string `json:",omitempty"` - Platform_version_all_preview_codenames []string `json:",omitempty"` - Platform_systemsdk_versions []string `json:",omitempty"` - Platform_security_patch *string `json:",omitempty"` - Platform_preview_sdk_version *string `json:",omitempty"` - Platform_min_supported_target_sdk_version *string `json:",omitempty"` - Platform_base_os *string `json:",omitempty"` - Platform_version_last_stable *string `json:",omitempty"` - Platform_version_known_codenames *string `json:",omitempty"` + Platform_display_version_name *string `json:",omitempty"` + Platform_version_name *string `json:",omitempty"` + Platform_sdk_version *int `json:",omitempty"` + Platform_sdk_codename *string `json:",omitempty"` + Platform_sdk_version_or_codename *string `json:",omitempty"` + Platform_sdk_final *bool `json:",omitempty"` + Platform_sdk_extension_version *int `json:",omitempty"` + Platform_base_sdk_extension_version *int `json:",omitempty"` + Platform_version_active_codenames []string `json:",omitempty"` + Platform_version_all_preview_codenames []string `json:",omitempty"` + Platform_systemsdk_versions []string `json:",omitempty"` + Platform_security_patch *string `json:",omitempty"` + Platform_preview_sdk_version *string `json:",omitempty"` + Platform_base_os *string `json:",omitempty"` + Platform_version_last_stable *string `json:",omitempty"` + Platform_version_known_codenames *string `json:",omitempty"` DeviceName *string `json:",omitempty"` DeviceProduct *string `json:",omitempty"` @@ -1995,6 +1995,20 @@ func (d *Defaults) GenerateAndroidBuildActions(ctx android.ModuleContext) { } func (c *Module) GenerateAndroidBuildActions(actx android.ModuleContext) { + ctx := moduleContextFromAndroidModuleContext(actx, c) + + // If Test_only is set on a module in bp file, respect the setting, otherwise + // see if is a known test module type. + testOnly := c.testModule || c.testLibrary() + if c.sourceProperties.Test_only != nil { + testOnly = Bool(c.sourceProperties.Test_only) + } + // Keep before any early returns. + android.SetProvider(ctx, android.TestOnlyProviderKey, android.TestModuleInformation{ + TestOnly: testOnly, + TopLevelTarget: c.testModule, + }) + // Handle the case of a test module split by `test_per_src` mutator. // // The `test_per_src` mutator adds an extra variation named "", depending on all the other @@ -2013,8 +2027,6 @@ func (c *Module) GenerateAndroidBuildActions(actx android.ModuleContext) { c.makeLinkType = GetMakeLinkType(actx, c) - ctx := moduleContextFromAndroidModuleContext(actx, c) - deps := c.depsToPaths(ctx) if ctx.Failed() { return @@ -2141,17 +2153,6 @@ func (c *Module) GenerateAndroidBuildActions(actx android.ModuleContext) { android.SetProvider(ctx, testing.TestModuleProviderKey, testing.TestModuleProviderData{}) } - // If Test_only is set on a module in bp file, respect the setting, otherwise - // see if is a known test module type. - testOnly := c.testModule || c.testLibrary() - if c.sourceProperties.Test_only != nil { - testOnly = Bool(c.sourceProperties.Test_only) - } - android.SetProvider(ctx, android.TestOnlyProviderKey, android.TestModuleInformation{ - TestOnly: testOnly, - TopLevelTarget: c.testModule, - }) - android.SetProvider(ctx, blueprint.SrcsFileProviderKey, blueprint.SrcsFileProviderData{SrcPaths: deps.GeneratedSources.Strings()}) android.CollectDependencyAconfigFiles(ctx, &c.mergedAconfigFiles) diff --git a/cc/cc_test_only_property_test.go b/cc/cc_test_only_property_test.go index 972e86bc5..c14f34ecb 100644 --- a/cc/cc_test_only_property_test.go +++ b/cc/cc_test_only_property_test.go @@ -78,6 +78,38 @@ func TestTestOnlyProvider(t *testing.T) { } } +func TestTestOnlyValueWithTestPerSrcProp(t *testing.T) { + t.Parallel() + ctx := android.GroupFixturePreparers( + prepareForCcTest, + ).RunTestWithBp(t, ` + // These should be test-only + cc_test { name: "cc-test", + gtest: false, + test_per_src: true, + srcs: ["foo_test.cpp"], + test_options: { unit_test: false, }, + } + `) + + // Ensure all variation of test-per-src tests are marked test-only. + ctx.VisitAllModules(func(m blueprint.Module) { + testOnly := false + if provider, ok := android.OtherModuleProvider(ctx.TestContext.OtherModuleProviderAdaptor(), m, android.TestOnlyProviderKey); ok { + if provider.TestOnly { + testOnly = true + } + } + if module, ok := m.(*Module); ok { + if testModule, ok := module.installer.(*testBinary); ok { + if !testOnly && *testModule.Properties.Test_per_src { + t.Errorf("%v is not test-only but should be", m) + } + } + } + }) +} + func TestTestOnlyInTeamsProto(t *testing.T) { t.Parallel() ctx := android.GroupFixturePreparers( diff --git a/dexpreopt/class_loader_context.go b/dexpreopt/class_loader_context.go index 57c7ae851..af1d33da6 100644 --- a/dexpreopt/class_loader_context.go +++ b/dexpreopt/class_loader_context.go @@ -323,6 +323,7 @@ func (clcMap ClassLoaderContextMap) addContext(ctx android.ModuleInstallPathCont } else if clc.Host == hostPath && clc.Device == devicePath { // Ok, the same library with the same paths. Don't re-add it, but don't raise an error // either, as the same library may be reachable via different transitional dependencies. + clc.Optional = clc.Optional && optional return nil } else { // Fail, as someone is trying to add the same library with different paths. This likely diff --git a/java/aar.go b/java/aar.go index a36626732..f8955ce90 100644 --- a/java/aar.go +++ b/java/aar.go @@ -356,12 +356,13 @@ type aaptBuildActionOptions struct { forceNonFinalResourceIDs bool extraLinkFlags []string aconfigTextFiles android.Paths + usesLibrary *usesLibrary } func (a *aapt) buildActions(ctx android.ModuleContext, opts aaptBuildActionOptions) { staticResourcesNodesDepSet, sharedResourcesNodesDepSet, staticRRODirsDepSet, staticManifestsDepSet, sharedExportPackages, libFlags := - aaptLibs(ctx, opts.sdkContext, opts.classLoaderContexts) + aaptLibs(ctx, opts.sdkContext, opts.classLoaderContexts, opts.usesLibrary) // Exclude any libraries from the supplied list. opts.classLoaderContexts = opts.classLoaderContexts.ExcludeLibs(opts.excludedLibs) @@ -703,7 +704,8 @@ func (t transitiveAarDeps) assets() android.Paths { } // aaptLibs collects libraries from dependencies and sdk_version and converts them into paths -func aaptLibs(ctx android.ModuleContext, sdkContext android.SdkContext, classLoaderContexts dexpreopt.ClassLoaderContextMap) ( +func aaptLibs(ctx android.ModuleContext, sdkContext android.SdkContext, + classLoaderContexts dexpreopt.ClassLoaderContextMap, usesLibrary *usesLibrary) ( staticResourcesNodes, sharedResourcesNodes *android.DepSet[*resourcesNode], staticRRODirs *android.DepSet[rroDir], staticManifests *android.DepSet[android.Path], sharedLibs android.Paths, flags []string) { @@ -753,6 +755,9 @@ func aaptLibs(ctx android.ModuleContext, sdkContext android.SdkContext, classLoa } addCLCFromDep(ctx, module, classLoaderContexts) + if usesLibrary != nil { + addMissingOptionalUsesLibsFromDep(ctx, module, usesLibrary) + } }) // AAPT2 overlays are in lowest to highest priority order, the topological order will be reversed later. @@ -805,12 +810,12 @@ func (a *AndroidLibrary) OutputFiles(tag string) (android.Paths, error) { var _ AndroidLibraryDependency = (*AndroidLibrary)(nil) func (a *AndroidLibrary) DepsMutator(ctx android.BottomUpMutatorContext) { + a.usesLibrary.deps(ctx, false) a.Module.deps(ctx) sdkDep := decodeSdkDep(ctx, android.SdkContext(a)) if sdkDep.hasFrameworkLibs() { a.aapt.deps(ctx, sdkDep) } - a.usesLibrary.deps(ctx, false) for _, aconfig_declaration := range a.aaptProperties.Flags_packages { ctx.AddDependency(ctx.Module(), aconfigDeclarationTag, aconfig_declaration) @@ -829,6 +834,7 @@ func (a *AndroidLibrary) GenerateAndroidBuildActions(ctx android.ModuleContext) classLoaderContexts: a.classLoaderContexts, enforceDefaultTargetSdkVersion: false, aconfigTextFiles: getAconfigFilePaths(ctx), + usesLibrary: &a.usesLibrary, }, ) @@ -1215,7 +1221,7 @@ func (a *AARImport) GenerateAndroidBuildActions(ctx android.ModuleContext) { linkDeps = append(linkDeps, a.manifest) staticResourcesNodesDepSet, sharedResourcesNodesDepSet, staticRRODirsDepSet, staticManifestsDepSet, sharedLibs, libFlags := - aaptLibs(ctx, android.SdkContext(a), nil) + aaptLibs(ctx, android.SdkContext(a), nil, nil) _ = sharedResourcesNodesDepSet _ = staticRRODirsDepSet @@ -1287,6 +1293,7 @@ func (a *AARImport) GenerateAndroidBuildActions(ctx android.ModuleContext) { } } addCLCFromDep(ctx, module, a.classLoaderContexts) + addMissingOptionalUsesLibsFromDep(ctx, module, &a.usesLibrary) }) var implementationJarFile android.OutputPath @@ -1405,6 +1412,12 @@ func (a *AARImport) ShouldSupportSdkVersion(ctx android.BaseModuleContext, var _ android.PrebuiltInterface = (*AARImport)(nil) +func (a *AARImport) UsesLibrary() *usesLibrary { + return &a.usesLibrary +} + +var _ ModuleWithUsesLibrary = (*AARImport)(nil) + // android_library_import imports an `.aar` file into the build graph as if it was built with android_library. // // This module is not suitable for installing on a device, but can be used as a `static_libs` dependency of diff --git a/java/app.go b/java/app.go index 1aa3afe8e..50d1a2f43 100644 --- a/java/app.go +++ b/java/app.go @@ -249,13 +249,13 @@ func (c Certificate) AndroidMkString() string { } func (a *AndroidApp) DepsMutator(ctx android.BottomUpMutatorContext) { - a.Module.deps(ctx) - if String(a.appProperties.Stl) == "c++_shared" && !a.SdkVersion(ctx).Specified() { ctx.PropertyErrorf("stl", "sdk_version must be set in order to use c++_shared") } sdkDep := decodeSdkDep(ctx, android.SdkContext(a)) + a.usesLibrary.deps(ctx, sdkDep.hasFrameworkLibs()) + a.Module.deps(ctx) if sdkDep.hasFrameworkLibs() { a.aapt.deps(ctx, sdkDep) } @@ -285,9 +285,6 @@ func (a *AndroidApp) DepsMutator(ctx android.BottomUpMutatorContext) { } ctx.AddFarVariationDependencies(variation, jniLibTag, a.appProperties.Jni_libs...) } - - a.usesLibrary.deps(ctx, sdkDep.hasFrameworkLibs()) - for _, aconfig_declaration := range a.aaptProperties.Flags_packages { ctx.AddDependency(ctx.Module(), aconfigDeclarationTag, aconfig_declaration) } @@ -534,6 +531,7 @@ func (a *AndroidApp) aaptBuildActions(ctx android.ModuleContext) { forceNonFinalResourceIDs: nonFinalIds, extraLinkFlags: aaptLinkFlags, aconfigTextFiles: getAconfigFilePaths(ctx), + usesLibrary: &a.usesLibrary, }, ) @@ -815,18 +813,10 @@ func (a *AndroidApp) generateAndroidBuildActions(ctx android.ModuleContext) { // The decision to enforce <uses-library> checks is made before adding implicit SDK libraries. a.usesLibrary.freezeEnforceUsesLibraries() - // Add implicit SDK libraries to <uses-library> list. - requiredUsesLibs, optionalUsesLibs := a.classLoaderContexts.UsesLibs() - for _, usesLib := range requiredUsesLibs { - a.usesLibrary.addLib(usesLib, false) - } - for _, usesLib := range optionalUsesLibs { - a.usesLibrary.addLib(usesLib, true) - } - // Check that the <uses-library> list is coherent with the manifest. if a.usesLibrary.enforceUsesLibraries() { - manifestCheckFile := a.usesLibrary.verifyUsesLibrariesManifest(ctx, a.mergedManifestFile) + manifestCheckFile := a.usesLibrary.verifyUsesLibrariesManifest( + ctx, a.mergedManifestFile, &a.classLoaderContexts) apkDeps = append(apkDeps, manifestCheckFile) } @@ -1596,6 +1586,9 @@ type UsesLibraryProperties struct { // provide the android.test.base statically and use jarjar to rename them so they do not collide // with the classes provided by the android.test.base library. Exclude_uses_libs []string + + // The module names of optional uses-library libraries that are missing from the source tree. + Missing_optional_uses_libs []string `blueprint:"mutated"` } // usesLibrary provides properties and helper functions for AndroidApp and AndroidAppImport to verify that the @@ -1612,20 +1605,11 @@ type usesLibrary struct { shouldDisableDexpreopt bool } -func (u *usesLibrary) addLib(lib string, optional bool) { - if !android.InList(lib, u.usesLibraryProperties.Uses_libs) && !android.InList(lib, u.usesLibraryProperties.Optional_uses_libs) { - if optional { - u.usesLibraryProperties.Optional_uses_libs = append(u.usesLibraryProperties.Optional_uses_libs, lib) - } else { - u.usesLibraryProperties.Uses_libs = append(u.usesLibraryProperties.Uses_libs, lib) - } - } -} - func (u *usesLibrary) deps(ctx android.BottomUpMutatorContext, addCompatDeps bool) { if !ctx.Config().UnbundledBuild() || ctx.Config().UnbundledBuildImage() { ctx.AddVariationDependencies(nil, usesLibReqTag, u.usesLibraryProperties.Uses_libs...) - ctx.AddVariationDependencies(nil, usesLibOptTag, u.presentOptionalUsesLibs(ctx)...) + presentOptionalUsesLibs := u.presentOptionalUsesLibs(ctx) + ctx.AddVariationDependencies(nil, usesLibOptTag, presentOptionalUsesLibs...) // Only add these extra dependencies if the module is an app that depends on framework // libs. This avoids creating a cyclic dependency: // e.g. framework-res -> org.apache.http.legacy -> ... -> framework-res. @@ -1636,6 +1620,8 @@ func (u *usesLibrary) deps(ctx android.BottomUpMutatorContext, addCompatDeps boo ctx.AddVariationDependencies(nil, usesLibCompat28OptTag, dexpreopt.OptionalCompatUsesLibs28...) ctx.AddVariationDependencies(nil, usesLibCompat30OptTag, dexpreopt.OptionalCompatUsesLibs30...) } + _, diff, _ := android.ListSetDifference(u.usesLibraryProperties.Optional_uses_libs, presentOptionalUsesLibs) + u.usesLibraryProperties.Missing_optional_uses_libs = diff } else { ctx.AddVariationDependencies(nil, r8LibraryJarTag, u.usesLibraryProperties.Uses_libs...) ctx.AddVariationDependencies(nil, r8LibraryJarTag, u.presentOptionalUsesLibs(ctx)...) @@ -1654,15 +1640,6 @@ func (u *usesLibrary) presentOptionalUsesLibs(ctx android.BaseModuleContext) []s return optionalUsesLibs } -// Helper function to replace string in a list. -func replaceInList(list []string, oldstr, newstr string) { - for i, str := range list { - if str == oldstr { - list[i] = newstr - } - } -} - // Returns a map of module names of shared library dependencies to the paths to their dex jars on // host and on device. func (u *usesLibrary) classLoaderContextForUsesLibDeps(ctx android.ModuleContext) dexpreopt.ClassLoaderContextMap { @@ -1704,11 +1681,6 @@ func (u *usesLibrary) classLoaderContextForUsesLibDeps(ctx android.ModuleContext libName := dep if ulib, ok := m.(ProvidesUsesLib); ok && ulib.ProvidesUsesLib() != nil { libName = *ulib.ProvidesUsesLib() - // Replace module name with library name in `uses_libs`/`optional_uses_libs` in - // order to pass verify_uses_libraries check (which compares these properties - // against library names written in the manifest). - replaceInList(u.usesLibraryProperties.Uses_libs, dep, libName) - replaceInList(u.usesLibraryProperties.Optional_uses_libs, dep, libName) } clcMap.AddContext(ctx, tag.sdkVersion, libName, tag.optional, lib.DexJarBuildPath(ctx).PathOrNil(), lib.DexJarInstallPath(), @@ -1742,7 +1714,7 @@ func (u *usesLibrary) freezeEnforceUsesLibraries() { // an APK with the manifest embedded in it (manifest_check will know which one it is by the file // extension: APKs are supposed to end with '.apk'). func (u *usesLibrary) verifyUsesLibraries(ctx android.ModuleContext, inputFile android.Path, - outputFile android.WritablePath) android.Path { + outputFile android.WritablePath, classLoaderContexts *dexpreopt.ClassLoaderContextMap) android.Path { statusFile := dexpreopt.UsesLibrariesStatusFile(ctx) @@ -1770,27 +1742,37 @@ func (u *usesLibrary) verifyUsesLibraries(ctx android.ModuleContext, inputFile a cmd.Flag("--enforce-uses-libraries-relax") } - for _, lib := range u.usesLibraryProperties.Uses_libs { + requiredUsesLibs, optionalUsesLibs := classLoaderContexts.UsesLibs() + for _, lib := range requiredUsesLibs { cmd.FlagWithArg("--uses-library ", lib) } - - for _, lib := range u.usesLibraryProperties.Optional_uses_libs { + for _, lib := range optionalUsesLibs { cmd.FlagWithArg("--optional-uses-library ", lib) } + // Also add missing optional uses libs, as the manifest check expects them. + // Note that what we add here are the module names of those missing libs, not library names, while + // the manifest check actually expects library names. However, the case where a library is missing + // and the module name != the library name is too rare for us to handle. + for _, lib := range u.usesLibraryProperties.Missing_optional_uses_libs { + cmd.FlagWithArg("--missing-optional-uses-library ", lib) + } + rule.Build("verify_uses_libraries", "verify <uses-library>") return outputFile } // verifyUsesLibrariesManifest checks the <uses-library> tags in an AndroidManifest.xml against // the build system and returns the path to a copy of the manifest. -func (u *usesLibrary) verifyUsesLibrariesManifest(ctx android.ModuleContext, manifest android.Path) android.Path { +func (u *usesLibrary) verifyUsesLibrariesManifest(ctx android.ModuleContext, manifest android.Path, + classLoaderContexts *dexpreopt.ClassLoaderContextMap) android.Path { outputFile := android.PathForModuleOut(ctx, "manifest_check", "AndroidManifest.xml") - return u.verifyUsesLibraries(ctx, manifest, outputFile) + return u.verifyUsesLibraries(ctx, manifest, outputFile, classLoaderContexts) } // verifyUsesLibrariesAPK checks the <uses-library> tags in the manifest of an APK against the build // system and returns the path to a copy of the APK. -func (u *usesLibrary) verifyUsesLibrariesAPK(ctx android.ModuleContext, apk android.Path) { - u.verifyUsesLibraries(ctx, apk, nil) // for APKs manifest_check does not write output file +func (u *usesLibrary) verifyUsesLibrariesAPK(ctx android.ModuleContext, apk android.Path, + classLoaderContexts *dexpreopt.ClassLoaderContextMap) { + u.verifyUsesLibraries(ctx, apk, nil, classLoaderContexts) // for APKs manifest_check does not write output file } diff --git a/java/app_import.go b/java/app_import.go index 7387e168c..bb07c423a 100644 --- a/java/app_import.go +++ b/java/app_import.go @@ -355,7 +355,7 @@ func (a *AndroidAppImport) generateAndroidBuildActions(ctx android.ModuleContext } if a.usesLibrary.enforceUsesLibraries() { - a.usesLibrary.verifyUsesLibrariesAPK(ctx, srcApk) + a.usesLibrary.verifyUsesLibrariesAPK(ctx, srcApk, &a.dexpreopter.classLoaderContexts) } a.dexpreopter.dexpreopt(ctx, android.RemoveOptionalPrebuiltPrefix(ctx.ModuleName()), jnisUncompressed) @@ -611,6 +611,12 @@ func createArchDpiVariantGroupType(archNames []string, dpiNames []string) reflec return return_struct } +func (a *AndroidAppImport) UsesLibrary() *usesLibrary { + return &a.usesLibrary +} + +var _ ModuleWithUsesLibrary = (*AndroidAppImport)(nil) + // android_app_import imports a prebuilt apk with additional processing specified in the module. // DPI-specific apk source files can be specified using dpi_variants. Example: // diff --git a/java/app_test.go b/java/app_test.go index 0c2800041..eab40e7da 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -3244,7 +3244,10 @@ func TestUsesLibraries(t *testing.T) { name: "static-y", srcs: ["a.java"], uses_libs: ["runtime-required-y"], - optional_uses_libs: ["runtime-optional-y"], + optional_uses_libs: [ + "runtime-optional-y", + "missing-lib-a", + ], sdk_version: "current", } @@ -3280,7 +3283,7 @@ func TestUsesLibraries(t *testing.T) { sdk_version: "current", optional_uses_libs: [ "bar", - "baz", + "missing-lib-b", ], } @@ -3295,7 +3298,7 @@ func TestUsesLibraries(t *testing.T) { ], optional_uses_libs: [ "bar", - "baz", + "missing-lib-b", ], } ` @@ -3317,10 +3320,10 @@ func TestUsesLibraries(t *testing.T) { // propagated from dependencies. actualManifestFixerArgs := app.Output("manifest_fixer/AndroidManifest.xml").Args["args"] expectManifestFixerArgs := `--extract-native-libs=true ` + - `--uses-library qux ` + - `--uses-library quuz ` + `--uses-library foo ` + `--uses-library com.non.sdk.lib ` + + `--uses-library qux ` + + `--uses-library quuz ` + `--uses-library runtime-library ` + `--uses-library runtime-required-x ` + `--uses-library runtime-required-y ` + @@ -3339,9 +3342,10 @@ func TestUsesLibraries(t *testing.T) { `--uses-library runtime-required-x ` + `--uses-library runtime-required-y ` + `--optional-uses-library bar ` + - `--optional-uses-library baz ` + `--optional-uses-library runtime-optional-x ` + - `--optional-uses-library runtime-optional-y ` + `--optional-uses-library runtime-optional-y ` + + `--missing-optional-uses-library missing-lib-b ` + + `--missing-optional-uses-library missing-lib-a` android.AssertStringDoesContain(t, "verify cmd args", verifyCmd, verifyArgs) // Test that all libraries are verified for an APK (library order matters). @@ -3350,7 +3354,7 @@ func TestUsesLibraries(t *testing.T) { `--uses-library com.non.sdk.lib ` + `--uses-library android.test.runner ` + `--optional-uses-library bar ` + - `--optional-uses-library baz ` + `--missing-optional-uses-library missing-lib-b ` android.AssertStringDoesContain(t, "verify apk cmd args", verifyApkCmd, verifyApkArgs) // Test that necessary args are passed for constructing CLC in Ninja phase. diff --git a/java/base.go b/java/base.go index ef61f1cc2..938ac5e82 100644 --- a/java/base.go +++ b/java/base.go @@ -843,9 +843,11 @@ func (j *Module) deps(ctx android.BottomUpMutatorContext) { if dep != nil { if component, ok := dep.(SdkLibraryComponentDependency); ok { if lib := component.OptionalSdkLibraryImplementation(); lib != nil { - // Add library as optional if it's one of the optional compatibility libs. + // Add library as optional if it's one of the optional compatibility libs or it's + // explicitly listed in the optional_uses_libs property. tag := usesLibReqTag - if android.InList(*lib, dexpreopt.OptionalCompatUsesLibs) { + if android.InList(*lib, dexpreopt.OptionalCompatUsesLibs) || + android.InList(*lib, j.usesLibrary.usesLibraryProperties.Optional_uses_libs) { tag = usesLibOptTag } ctx.AddVariationDependencies(nil, tag, *lib) @@ -2387,6 +2389,7 @@ func (j *Module) collectDeps(ctx android.ModuleContext) deps { } addCLCFromDep(ctx, module, j.classLoaderContexts) + addMissingOptionalUsesLibsFromDep(ctx, module, &j.usesLibrary) }) return deps @@ -2720,3 +2723,13 @@ type ModuleWithStem interface { } var _ ModuleWithStem = (*Module)(nil) + +type ModuleWithUsesLibrary interface { + UsesLibrary() *usesLibrary +} + +func (j *Module) UsesLibrary() *usesLibrary { + return &j.usesLibrary +} + +var _ ModuleWithUsesLibrary = (*Module)(nil) diff --git a/java/java.go b/java/java.go index ca99e6991..fb35a9a0e 100644 --- a/java/java.go +++ b/java/java.go @@ -966,8 +966,8 @@ func (j *Library) GenerateAndroidBuildActions(ctx android.ModuleContext) { } func (j *Library) DepsMutator(ctx android.BottomUpMutatorContext) { - j.deps(ctx) j.usesLibrary.deps(ctx, false) + j.deps(ctx) } const ( @@ -3162,13 +3162,35 @@ func addCLCFromDep(ctx android.ModuleContext, depModule android.Module, // <uses_library> and should not be added to CLC, but the transitive <uses-library> dependencies // from its CLC should be added to the current CLC. if sdkLib != nil { - clcMap.AddContext(ctx, dexpreopt.AnySdkVersion, *sdkLib, false, + optional := false + if module, ok := ctx.Module().(ModuleWithUsesLibrary); ok { + if android.InList(*sdkLib, module.UsesLibrary().usesLibraryProperties.Optional_uses_libs) { + optional = true + } + } + clcMap.AddContext(ctx, dexpreopt.AnySdkVersion, *sdkLib, optional, dep.DexJarBuildPath(ctx).PathOrNil(), dep.DexJarInstallPath(), dep.ClassLoaderContexts()) } else { clcMap.AddContextMap(dep.ClassLoaderContexts(), depName) } } +func addMissingOptionalUsesLibsFromDep(ctx android.ModuleContext, depModule android.Module, + usesLibrary *usesLibrary) { + + dep, ok := depModule.(ModuleWithUsesLibrary) + if !ok { + return + } + + for _, lib := range dep.UsesLibrary().usesLibraryProperties.Missing_optional_uses_libs { + if !android.InList(lib, usesLibrary.usesLibraryProperties.Missing_optional_uses_libs) { + usesLibrary.usesLibraryProperties.Missing_optional_uses_libs = + append(usesLibrary.usesLibraryProperties.Missing_optional_uses_libs, lib) + } + } +} + type JavaApiContributionImport struct { JavaApiContribution diff --git a/scripts/manifest_check.py b/scripts/manifest_check.py index c33b104bd..b10125994 100755 --- a/scripts/manifest_check.py +++ b/scripts/manifest_check.py @@ -52,6 +52,14 @@ def parse_args(): 'required:false' ) parser.add_argument( + '--missing-optional-uses-library', + dest='missing_optional_uses_libraries', + action='append', + help='specify uses-library entries missing from the build system with ' + 'required:false', + default=[] + ) + parser.add_argument( '--enforce-uses-libraries', dest='enforce_uses_libraries', action='store_true', @@ -91,7 +99,7 @@ C_OFF = "\033[0m" C_BOLD = "\033[1m" -def enforce_uses_libraries(manifest, required, optional, relax, is_apk, path): +def enforce_uses_libraries(manifest, required, optional, missing_optional, relax, is_apk, path): """Verify that the <uses-library> tags in the manifest match those provided by the build system. @@ -119,7 +127,12 @@ def enforce_uses_libraries(manifest, required, optional, relax, is_apk, path): required = trim_namespace_parts(required) optional = trim_namespace_parts(optional) - if manifest_required == required and manifest_optional == optional: + existing_manifest_optional = [ + lib for lib in manifest_optional if lib not in missing_optional] + + # The order of the existing libraries matter, while the order of the missing + # ones doesn't. + if manifest_required == required and existing_manifest_optional == optional: return None #pylint: disable=line-too-long @@ -129,6 +142,7 @@ def enforce_uses_libraries(manifest, required, optional, relax, is_apk, path): '\t- required libraries in build system: %s[%s]%s\n' % (C_RED, ', '.join(required), C_OFF), '\t vs. in the manifest: %s[%s]%s\n' % (C_RED, ', '.join(manifest_required), C_OFF), '\t- optional libraries in build system: %s[%s]%s\n' % (C_RED, ', '.join(optional), C_OFF), + '\t and missing ones in build system: %s[%s]%s\n' % (C_RED, ', '.join(missing_optional), C_OFF), '\t vs. in the manifest: %s[%s]%s\n' % (C_RED, ', '.join(manifest_optional), C_OFF), '\t- tags in the manifest (%s):\n' % path, '\t\t%s\n' % '\t\t'.join(tags), @@ -340,11 +354,14 @@ def main(): if args.enforce_uses_libraries: # Load dexpreopt.config files and build a mapping from module - # names to library names. This is necessary because build system - # addresses libraries by their module name (`uses_libs`, - # `optional_uses_libs`, `LOCAL_USES_LIBRARIES`, - # `LOCAL_OPTIONAL_LIBRARY_NAMES` all contain module names), while - # the manifest addresses libraries by their name. + # names to library names. This is for Make only and it's necessary + # because Make passes module names from `LOCAL_USES_LIBRARIES`, + # `LOCAL_OPTIONAL_LIBRARY_NAMES`, while the manifest addresses + # libraries by their name. Soong doesn't use it and doesn't need it + # because it converts the module names to the library names and + # passes the library names. There is no need to translate missing + # optional libs because they are missing and therefore there is no + # mapping for them. mod_to_lib = load_dexpreopt_configs(args.dexpreopt_configs) required = translate_libnames(args.uses_libraries, mod_to_lib) optional = translate_libnames(args.optional_uses_libraries, @@ -354,8 +371,8 @@ def main(): # those in the manifest. Raise an exception on mismatch, unless the # script was passed a special parameter to suppress exceptions. errmsg = enforce_uses_libraries(manifest, required, optional, - args.enforce_uses_libraries_relax, - is_apk, args.input) + args.missing_optional_uses_libraries, + args.enforce_uses_libraries_relax, is_apk, args.input) # Create a status file that is empty on success, or contains an # error message on failure. When exceptions are suppressed, diff --git a/scripts/manifest_check_test.py b/scripts/manifest_check_test.py index 3be7a30bf..8003b3e19 100755 --- a/scripts/manifest_check_test.py +++ b/scripts/manifest_check_test.py @@ -44,15 +44,17 @@ def required_apk(value): class EnforceUsesLibrariesTest(unittest.TestCase): """Unit tests for add_extract_native_libs function.""" - def run_test(self, xml, apk, uses_libraries=[], optional_uses_libraries=[]): #pylint: disable=dangerous-default-value + def run_test(self, xml, apk, uses_libraries=[], optional_uses_libraries=[], + missing_optional_uses_libraries=[]): #pylint: disable=dangerous-default-value doc = minidom.parseString(xml) try: relax = False manifest_check.enforce_uses_libraries( - doc, uses_libraries, optional_uses_libraries, relax, False, - 'path/to/X/AndroidManifest.xml') + doc, uses_libraries, optional_uses_libraries, missing_optional_uses_libraries, + relax, False, 'path/to/X/AndroidManifest.xml') manifest_check.enforce_uses_libraries(apk, uses_libraries, optional_uses_libraries, + missing_optional_uses_libraries, relax, True, 'path/to/X/X.apk') return True @@ -102,6 +104,15 @@ class EnforceUsesLibrariesTest(unittest.TestCase): matches = self.run_test(xml, apk, optional_uses_libraries=['foo']) self.assertFalse(matches) + def test_expected_missing_optional_uses_library(self): + xml = self.xml_tmpl % ( + uses_library_xml('foo') + uses_library_xml('missing') + uses_library_xml('bar')) + apk = self.apk_tmpl % ( + uses_library_apk('foo') + uses_library_apk('missing') + uses_library_apk('bar')) + matches = self.run_test(xml, apk, optional_uses_libraries=['foo', 'bar'], + missing_optional_uses_libraries=['missing']) + self.assertFalse(matches) + def test_missing_uses_library(self): xml = self.xml_tmpl % ('') apk = self.apk_tmpl % ('') |