diff options
| -rw-r--r-- | android/bazel.go | 4 | ||||
| -rw-r--r-- | apex/apex.go | 52 | ||||
| -rw-r--r-- | apex/apex_test.go | 17 | ||||
| -rw-r--r-- | apex/builder.go | 2 | ||||
| -rw-r--r-- | java/Android.bp | 1 | ||||
| -rw-r--r-- | java/java_test.go | 101 | ||||
| -rw-r--r-- | java/lint.go | 26 | ||||
| -rw-r--r-- | java/lint_test.go | 204 |
8 files changed, 275 insertions, 132 deletions
diff --git a/android/bazel.go b/android/bazel.go index 6e87d5770..3e57e222b 100644 --- a/android/bazel.go +++ b/android/bazel.go @@ -180,8 +180,6 @@ var ( "libc_init_dynamic", // ruperts@, cc_library_static, 'private/bionic_defs.h' file not found "libc_tzcode", // ruperts@, cc_library_static, error: expected expression "libc_netbsd", // ruperts@, cc_library_static, 'engine.c' file not found - "libc_openbsd_large_stack", // ruperts@, cc_library_static, 'android/log.h' file not found - "libc_openbsd", // ruperts@, cc_library_static, 'android/log.h' file not found "libc_fortify", // ruperts@, cc_library_static, 'private/bionic_fortify.h' file not found "libc_bionic", // ruperts@, cc_library_static, 'private/bionic_asm.h' file not found "libc_bionic_ndk", // ruperts@, cc_library_static, depends on //bionic/libc/system_properties @@ -203,7 +201,7 @@ var ( "liblinker_malloc", // ruperts@, cc_library_static, depends on //system/logging/liblog:liblog "liblinker_debuggerd_stub", // ruperts@, cc_library_static, depends on //system/libbase "libbionic_tests_headers_posix", // ruperts@, cc_library_static, 'complex.h' file not found - "libc_dns", // ruperts@, cc_library_static, 'android/log.h' file not found + "libc_dns", // ruperts@, cc_library_static, 'private/android_filesystem_config.h' file not found "libc_static_dispatch", // eakammer@, cc_library_static, 'private/bionic_asm.h' file not found "libc_dynamic_dispatch", // eakammer@, cc_library_static, 'private/bionic_ifuncs.h' file not found "note_memtag_heap_async", // jingwen@, cc_library_static, 'private/bionic_asm.h' file not found (arm64) diff --git a/apex/apex.go b/apex/apex.go index efeb0e222..088a462b3 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -110,16 +110,6 @@ type apexBundleProperties struct { // List of filesystem images that are embedded inside this APEX bundle. Filesystems []string - // Name of the apex_key module that provides the private key to sign this APEX bundle. - Key *string - - // Specifies the certificate and the private key to sign the zip container of this APEX. If - // this is "foo", foo.x509.pem and foo.pk8 under PRODUCT_DEFAULT_DEV_CERTIFICATE are used - // as the certificate and the private key, respectively. If this is ":module", then the - // certificate and the private key are provided from the android_app_certificate module - // named "module". - Certificate *string - // The minimum SDK version that this APEX must support at minimum. This is usually set to // the SDK version that the APEX was first introduced. Min_sdk_version *string @@ -299,6 +289,16 @@ type overridableProperties struct { // A txt file containing list of files that are allowed to be included in this APEX. Allowed_files *string `android:"path"` + + // Name of the apex_key module that provides the private key to sign this APEX bundle. + Key *string + + // Specifies the certificate and the private key to sign the zip container of this APEX. If + // this is "foo", foo.x509.pem and foo.pk8 under PRODUCT_DEFAULT_DEV_CERTIFICATE are used + // as the certificate and the private key, respectively. If this is ":module", then the + // certificate and the private key are provided from the android_app_certificate module + // named "module". + Certificate *string } type apexBundle struct { @@ -760,20 +760,6 @@ func (a *apexBundle) DepsMutator(ctx android.BottomUpMutatorContext) { } } - // Dependencies for signing - if String(a.properties.Key) == "" { - ctx.PropertyErrorf("key", "missing") - return - } - ctx.AddDependency(ctx.Module(), keyTag, String(a.properties.Key)) - - cert := android.SrcIsModule(a.getCertString(ctx)) - if cert != "" { - ctx.AddDependency(ctx.Module(), certificateTag, cert) - // empty cert is not an error. Cert and private keys will be directly found under - // PRODUCT_DEFAULT_DEV_CERTIFICATE - } - // Marks that this APEX (in fact all the modules in it) has to be built with the given SDKs. // This field currently isn't used. // TODO(jiyong): consider dropping this feature @@ -797,6 +783,20 @@ func (a *apexBundle) OverridablePropertiesDepsMutator(ctx android.BottomUpMutato commonVariation := ctx.Config().AndroidCommonTarget.Variations() ctx.AddFarVariationDependencies(commonVariation, androidAppTag, a.overridableProperties.Apps...) ctx.AddFarVariationDependencies(commonVariation, rroTag, a.overridableProperties.Rros...) + + // Dependencies for signing + if String(a.overridableProperties.Key) == "" { + ctx.PropertyErrorf("key", "missing") + return + } + ctx.AddDependency(ctx.Module(), keyTag, String(a.overridableProperties.Key)) + + cert := android.SrcIsModule(a.getCertString(ctx)) + if cert != "" { + ctx.AddDependency(ctx.Module(), certificateTag, cert) + // empty cert is not an error. Cert and private keys will be directly found under + // PRODUCT_DEFAULT_DEV_CERTIFICATE + } } type ApexBundleInfo struct { @@ -1292,7 +1292,7 @@ func (a *apexBundle) getCertString(ctx android.BaseModuleContext) string { if overridden { return ":" + certificate } - return String(a.properties.Certificate) + return String(a.overridableProperties.Certificate) } // See the installable property @@ -1949,7 +1949,7 @@ func (a *apexBundle) GenerateAndroidBuildActions(ctx android.ModuleContext) { return false }) if a.privateKeyFile == nil { - ctx.PropertyErrorf("key", "private_key for %q could not be found", String(a.properties.Key)) + ctx.PropertyErrorf("key", "private_key for %q could not be found", String(a.overridableProperties.Key)) return } diff --git a/apex/apex_test.go b/apex/apex_test.go index 977a9544c..a7ae6f02b 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -5599,6 +5599,8 @@ func TestOverrideApex(t *testing.T) { overrides: ["unknownapex"], logging_parent: "com.foo.bar", package_name: "test.overridden.package", + key: "mynewapex.key", + certificate: ":myapex.certificate", } apex_key { @@ -5607,6 +5609,17 @@ func TestOverrideApex(t *testing.T) { private_key: "testkey.pem", } + apex_key { + name: "mynewapex.key", + public_key: "testkey2.avbpubkey", + private_key: "testkey2.pem", + } + + android_app_certificate { + name: "myapex.certificate", + certificate: "testkey", + } + android_app { name: "app", srcs: ["foo/bar/MyClass.java"], @@ -5651,6 +5664,10 @@ func TestOverrideApex(t *testing.T) { optFlags := apexRule.Args["opt_flags"] ensureContains(t, optFlags, "--override_apk_package_name test.overridden.package") + ensureContains(t, optFlags, "--pubkey testkey2.avbpubkey") + + signApkRule := module.Rule("signapk") + ensureEquals(t, signApkRule.Args["certificates"], "testkey.x509.pem testkey.pk8") data := android.AndroidMkDataForTest(t, ctx, apexBundle) var builder strings.Builder diff --git a/apex/builder.go b/apex/builder.go index b382a536e..da8841ce7 100644 --- a/apex/builder.go +++ b/apex/builder.go @@ -871,7 +871,7 @@ func (a *apexBundle) getCertificateAndPrivateKey(ctx android.PathContext) (pem, return a.containerCertificateFile, a.containerPrivateKeyFile } - cert := String(a.properties.Certificate) + cert := String(a.overridableProperties.Certificate) if cert == "" { return ctx.Config().DefaultAppCertificate(ctx) } diff --git a/java/Android.bp b/java/Android.bp index 888b01b2a..8e3e10d9d 100644 --- a/java/Android.bp +++ b/java/Android.bp @@ -83,6 +83,7 @@ bootstrap_go_package { "java_test.go", "jdeps_test.go", "kotlin_test.go", + "lint_test.go", "platform_bootclasspath_test.go", "platform_compat_config_test.go", "plugin_test.go", diff --git a/java/java_test.go b/java/java_test.go index ed70b7092..1b8aec286 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -1203,107 +1203,6 @@ func TestIncludeSrcs(t *testing.T) { } } -func TestJavaLint(t *testing.T) { - ctx, _ := testJavaWithFS(t, ` - java_library { - name: "foo", - srcs: [ - "a.java", - "b.java", - "c.java", - ], - min_sdk_version: "29", - sdk_version: "system_current", - } - `, map[string][]byte{ - "lint-baseline.xml": nil, - }) - - foo := ctx.ModuleForTests("foo", "android_common") - - sboxProto := android.RuleBuilderSboxProtoForTests(t, foo.Output("lint.sbox.textproto")) - if !strings.Contains(*sboxProto.Commands[0].Command, "--baseline lint-baseline.xml") { - t.Error("did not pass --baseline flag") - } -} - -func TestJavaLintWithoutBaseline(t *testing.T) { - ctx, _ := testJavaWithFS(t, ` - java_library { - name: "foo", - srcs: [ - "a.java", - "b.java", - "c.java", - ], - min_sdk_version: "29", - sdk_version: "system_current", - } - `, map[string][]byte{}) - - foo := ctx.ModuleForTests("foo", "android_common") - - sboxProto := android.RuleBuilderSboxProtoForTests(t, foo.Output("lint.sbox.textproto")) - if strings.Contains(*sboxProto.Commands[0].Command, "--baseline") { - t.Error("passed --baseline flag for non existent file") - } -} - -func TestJavaLintRequiresCustomLintFileToExist(t *testing.T) { - android.GroupFixturePreparers( - PrepareForTestWithJavaDefaultModules, - android.PrepareForTestDisallowNonExistentPaths, - ).ExtendWithErrorHandler(android.FixtureExpectsAllErrorsToMatchAPattern([]string{`source path "mybaseline.xml" does not exist`})). - RunTestWithBp(t, ` - java_library { - name: "foo", - srcs: [ - ], - min_sdk_version: "29", - sdk_version: "system_current", - lint: { - baseline_filename: "mybaseline.xml", - }, - } - `) -} - -func TestJavaLintUsesCorrectBpConfig(t *testing.T) { - ctx, _ := testJavaWithFS(t, ` - java_library { - name: "foo", - srcs: [ - "a.java", - "b.java", - "c.java", - ], - min_sdk_version: "29", - sdk_version: "system_current", - lint: { - error_checks: ["SomeCheck"], - baseline_filename: "mybaseline.xml", - }, - } - `, map[string][]byte{ - "mybaseline.xml": nil, - }) - - foo := ctx.ModuleForTests("foo", "android_common") - - sboxProto := android.RuleBuilderSboxProtoForTests(t, foo.Output("lint.sbox.textproto")) - if !strings.Contains(*sboxProto.Commands[0].Command, "--baseline mybaseline.xml") { - t.Error("did not use the correct file for baseline") - } - - if !strings.Contains(*sboxProto.Commands[0].Command, "--error_check NewApi") { - t.Error("should check NewApi errors") - } - - if !strings.Contains(*sboxProto.Commands[0].Command, "--error_check SomeCheck") { - t.Error("should combine NewApi errors with SomeCheck errors") - } -} - func TestGeneratedSources(t *testing.T) { ctx, _ := testJavaWithFS(t, ` java_library { diff --git a/java/lint.go b/java/lint.go index a77daa85d..862c9b4d9 100644 --- a/java/lint.go +++ b/java/lint.go @@ -26,6 +26,10 @@ import ( "android/soong/remoteexec" ) +// lint checks automatically enforced for modules that have different min_sdk_version than +// sdk_version +var updatabilityChecks = []string{"NewApi"} + type LintProperties struct { // Controls for running Android Lint on the module. Lint struct { @@ -53,6 +57,9 @@ type LintProperties struct { // Name of the file that lint uses as the baseline. Defaults to "lint-baseline.xml". Baseline_filename *string + + // If true, baselining updatability lint checks (e.g. NewApi) is prohibited. Defaults to false. + Strict_updatability_linting *bool } } @@ -253,6 +260,13 @@ func (l *linter) writeLintProjectXML(ctx android.ModuleContext, rule *android.Ru cmd.FlagForEachArg("--error_check ", l.properties.Lint.Error_checks) cmd.FlagForEachArg("--fatal_check ", l.properties.Lint.Fatal_checks) + if BoolDefault(l.properties.Lint.Strict_updatability_linting, false) { + if baselinePath := l.getBaselineFilepath(ctx); baselinePath.Valid() { + cmd.FlagWithInput("--baseline ", baselinePath.Path()) + cmd.FlagForEachArg("--disallowed_issues ", updatabilityChecks) + } + } + return lintPaths{ projectXML: projectXMLPath, configXML: configXMLPath, @@ -298,7 +312,17 @@ func (l *linter) lint(ctx android.ModuleContext) { } if l.minSdkVersion != l.compileSdkVersion { - l.extraMainlineLintErrors = append(l.extraMainlineLintErrors, "NewApi") + l.extraMainlineLintErrors = append(l.extraMainlineLintErrors, updatabilityChecks...) + _, filtered := android.FilterList(l.properties.Lint.Warning_checks, updatabilityChecks) + if len(filtered) != 0 { + ctx.PropertyErrorf("lint.warning_checks", + "Can't treat %v checks as warnings if min_sdk_version is different from sdk_version.", filtered) + } + _, filtered = android.FilterList(l.properties.Lint.Disabled_checks, updatabilityChecks) + if len(filtered) != 0 { + ctx.PropertyErrorf("lint.disabled_checks", + "Can't disable %v checks if min_sdk_version is different from sdk_version.", filtered) + } } extraLintCheckModules := ctx.GetDirectDepsWithTag(extraLintCheckTag) diff --git a/java/lint_test.go b/java/lint_test.go new file mode 100644 index 000000000..a253df979 --- /dev/null +++ b/java/lint_test.go @@ -0,0 +1,204 @@ +// Copyright 2021 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package java + +import ( + "strings" + "testing" + + "android/soong/android" +) + +func TestJavaLint(t *testing.T) { + ctx, _ := testJavaWithFS(t, ` + java_library { + name: "foo", + srcs: [ + "a.java", + "b.java", + "c.java", + ], + min_sdk_version: "29", + sdk_version: "system_current", + } + `, map[string][]byte{ + "lint-baseline.xml": nil, + }) + + foo := ctx.ModuleForTests("foo", "android_common") + + sboxProto := android.RuleBuilderSboxProtoForTests(t, foo.Output("lint.sbox.textproto")) + if !strings.Contains(*sboxProto.Commands[0].Command, "--baseline lint-baseline.xml") { + t.Error("did not pass --baseline flag") + } +} + +func TestJavaLintWithoutBaseline(t *testing.T) { + ctx, _ := testJavaWithFS(t, ` + java_library { + name: "foo", + srcs: [ + "a.java", + "b.java", + "c.java", + ], + min_sdk_version: "29", + sdk_version: "system_current", + } + `, map[string][]byte{}) + + foo := ctx.ModuleForTests("foo", "android_common") + + sboxProto := android.RuleBuilderSboxProtoForTests(t, foo.Output("lint.sbox.textproto")) + if strings.Contains(*sboxProto.Commands[0].Command, "--baseline") { + t.Error("passed --baseline flag for non existent file") + } +} + +func TestJavaLintRequiresCustomLintFileToExist(t *testing.T) { + android.GroupFixturePreparers( + PrepareForTestWithJavaDefaultModules, + android.PrepareForTestDisallowNonExistentPaths, + ).ExtendWithErrorHandler(android.FixtureExpectsAllErrorsToMatchAPattern([]string{`source path "mybaseline.xml" does not exist`})). + RunTestWithBp(t, ` + java_library { + name: "foo", + srcs: [ + ], + min_sdk_version: "29", + sdk_version: "system_current", + lint: { + baseline_filename: "mybaseline.xml", + }, + } + `) +} + +func TestJavaLintUsesCorrectBpConfig(t *testing.T) { + ctx, _ := testJavaWithFS(t, ` + java_library { + name: "foo", + srcs: [ + "a.java", + "b.java", + "c.java", + ], + min_sdk_version: "29", + sdk_version: "system_current", + lint: { + error_checks: ["SomeCheck"], + baseline_filename: "mybaseline.xml", + }, + } + `, map[string][]byte{ + "mybaseline.xml": nil, + }) + + foo := ctx.ModuleForTests("foo", "android_common") + + sboxProto := android.RuleBuilderSboxProtoForTests(t, foo.Output("lint.sbox.textproto")) + if !strings.Contains(*sboxProto.Commands[0].Command, "--baseline mybaseline.xml") { + t.Error("did not use the correct file for baseline") + } + + if !strings.Contains(*sboxProto.Commands[0].Command, "--error_check NewApi") { + t.Error("should check NewApi errors") + } + + if !strings.Contains(*sboxProto.Commands[0].Command, "--error_check SomeCheck") { + t.Error("should combine NewApi errors with SomeCheck errors") + } +} + +func TestJavaLintBypassUpdatableChecks(t *testing.T) { + testCases := []struct { + name string + bp string + error string + }{ + { + name: "warning_checks", + bp: ` + java_library { + name: "foo", + srcs: [ + "a.java", + ], + min_sdk_version: "29", + sdk_version: "current", + lint: { + warning_checks: ["NewApi"], + }, + } + `, + error: "lint.warning_checks: Can't treat \\[NewApi\\] checks as warnings if min_sdk_version is different from sdk_version.", + }, + { + name: "disable_checks", + bp: ` + java_library { + name: "foo", + srcs: [ + "a.java", + ], + min_sdk_version: "29", + sdk_version: "current", + lint: { + disabled_checks: ["NewApi"], + }, + } + `, + error: "lint.disabled_checks: Can't disable \\[NewApi\\] checks if min_sdk_version is different from sdk_version.", + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + errorHandler := android.FixtureExpectsAtLeastOneErrorMatchingPattern(testCase.error) + android.GroupFixturePreparers(PrepareForTestWithJavaDefaultModules). + ExtendWithErrorHandler(errorHandler). + RunTestWithBp(t, testCase.bp) + }) + } +} + +func TestJavaLintStrictUpdatabilityLinting(t *testing.T) { + bp := ` + java_library { + name: "foo", + srcs: [ + "a.java", + ], + min_sdk_version: "29", + sdk_version: "current", + lint: { + strict_updatability_linting: true, + }, + } + ` + fs := android.MockFS{ + "lint-baseline.xml": nil, + } + + result := android.GroupFixturePreparers(PrepareForTestWithJavaDefaultModules, fs.AddToFixture()). + RunTestWithBp(t, bp) + + foo := result.ModuleForTests("foo", "android_common") + sboxProto := android.RuleBuilderSboxProtoForTests(t, foo.Output("lint.sbox.textproto")) + if !strings.Contains(*sboxProto.Commands[0].Command, + "--baseline lint-baseline.xml --disallowed_issues NewApi") { + t.Error("did not restrict baselining NewApi") + } +} |