From 7416d67f2878b960474afe8c420af8b14b7d9a5e Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Thu, 4 Jan 2024 23:20:42 +0000 Subject: [PATCH] Revert "Revert "Limit System SDK to 34 for Java modules in the v..." Revert submission 2897568-revert-2894701-limit_systemsdk-WNEMOTGMRS Reason for revert: Forward fix was merged Reverted changes: /q/submissionid:2897568-revert-2894701-limit_systemsdk-WNEMOTGMRS Change-Id: Id857406513fbf33a20e5d3836742ebd8a0105516 --- android/sdk_version.go | 85 ++++++++++++++++++++++++++++++++++------ android/test_config.go | 5 ++- java/Android.bp | 3 +- java/sdk_version_test.go | 66 +++++++++++++++++++++++++++++++ sysprop/sysprop_test.go | 6 +++ 5 files changed, 151 insertions(+), 14 deletions(-) create mode 100644 java/sdk_version_test.go diff --git a/android/sdk_version.go b/android/sdk_version.go index aafcee79a..9c84a27f8 100644 --- a/android/sdk_version.go +++ b/android/sdk_version.go @@ -16,6 +16,7 @@ package android import ( "fmt" + "reflect" "strconv" "strings" ) @@ -162,6 +163,17 @@ func (s SdkSpec) ForVendorPartition(ctx EarlyModuleContext) SdkSpec { // If BOARD_CURRENT_API_LEVEL_FOR_VENDOR_MODULES has a numeric value, // use it instead of "current" for the vendor partition. currentSdkVersion := ctx.DeviceConfig().CurrentApiLevelForVendorModules() + // b/314011075: special case for Java modules in vendor partition. They can no longer use + // SDK 35 or later. Their maximum API level is limited to 34 (Android U). This is to + // discourage the use of Java APIs in the vendor partition which hasn't been officially + // supported since the Project Treble back in Android 10. We would like to eventually + // evacuate all Java modules from the partition, but that shall be done progressively. + // Note that the check for the availability of SDK 34 is to not break existing tests where + // any of the frozen SDK version is unavailable. + if isJava(ctx.Module()) && isSdkVersion34AvailableIn(ctx.Config()) { + currentSdkVersion = "34" + } + if currentSdkVersion == "current" { return s } @@ -290,28 +302,79 @@ func SdkSpecFromWithConfig(config Config, str string) SdkSpec { } } +// Checks if the use of this SDK `s` is valid for the given module context `ctx`. func (s SdkSpec) ValidateSystemSdk(ctx EarlyModuleContext) bool { - // Ensures that the specified system SDK version is one of BOARD_SYSTEMSDK_VERSIONS (for vendor/product Java module) - // Assuming that BOARD_SYSTEMSDK_VERSIONS := 28 29, - // sdk_version of the modules in vendor/product that use system sdk must be either system_28, system_29 or system_current - if s.Kind != SdkSystem || s.ApiLevel.IsPreview() { + // Do some early checks. This check is currently only for Java modules. And our only concern + // is the use of "system" SDKs. + if !isJava(ctx.Module()) || s.Kind != SdkSystem { return true } - allowedVersions := ctx.DeviceConfig().PlatformSystemSdkVersions() - if ctx.DeviceSpecific() || ctx.SocSpecific() || (ctx.ProductSpecific() && ctx.Config().EnforceProductPartitionInterface()) { - systemSdkVersions := ctx.DeviceConfig().SystemSdkVersions() - if len(systemSdkVersions) > 0 { - allowedVersions = systemSdkVersions + + inVendor := ctx.DeviceSpecific() || ctx.SocSpecific() + inProduct := ctx.ProductSpecific() + isProductUnbundled := ctx.Config().EnforceProductPartitionInterface() + inApex := false + if am, ok := ctx.Module().(ApexModule); ok { + inApex = am.InAnyApex() + } + isUnbundled := inVendor || (inProduct && isProductUnbundled) || inApex + + // Bundled modules can use any SDK + if !isUnbundled { + return true + } + + // Unbundled modules are allowed to use BOARD_SYSTEMSDK_VERSIONS + supportedVersions := ctx.DeviceConfig().SystemSdkVersions() + + // b/314011075: special case for vendor modules. Java modules in the vendor partition can + // not use SDK 35 or later. This is to discourage the use of Java APIs in the vendor + // partition which hasn't been officially supported since the Project Treble back in Android + // 10. We would like to eventually evacuate all Java modules from the partition, but that + // shall be done progressively. + if inVendor { + // 28 was the API when BOARD_SYSTEMSDK_VERSIONS was introduced, so that's the oldest + // we should allow. + supportedVersions = []string{} + for v := 28; v <= 34; v++ { + supportedVersions = append(supportedVersions, strconv.Itoa(v)) } } - if len(allowedVersions) > 0 && !InList(s.ApiLevel.String(), allowedVersions) { + + // APEXes in the system partition are still considered as part of the platform, thus can use + // more SDKs from PLATFORM_SYSTEMSDK_VERSIONS + if inApex && !inVendor { + supportedVersions = ctx.DeviceConfig().PlatformSystemSdkVersions() + } + + thisVer, err := s.EffectiveVersion(ctx) + if err != nil { + ctx.PropertyErrorf("sdk_version", "invalid sdk version %q", s.Raw) + return false + } + + thisVerString := strconv.Itoa(thisVer.FinalOrPreviewInt()) + if thisVer.IsPreview() { + thisVerString = *ctx.Config().productVariables.Platform_sdk_version_or_codename + } + + if !InList(thisVerString, supportedVersions) { ctx.PropertyErrorf("sdk_version", "incompatible sdk version %q. System SDK version should be one of %q", - s.Raw, allowedVersions) + s.Raw, supportedVersions) return false } return true } +func isJava(m Module) bool { + moduleType := reflect.TypeOf(m).String() + return strings.HasPrefix(moduleType, "*java.") +} + +func isSdkVersion34AvailableIn(c Config) bool { + return c.PlatformSdkVersion().FinalInt() >= 34 +} + func init() { RegisterMakeVarsProvider(pctx, javaSdkMakeVars) } diff --git a/android/test_config.go b/android/test_config.go index 9e1ac70df..a15343adb 100644 --- a/android/test_config.go +++ b/android/test_config.go @@ -39,11 +39,12 @@ func TestConfig(buildDir string, env map[string]string, bp string, fs map[string DeviceName: stringPtr("test_device"), DeviceProduct: stringPtr("test_product"), Platform_sdk_version: intPtr(30), + Platform_sdk_version_or_codename: stringPtr("S"), Platform_sdk_codename: stringPtr("S"), Platform_base_sdk_extension_version: intPtr(1), Platform_version_active_codenames: []string{"S", "Tiramisu"}, - DeviceSystemSdkVersions: []string{"14", "15"}, - Platform_systemsdk_versions: []string{"29", "30"}, + DeviceSystemSdkVersions: []string{"29", "30", "S"}, + Platform_systemsdk_versions: []string{"29", "30", "S", "Tiramisu"}, AAPTConfig: []string{"normal", "large", "xlarge", "hdpi", "xhdpi", "xxhdpi"}, AAPTPreferredConfig: stringPtr("xhdpi"), AAPTCharacteristics: stringPtr("nosdcard"), diff --git a/java/Android.bp b/java/Android.bp index 79cd3f923..2585cd23f 100644 --- a/java/Android.bp +++ b/java/Android.bp @@ -108,8 +108,9 @@ bootstrap_go_package { "prebuilt_apis_test.go", "proto_test.go", "rro_test.go", - "sdk_test.go", "sdk_library_test.go", + "sdk_test.go", + "sdk_version_test.go", "system_modules_test.go", "systemserver_classpath_fragment_test.go", "test_spec_test.go", diff --git a/java/sdk_version_test.go b/java/sdk_version_test.go new file mode 100644 index 000000000..88351d2ef --- /dev/null +++ b/java/sdk_version_test.go @@ -0,0 +1,66 @@ +// Copyright 2024 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 ( + "testing" + + "android/soong/android" +) + +func stringPtr(v string) *string { + return &v +} + +func TestSystemSdkFromVendor(t *testing.T) { + fixtures := android.GroupFixturePreparers( + PrepareForTestWithJavaDefaultModules, + android.FixtureModifyProductVariables(func(variables android.FixtureProductVariables) { + variables.Platform_sdk_version = intPtr(34) + variables.Platform_sdk_codename = stringPtr("VanillaIceCream") + variables.Platform_version_active_codenames = []string{"VanillaIceCream"} + variables.Platform_systemsdk_versions = []string{"33", "34", "VanillaIceCream"} + variables.DeviceSystemSdkVersions = []string{"VanillaIceCream"} + }), + FixtureWithPrebuiltApis(map[string][]string{ + "33": {}, + "34": {}, + "35": {}, + }), + ) + + fixtures.ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern("incompatible sdk version")). + RunTestWithBp(t, ` + android_app { + name: "foo", + srcs: ["a.java"], + vendor: true, + sdk_version: "system_35", + }`) + + result := fixtures.RunTestWithBp(t, ` + android_app { + name: "foo", + srcs: ["a.java"], + vendor: true, + sdk_version: "system_current", + }`) + fooModule := result.ModuleForTests("foo", "android_common") + fooClasspath := fooModule.Rule("javac").Args["classpath"] + + android.AssertStringDoesContain(t, "foo classpath", fooClasspath, "prebuilts/sdk/34/system/android.jar") + android.AssertStringDoesNotContain(t, "foo classpath", fooClasspath, "prebuilts/sdk/35/system/android.jar") + android.AssertStringDoesNotContain(t, "foo classpath", fooClasspath, "prebuilts/sdk/current/system/android.jar") +} diff --git a/sysprop/sysprop_test.go b/sysprop/sysprop_test.go index e51fe39c9..e5b3dea05 100644 --- a/sysprop/sysprop_test.go +++ b/sysprop/sysprop_test.go @@ -125,6 +125,12 @@ func test(t *testing.T, bp string) *android.TestResult { variables.DeviceSystemSdkVersions = []string{"28"} variables.DeviceVndkVersion = proptools.StringPtr("current") variables.Platform_vndk_version = proptools.StringPtr("29") + variables.DeviceCurrentApiLevelForVendorModules = proptools.StringPtr("28") + }), + java.FixtureWithPrebuiltApis(map[string][]string{ + "28": {}, + "29": {}, + "30": {}, }), mockFS.AddToFixture(), android.FixtureWithRootAndroidBp(bp),