Merge changes I4c373b29,I9ccda6fc,I1b390b0e,I30151217

* changes:
  Do not propagate strict updatability linting to libcore/
  Create allowlist to skip strict updatability lint check
  Propagate strict_updatability_linting to transitive deps of updatable apexes
  Export LintDepSetsIntf from java package
This commit is contained in:
Spandan Das
2022-03-24 16:38:20 +00:00
committed by Gerrit Code Review
4 changed files with 242 additions and 15 deletions

View File

@@ -76,6 +76,8 @@ func RegisterPostDepsMutators(ctx android.RegisterMutatorsContext) {
ctx.BottomUp("apex", apexMutator).Parallel() ctx.BottomUp("apex", apexMutator).Parallel()
ctx.BottomUp("apex_directly_in_any", apexDirectlyInAnyMutator).Parallel() ctx.BottomUp("apex_directly_in_any", apexDirectlyInAnyMutator).Parallel()
ctx.BottomUp("apex_flattened", apexFlattenedMutator).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 { 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 // 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. // unique apex variations for this module. See android/apex.go for more about unique apex variant.
// TODO(jiyong): move this to android/apex.go? // TODO(jiyong): move this to android/apex.go?

View File

@@ -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) { func TestMain(m *testing.M) {
os.Exit(m.Run()) os.Exit(m.Run())
} }

View File

@@ -102,12 +102,12 @@ type lintOutputsIntf interface {
lintOutputs() *lintOutputs lintOutputs() *lintOutputs
} }
type lintDepSetsIntf interface { type LintDepSetsIntf interface {
LintDepSets() LintDepSets LintDepSets() LintDepSets
// Methods used to propagate strict_updatability_linting values. // Methods used to propagate strict_updatability_linting values.
getStrictUpdatabilityLinting() bool GetStrictUpdatabilityLinting() bool
setStrictUpdatabilityLinting(bool) SetStrictUpdatabilityLinting(bool)
} }
type LintDepSets struct { type LintDepSets struct {
@@ -158,15 +158,15 @@ func (l *linter) LintDepSets() LintDepSets {
return l.outputs.depSets return l.outputs.depSets
} }
func (l *linter) getStrictUpdatabilityLinting() bool { func (l *linter) GetStrictUpdatabilityLinting() bool {
return BoolDefault(l.properties.Lint.Strict_updatability_linting, false) 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 l.properties.Lint.Strict_updatability_linting = &strictLinting
} }
var _ lintDepSetsIntf = (*linter)(nil) var _ LintDepSetsIntf = (*linter)(nil)
var _ lintOutputsIntf = (*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("--error_check ", l.properties.Lint.Error_checks)
cmd.FlagForEachArg("--fatal_check ", l.properties.Lint.Fatal_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. // Verify the module does not baseline issues that endanger safe updatability.
if baselinePath := l.getBaselineFilepath(ctx); baselinePath.Valid() { if baselinePath := l.getBaselineFilepath(ctx); baselinePath.Valid() {
cmd.FlagWithInput("--baseline ", baselinePath.Path()) cmd.FlagWithInput("--baseline ", baselinePath.Path())
@@ -382,7 +382,7 @@ func (l *linter) lint(ctx android.ModuleContext) {
depSetsBuilder := NewLintDepSetBuilder().Direct(html, text, xml) depSetsBuilder := NewLintDepSetBuilder().Direct(html, text, xml)
ctx.VisitDirectDepsWithTag(staticLibTag, func(dep android.Module) { ctx.VisitDirectDepsWithTag(staticLibTag, func(dep android.Module) {
if depLint, ok := dep.(lintDepSetsIntf); ok { if depLint, ok := dep.(LintDepSetsIntf); ok {
depSetsBuilder.Transitive(depLint.LintDepSets()) 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. // Enforce the strict updatability linting to all applicable transitive dependencies.
func enforceStrictUpdatabilityLintingMutator(ctx android.TopDownMutatorContext) { func enforceStrictUpdatabilityLintingMutator(ctx android.TopDownMutatorContext) {
m := ctx.Module() 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) { ctx.VisitDirectDepsWithTag(staticLibTag, func(d android.Module) {
if a, ok := d.(lintDepSetsIntf); ok { if a, ok := d.(LintDepSetsIntf); ok {
a.setStrictUpdatabilityLinting(true) a.SetStrictUpdatabilityLinting(true)
} }
}) })
} }

View File

@@ -2362,17 +2362,17 @@ func (module *SdkLibraryImport) LintDepSets() LintDepSets {
} }
} }
func (module *SdkLibraryImport) getStrictUpdatabilityLinting() bool { func (module *SdkLibraryImport) GetStrictUpdatabilityLinting() bool {
if module.implLibraryModule == nil { if module.implLibraryModule == nil {
return false return false
} else { } 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 { if module.implLibraryModule != nil {
module.implLibraryModule.setStrictUpdatabilityLinting(strictLinting) module.implLibraryModule.SetStrictUpdatabilityLinting(strictLinting)
} }
} }