diff --git a/apex/apex.go b/apex/apex.go index ac67feea2..56313714d 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -76,6 +76,8 @@ func RegisterPostDepsMutators(ctx android.RegisterMutatorsContext) { ctx.BottomUp("apex", apexMutator).Parallel() ctx.BottomUp("apex_directly_in_any", apexDirectlyInAnyMutator).Parallel() ctx.BottomUp("apex_flattened", apexFlattenedMutator).Parallel() + // Register after apex_info mutator so that it can use ApexVariationName + ctx.TopDown("apex_strict_updatability_lint", apexStrictUpdatibilityLintMutator).Parallel() } type apexBundleProperties struct { @@ -1005,6 +1007,52 @@ func apexInfoMutator(mctx android.TopDownMutatorContext) { } } +// apexStrictUpdatibilityLintMutator propagates strict_updatability_linting to transitive deps of a mainline module +// This check is enforced for updatable modules +func apexStrictUpdatibilityLintMutator(mctx android.TopDownMutatorContext) { + if !mctx.Module().Enabled() { + return + } + if apex, ok := mctx.Module().(*apexBundle); ok && apex.checkStrictUpdatabilityLinting() { + mctx.WalkDeps(func(child, parent android.Module) bool { + // b/208656169 Do not propagate strict updatability linting to libcore/ + // These libs are available on the classpath during compilation + // These libs are transitive deps of the sdk. See java/sdk.go:decodeSdkDep + // Only skip libraries defined in libcore root, not subdirectories + if mctx.OtherModuleDir(child) == "libcore" { + // Do not traverse transitive deps of libcore/ libs + return false + } + if lintable, ok := child.(java.LintDepSetsIntf); ok { + lintable.SetStrictUpdatabilityLinting(true) + } + // visit transitive deps + return true + }) + } +} + +// TODO: b/215736885 Whittle the denylist +// Transitive deps of certain mainline modules baseline NewApi errors +// Skip these mainline modules for now +var ( + skipStrictUpdatabilityLintAllowlist = []string{ + "com.android.art", + "com.android.art.debug", + "com.android.conscrypt", + "com.android.media", + // test apexes + "test_com.android.art", + "test_com.android.conscrypt", + "test_com.android.media", + "test_jitzygote_com.android.art", + } +) + +func (a *apexBundle) checkStrictUpdatabilityLinting() bool { + return a.Updatable() && !android.InList(a.ApexVariationName(), skipStrictUpdatabilityLintAllowlist) +} + // apexUniqueVariationsMutator checks if any dependencies use unique apex variations. If so, use // unique apex variations for this module. See android/apex.go for more about unique apex variant. // TODO(jiyong): move this to android/apex.go? diff --git a/apex/apex_test.go b/apex/apex_test.go index 4f2a58348..85bd59526 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -9051,6 +9051,185 @@ func ensureDoesNotContainRequiredDeps(t *testing.T, ctx *android.TestContext, mo } } +func TestApexStrictUpdtabilityLint(t *testing.T) { + bpTemplate := ` + apex { + name: "myapex", + key: "myapex.key", + java_libs: ["myjavalib"], + updatable: %v, + min_sdk_version: "29", + } + apex_key { + name: "myapex.key", + } + java_library { + name: "myjavalib", + srcs: ["MyClass.java"], + apex_available: [ "myapex" ], + lint: { + strict_updatability_linting: %v, + }, + sdk_version: "current", + min_sdk_version: "29", + } + ` + fs := android.MockFS{ + "lint-baseline.xml": nil, + } + + testCases := []struct { + testCaseName string + apexUpdatable bool + javaStrictUpdtabilityLint bool + lintFileExists bool + disallowedFlagExpected bool + }{ + { + testCaseName: "lint-baseline.xml does not exist, no disallowed flag necessary in lint cmd", + apexUpdatable: true, + javaStrictUpdtabilityLint: true, + lintFileExists: false, + disallowedFlagExpected: false, + }, + { + testCaseName: "non-updatable apex respects strict_updatability of javalib", + apexUpdatable: false, + javaStrictUpdtabilityLint: false, + lintFileExists: true, + disallowedFlagExpected: false, + }, + { + testCaseName: "non-updatable apex respects strict updatability of javalib", + apexUpdatable: false, + javaStrictUpdtabilityLint: true, + lintFileExists: true, + disallowedFlagExpected: true, + }, + { + testCaseName: "updatable apex sets strict updatability of javalib to true", + apexUpdatable: true, + javaStrictUpdtabilityLint: false, // will be set to true by mutator + lintFileExists: true, + disallowedFlagExpected: true, + }, + } + + for _, testCase := range testCases { + bp := fmt.Sprintf(bpTemplate, testCase.apexUpdatable, testCase.javaStrictUpdtabilityLint) + fixtures := []android.FixturePreparer{} + if testCase.lintFileExists { + fixtures = append(fixtures, fs.AddToFixture()) + } + + result := testApex(t, bp, fixtures...) + myjavalib := result.ModuleForTests("myjavalib", "android_common_apex29") + sboxProto := android.RuleBuilderSboxProtoForTests(t, myjavalib.Output("lint.sbox.textproto")) + disallowedFlagActual := strings.Contains(*sboxProto.Commands[0].Command, "--baseline lint-baseline.xml --disallowed_issues NewApi") + + if disallowedFlagActual != testCase.disallowedFlagExpected { + t.Errorf("Failed testcase: %v \nActual lint cmd: %v", testCase.testCaseName, *sboxProto.Commands[0].Command) + } + } +} + +func TestUpdatabilityLintSkipLibcore(t *testing.T) { + bp := ` + apex { + name: "myapex", + key: "myapex.key", + java_libs: ["myjavalib"], + updatable: true, + min_sdk_version: "29", + } + apex_key { + name: "myapex.key", + } + java_library { + name: "myjavalib", + srcs: ["MyClass.java"], + apex_available: [ "myapex" ], + sdk_version: "current", + min_sdk_version: "29", + } + ` + + testCases := []struct { + testCaseName string + moduleDirectory string + disallowedFlagExpected bool + }{ + { + testCaseName: "lintable module defined outside libcore", + moduleDirectory: "", + disallowedFlagExpected: true, + }, + { + testCaseName: "lintable module defined in libcore root directory", + moduleDirectory: "libcore/", + disallowedFlagExpected: false, + }, + { + testCaseName: "lintable module defined in libcore child directory", + moduleDirectory: "libcore/childdir/", + disallowedFlagExpected: true, + }, + } + + for _, testCase := range testCases { + lintFileCreator := android.FixtureAddTextFile(testCase.moduleDirectory+"lint-baseline.xml", "") + bpFileCreator := android.FixtureAddTextFile(testCase.moduleDirectory+"Android.bp", bp) + result := testApex(t, "", lintFileCreator, bpFileCreator) + myjavalib := result.ModuleForTests("myjavalib", "android_common_apex29") + sboxProto := android.RuleBuilderSboxProtoForTests(t, myjavalib.Output("lint.sbox.textproto")) + cmdFlags := fmt.Sprintf("--baseline %vlint-baseline.xml --disallowed_issues NewApi", testCase.moduleDirectory) + disallowedFlagActual := strings.Contains(*sboxProto.Commands[0].Command, cmdFlags) + + if disallowedFlagActual != testCase.disallowedFlagExpected { + t.Errorf("Failed testcase: %v \nActual lint cmd: %v", testCase.testCaseName, *sboxProto.Commands[0].Command) + } + } +} + +// checks transtive deps of an apex coming from bootclasspath_fragment +func TestApexStrictUpdtabilityLintBcpFragmentDeps(t *testing.T) { + bp := ` + apex { + name: "myapex", + key: "myapex.key", + bootclasspath_fragments: ["mybootclasspathfragment"], + updatable: true, + min_sdk_version: "29", + } + apex_key { + name: "myapex.key", + } + bootclasspath_fragment { + name: "mybootclasspathfragment", + contents: ["myjavalib"], + apex_available: ["myapex"], + } + java_library { + name: "myjavalib", + srcs: ["MyClass.java"], + apex_available: [ "myapex" ], + sdk_version: "current", + min_sdk_version: "29", + compile_dex: true, + } + ` + fs := android.MockFS{ + "lint-baseline.xml": nil, + } + + result := testApex(t, bp, dexpreopt.FixtureSetApexBootJars("myapex:myjavalib"), fs.AddToFixture()) + myjavalib := result.ModuleForTests("myjavalib", "android_common_apex29") + sboxProto := android.RuleBuilderSboxProtoForTests(t, myjavalib.Output("lint.sbox.textproto")) + if !strings.Contains(*sboxProto.Commands[0].Command, "--baseline lint-baseline.xml --disallowed_issues NewApi") { + t.Errorf("Strict updabality lint missing in myjavalib coming from bootclasspath_fragment mybootclasspath-fragment\nActual lint cmd: %v", *sboxProto.Commands[0].Command) + } +} + func TestMain(m *testing.M) { os.Exit(m.Run()) } diff --git a/java/lint.go b/java/lint.go index 7845c336b..e97c9c225 100644 --- a/java/lint.go +++ b/java/lint.go @@ -102,12 +102,12 @@ type lintOutputsIntf interface { lintOutputs() *lintOutputs } -type lintDepSetsIntf interface { +type LintDepSetsIntf interface { LintDepSets() LintDepSets // Methods used to propagate strict_updatability_linting values. - getStrictUpdatabilityLinting() bool - setStrictUpdatabilityLinting(bool) + GetStrictUpdatabilityLinting() bool + SetStrictUpdatabilityLinting(bool) } type LintDepSets struct { @@ -158,15 +158,15 @@ func (l *linter) LintDepSets() LintDepSets { return l.outputs.depSets } -func (l *linter) getStrictUpdatabilityLinting() bool { +func (l *linter) GetStrictUpdatabilityLinting() bool { return BoolDefault(l.properties.Lint.Strict_updatability_linting, false) } -func (l *linter) setStrictUpdatabilityLinting(strictLinting bool) { +func (l *linter) SetStrictUpdatabilityLinting(strictLinting bool) { l.properties.Lint.Strict_updatability_linting = &strictLinting } -var _ lintDepSetsIntf = (*linter)(nil) +var _ LintDepSetsIntf = (*linter)(nil) var _ lintOutputsIntf = (*linter)(nil) @@ -273,7 +273,7 @@ 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 l.getStrictUpdatabilityLinting() { + if l.GetStrictUpdatabilityLinting() { // Verify the module does not baseline issues that endanger safe updatability. if baselinePath := l.getBaselineFilepath(ctx); baselinePath.Valid() { cmd.FlagWithInput("--baseline ", baselinePath.Path()) @@ -382,7 +382,7 @@ func (l *linter) lint(ctx android.ModuleContext) { depSetsBuilder := NewLintDepSetBuilder().Direct(html, text, xml) ctx.VisitDirectDepsWithTag(staticLibTag, func(dep android.Module) { - if depLint, ok := dep.(lintDepSetsIntf); ok { + if depLint, ok := dep.(LintDepSetsIntf); ok { depSetsBuilder.Transitive(depLint.LintDepSets()) } }) @@ -660,10 +660,10 @@ func lintZip(ctx android.BuilderContext, paths android.Paths, outputPath android // Enforce the strict updatability linting to all applicable transitive dependencies. func enforceStrictUpdatabilityLintingMutator(ctx android.TopDownMutatorContext) { m := ctx.Module() - if d, ok := m.(lintDepSetsIntf); ok && d.getStrictUpdatabilityLinting() { + if d, ok := m.(LintDepSetsIntf); ok && d.GetStrictUpdatabilityLinting() { ctx.VisitDirectDepsWithTag(staticLibTag, func(d android.Module) { - if a, ok := d.(lintDepSetsIntf); ok { - a.setStrictUpdatabilityLinting(true) + if a, ok := d.(LintDepSetsIntf); ok { + a.SetStrictUpdatabilityLinting(true) } }) } diff --git a/java/sdk_library.go b/java/sdk_library.go index bfdfffcd9..c37ed1a27 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -2362,17 +2362,17 @@ func (module *SdkLibraryImport) LintDepSets() LintDepSets { } } -func (module *SdkLibraryImport) getStrictUpdatabilityLinting() bool { +func (module *SdkLibraryImport) GetStrictUpdatabilityLinting() bool { if module.implLibraryModule == nil { return false } else { - return module.implLibraryModule.getStrictUpdatabilityLinting() + return module.implLibraryModule.GetStrictUpdatabilityLinting() } } -func (module *SdkLibraryImport) setStrictUpdatabilityLinting(strictLinting bool) { +func (module *SdkLibraryImport) SetStrictUpdatabilityLinting(strictLinting bool) { if module.implLibraryModule != nil { - module.implLibraryModule.setStrictUpdatabilityLinting(strictLinting) + module.implLibraryModule.SetStrictUpdatabilityLinting(strictLinting) } }