diff options
-rw-r--r-- | apex/apex.go | 20 | ||||
-rw-r--r-- | apex/apex_test.go | 57 | ||||
-rw-r--r-- | apex/builder.go | 35 | ||||
-rw-r--r-- | cc/config/tidy.go | 1 | ||||
-rw-r--r-- | cc/test.go | 14 | ||||
-rw-r--r-- | docs/tidy.md | 15 | ||||
-rw-r--r-- | java/base.go | 9 | ||||
-rw-r--r-- | java/kotlin_test.go | 15 |
8 files changed, 120 insertions, 46 deletions
diff --git a/apex/apex.go b/apex/apex.go index 73a3fc2c7..b3398156d 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -19,6 +19,7 @@ package apex import ( "fmt" "path/filepath" + "regexp" "sort" "strings" @@ -1653,7 +1654,20 @@ type androidApp interface { var _ androidApp = (*java.AndroidApp)(nil) var _ androidApp = (*java.AndroidAppImport)(nil) -const APEX_VERSION_PLACEHOLDER = "__APEX_VERSION_PLACEHOLDER__" +func sanitizedBuildIdForPath(ctx android.BaseModuleContext) string { + buildId := ctx.Config().BuildId() + + // The build ID is used as a suffix for a filename, so ensure that + // the set of characters being used are sanitized. + // - any word character: [a-zA-Z0-9_] + // - dots: . + // - dashes: - + validRegex := regexp.MustCompile(`^[\w\.\-\_]+$`) + if !validRegex.MatchString(buildId) { + ctx.ModuleErrorf("Unable to use build id %s as filename suffix, valid characters are [a-z A-Z 0-9 _ . -].", buildId) + } + return buildId +} func apexFileForAndroidApp(ctx android.BaseModuleContext, aapp androidApp) apexFile { appDir := "app" @@ -1664,7 +1678,7 @@ func apexFileForAndroidApp(ctx android.BaseModuleContext, aapp androidApp) apexF // TODO(b/224589412, b/226559955): Ensure that the subdirname is suffixed // so that PackageManager correctly invalidates the existing installed apk // in favour of the new APK-in-APEX. See bugs for more information. - dirInApex := filepath.Join(appDir, aapp.InstallApkName()+"@"+APEX_VERSION_PLACEHOLDER) + dirInApex := filepath.Join(appDir, aapp.InstallApkName()+"@"+sanitizedBuildIdForPath(ctx)) fileToCopy := aapp.OutputFile() af := newApexFile(ctx, fileToCopy, aapp.BaseModuleName(), dirInApex, app, aapp) @@ -1903,7 +1917,7 @@ func (a *apexBundle) GenerateAndroidBuildActions(ctx android.ModuleContext) { // suffixed so that PackageManager correctly invalidates the // existing installed apk in favour of the new APK-in-APEX. // See bugs for more information. - appDirName := filepath.Join(appDir, ap.BaseModuleName()+"@"+APEX_VERSION_PLACEHOLDER) + appDirName := filepath.Join(appDir, ap.BaseModuleName()+"@"+sanitizedBuildIdForPath(ctx)) af := newApexFile(ctx, ap.OutputFile(), ap.BaseModuleName(), appDirName, appSet, ap) af.certificate = java.PresignedCertificate filesInfo = append(filesInfo, af) diff --git a/apex/apex_test.go b/apex/apex_test.go index 4a52115db..7b2905884 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -223,6 +223,7 @@ var prepareForApexTest = android.GroupFixturePreparers( // not because of these tests specifically (it's not used by the tests) variables.Platform_version_active_codenames = []string{"Q", "Tiramisu"} variables.Platform_vndk_version = proptools.StringPtr("29") + variables.BuildId = proptools.StringPtr("TEST.BUILD_ID") }), ) @@ -682,7 +683,7 @@ func TestDefaults(t *testing.T) { "etc/myetc", "javalib/myjar.jar", "lib64/mylib.so", - "app/AppFoo@__APEX_VERSION_PLACEHOLDER__/AppFoo.apk", + "app/AppFoo@TEST.BUILD_ID/AppFoo.apk", "overlay/blue/rro.apk", "etc/bpf/bpf.o", "etc/bpf/bpf2.o", @@ -5682,8 +5683,8 @@ func TestApexWithApps(t *testing.T) { apexRule := module.Rule("apexRule") copyCmds := apexRule.Args["copy_commands"] - ensureContains(t, copyCmds, "image.apex/app/AppFoo@__APEX_VERSION_PLACEHOLDER__/AppFoo.apk") - ensureContains(t, copyCmds, "image.apex/priv-app/AppFooPriv@__APEX_VERSION_PLACEHOLDER__/AppFooPriv.apk") + ensureContains(t, copyCmds, "image.apex/app/AppFoo@TEST.BUILD_ID/AppFoo.apk") + ensureContains(t, copyCmds, "image.apex/priv-app/AppFooPriv@TEST.BUILD_ID/AppFooPriv.apk") appZipRule := ctx.ModuleForTests("AppFoo", "android_common_apex10000").Description("zip jni libs") // JNI libraries are uncompressed @@ -5700,6 +5701,36 @@ func TestApexWithApps(t *testing.T) { } } +func TestApexWithAppImportBuildId(t *testing.T) { + invalidBuildIds := []string{"../", "a b", "a/b", "a/b/../c", "/a"} + for _, id := range invalidBuildIds { + message := fmt.Sprintf("Unable to use build id %s as filename suffix", id) + fixture := android.FixtureModifyProductVariables(func(variables android.FixtureProductVariables) { + variables.BuildId = proptools.StringPtr(id) + }) + testApexError(t, message, `apex { + name: "myapex", + key: "myapex.key", + apps: ["AppFooPrebuilt"], + updatable: false, + } + + apex_key { + name: "myapex.key", + public_key: "testkey.avbpubkey", + private_key: "testkey.pem", + } + + android_app_import { + name: "AppFooPrebuilt", + apk: "PrebuiltAppFoo.apk", + presigned: true, + apex_available: ["myapex"], + } + `, fixture) + } +} + func TestApexWithAppImports(t *testing.T) { ctx := testApex(t, ` apex { @@ -5745,8 +5776,8 @@ func TestApexWithAppImports(t *testing.T) { apexRule := module.Rule("apexRule") copyCmds := apexRule.Args["copy_commands"] - ensureContains(t, copyCmds, "image.apex/app/AppFooPrebuilt@__APEX_VERSION_PLACEHOLDER__/AppFooPrebuilt.apk") - ensureContains(t, copyCmds, "image.apex/priv-app/AppFooPrivPrebuilt@__APEX_VERSION_PLACEHOLDER__/AwesomePrebuiltAppFooPriv.apk") + ensureContains(t, copyCmds, "image.apex/app/AppFooPrebuilt@TEST.BUILD_ID/AppFooPrebuilt.apk") + ensureContains(t, copyCmds, "image.apex/priv-app/AppFooPrivPrebuilt@TEST.BUILD_ID/AwesomePrebuiltAppFooPriv.apk") } func TestApexWithAppImportsPrefer(t *testing.T) { @@ -5787,7 +5818,7 @@ func TestApexWithAppImportsPrefer(t *testing.T) { })) ensureExactContents(t, ctx, "myapex", "android_common_myapex_image", []string{ - "app/AppFoo@__APEX_VERSION_PLACEHOLDER__/AppFooPrebuilt.apk", + "app/AppFoo@TEST.BUILD_ID/AppFooPrebuilt.apk", }) } @@ -5820,7 +5851,7 @@ func TestApexWithTestHelperApp(t *testing.T) { apexRule := module.Rule("apexRule") copyCmds := apexRule.Args["copy_commands"] - ensureContains(t, copyCmds, "image.apex/app/TesterHelpAppFoo@__APEX_VERSION_PLACEHOLDER__/TesterHelpAppFoo.apk") + ensureContains(t, copyCmds, "image.apex/app/TesterHelpAppFoo@TEST.BUILD_ID/TesterHelpAppFoo.apk") } func TestApexPropertiesShouldBeDefaultable(t *testing.T) { @@ -6263,8 +6294,8 @@ func TestOverrideApex(t *testing.T) { apexRule := module.Rule("apexRule") copyCmds := apexRule.Args["copy_commands"] - ensureNotContains(t, copyCmds, "image.apex/app/app@__APEX_VERSION_PLACEHOLDER__/app.apk") - ensureContains(t, copyCmds, "image.apex/app/override_app@__APEX_VERSION_PLACEHOLDER__/override_app.apk") + ensureNotContains(t, copyCmds, "image.apex/app/app@TEST.BUILD_ID/app.apk") + ensureContains(t, copyCmds, "image.apex/app/override_app@TEST.BUILD_ID/override_app.apk") ensureNotContains(t, copyCmds, "image.apex/etc/bpf/bpf.o") ensureContains(t, copyCmds, "image.apex/etc/bpf/override_bpf.o") @@ -7168,7 +7199,7 @@ func TestAppBundle(t *testing.T) { content := bundleConfigRule.Args["content"] ensureContains(t, content, `"compression":{"uncompressed_glob":["apex_payload.img","apex_manifest.*"]}`) - ensureContains(t, content, `"apex_config":{"apex_embedded_apk_config":[{"package_name":"com.android.foo","path":"app/AppFoo@__APEX_VERSION_PLACEHOLDER__/AppFoo.apk"}]}`) + ensureContains(t, content, `"apex_config":{"apex_embedded_apk_config":[{"package_name":"com.android.foo","path":"app/AppFoo@TEST.BUILD_ID/AppFoo.apk"}]}`) } func TestAppSetBundle(t *testing.T) { @@ -7199,9 +7230,9 @@ func TestAppSetBundle(t *testing.T) { if len(copyCmds) != 3 { t.Fatalf("Expected 3 commands, got %d in:\n%s", len(copyCmds), s) } - ensureMatches(t, copyCmds[0], "^rm -rf .*/app/AppSet@__APEX_VERSION_PLACEHOLDER__$") - ensureMatches(t, copyCmds[1], "^mkdir -p .*/app/AppSet@__APEX_VERSION_PLACEHOLDER__$") - ensureMatches(t, copyCmds[2], "^unzip .*-d .*/app/AppSet@__APEX_VERSION_PLACEHOLDER__ .*/AppSet.zip$") + ensureMatches(t, copyCmds[0], "^rm -rf .*/app/AppSet@TEST.BUILD_ID$") + ensureMatches(t, copyCmds[1], "^mkdir -p .*/app/AppSet@TEST.BUILD_ID$") + ensureMatches(t, copyCmds[2], "^unzip .*-d .*/app/AppSet@TEST.BUILD_ID .*/AppSet.zip$") } func TestAppSetBundlePrebuilt(t *testing.T) { diff --git a/apex/builder.go b/apex/builder.go index d4765d022..abbf8ad25 100644 --- a/apex/builder.go +++ b/apex/builder.go @@ -107,16 +107,14 @@ var ( `--canned_fs_config ${canned_fs_config} ` + `--include_build_info ` + `--payload_type image ` + - `--key ${key} ` + - `--apex_version_placeholder ${apex_version_placeholder} ` + - `${opt_flags} ${image_dir} ${out} `, + `--key ${key} ${opt_flags} ${image_dir} ${out} `, CommandDeps: []string{"${apexer}", "${avbtool}", "${e2fsdroid}", "${merge_zips}", "${mke2fs}", "${resize2fs}", "${sefcontext_compile}", "${make_f2fs}", "${sload_f2fs}", "${make_erofs}", "${soong_zip}", "${zipalign}", "${aapt2}", "prebuilts/sdk/current/public/android.jar"}, Rspfile: "${out}.copy_commands", RspfileContent: "${copy_commands}", Description: "APEX ${image_dir} => ${out}", - }, "tool_path", "image_dir", "copy_commands", "file_contexts", "canned_fs_config", "key", "opt_flags", "manifest", "payload_fs_type", "apex_version_placeholder") + }, "tool_path", "image_dir", "copy_commands", "file_contexts", "canned_fs_config", "key", "opt_flags", "manifest", "payload_fs_type") zipApexRule = pctx.StaticRule("zipApexRule", blueprint.RuleParams{ Command: `rm -rf ${image_dir} && mkdir -p ${image_dir} && ` + @@ -124,13 +122,12 @@ var ( `APEXER_TOOL_PATH=${tool_path} ` + `${apexer} --force --manifest ${manifest} ` + `--payload_type zip ` + - `--apex_version_placeholder ${apex_version_placeholder} ` + `${image_dir} ${out} `, CommandDeps: []string{"${apexer}", "${merge_zips}", "${soong_zip}", "${zipalign}", "${aapt2}"}, Rspfile: "${out}.copy_commands", RspfileContent: "${copy_commands}", Description: "ZipAPEX ${image_dir} => ${out}", - }, "tool_path", "image_dir", "copy_commands", "manifest", "apex_version_placeholder") + }, "tool_path", "image_dir", "copy_commands", "manifest") apexProtoConvertRule = pctx.AndroidStaticRule("apexProtoConvertRule", blueprint.RuleParams{ @@ -661,15 +658,14 @@ func (a *apexBundle) buildUnflattenedApex(ctx android.ModuleContext) { Output: unsignedOutputFile, Description: "apex (" + apexType.name() + ")", Args: map[string]string{ - "tool_path": outHostBinDir + ":" + prebuiltSdkToolsBinDir, - "image_dir": imageDir.String(), - "copy_commands": strings.Join(copyCommands, " && "), - "manifest": a.manifestPbOut.String(), - "file_contexts": fileContexts.String(), - "canned_fs_config": cannedFsConfig.String(), - "key": a.privateKeyFile.String(), - "opt_flags": strings.Join(optFlags, " "), - "apex_version_placeholder": APEX_VERSION_PLACEHOLDER, + "tool_path": outHostBinDir + ":" + prebuiltSdkToolsBinDir, + "image_dir": imageDir.String(), + "copy_commands": strings.Join(copyCommands, " && "), + "manifest": a.manifestPbOut.String(), + "file_contexts": fileContexts.String(), + "canned_fs_config": cannedFsConfig.String(), + "key": a.privateKeyFile.String(), + "opt_flags": strings.Join(optFlags, " "), }, }) @@ -761,11 +757,10 @@ func (a *apexBundle) buildUnflattenedApex(ctx android.ModuleContext) { Output: unsignedOutputFile, Description: "apex (" + apexType.name() + ")", Args: map[string]string{ - "tool_path": outHostBinDir + ":" + prebuiltSdkToolsBinDir, - "image_dir": imageDir.String(), - "copy_commands": strings.Join(copyCommands, " && "), - "manifest": a.manifestPbOut.String(), - "apex_version_placeholder": APEX_VERSION_PLACEHOLDER, + "tool_path": outHostBinDir + ":" + prebuiltSdkToolsBinDir, + "image_dir": imageDir.String(), + "copy_commands": strings.Join(copyCommands, " && "), + "manifest": a.manifestPbOut.String(), }, }) } diff --git a/cc/config/tidy.go b/cc/config/tidy.go index ba1043b0d..fb9ac49d3 100644 --- a/cc/config/tidy.go +++ b/cc/config/tidy.go @@ -122,6 +122,7 @@ var DefaultLocalTidyChecks = []PathBasedTidyCheck{ {"hardware/qcom", tidyExternalVendor}, {"vendor/", tidyExternalVendor}, {"vendor/google", tidyDefault}, + {"vendor/google_arc/libs/org.chromium.arc.mojom", tidyExternalVendor}, {"vendor/google_devices", tidyExternalVendor}, } diff --git a/cc/test.go b/cc/test.go index ead7877e2..57035711e 100644 --- a/cc/test.go +++ b/cc/test.go @@ -30,7 +30,8 @@ type TestLinkerProperties struct { // if set, build against the gtest library. Defaults to true. Gtest *bool - // if set, use the isolated gtest runner. Defaults to false. + // if set, use the isolated gtest runner. Defaults to true if gtest is also true and the arch is Windows, false + // otherwise. Isolated *bool } @@ -256,6 +257,13 @@ func (test *testDecorator) gtest() bool { return BoolDefault(test.LinkerProperties.Gtest, true) } +func (test *testDecorator) isolated(ctx BaseModuleContext) bool { + if !ctx.Windows() { + return BoolDefault(test.LinkerProperties.Isolated, false) + } + return BoolDefault(test.LinkerProperties.Isolated, false) +} + func (test *testDecorator) testBinary() bool { return true } @@ -288,7 +296,7 @@ func (test *testDecorator) linkerDeps(ctx BaseModuleContext, deps Deps) Deps { if test.gtest() { if ctx.useSdk() && ctx.Device() { deps.StaticLibs = append(deps.StaticLibs, "libgtest_main_ndk_c++", "libgtest_ndk_c++") - } else if BoolDefault(test.LinkerProperties.Isolated, false) { + } else if test.isolated(ctx) { deps.StaticLibs = append(deps.StaticLibs, "libgtest_isolated_main") // The isolated library requires liblog, but adding it // as a static library means unit tests cannot override @@ -424,7 +432,7 @@ func (test *testBinary) install(ctx ModuleContext, file android.Path) { var options []tradefed.Option configs = append(configs, tradefed.Object{"target_preparer", "com.android.tradefed.targetprep.StopServicesSetup", options}) } - if Bool(test.testDecorator.LinkerProperties.Isolated) { + if test.isolated(ctx) { configs = append(configs, tradefed.Option{Name: "not-shardable", Value: "true"}) } if test.Properties.Test_options.Run_test_as != nil { diff --git a/docs/tidy.md b/docs/tidy.md index 890c3a036..2eb8234c7 100644 --- a/docs/tidy.md +++ b/docs/tidy.md @@ -31,7 +31,7 @@ The default global clang-tidy checks and flags are defined in The global default can be overwritten by module properties in Android.bp. -### `tidy` and `tidy_checks` +### `tidy`, `tidy_checks`, and `ALLOW_LOCAL_TIDY_TRUE` For example, in [system/bpf/Android.bp](https://android.googlesource.com/platform/system/bpf/+/refs/heads/master/Android.bp), @@ -52,8 +52,16 @@ cc_defaults { } ``` That means in normal builds, even without `WITH_TIDY=1`, -the modules that use `bpf_defaults` will run clang-tidy +the modules that use `bpf_defaults` _should_ run clang-tidy over C/C++ source files with the given `tidy_checks`. + +However since clang-tidy warnings and its runtime cost might +not be wanted by all people, the default is to ignore the +`tidy:true` property unless the environment variable +`ALLOW_LOCAL_TIDY_TRUE` is set to true or 1. +To run clang-tidy on all modules that should be tested with clang-tidy, +`ALLOW_LOCAL_TIDY_TRUE` or `WITH_TIDY` should be set to true or 1. + Note that `clang-analyzer-security*` is included in `tidy_checks` but not all `clang-analyzer-*` checks. Check `cert-err34-c` is disabled, although `cert-*` is selected. @@ -80,6 +88,9 @@ cc_test_library { } ``` +Note that `tidy:false` always disables clang-tidy, no matter +`ALLOW_LOCAL_TIDY_TRUE` is set or not. + ### `tidy_checks_as_errors` The global tidy checks are enabled as warnings. diff --git a/java/base.go b/java/base.go index e60e54ec1..1e8095baa 100644 --- a/java/base.go +++ b/java/base.go @@ -748,9 +748,7 @@ func (j *Module) deps(ctx android.BottomUpMutatorContext) { // Kotlin files ctx.AddVariationDependencies(nil, kotlinStdlibTag, "kotlin-stdlib", "kotlin-stdlib-jdk7", "kotlin-stdlib-jdk8") - if len(j.properties.Plugins) > 0 { - ctx.AddVariationDependencies(nil, kotlinAnnotationsTag, "kotlin-annotations") - } + ctx.AddVariationDependencies(nil, kotlinAnnotationsTag, "kotlin-annotations") } // Framework libraries need special handling in static coverage builds: they should not have @@ -1106,8 +1104,6 @@ func (j *Module) compile(ctx android.ModuleContext, aaptSrcJar android.Path) { flags.classpath = append(flags.classpath, deps.kotlinStdlib...) flags.classpath = append(flags.classpath, deps.kotlinAnnotations...) - flags.dexClasspath = append(flags.dexClasspath, deps.kotlinAnnotations...) - flags.kotlincClasspath = append(flags.kotlincClasspath, flags.bootClasspath...) flags.kotlincClasspath = append(flags.kotlincClasspath, flags.classpath...) @@ -1139,9 +1135,12 @@ func (j *Module) compile(ctx android.ModuleContext, aaptSrcJar android.Path) { // Jar kotlin classes into the final jar after javac if BoolDefault(j.properties.Static_kotlin_stdlib, true) { kotlinJars = append(kotlinJars, deps.kotlinStdlib...) + kotlinJars = append(kotlinJars, deps.kotlinAnnotations...) kotlinHeaderJars = append(kotlinHeaderJars, deps.kotlinStdlib...) + kotlinHeaderJars = append(kotlinHeaderJars, deps.kotlinAnnotations...) } else { flags.dexClasspath = append(flags.dexClasspath, deps.kotlinStdlib...) + flags.dexClasspath = append(flags.dexClasspath, deps.kotlinAnnotations...) } } diff --git a/java/kotlin_test.go b/java/kotlin_test.go index 435d78294..491ce2939 100644 --- a/java/kotlin_test.go +++ b/java/kotlin_test.go @@ -42,6 +42,11 @@ func TestKotlin(t *testing.T) { } `) + kotlinStdlib := ctx.ModuleForTests("kotlin-stdlib", "android_common"). + Output("turbine-combined/kotlin-stdlib.jar").Output + kotlinAnnotations := ctx.ModuleForTests("kotlin-annotations", "android_common"). + Output("turbine-combined/kotlin-annotations.jar").Output + fooKotlinc := ctx.ModuleForTests("foo", "android_common").Rule("kotlinc") fooJavac := ctx.ModuleForTests("foo", "android_common").Rule("javac") fooJar := ctx.ModuleForTests("foo", "android_common").Output("combined/foo.jar") @@ -69,6 +74,16 @@ func TestKotlin(t *testing.T) { fooJar.Inputs.Strings(), fooKotlincClasses.String()) } + if !inList(kotlinStdlib.String(), fooJar.Inputs.Strings()) { + t.Errorf("foo jar inputs %v does not contain %v", + fooJar.Inputs.Strings(), kotlinStdlib.String()) + } + + if !inList(kotlinAnnotations.String(), fooJar.Inputs.Strings()) { + t.Errorf("foo jar inputs %v does not contain %v", + fooJar.Inputs.Strings(), kotlinAnnotations.String()) + } + if !inList(fooKotlincHeaderClasses.String(), fooHeaderJar.Inputs.Strings()) { t.Errorf("foo header jar inputs %v does not contain %q", fooHeaderJar.Inputs.Strings(), fooKotlincHeaderClasses.String()) |