From cded5ecfc3e51052cd8353c8867f2bdcd0535756 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Fri, 10 Jan 2020 17:30:36 +0000 Subject: [PATCH 1/5] Document java_system_modules Bug: 142940300 Test: m nothing Change-Id: Ic658226088615dbeeef15f17dbb095a968e0742d --- java/system_modules.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/java/system_modules.go b/java/system_modules.go index ed2fc1856..81e5cb462 100644 --- a/java/system_modules.go +++ b/java/system_modules.go @@ -92,6 +92,9 @@ func TransformJarsToSystemModules(ctx android.ModuleContext, jars android.Paths) return outDir, outputs.Paths() } +// java_system_modules creates a system module from a set of java libraries that can +// be referenced from the system_modules property. It must contain at a minimum the +// java.base module which must include classes from java.lang amongst other java packages. func SystemModulesFactory() android.Module { module := &SystemModules{} module.AddProperties(&module.properties) From 90169baf0ec21c57cb89132c91a4119e12de51df Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Fri, 10 Jan 2020 17:16:44 +0000 Subject: [PATCH 2/5] Added java_system_modules_import A prebuilt version of java_system_modules. It does not import the generated system module, it generates the system module from imported java libraries in the same way that java_system_modules does. It just acts as a prebuilt, i.e. can have the same base name as another module type and the one to use is selected at runtime. Bug: 142940300 Test: m nothing Change-Id: I126db49d18294fcd6e2b7ad0237f83e9c2fdef7a --- java/java_test.go | 30 ++++++++++++++++++++++++++++++ java/system_modules.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/java/java_test.go b/java/java_test.go index 5791619c1..a2788cb8e 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -1261,3 +1261,33 @@ func TestJavaSystemModules(t *testing.T) { } } } + +func TestJavaSystemModulesImport(t *testing.T) { + ctx, _ := testJava(t, ` + java_system_modules_import { + name: "system-modules", + libs: ["system-module1", "system-module2"], + } + java_import { + name: "system-module1", + jars: ["a.jar"], + } + java_import { + name: "system-module2", + jars: ["b.jar"], + } + `) + + // check the existence of the module + systemModules := ctx.ModuleForTests("system-modules", "android_common") + + cmd := systemModules.Rule("jarsTosystemModules") + + // make sure the command compiles against the supplied modules. + for _, module := range []string{"system-module1.jar", "system-module2.jar"} { + if !strings.Contains(cmd.Args["classpath"], module) { + t.Errorf("system modules classpath %v does not contain %q", cmd.Args["classpath"], + module) + } + } +} diff --git a/java/system_modules.go b/java/system_modules.go index 81e5cb462..92297c416 100644 --- a/java/system_modules.go +++ b/java/system_modules.go @@ -35,6 +35,7 @@ func init() { func RegisterSystemModulesBuildComponents(ctx android.RegistrationContext) { ctx.RegisterModuleType("java_system_modules", SystemModulesFactory) + ctx.RegisterModuleType("java_system_modules_import", systemModulesImportFactory) } var ( @@ -160,3 +161,30 @@ func (system *SystemModules) AndroidMk() android.AndroidMkData { }, } } + +// A prebuilt version of java_system_modules. It does not import the +// generated system module, it generates the system module from imported +// java libraries in the same way that java_system_modules does. It just +// acts as a prebuilt, i.e. can have the same base name as another module +// type and the one to use is selected at runtime. +func systemModulesImportFactory() android.Module { + module := &systemModulesImport{} + module.AddProperties(&module.properties) + android.InitPrebuiltModule(module, &module.properties.Libs) + android.InitAndroidArchModule(module, android.HostAndDeviceSupported, android.MultilibCommon) + android.InitDefaultableModule(module) + return module +} + +type systemModulesImport struct { + SystemModules + prebuilt android.Prebuilt +} + +func (system *systemModulesImport) Name() string { + return system.prebuilt.Name(system.ModuleBase.Name()) +} + +func (system *systemModulesImport) Prebuilt() *android.Prebuilt { + return &system.prebuilt +} From a80ef84652eda6dbb14a0bec358e7213e3b2dd09 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Tue, 14 Jan 2020 12:09:36 +0000 Subject: [PATCH 3/5] Support registering hard coded pre arch mutators Some pre arch mutators are hard coded into mutator.go and so could not share code for registering those mutators between tests and runtime. This change adds a new HardCodedPreArchMutators(RegisterMutatorFunc) method to RegistrationContext which allows hard coded mutators to be registered alongside other build components during init() and testing. The method is treated as a noop on the InitRegistrationContext and behaves just like the PreArchMutators(RegisterMutatorFunc) method on the TestingContext. Bug: 146540677 Test: m nothing Change-Id: I6f8b1e2d54d9dc4e86f951ced61d1ee7b0fe4b2e --- android/register.go | 10 ++++++++++ android/testing.go | 5 +++++ 2 files changed, 15 insertions(+) diff --git a/android/register.go b/android/register.go index b48d3d1bc..d5aa1a537 100644 --- a/android/register.go +++ b/android/register.go @@ -127,6 +127,12 @@ type RegistrationContext interface { RegisterModuleType(name string, factory ModuleFactory) RegisterSingletonType(name string, factory SingletonFactory) PreArchMutators(f RegisterMutatorFunc) + + // Register pre arch mutators that are hard coded into mutator.go. + // + // Only registers mutators for testing, is a noop on the InitRegistrationContext. + HardCodedPreArchMutators(f RegisterMutatorFunc) + PreDepsMutators(f RegisterMutatorFunc) PostDepsMutators(f RegisterMutatorFunc) } @@ -180,6 +186,10 @@ func (ctx *initRegistrationContext) PreArchMutators(f RegisterMutatorFunc) { PreArchMutators(f) } +func (ctx *initRegistrationContext) HardCodedPreArchMutators(f RegisterMutatorFunc) { + // Nothing to do as the mutators are hard code in preArch in mutator.go +} + func (ctx *initRegistrationContext) PreDepsMutators(f RegisterMutatorFunc) { PreDepsMutators(f) } diff --git a/android/testing.go b/android/testing.go index 6663728e3..c07af7f32 100644 --- a/android/testing.go +++ b/android/testing.go @@ -59,6 +59,11 @@ func (ctx *TestContext) PreArchMutators(f RegisterMutatorFunc) { ctx.preArch = append(ctx.preArch, f) } +func (ctx *TestContext) HardCodedPreArchMutators(f RegisterMutatorFunc) { + // Register mutator function as normal for testing. + ctx.PreArchMutators(f) +} + func (ctx *TestContext) PreDepsMutators(f RegisterMutatorFunc) { ctx.preDeps = append(ctx.preDeps, f) } From c132742c963ed5501c82deb09a7e7c624dbafd44 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Tue, 14 Jan 2020 12:15:29 +0000 Subject: [PATCH 4/5] Dedup package build components registration Bug: 146540677 Test: m nothing Change-Id: Iff2d7063b7f06313e9068c61a5627229463c98dd --- android/package.go | 14 +++++++++++++- android/package_test.go | 3 +-- android/visibility_test.go | 3 +-- sdk/testing.go | 4 +--- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/android/package.go b/android/package.go index ed604c606..077c4a464 100644 --- a/android/package.go +++ b/android/package.go @@ -22,7 +22,19 @@ import ( ) func init() { - RegisterModuleType("package", PackageFactory) + RegisterPackageBuildComponents(InitRegistrationContext) +} + +// Register the package module type and supporting mutators. +// +// This must be called in the correct order (relative to other methods that also +// register mutators) to match the order of mutator registration in mutator.go. +// Failing to do so will result in an unrealistic test environment. +func RegisterPackageBuildComponents(ctx RegistrationContext) { + ctx.RegisterModuleType("package", PackageFactory) + + // Register mutators that are hard coded in to mutator.go. + ctx.HardCodedPreArchMutators(RegisterPackageRenamer) } // The information maintained about each package. diff --git a/android/package_test.go b/android/package_test.go index bc66928ce..4710e3954 100644 --- a/android/package_test.go +++ b/android/package_test.go @@ -87,8 +87,7 @@ func testPackage(fs map[string][]byte) (*TestContext, []error) { config := TestArchConfig(buildDir, nil, "", fs) ctx := NewTestArchContext() - ctx.RegisterModuleType("package", PackageFactory) - ctx.PreArchMutators(RegisterPackageRenamer) + RegisterPackageBuildComponents(ctx) ctx.Register(config) _, errs := ctx.ParseBlueprintsFiles(".") diff --git a/android/visibility_test.go b/android/visibility_test.go index fbf2fb72d..c74271a90 100644 --- a/android/visibility_test.go +++ b/android/visibility_test.go @@ -871,10 +871,9 @@ func testVisibility(buildDir string, fs map[string][]byte) (*TestContext, []erro config := TestArchConfig(buildDir, nil, "", fs) ctx := NewTestArchContext() - ctx.RegisterModuleType("package", PackageFactory) ctx.RegisterModuleType("mock_library", newMockLibraryModule) ctx.RegisterModuleType("mock_defaults", defaultsFactory) - ctx.PreArchMutators(RegisterPackageRenamer) + RegisterPackageBuildComponents(ctx) ctx.PreArchMutators(RegisterVisibilityRuleChecker) ctx.PreArchMutators(RegisterDefaultsPreArchMutators) ctx.PreArchMutators(RegisterVisibilityRuleGatherer) diff --git a/sdk/testing.go b/sdk/testing.go index 809788912..c9cc30f1c 100644 --- a/sdk/testing.go +++ b/sdk/testing.go @@ -62,14 +62,12 @@ func testSdkContext(bp string, fs map[string][]byte) (*android.TestContext, andr ctx := android.NewTestArchContext() // from android package - ctx.PreArchMutators(android.RegisterPackageRenamer) + android.RegisterPackageBuildComponents(ctx) ctx.PreArchMutators(android.RegisterVisibilityRuleChecker) ctx.PreArchMutators(android.RegisterDefaultsPreArchMutators) ctx.PreArchMutators(android.RegisterVisibilityRuleGatherer) ctx.PostDepsMutators(android.RegisterVisibilityRuleEnforcer) - ctx.RegisterModuleType("package", android.PackageFactory) - // from java package java.RegisterJavaBuildComponents(ctx) java.RegisterAppBuildComponents(ctx) From 78ac5b962e18e96ce1fb6c2619ed2f12faeb9dad Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Tue, 14 Jan 2020 12:42:08 +0000 Subject: [PATCH 5/5] Exclude source->prebuilt deps from visibility enforcement When both prebuilt and source versions of a module are present in the build an implicit dependency is added from source -> prebuilt. This change excludes that dependency from visibility enforcement as it provides no value and would otherwise require an sdk snapshot to rewrite the visibility of the generated prebuilt modules which would be complicated. The rewriting would include: * Detecting //visibility:public and just passing that straight through. * Detecting //visibility:private and replacing that with the location of the source. * Otherwise, adding the location of the source to the visibility property. This adds a general mechanism to allow any dependency to be excluded from visibility enforcement by simply using a tag that implements the ExcludeFromVisibilityEnforcementTag interface. Bug: 142940300 Test: m nothing Change-Id: I0668ff5aa798152d17faf3aac1bb8eff8d6350c3 --- android/prebuilt.go | 3 +++ android/prebuilt_test.go | 11 ++++++--- android/visibility.go | 17 ++++++++++++++ android/visibility_test.go | 48 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 76 insertions(+), 3 deletions(-) diff --git a/android/prebuilt.go b/android/prebuilt.go index 2c99f1f7a..c780cb24f 100644 --- a/android/prebuilt.go +++ b/android/prebuilt.go @@ -36,6 +36,9 @@ type prebuiltDependencyTag struct { var PrebuiltDepTag prebuiltDependencyTag +// Mark this tag so dependencies that use it are excluded from visibility enforcement. +func (t prebuiltDependencyTag) ExcludeFromVisibilityEnforcement() {} + type PrebuiltProperties struct { // When prefer is set to true the prebuilt will be used instead of any source module with // a matching name. diff --git a/android/prebuilt_test.go b/android/prebuilt_test.go index 8648b93e2..8ff5c4034 100644 --- a/android/prebuilt_test.go +++ b/android/prebuilt_test.go @@ -141,10 +141,8 @@ func TestPrebuilts(t *testing.T) { config := TestConfig(buildDir, nil, bp, fs) ctx := NewTestContext() - RegisterPrebuiltMutators(ctx) + registerTestPrebuiltBuildComponents(ctx) ctx.RegisterModuleType("filegroup", FileGroupFactory) - ctx.RegisterModuleType("prebuilt", newPrebuiltModule) - ctx.RegisterModuleType("source", newSourceModule) ctx.Register(config) _, errs := ctx.ParseBlueprintsFiles("Android.bp") @@ -212,6 +210,13 @@ func TestPrebuilts(t *testing.T) { } } +func registerTestPrebuiltBuildComponents(ctx RegistrationContext) { + ctx.RegisterModuleType("prebuilt", newPrebuiltModule) + ctx.RegisterModuleType("source", newSourceModule) + + RegisterPrebuiltMutators(ctx) +} + type prebuiltModule struct { ModuleBase prebuilt Prebuilt diff --git a/android/visibility.go b/android/visibility.go index c28ec93f4..a597687c0 100644 --- a/android/visibility.go +++ b/android/visibility.go @@ -19,6 +19,8 @@ import ( "regexp" "strings" "sync" + + "github.com/google/blueprint" ) // Enforces visibility rules between modules. @@ -190,6 +192,15 @@ func moduleToVisibilityRuleMap(ctx BaseModuleContext) *sync.Map { }).(*sync.Map) } +// Marker interface that identifies dependencies that are excluded from visibility +// enforcement. +type ExcludeFromVisibilityEnforcementTag interface { + blueprint.DependencyTag + + // Method that differentiates this interface from others. + ExcludeFromVisibilityEnforcement() +} + // The rule checker needs to be registered before defaults expansion to correctly check that // //visibility:xxx isn't combined with other packages in the same list in any one module. func RegisterVisibilityRuleChecker(ctx RegisterMutatorsContext) { @@ -389,6 +400,12 @@ func visibilityRuleEnforcer(ctx TopDownMutatorContext) { // Visit all the dependencies making sure that this module has access to them all. ctx.VisitDirectDeps(func(dep Module) { + // Ignore dependencies that have an ExcludeFromVisibilityEnforcementTag + tag := ctx.OtherModuleDependencyTag(dep) + if _, ok := tag.(ExcludeFromVisibilityEnforcementTag); ok { + return + } + depName := ctx.OtherModuleName(dep) depDir := ctx.OtherModuleDir(dep) depQualified := qualifiedModuleName{depDir, depName} diff --git a/android/visibility_test.go b/android/visibility_test.go index c74271a90..6006072da 100644 --- a/android/visibility_test.go +++ b/android/visibility_test.go @@ -853,6 +853,51 @@ var visibilityTests = []struct { ` not visible to this module`, }, }, + { + name: "verify that prebuilt dependencies are ignored for visibility reasons (not preferred)", + fs: map[string][]byte{ + "prebuilts/Blueprints": []byte(` + prebuilt { + name: "module", + visibility: ["//top/other"], + }`), + "top/sources/source_file": nil, + "top/sources/Blueprints": []byte(` + source { + name: "module", + visibility: ["//top/other"], + }`), + "top/other/source_file": nil, + "top/other/Blueprints": []byte(` + source { + name: "other", + deps: [":module"], + }`), + }, + }, + { + name: "verify that prebuilt dependencies are ignored for visibility reasons (preferred)", + fs: map[string][]byte{ + "prebuilts/Blueprints": []byte(` + prebuilt { + name: "module", + visibility: ["//top/other"], + prefer: true, + }`), + "top/sources/source_file": nil, + "top/sources/Blueprints": []byte(` + source { + name: "module", + visibility: ["//top/other"], + }`), + "top/other/source_file": nil, + "top/other/Blueprints": []byte(` + source { + name: "other", + deps: [":module"], + }`), + }, + }, } func TestVisibility(t *testing.T) { @@ -873,7 +918,10 @@ func testVisibility(buildDir string, fs map[string][]byte) (*TestContext, []erro ctx := NewTestArchContext() ctx.RegisterModuleType("mock_library", newMockLibraryModule) ctx.RegisterModuleType("mock_defaults", defaultsFactory) + + // Order of the following method calls is significant. RegisterPackageBuildComponents(ctx) + registerTestPrebuiltBuildComponents(ctx) ctx.PreArchMutators(RegisterVisibilityRuleChecker) ctx.PreArchMutators(RegisterDefaultsPreArchMutators) ctx.PreArchMutators(RegisterVisibilityRuleGatherer)