From 3ea9b659905b72332885a4d55527f8ee4bbc1326 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Wed, 15 May 2024 23:01:54 +0900 Subject: [PATCH] filesystem modules gathers first target only With this change, the deps property in filesystem modules gather the first target of the filesystem module only. To gather dependencies across both targets, use multilib.both.deps instead. Bug: N/A Test: go test ./... Change-Id: Ie2ff0c48f08c61c8b219fc2c1540476ff8e4b1fc --- android/packaging.go | 32 +++++- android/packaging_test.go | 201 ++++++++++++++++++++++++++++------ filesystem/filesystem.go | 1 + filesystem/filesystem_test.go | 6 +- 4 files changed, 199 insertions(+), 41 deletions(-) diff --git a/android/packaging.go b/android/packaging.go index 383f828bd..080dcfea0 100644 --- a/android/packaging.go +++ b/android/packaging.go @@ -142,6 +142,10 @@ type PackagingBase struct { // for rare cases like when there's a dependency to a module which exists in certain repo // checkouts, this is needed. IgnoreMissingDependencies bool + + // If this is set to true by a module type inheriting PackagingBase, the deps property + // collects the first target only even with compile_multilib: true. + DepsCollectFirstTargetOnly bool } type depsProperty struct { @@ -154,6 +158,7 @@ type packagingMultilibProperties struct { Common depsProperty `android:"arch_variant"` Lib32 depsProperty `android:"arch_variant"` Lib64 depsProperty `android:"arch_variant"` + Both depsProperty `android:"arch_variant"` } type packagingArchProperties struct { @@ -194,11 +199,28 @@ func (p *PackagingBase) getDepsForArch(ctx BaseModuleContext, arch ArchType) []s ret = append(ret, p.properties.Multilib.Common.Deps...) } - for i, t := range ctx.MultiTargets() { - if t.Arch.ArchType == arch { - ret = append(ret, p.properties.Deps...) - if i == 0 { - ret = append(ret, p.properties.Multilib.First.Deps...) + if p.DepsCollectFirstTargetOnly { + if len(p.properties.Multilib.First.Deps) > 0 { + ctx.PropertyErrorf("multilib.first.deps", "not supported. use \"deps\" instead") + } + for i, t := range ctx.MultiTargets() { + if t.Arch.ArchType == arch { + ret = append(ret, p.properties.Multilib.Both.Deps...) + if i == 0 { + ret = append(ret, p.properties.Deps...) + } + } + } + } else { + if len(p.properties.Multilib.Both.Deps) > 0 { + ctx.PropertyErrorf("multilib.both.deps", "not supported. use \"deps\" instead") + } + for i, t := range ctx.MultiTargets() { + if t.Arch.ArchType == arch { + ret = append(ret, p.properties.Deps...) + if i == 0 { + ret = append(ret, p.properties.Multilib.First.Deps...) + } } } } diff --git a/android/packaging_test.go b/android/packaging_test.go index 383343723..f99bb91cd 100644 --- a/android/packaging_test.go +++ b/android/packaging_test.go @@ -67,18 +67,15 @@ type packageTestModule struct { entries []string } -func packageMultiTargetTestModuleFactory() Module { +func packageTestModuleFactory(multiTarget bool, depsCollectFirstTargetOnly bool) Module { module := &packageTestModule{} InitPackageModule(module) - InitAndroidMultiTargetsArchModule(module, DeviceSupported, MultilibCommon) - module.AddProperties(&module.properties) - return module -} - -func packageTestModuleFactory() Module { - module := &packageTestModule{} - InitPackageModule(module) - InitAndroidArchModule(module, DeviceSupported, MultilibBoth) + module.DepsCollectFirstTargetOnly = depsCollectFirstTargetOnly + if multiTarget { + InitAndroidMultiTargetsArchModule(module, DeviceSupported, MultilibCommon) + } else { + InitAndroidArchModule(module, DeviceSupported, MultilibBoth) + } module.AddProperties(&module.properties) return module } @@ -98,17 +95,23 @@ func (m *packageTestModule) GenerateAndroidBuildActions(ctx ModuleContext) { m.entries = m.CopyDepsToZip(ctx, m.GatherPackagingSpecs(ctx), zipFile) } -func runPackagingTest(t *testing.T, multitarget bool, bp string, expected []string) { +type packageTestModuleConfig struct { + multiTarget bool + depsCollectFirstTargetOnly bool +} + +func runPackagingTest(t *testing.T, config packageTestModuleConfig, bp string, expected []string) { t.Helper() var archVariant string - var moduleFactory ModuleFactory - if multitarget { + if config.multiTarget { archVariant = "android_common" - moduleFactory = packageMultiTargetTestModuleFactory } else { archVariant = "android_arm64_armv8-a" - moduleFactory = packageTestModuleFactory + } + + moduleFactory := func() Module { + return packageTestModuleFactory(config.multiTarget, config.depsCollectFirstTargetOnly) } result := GroupFixturePreparers( @@ -128,8 +131,11 @@ func runPackagingTest(t *testing.T, multitarget bool, bp string, expected []stri } func TestPackagingBaseMultiTarget(t *testing.T) { - multiTarget := true - runPackagingTest(t, multiTarget, + config := packageTestModuleConfig{ + multiTarget: true, + depsCollectFirstTargetOnly: false, + } + runPackagingTest(t, config, ` component { name: "foo", @@ -141,7 +147,7 @@ func TestPackagingBaseMultiTarget(t *testing.T) { } `, []string{"lib64/foo"}) - runPackagingTest(t, multiTarget, + runPackagingTest(t, config, ` component { name: "foo", @@ -158,7 +164,7 @@ func TestPackagingBaseMultiTarget(t *testing.T) { } `, []string{"lib64/foo", "lib64/bar"}) - runPackagingTest(t, multiTarget, + runPackagingTest(t, config, ` component { name: "foo", @@ -176,7 +182,7 @@ func TestPackagingBaseMultiTarget(t *testing.T) { } `, []string{"lib32/foo", "lib32/bar", "lib64/foo", "lib64/bar"}) - runPackagingTest(t, multiTarget, + runPackagingTest(t, config, ` component { name: "foo", @@ -199,7 +205,7 @@ func TestPackagingBaseMultiTarget(t *testing.T) { } `, []string{"lib32/foo", "lib32/bar", "lib64/foo"}) - runPackagingTest(t, multiTarget, + runPackagingTest(t, config, ` component { name: "foo", @@ -221,7 +227,7 @@ func TestPackagingBaseMultiTarget(t *testing.T) { } `, []string{"lib32/foo", "lib64/foo", "lib64/bar"}) - runPackagingTest(t, multiTarget, + runPackagingTest(t, config, ` component { name: "foo", @@ -252,8 +258,11 @@ func TestPackagingBaseMultiTarget(t *testing.T) { } func TestPackagingBaseSingleTarget(t *testing.T) { - multiTarget := false - runPackagingTest(t, multiTarget, + config := packageTestModuleConfig{ + multiTarget: false, + depsCollectFirstTargetOnly: false, + } + runPackagingTest(t, config, ` component { name: "foo", @@ -265,7 +274,7 @@ func TestPackagingBaseSingleTarget(t *testing.T) { } `, []string{"lib64/foo"}) - runPackagingTest(t, multiTarget, + runPackagingTest(t, config, ` component { name: "foo", @@ -282,7 +291,7 @@ func TestPackagingBaseSingleTarget(t *testing.T) { } `, []string{"lib64/foo", "lib64/bar"}) - runPackagingTest(t, multiTarget, + runPackagingTest(t, config, ` component { name: "foo", @@ -304,7 +313,7 @@ func TestPackagingBaseSingleTarget(t *testing.T) { } `, []string{"lib64/foo"}) - runPackagingTest(t, multiTarget, + runPackagingTest(t, config, ` component { name: "foo", @@ -325,7 +334,7 @@ func TestPackagingBaseSingleTarget(t *testing.T) { } `, []string{"lib64/foo", "lib64/bar"}) - runPackagingTest(t, multiTarget, + runPackagingTest(t, config, ` component { name: "foo", @@ -353,7 +362,7 @@ func TestPackagingBaseSingleTarget(t *testing.T) { } `, []string{"lib64/foo", "lib64/bar"}) - runPackagingTest(t, multiTarget, + runPackagingTest(t, config, ` component { name: "foo", @@ -374,8 +383,11 @@ func TestPackagingBaseSingleTarget(t *testing.T) { func TestPackagingWithSkipInstallDeps(t *testing.T) { // package -[dep]-> foo -[dep]-> bar -[dep]-> baz // Packaging should continue transitively through modules that are not installed. - multiTarget := false - runPackagingTest(t, multiTarget, + config := packageTestModuleConfig{ + multiTarget: false, + depsCollectFirstTargetOnly: false, + } + runPackagingTest(t, config, ` component { name: "foo", @@ -398,3 +410,130 @@ func TestPackagingWithSkipInstallDeps(t *testing.T) { } `, []string{"lib64/foo", "lib64/bar", "lib64/baz"}) } + +func TestPackagingWithDepsCollectFirstTargetOnly(t *testing.T) { + config := packageTestModuleConfig{ + multiTarget: true, + depsCollectFirstTargetOnly: true, + } + runPackagingTest(t, config, + ` + component { + name: "foo", + } + + package_module { + name: "package", + deps: ["foo"], + } + `, []string{"lib64/foo"}) + + runPackagingTest(t, config, + ` + component { + name: "foo", + deps: ["bar"], + } + + component { + name: "bar", + } + + package_module { + name: "package", + deps: ["foo"], + } + `, []string{"lib64/foo", "lib64/bar"}) + + runPackagingTest(t, config, + ` + component { + name: "foo", + deps: ["bar"], + } + + component { + name: "bar", + } + + package_module { + name: "package", + deps: ["foo"], + compile_multilib: "both", + } + `, []string{"lib64/foo", "lib64/bar"}) + + runPackagingTest(t, config, + ` + component { + name: "foo", + } + + component { + name: "bar", + compile_multilib: "32", + } + + package_module { + name: "package", + deps: ["foo"], + multilib: { + lib32: { + deps: ["bar"], + }, + }, + compile_multilib: "both", + } + `, []string{"lib32/bar", "lib64/foo"}) + + runPackagingTest(t, config, + ` + component { + name: "foo", + } + + component { + name: "bar", + } + + package_module { + name: "package", + deps: ["foo"], + multilib: { + both: { + deps: ["bar"], + }, + }, + compile_multilib: "both", + } + `, []string{"lib64/foo", "lib32/bar", "lib64/bar"}) + + runPackagingTest(t, config, + ` + component { + name: "foo", + } + + component { + name: "bar", + } + + component { + name: "baz", + } + + package_module { + name: "package", + deps: ["foo"], + arch: { + arm64: { + deps: ["bar"], + }, + x86_64: { + deps: ["baz"], + }, + }, + compile_multilib: "both", + } + `, []string{"lib64/foo", "lib64/bar"}) +} diff --git a/filesystem/filesystem.go b/filesystem/filesystem.go index d8a00e2c9..8b71e85c7 100644 --- a/filesystem/filesystem.go +++ b/filesystem/filesystem.go @@ -152,6 +152,7 @@ func filesystemFactory() android.Module { func initFilesystemModule(module *filesystem) { module.AddProperties(&module.properties) android.InitPackageModule(module) + module.PackagingBase.DepsCollectFirstTargetOnly = true android.InitAndroidMultiTargetsArchModule(module, android.DeviceSupported, android.MultilibCommon) android.InitDefaultableModule(module) } diff --git a/filesystem/filesystem_test.go b/filesystem/filesystem_test.go index 015d39aab..2dc8c21e0 100644 --- a/filesystem/filesystem_test.go +++ b/filesystem/filesystem_test.go @@ -508,11 +508,7 @@ func TestFilterOutUnsupportedArches(t *testing.T) { android_filesystem { name: "fs_64_32", compile_multilib: "both", - multilib: { - first: { - deps: ["foo"], - }, - }, + deps: ["foo"], } cc_binary {