Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 18cb570c authored by Colin Cross's avatar Colin Cross
Browse files

Only propgate strict updatability linting to modules that are in the apex 

apex_strict_updatability_lint was propagating strict updatability
linting much further than necessary by following all transitive
dependencies, not just static dependencies of modules that are in
the apex.  This caused unnecessary lint checks, requiring the
libcore exception in b/208656169 and the allowlist in b/215736885.
Use apex.WalkPayloadDeps instead, which tracks whether the dependency
is inside the same apex or not, and stop propagating the flag when
a module outside the apex is reached.

Bug: 208656169
Bug: 215736885
Test: m lint-check
Flag: EXEMPT refactor
Change-Id: I8d801ff1414e0bdb5997d29ab453488d3ab76d64
parent 8bf14fcb

apex/apex.go

+3 −22
Original line number Diff line number Diff line
@@ -1127,19 +1127,11 @@ func apexStrictUpdatibilityLintMutator(mctx android.TopDownMutatorContext) { 
		return

	}

	if apex, ok := mctx.Module().(*apexBundle); ok && apex.checkStrictUpdatabilityLinting(mctx) {

		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 android.InList(child.Name(), skipLintJavalibAllowlist) {

		apex.WalkPayloadDeps(mctx, func(mctx android.BaseModuleContext, from blueprint.Module, to android.ApexModule, externalDep bool) bool {

			if externalDep {

				return false

			}

			if lintable, ok := child.(java.LintDepSetsIntf); ok {

			if lintable, ok := to.(java.LintDepSetsIntf); ok {

				lintable.SetStrictUpdatabilityLinting(true)

			}

			// visit transitive deps
@@ -1208,17 +1200,6 @@ var ( 
		"test_jitzygote_com.android.art",

		// go/keep-sorted end

	}



	// TODO: b/215736885 Remove this list

	skipLintJavalibAllowlist = []string{

		"conscrypt.module.platform.api.stubs",

		"conscrypt.module.public.api.stubs",

		"conscrypt.module.public.api.stubs.system",

		"conscrypt.module.public.api.stubs.module_lib",

		"framework-media.stubs",

		"framework-media.stubs.system",

		"framework-media.stubs.module_lib",

	}

)



func (a *apexBundle) checkStrictUpdatabilityLinting(mctx android.TopDownMutatorContext) bool {

apex/apex_test.go

+0 −61
Original line number Diff line number Diff line
@@ -10055,67 +10055,6 @@ func TestApexStrictUpdtabilityLint(t *testing.T) { 
	}

}



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",

			lint: {

				baseline_filename: "lint-baseline.xml",

			}

		}

		`



	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, result, 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 := `