From 0449a6337e01e05b62abc80da45151ec87202f08 Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Fri, 26 Jun 2020 10:12:36 -0700 Subject: [PATCH] Add relative_install_path property to prebuilt_etc This supports a more consistent property across modules for specifying a subdirectory to install a file into for prebuilt_etc modules. Updates bpfix to rewrite `sub_dir` to `relative_install_path`. Test: gotest prebuilt_etc_test Test: gotest bpfix_test Bug: 156568187 Change-Id: Idd05cd2178c46e290764a3b708faa8275818ca1e --- androidmk/androidmk/androidmk_test.go | 34 ++++++++--------- bpfix/bpfix/bpfix.go | 4 +- bpfix/bpfix/bpfix_test.go | 16 ++++++++ etc/prebuilt_etc.go | 16 ++++++-- etc/prebuilt_etc_test.go | 55 ++++++++++++++++++++++++++- 5 files changed, 103 insertions(+), 22 deletions(-) diff --git a/androidmk/androidmk/androidmk_test.go b/androidmk/androidmk/androidmk_test.go index 726746bdd..2448acc21 100644 --- a/androidmk/androidmk/androidmk_test.go +++ b/androidmk/androidmk/androidmk_test.go @@ -876,7 +876,7 @@ include $(BUILD_PREBUILT) prebuilt_etc { name: "etc.test1", src: "mymod", - sub_dir: "foo/bar", + relative_install_path: "foo/bar", } `, @@ -896,7 +896,7 @@ prebuilt_etc { name: "etc.test1", src: "etc.test1", - sub_dir: "foo/bar", + relative_install_path: "foo/bar", } `, @@ -913,7 +913,7 @@ include $(BUILD_PREBUILT) expected: ` prebuilt_etc { name: "etc.test1", - sub_dir: "foo/bar", + relative_install_path: "foo/bar", device_specific: true, } @@ -931,7 +931,7 @@ include $(BUILD_PREBUILT) expected: ` prebuilt_etc { name: "etc.test1", - sub_dir: "foo/bar", + relative_install_path: "foo/bar", product_specific: true, @@ -950,7 +950,7 @@ include $(BUILD_PREBUILT) expected: ` prebuilt_etc { name: "etc.test1", - sub_dir: "foo/bar", + relative_install_path: "foo/bar", product_specific: true, } @@ -968,7 +968,7 @@ include $(BUILD_PREBUILT) expected: ` prebuilt_etc { name: "etc.test1", - sub_dir: "foo/bar", + relative_install_path: "foo/bar", system_ext_specific: true, } @@ -986,7 +986,7 @@ include $(BUILD_PREBUILT) expected: ` prebuilt_etc { name: "etc.test1", - sub_dir: "foo/bar", + relative_install_path: "foo/bar", system_ext_specific: true, @@ -1005,7 +1005,7 @@ include $(BUILD_PREBUILT) expected: ` prebuilt_etc { name: "etc.test1", - sub_dir: "foo/bar", + relative_install_path: "foo/bar", proprietary: true, } @@ -1023,7 +1023,7 @@ include $(BUILD_PREBUILT) expected: ` prebuilt_etc { name: "etc.test1", - sub_dir: "foo/bar", + relative_install_path: "foo/bar", proprietary: true, } @@ -1041,7 +1041,7 @@ include $(BUILD_PREBUILT) expected: ` prebuilt_etc { name: "etc.test1", - sub_dir: "foo/bar", + relative_install_path: "foo/bar", proprietary: true, } @@ -1059,7 +1059,7 @@ include $(BUILD_PREBUILT) expected: ` prebuilt_etc { name: "etc.test1", - sub_dir: "foo/bar", + relative_install_path: "foo/bar", recovery: true, } @@ -1098,7 +1098,7 @@ prebuilt_usr_share { name: "foo", src: "foo.txt", - sub_dir: "bar", + relative_install_path: "bar", } `, }, @@ -1174,7 +1174,7 @@ prebuilt_usr_share_host { name: "foo", src: "foo.txt", - sub_dir: "bar", + relative_install_path: "bar", } `, }, @@ -1193,7 +1193,7 @@ prebuilt_firmware { name: "foo", src: "foo.fw", - sub_dir: "bar", + relative_install_path: "bar", } `, }, @@ -1212,7 +1212,7 @@ prebuilt_firmware { name: "foo", src: "foo.fw", - sub_dir: "bar", + relative_install_path: "bar", } `, }, @@ -1231,7 +1231,7 @@ prebuilt_firmware { name: "foo", src: "foo.fw", - sub_dir: "bar", + relative_install_path: "bar", proprietary: true, } `, @@ -1251,7 +1251,7 @@ prebuilt_firmware { name: "foo", src: "foo.fw", - sub_dir: "bar", + relative_install_path: "bar", proprietary: true, } `, diff --git a/bpfix/bpfix/bpfix.go b/bpfix/bpfix/bpfix.go index e731750fb..689cbd15b 100644 --- a/bpfix/bpfix/bpfix.go +++ b/bpfix/bpfix/bpfix.go @@ -533,7 +533,7 @@ func (f etcPrebuiltModuleUpdate) update(m *parser.Module, path string) bool { updated = true } else if trimmedPath := strings.TrimPrefix(path, f.prefix+"/"); trimmedPath != path { m.Properties = append(m.Properties, &parser.Property{ - Name: "sub_dir", + Name: "relative_install_path", Value: &parser.String{Value: trimmedPath}, }) updated = true @@ -581,6 +581,8 @@ func rewriteAndroidmkPrebuiltEtc(f *Fixer) error { // 'srcs' --> 'src' conversion convertToSingleSource(mod, "src") + renameProperty(mod, "sub_dir", "relative_install_dir") + // The rewriter converts LOCAL_MODULE_PATH attribute into a struct attribute // 'local_module_path'. Analyze its contents and create the correct sub_dir:, // filename: and boolean attributes combination diff --git a/bpfix/bpfix/bpfix_test.go b/bpfix/bpfix/bpfix_test.go index 64a7b93b2..8988177b1 100644 --- a/bpfix/bpfix/bpfix_test.go +++ b/bpfix/bpfix/bpfix_test.go @@ -742,6 +742,22 @@ func TestRewritePrebuiltEtc(t *testing.T) { } `, }, + { + name: "prebuilt_etc sub_dir", + in: ` + prebuilt_etc { + name: "foo", + src: "bar", + sub_dir: "baz", + } + `, + out: `prebuilt_etc { + name: "foo", + src: "bar", + relative_install_dir: "baz", + } + `, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { diff --git a/etc/prebuilt_etc.go b/etc/prebuilt_etc.go index df6d79d76..0f7b8dffc 100644 --- a/etc/prebuilt_etc.go +++ b/etc/prebuilt_etc.go @@ -42,9 +42,12 @@ type prebuiltEtcProperties struct { // Source file of this prebuilt. Src *string `android:"path,arch_variant"` - // optional subdirectory under which this file is installed into + // optional subdirectory under which this file is installed into, cannot be specified with relative_install_path, prefer relative_install_path Sub_dir *string `android:"arch_variant"` + // optional subdirectory under which this file is installed into, cannot be specified with sub_dir + Relative_install_path *string `android:"arch_variant"` + // optional name for the installed file. If unspecified, name of the module is used as the file name Filename *string `android:"arch_variant"` @@ -158,7 +161,10 @@ func (p *PrebuiltEtc) OutputFile() android.OutputPath { } func (p *PrebuiltEtc) SubDir() string { - return android.String(p.properties.Sub_dir) + if subDir := proptools.String(p.properties.Sub_dir); subDir != "" { + return subDir + } + return proptools.String(p.properties.Relative_install_path) } func (p *PrebuiltEtc) Installable() bool { @@ -181,13 +187,17 @@ func (p *PrebuiltEtc) GenerateAndroidBuildActions(ctx android.ModuleContext) { } p.outputFilePath = android.PathForModuleOut(ctx, filename).OutputPath + if p.properties.Sub_dir != nil && p.properties.Relative_install_path != nil { + ctx.PropertyErrorf("sub_dir", "relative_install_path is set. Cannot set sub_dir") + } + // If soc install dir was specified and SOC specific is set, set the installDirPath to the specified // socInstallDirBase. installBaseDir := p.installDirBase if ctx.SocSpecific() && p.socInstallDirBase != "" { installBaseDir = p.socInstallDirBase } - p.installDirPath = android.PathForModuleInstall(ctx, installBaseDir, proptools.String(p.properties.Sub_dir)) + p.installDirPath = android.PathForModuleInstall(ctx, installBaseDir, p.SubDir()) // This ensures that outputFilePath has the correct name for others to // use, as the source file may have a different name. diff --git a/etc/prebuilt_etc_test.go b/etc/prebuilt_etc_test.go index 4ce1984c0..8fc36c274 100644 --- a/etc/prebuilt_etc_test.go +++ b/etc/prebuilt_etc_test.go @@ -49,7 +49,7 @@ func TestMain(m *testing.M) { os.Exit(run()) } -func testPrebuiltEtc(t *testing.T, bp string) (*android.TestContext, android.Config) { +func testPrebuiltEtcContext(t *testing.T, bp string) (*android.TestContext, android.Config) { fs := map[string][]byte{ "foo.conf": nil, "bar.conf": nil, @@ -67,6 +67,14 @@ func testPrebuiltEtc(t *testing.T, bp string) (*android.TestContext, android.Con ctx.RegisterModuleType("prebuilt_firmware", PrebuiltFirmwareFactory) ctx.RegisterModuleType("prebuilt_dsp", PrebuiltDSPFactory) ctx.Register(config) + + return ctx, config +} + +func testPrebuiltEtc(t *testing.T, bp string) (*android.TestContext, android.Config) { + t.Helper() + + ctx, config := testPrebuiltEtcContext(t, bp) _, errs := ctx.ParseFileList(".", []string{"Android.bp"}) android.FailIfErrored(t, errs) _, errs = ctx.PrepareBuildActions(config) @@ -75,6 +83,24 @@ func testPrebuiltEtc(t *testing.T, bp string) (*android.TestContext, android.Con return ctx, config } +func testPrebuiltEtcError(t *testing.T, pattern, bp string) { + t.Helper() + + ctx, config := testPrebuiltEtcContext(t, bp) + _, errs := ctx.ParseFileList(".", []string{"Android.bp"}) + if len(errs) > 0 { + android.FailIfNoMatchingErrors(t, pattern, errs) + return + } + + _, errs = ctx.PrepareBuildActions(config) + if len(errs) > 0 { + android.FailIfNoMatchingErrors(t, pattern, errs) + return + } + + t.Fatalf("missing expected error %q (0 errors are returned)", pattern) +} func TestPrebuiltEtcVariants(t *testing.T) { ctx, _ := testPrebuiltEtc(t, ` prebuilt_etc { @@ -184,6 +210,33 @@ func TestPrebuiltEtcAndroidMk(t *testing.T) { } } +func TestPrebuiltEtcRelativeInstallPathInstallDirPath(t *testing.T) { + ctx, _ := testPrebuiltEtc(t, ` + prebuilt_etc { + name: "foo.conf", + src: "foo.conf", + relative_install_path: "bar", + } + `) + + p := ctx.ModuleForTests("foo.conf", "android_arm64_armv8-a").Module().(*PrebuiltEtc) + expected := buildDir + "/target/product/test_device/system/etc/bar" + if p.installDirPath.String() != expected { + t.Errorf("expected %q, got %q", expected, p.installDirPath.String()) + } +} + +func TestPrebuiltEtcCannotSetRelativeInstallPathAndSubDir(t *testing.T) { + testPrebuiltEtcError(t, "relative_install_path is set. Cannot set sub_dir", ` + prebuilt_etc { + name: "foo.conf", + src: "foo.conf", + sub_dir: "bar", + relative_install_path: "bar", + } + `) +} + func TestPrebuiltEtcHost(t *testing.T) { ctx, _ := testPrebuiltEtc(t, ` prebuilt_etc_host {