Browse Source

Merge changes Ie8881b85,I6c064f8a

* changes:
  Enforce updatable=true on apps of updatable apexes
  Update sdk_version check for jni_libs of updatable apps
Spandan Das 2 years ago
parent
commit
f923333a93
4 changed files with 96 additions and 11 deletions
  1. 18 1
      apex/apex.go
  2. 64 0
      apex/apex_test.go
  3. 7 7
      java/app.go
  4. 7 3
      java/app_test.go

+ 18 - 1
apex/apex.go

@@ -977,6 +977,7 @@ type ApexInfoMutator interface {
 
 // apexInfoMutator delegates the work of identifying which modules need an ApexInfo and apex
 // specific variant to modules that support the ApexInfoMutator.
+// It also propagates updatable=true to apps of updatable apexes
 func apexInfoMutator(mctx android.TopDownMutatorContext) {
 	if !mctx.Module().Enabled() {
 		return
@@ -984,8 +985,8 @@ func apexInfoMutator(mctx android.TopDownMutatorContext) {
 
 	if a, ok := mctx.Module().(ApexInfoMutator); ok {
 		a.ApexInfoMutator(mctx)
-		return
 	}
+	enforceAppUpdatability(mctx)
 }
 
 // apexStrictUpdatibilityLintMutator propagates strict_updatability_linting to transitive deps of a mainline module
@@ -1016,6 +1017,22 @@ func apexStrictUpdatibilityLintMutator(mctx android.TopDownMutatorContext) {
 	}
 }
 
+// enforceAppUpdatability propagates updatable=true to apps of updatable apexes
+func enforceAppUpdatability(mctx android.TopDownMutatorContext) {
+	if !mctx.Module().Enabled() {
+		return
+	}
+	if apex, ok := mctx.Module().(*apexBundle); ok && apex.Updatable() {
+		// checking direct deps is sufficient since apex->apk is a direct edge, even when inherited via apex_defaults
+		mctx.VisitDirectDeps(func(module android.Module) {
+			// ignore android_test_app
+			if app, ok := module.(*java.AndroidApp); ok {
+				app.SetUpdatable(true)
+			}
+		})
+	}
+}
+
 // TODO: b/215736885 Whittle the denylist
 // Transitive deps of certain mainline modules baseline NewApi errors
 // Skip these mainline modules for now

+ 64 - 0
apex/apex_test.go

@@ -2394,6 +2394,7 @@ func TestApexMinSdkVersion_ErrorIfDepIsNewer_Java(t *testing.T) {
 			key: "myapex.key",
 			apps: ["AppFoo"],
 			min_sdk_version: "29",
+			updatable: false,
 		}
 
 		apex_key {
@@ -9355,6 +9356,69 @@ func TestApexStrictUpdtabilityLintBcpFragmentDeps(t *testing.T) {
 	}
 }
 
+// updatable apexes should propagate updatable=true to its apps
+func TestUpdatableApexEnforcesAppUpdatability(t *testing.T) {
+	bp := `
+		apex {
+			name: "myapex",
+			key: "myapex.key",
+			updatable: %v,
+			apps: [
+				"myapp",
+			],
+			min_sdk_version: "30",
+		}
+		apex_key {
+			name: "myapex.key",
+		}
+		android_app {
+			name: "myapp",
+			updatable: %v,
+			apex_available: [
+				"myapex",
+			],
+			sdk_version: "current",
+			min_sdk_version: "30",
+		}
+		`
+	testCases := []struct {
+		name                      string
+		apex_is_updatable_bp      bool
+		app_is_updatable_bp       bool
+		app_is_updatable_expected bool
+	}{
+		{
+			name:                      "Non-updatable apex respects updatable property of non-updatable app",
+			apex_is_updatable_bp:      false,
+			app_is_updatable_bp:       false,
+			app_is_updatable_expected: false,
+		},
+		{
+			name:                      "Non-updatable apex respects updatable property of updatable app",
+			apex_is_updatable_bp:      false,
+			app_is_updatable_bp:       true,
+			app_is_updatable_expected: true,
+		},
+		{
+			name:                      "Updatable apex respects updatable property of updatable app",
+			apex_is_updatable_bp:      true,
+			app_is_updatable_bp:       true,
+			app_is_updatable_expected: true,
+		},
+		{
+			name:                      "Updatable apex sets updatable=true on non-updatable app",
+			apex_is_updatable_bp:      true,
+			app_is_updatable_bp:       false,
+			app_is_updatable_expected: true,
+		},
+	}
+	for _, testCase := range testCases {
+		result := testApex(t, fmt.Sprintf(bp, testCase.apex_is_updatable_bp, testCase.app_is_updatable_bp))
+		myapp := result.ModuleForTests("myapp", "android_common").Module().(*java.AndroidApp)
+		android.AssertBoolEquals(t, testCase.name, testCase.app_is_updatable_expected, myapp.Updatable())
+	}
+}
+
 func TestMain(m *testing.M) {
 	os.Exit(m.Run())
 }

+ 7 - 7
java/app.go

@@ -299,10 +299,6 @@ func (a *AndroidApp) checkAppSdkVersions(ctx android.ModuleContext) {
 
 // If an updatable APK sets min_sdk_version, min_sdk_vesion of JNI libs should match with it.
 // This check is enforced for "updatable" APKs (including APK-in-APEX).
-// b/155209650: until min_sdk_version is properly supported, use sdk_version instead.
-// because, sdk_version is overridden by min_sdk_version (if set as smaller)
-// and sdkLinkType is checked with dependencies so we can be sure that the whole dependency tree
-// will meet the requirements.
 func (a *AndroidApp) checkJniLibsSdkVersion(ctx android.ModuleContext, minSdkVersion android.ApiLevel) {
 	// It's enough to check direct JNI deps' sdk_version because all transitive deps from JNI deps are checked in cc.checkLinkType()
 	ctx.VisitDirectDeps(func(m android.Module) {
@@ -313,10 +309,10 @@ func (a *AndroidApp) checkJniLibsSdkVersion(ctx android.ModuleContext, minSdkVer
 		// The domain of cc.sdk_version is "current" and <number>
 		// We can rely on android.SdkSpec to convert it to <number> so that "current" is
 		// handled properly regardless of sdk finalization.
-		jniSdkVersion, err := android.SdkSpecFrom(ctx, dep.SdkVersion()).EffectiveVersion(ctx)
+		jniSdkVersion, err := android.SdkSpecFrom(ctx, dep.MinSdkVersion()).EffectiveVersion(ctx)
 		if err != nil || minSdkVersion.LessThan(jniSdkVersion) {
-			ctx.OtherModuleErrorf(dep, "sdk_version(%v) is higher than min_sdk_version(%v) of the containing android_app(%v)",
-				dep.SdkVersion(), minSdkVersion, ctx.ModuleName())
+			ctx.OtherModuleErrorf(dep, "min_sdk_version(%v) is higher than min_sdk_version(%v) of the containing android_app(%v)",
+				dep.MinSdkVersion(), minSdkVersion, ctx.ModuleName())
 			return
 		}
 
@@ -842,6 +838,10 @@ func (a *AndroidApp) Updatable() bool {
 	return Bool(a.appProperties.Updatable)
 }
 
+func (a *AndroidApp) SetUpdatable(val bool) {
+	a.appProperties.Updatable = &val
+}
+
 func (a *AndroidApp) getCertString(ctx android.BaseModuleContext) string {
 	certificate, overridden := ctx.DeviceConfig().OverrideCertificateFor(ctx.ModuleName())
 	if overridden {

+ 7 - 3
java/app_test.go

@@ -427,7 +427,8 @@ func TestUpdatableApps_JniLibShouldBeBuiltAgainstMinSdkVersion(t *testing.T) {
 			name: "libjni",
 			stl: "none",
 			system_shared_libs: [],
-			sdk_version: "29",
+			sdk_version: "current",
+			min_sdk_version: "29",
 		}
 	`
 	fs := map[string][]byte{
@@ -481,12 +482,13 @@ func TestUpdatableApps_ErrorIfJniLibDoesntSupportMinSdkVersion(t *testing.T) {
 			name: "libjni",
 			stl: "none",
 			sdk_version: "current",
+			min_sdk_version: "current",
 		}
 	`
-	testJavaError(t, `"libjni" .*: sdk_version\(current\) is higher than min_sdk_version\(29\)`, bp)
+	testJavaError(t, `"libjni" .*: min_sdk_version\(current\) is higher than min_sdk_version\(29\)`, bp)
 }
 
-func TestUpdatableApps_ErrorIfDepSdkVersionIsHigher(t *testing.T) {
+func TestUpdatableApps_ErrorIfDepMinSdkVersionIsHigher(t *testing.T) {
 	bp := cc.GatherRequiredDepsForTest(android.Android) + `
 		android_app {
 			name: "foo",
@@ -503,6 +505,7 @@ func TestUpdatableApps_ErrorIfDepSdkVersionIsHigher(t *testing.T) {
 			shared_libs: ["libbar"],
 			system_shared_libs: [],
 			sdk_version: "27",
+			min_sdk_version: "27",
 		}
 
 		cc_library {
@@ -510,6 +513,7 @@ func TestUpdatableApps_ErrorIfDepSdkVersionIsHigher(t *testing.T) {
 			stl: "none",
 			system_shared_libs: [],
 			sdk_version: "current",
+			min_sdk_version: "current",
 		}
 	`
 	testJavaError(t, `"libjni" .*: links "libbar" built against newer API version "current"`, bp)