From 502da3987a488307ffab5b5b2cedc119892e40d6 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Tue, 28 Feb 2023 17:54:17 -0800 Subject: [PATCH] Export non-apex variants of modules to make Currently, non-apex variants of modules that are in apexes are not exported to make unless they're apex_available to the platform. This means that you can't `m` those modules directly. However, there is a workaround in the apex androidmk implementation that emits make rules for the removed modules, but just redirects them to build the apex itself. We want to remove that, but one of the problems with doing so is that you can no longer `m` many modules afterwards. To fix that, unhide the apex's dependencies from make. To ensure they're not installed, call SkipInstall() on them, and update SkipInstall() to be more strict by setting `LOCAL_UNINSTALLABLE_MODULE := true`. Bug: 254205429 Test: Presubmits Change-Id: Ib971981559f3b642ce6be8890679e994e1b44be0 --- android/androidmk.go | 2 ++ android/apex.go | 2 +- android/module.go | 11 +--------- android/packaging_test.go | 44 +++++++++++++++++++++++++++++++++++++-- cc/cc.go | 9 -------- cc/installer.go | 4 ---- cc/library.go | 11 ---------- cc/prebuilt.go | 2 +- java/androidmk.go | 6 ++---- 9 files changed, 49 insertions(+), 42 deletions(-) diff --git a/android/androidmk.go b/android/androidmk.go index aa411d116..a7b69f668 100644 --- a/android/androidmk.go +++ b/android/androidmk.go @@ -560,6 +560,8 @@ func (a *AndroidMkEntries) fillInEntries(ctx fillInEntriesContext, mod blueprint a.SetPaths("LOCAL_SOONG_INSTALL_SYMLINKS", base.katiSymlinks.InstallPaths().Paths()) } + a.SetBoolIfTrue("LOCAL_UNINSTALLABLE_MODULE", base.commonProperties.SkipInstall) + if am, ok := mod.(ApexModule); ok { a.SetBoolIfTrue("LOCAL_NOT_AVAILABLE_FOR_PLATFORM", am.NotAvailableForPlatform()) } diff --git a/android/apex.go b/android/apex.go index 358818f47..a0ac5b862 100644 --- a/android/apex.go +++ b/android/apex.go @@ -603,7 +603,7 @@ func CreateApexVariations(mctx BottomUpMutatorContext, module ApexModule) []Modu // Do not install the module for platform, but still allow it to output // uninstallable AndroidMk entries in certain cases when they have side // effects. TODO(jiyong): move this routine to somewhere else - mod.MakeUninstallable() + mod.SkipInstall() } if !platformVariation { mctx.SetVariationProvider(mod, ApexInfoProvider, apexInfos[i-1]) diff --git a/android/module.go b/android/module.go index 773d77be0..b45ed9530 100644 --- a/android/module.go +++ b/android/module.go @@ -505,8 +505,8 @@ type Module interface { PartitionTag(DeviceConfig) string HideFromMake() IsHideFromMake() bool + SkipInstall() IsSkipInstall() bool - MakeUninstallable() ReplacedByPrebuilt() IsReplacedByPrebuilt() bool ExportedToMake() bool @@ -1964,15 +1964,6 @@ func (m *ModuleBase) IsSkipInstall() bool { return m.commonProperties.SkipInstall } -// Similar to HideFromMake, but if the AndroidMk entry would set -// LOCAL_UNINSTALLABLE_MODULE then this variant may still output that entry -// rather than leaving it out altogether. That happens in cases where it would -// have other side effects, in particular when it adds a NOTICE file target, -// which other install targets might depend on. -func (m *ModuleBase) MakeUninstallable() { - m.HideFromMake() -} - func (m *ModuleBase) ReplacedByPrebuilt() { m.commonProperties.ReplacedByPrebuilt = true m.HideFromMake() diff --git a/android/packaging_test.go b/android/packaging_test.go index 91ac1f386..f32d1c396 100644 --- a/android/packaging_test.go +++ b/android/packaging_test.go @@ -15,6 +15,7 @@ package android import ( + "strings" "testing" "github.com/google/blueprint" @@ -28,6 +29,8 @@ type componentTestModule struct { Deps []string Skip_install *bool } + + builtFile Path } // dep tag used in this test. All dependencies are considered as installable. @@ -48,13 +51,21 @@ func (m *componentTestModule) DepsMutator(ctx BottomUpMutatorContext) { } func (m *componentTestModule) GenerateAndroidBuildActions(ctx ModuleContext) { - builtFile := PathForModuleOut(ctx, m.Name()) + m.builtFile = PathForModuleOut(ctx, m.Name()) dir := ctx.Target().Arch.ArchType.Multilib installDir := PathForModuleInstall(ctx, dir) if proptools.Bool(m.props.Skip_install) { m.SkipInstall() } - ctx.InstallFile(installDir, m.Name(), builtFile) + ctx.InstallFile(installDir, m.Name(), m.builtFile) +} + +func (m *componentTestModule) AndroidMkEntries() []AndroidMkEntries { + return []AndroidMkEntries{ + { + OutputFile: OptionalPathForPath(m.builtFile), + }, + } } // Module that itself is a package @@ -251,6 +262,35 @@ func TestPackagingBaseMultiTarget(t *testing.T) { `, []string{"lib32/foo", "lib64/foo", "lib64/bar"}) } +func TestSkipInstallProducesLocalUninstallableModule(t *testing.T) { + result := GroupFixturePreparers( + PrepareForTestWithArchMutator, + FixtureRegisterWithContext(func(ctx RegistrationContext) { + ctx.RegisterModuleType("component", componentTestModuleFactory) + ctx.RegisterModuleType("package_module", packageTestModuleFactory) + }), + FixtureWithRootAndroidBp(` +component { + name: "foo", + skip_install: true, +} + +package_module { + name: "package", + deps: ["foo"], +} +`), + ).RunTest(t) + module := result.ModuleForTests("foo", "android_arm64_armv8-a").Module().(*componentTestModule) + entries := AndroidMkEntriesForTest(t, result.TestContext, module) + builder := &strings.Builder{} + entries[0].write(builder) + androidMkString := builder.String() + if !strings.Contains(androidMkString, "LOCAL_UNINSTALLABLE_MODULE := true") { + t.Errorf("Expected android mk entries to contain \"LOCAL_UNINSTALLABLE_MODULE := true\", got: \n%s", androidMkString) + } +} + func TestPackagingBaseSingleTarget(t *testing.T) { multiTarget := false runPackagingTest(t, multiTarget, diff --git a/cc/cc.go b/cc/cc.go index 0e88c5686..e592cc5e8 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -609,7 +609,6 @@ type installer interface { inSanitizerDir() bool hostToolPath() android.OptionalPath relativeInstallPath() string - makeUninstallable(mod *Module) installInRoot() bool } @@ -3535,14 +3534,6 @@ func (c *Module) InstallInRecovery() bool { return c.InRecovery() } -func (c *Module) MakeUninstallable() { - if c.installer == nil { - c.ModuleBase.MakeUninstallable() - return - } - c.installer.makeUninstallable(c) -} - func (c *Module) HostToolPath() android.OptionalPath { if c.installer == nil { return android.OptionalPath{} diff --git a/cc/installer.go b/cc/installer.go index e2c0e7b8d..c3618b7c1 100644 --- a/cc/installer.go +++ b/cc/installer.go @@ -121,10 +121,6 @@ func (installer *baseInstaller) relativeInstallPath() string { return String(installer.Properties.Relative_install_path) } -func (installer *baseInstaller) makeUninstallable(mod *Module) { - mod.ModuleBase.MakeUninstallable() -} - func (installer *baseInstaller) installInRoot() bool { return Bool(installer.Properties.Install_in_root) } diff --git a/cc/library.go b/cc/library.go index 27f06230b..574c4c32a 100644 --- a/cc/library.go +++ b/cc/library.go @@ -2439,17 +2439,6 @@ func (library *libraryDecorator) installable() *bool { return nil } -func (library *libraryDecorator) makeUninstallable(mod *Module) { - if library.static() && library.buildStatic() && !library.buildStubs() { - // If we're asked to make a static library uninstallable we don't do - // anything since AndroidMkEntries always sets LOCAL_UNINSTALLABLE_MODULE - // for these entries. This is done to still get the make targets for NOTICE - // files from notice_files.mk, which other libraries might depend on. - return - } - mod.ModuleBase.MakeUninstallable() -} - func (library *libraryDecorator) getPartition() string { return library.path.Partition() } diff --git a/cc/prebuilt.go b/cc/prebuilt.go index 5b7ba4346..c237d751f 100644 --- a/cc/prebuilt.go +++ b/cc/prebuilt.go @@ -214,7 +214,7 @@ func (p *prebuiltLibraryLinker) link(ctx ModuleContext, // without the prefix hack below. if p.hasStubsVariants() && !p.buildStubs() && !ctx.Host() && !strings.HasPrefix(ctx.baseModuleName(), "libclang_rt.") { - ctx.Module().MakeUninstallable() + ctx.Module().SkipInstall() } return outputFile diff --git a/java/androidmk.go b/java/androidmk.go index a4dac80d4..d73ff4616 100644 --- a/java/androidmk.go +++ b/java/androidmk.go @@ -76,9 +76,6 @@ func (library *Library) AndroidMkEntries() []android.AndroidMkEntries { entriesList = append(entriesList, dexpreoptEntries...) } entriesList = append(entriesList, android.AndroidMkEntries{Disabled: true}) - } else if !library.ApexModuleBase.AvailableFor(android.AvailableToPlatform) { - // Platform variant. If not available for the platform, we don't need Make module. - entriesList = append(entriesList, android.AndroidMkEntries{Disabled: true}) } else { entriesList = append(entriesList, android.AndroidMkEntries{ Class: "JAVA_LIBRARIES", @@ -94,7 +91,8 @@ func (library *Library) AndroidMkEntries() []android.AndroidMkEntries { entries.AddStrings("LOCAL_LOGTAGS_FILES", logtags...) } - if library.installFile == nil { + if library.installFile == nil || !library.ApexModuleBase.AvailableFor(android.AvailableToPlatform) { + // If the ApexModule is not available for the platform, it shouldn't be installed. entries.SetBoolIfTrue("LOCAL_UNINSTALLABLE_MODULE", true) } if library.dexJarFile.IsSet() {