From a3d0986cfc352eb784d659825ff93772963dcbd5 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Tue, 11 Jun 2019 13:40:47 +0100 Subject: [PATCH 1/2] Remove the no_standard_libs property Corrects an error message that refers to no_standard_libs. Removes any tests that use no_standard_libs:true where possible as there are duplicate tests for sdk_version:"none". Otherwise, switches them over to use sdk_version:"none". The androidmk mapping from LOCAL_NO_STANDARD_LIBRARIES to no_standard_libs has also been removed. There was little point in updating the tool to map it through to sdk_version:"none" as there are only a couple of places where it is used, in art's test running mk targets and in some unbundled packages to work around some limitation in .mk based build. Bug: 134566750 Test: m droid Change-Id: I6413c9b1fe3e63b93753a6a017d2981e32b7e013 --- android/neverallow.go | 18 ++--------- android/neverallow_test.go | 48 ++---------------------------- androidmk/cmd/androidmk/android.go | 1 - java/aar.go | 4 --- java/droiddoc.go | 8 ----- java/java.go | 12 ++------ java/java_test.go | 2 +- java/sdk.go | 12 ++------ java/sdk_library.go | 1 - java/sdk_test.go | 22 -------------- java/testing.go | 8 ++--- 11 files changed, 14 insertions(+), 122 deletions(-) diff --git a/android/neverallow.go b/android/neverallow.go index f75138991..f35d1fed9 100644 --- a/android/neverallow.go +++ b/android/neverallow.go @@ -52,7 +52,6 @@ func createNeverAllows() []*rule { rules = append(rules, createTrebleRules()...) rules = append(rules, createLibcoreRules()...) rules = append(rules, createJavaDeviceForHostRules()...) - rules = append(rules, createJavaLibraryHostRules()...) return rules } @@ -101,13 +100,9 @@ func createLibcoreRules() []*rule { "development", } - // Core library constraints. The no_standard_libs can only be used in core - // library projects. Access to core library targets is restricted using - // visibility rules. + // Core library constraints. The sdk_version: "none" can only be used in core library projects. + // Access to core library targets is restricted using visibility rules. rules := []*rule{ - neverallow(). - notIn(coreLibraryProjects...). - with("no_standard_libs", "true"), neverallow(). notIn(coreLibraryProjects...). with("sdk_version", "none"), @@ -131,15 +126,6 @@ func createJavaDeviceForHostRules() []*rule { } } -func createJavaLibraryHostRules() []*rule { - return []*rule{ - neverallow(). - moduleType("java_library_host"). - with("no_standard_libs", "true"). - because("no_standard_libs makes no sense with java_library_host"), - } -} - func neverallowMutator(ctx BottomUpMutatorContext) { m, ok := ctx.Module().(Module) if !ok { diff --git a/android/neverallow_test.go b/android/neverallow_test.go index 62c5142d2..ee3c94f0c 100644 --- a/android/neverallow_test.go +++ b/android/neverallow_test.go @@ -147,37 +147,6 @@ var neverallowTests = []struct { expectedError: "java_device_for_host can only be used in whitelisted projects", }, // Libcore rule tests - { - name: "no_standard_libs: true inside core libraries", - fs: map[string][]byte{ - "libcore/Blueprints": []byte(` - java_library { - name: "inside_core_libraries", - no_standard_libs: true, - }`), - }, - }, - { - name: "no_standard_libs: true outside core libraries", - fs: map[string][]byte{ - "Blueprints": []byte(` - java_library { - name: "outside_core_libraries", - no_standard_libs: true, - }`), - }, - expectedError: "module \"outside_core_libraries\": violates neverallow", - }, - { - name: "no_standard_libs: false", - fs: map[string][]byte{ - "Blueprints": []byte(` - java_library { - name: "outside_core_libraries", - no_standard_libs: false, - }`), - }, - }, { name: "sdk_version: \"none\" inside core libraries", fs: map[string][]byte{ @@ -209,18 +178,6 @@ var neverallowTests = []struct { }`), }, }, - // java_library_host rule tests - { - name: "java_library_host with no_standard_libs: true", - fs: map[string][]byte{ - "libcore/Blueprints": []byte(` - java_library_host { - name: "inside_core_libraries", - no_standard_libs: true, - }`), - }, - expectedError: "module \"inside_core_libraries\": violates neverallow", - }, } func TestNeverallow(t *testing.T) { @@ -295,9 +252,8 @@ func (p *mockCcLibraryModule) GenerateAndroidBuildActions(ModuleContext) { } type mockJavaLibraryProperties struct { - Libs []string - No_standard_libs *bool - Sdk_version *string + Libs []string + Sdk_version *string } type mockJavaLibraryModule struct { diff --git a/androidmk/cmd/androidmk/android.go b/androidmk/cmd/androidmk/android.go index 62aa3dc48..af81e439b 100644 --- a/androidmk/cmd/androidmk/android.go +++ b/androidmk/cmd/androidmk/android.go @@ -185,7 +185,6 @@ func init() { "LOCAL_NO_CRT": "nocrt", "LOCAL_ALLOW_UNDEFINED_SYMBOLS": "allow_undefined_symbols", "LOCAL_RTTI_FLAG": "rtti", - "LOCAL_NO_STANDARD_LIBRARIES": "no_standard_libs", "LOCAL_PACK_MODULE_RELOCATIONS": "pack_relocations", "LOCAL_TIDY": "tidy", "LOCAL_USE_CLANG_LLD": "use_clang_lld", diff --git a/java/aar.go b/java/aar.go index adea87ff6..47f6e5f6a 100644 --- a/java/aar.go +++ b/java/aar.go @@ -517,10 +517,6 @@ func (a *AARImport) noFrameworkLibs() bool { return false } -func (a *AARImport) noStandardLibs() bool { - return false -} - var _ AndroidLibraryDependency = (*AARImport)(nil) func (a *AARImport) ExportPackage() android.Path { diff --git a/java/droiddoc.go b/java/droiddoc.go index e1476a27f..a8cf1c034 100644 --- a/java/droiddoc.go +++ b/java/droiddoc.go @@ -171,10 +171,6 @@ type JavadocProperties struct { // list of java libraries that will be in the classpath. Libs []string `android:"arch_variant"` - // don't build against the default libraries (bootclasspath, ext, and framework for device - // targets) - No_standard_libs *bool - // don't build against the framework libraries (ext, and framework for device targets) No_framework_libs *bool @@ -542,10 +538,6 @@ func (j *Javadoc) noFrameworkLibs() bool { return Bool(j.properties.No_framework_libs) } -func (j *Javadoc) noStandardLibs() bool { - return Bool(j.properties.No_standard_libs) -} - func (j *Javadoc) addDeps(ctx android.BottomUpMutatorContext) { if ctx.Device() { sdkDep := decodeSdkDep(ctx, sdkContext(j)) diff --git a/java/java.go b/java/java.go index c42ca28e8..2a5c5d3ed 100644 --- a/java/java.go +++ b/java/java.go @@ -82,10 +82,6 @@ type CompilerProperties struct { // list of files that should be excluded from java_resources and java_resource_dirs Exclude_java_resources []string `android:"path,arch_variant"` - // don't build against the default libraries (bootclasspath, ext, and framework for device - // targets) - No_standard_libs *bool - // don't build against the framework libraries (ext, and framework for device targets) No_framework_libs *bool @@ -490,10 +486,6 @@ func (j *Module) noFrameworkLibs() bool { return Bool(j.properties.No_framework_libs) } -func (j *Module) noStandardLibs() bool { - return Bool(j.properties.No_standard_libs) -} - func (j *Module) deps(ctx android.BottomUpMutatorContext) { if ctx.Device() { sdkDep := decodeSdkDep(ctx, sdkContext(j)) @@ -513,8 +505,8 @@ func (j *Module) deps(ctx android.BottomUpMutatorContext) { } } } else if j.deviceProperties.System_modules == nil { - ctx.PropertyErrorf("no_standard_libs", - "system_modules is required to be set when no_standard_libs is true, did you mean no_framework_libs?") + ctx.PropertyErrorf("sdk_version", + `system_modules is required to be set when sdk_version is "none", did you mean no_framework_libs?`) } else if *j.deviceProperties.System_modules != "none" { ctx.AddVariationDependencies(nil, systemModulesTag, *j.deviceProperties.System_modules) } diff --git a/java/java_test.go b/java/java_test.go index cecc8dacd..22dec073f 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -1013,7 +1013,7 @@ func TestPatchModule(t *testing.T) { java_library { name: "bar", srcs: ["b.java"], - no_standard_libs: true, + sdk_version: "none", system_modules: "none", patch_module: "java.base", } diff --git a/java/sdk.go b/java/sdk.go index 9dfb38bf4..e01b730df 100644 --- a/java/sdk.go +++ b/java/sdk.go @@ -45,9 +45,6 @@ type sdkContext interface { // targetSdkVersion returns the target_sdk_version property of the current module, or sdkVersion() if it is not set. targetSdkVersion() string - // Temporarily provide access to the no_standard_libs property (where present). - noStandardLibs() bool - // Temporarily provide access to the no_frameworks_libs property (where present). noFrameworkLibs() bool } @@ -145,8 +142,7 @@ func decodeSdkDep(ctx android.BaseModuleContext, sdkContext sdkContext) sdkDep { jars: android.Paths{jarPath.Path(), lambdaStubsPath}, aidl: android.OptionalPathForPath(aidlPath.Path()), - // Pass values straight through for now to match previous behavior. - noStandardLibs: sdkContext.noStandardLibs(), + // Pass value straight through for now to match previous behavior. noFrameworksLibs: sdkContext.noFrameworkLibs(), } } @@ -159,8 +155,7 @@ func decodeSdkDep(ctx android.BaseModuleContext, sdkContext sdkContext) sdkDep { frameworkResModule: r, aidl: android.OptionalPathForPath(aidl), - // Pass values straight through for now to match previous behavior. - noStandardLibs: sdkContext.noStandardLibs(), + // Pass value straight through for now to match previous behavior. noFrameworksLibs: sdkContext.noFrameworkLibs(), } @@ -197,8 +192,7 @@ func decodeSdkDep(ctx android.BaseModuleContext, sdkContext sdkContext) sdkDep { useDefaultLibs: true, frameworkResModule: "framework-res", - // Pass values straight through for now to match previous behavior. - noStandardLibs: sdkContext.noStandardLibs(), + // Pass value straight through for now to match previous behavior. noFrameworksLibs: sdkContext.noFrameworkLibs(), } case "none": diff --git a/java/sdk_library.go b/java/sdk_library.go index 01531c5c6..b4a3f296c 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -385,7 +385,6 @@ func (module *SdkLibrary) createStubsLibrary(mctx android.LoadHookContext, apiSc Device_specific *bool Product_specific *bool Compile_dex *bool - No_standard_libs *bool System_modules *string Java_version *string Product_variables struct { diff --git a/java/sdk_test.go b/java/sdk_test.go index 1efe83b74..32de8802a 100644 --- a/java/sdk_test.go +++ b/java/sdk_test.go @@ -111,14 +111,6 @@ func TestClasspath(t *testing.T) { bootclasspath: []string{"core.current.stubs", "core-lambda-stubs"}, system: "bootclasspath", // special value to tell 1.9 test to expect bootclasspath }, - { - - name: "nostdlib - no_standard_libs: true", - properties: `no_standard_libs: true, system_modules: "none"`, - system: "none", - bootclasspath: []string{`""`}, - classpath: []string{}, - }, { name: "nostdlib", @@ -127,14 +119,6 @@ func TestClasspath(t *testing.T) { bootclasspath: []string{`""`}, classpath: []string{}, }, - { - - name: "nostdlib system_modules - no_standard_libs: true", - properties: `no_standard_libs: true, system_modules: "core-platform-api-stubs-system-modules"`, - system: "core-platform-api-stubs-system-modules", - bootclasspath: []string{`""`}, - classpath: []string{}, - }, { name: "nostdlib system_modules", @@ -160,12 +144,6 @@ func TestClasspath(t *testing.T) { classpath: []string{}, bootclasspath: []string{"jdk8/jre/lib/jce.jar", "jdk8/jre/lib/rt.jar"}, }, - { - name: "host supported nostdlib - no_standard_libs: true", - host: android.Host, - properties: `host_supported: true, no_standard_libs: true, system_modules: "none"`, - classpath: []string{}, - }, { name: "host supported nostdlib", host: android.Host, diff --git a/java/testing.go b/java/testing.go index 7cd187164..18fd30af2 100644 --- a/java/testing.go +++ b/java/testing.go @@ -64,7 +64,7 @@ func GatherRequiredDepsForTest() string { java_library { name: "framework", srcs: ["a.java"], - no_standard_libs: true, + sdk_version: "none", system_modules: "core-platform-api-stubs-system-modules", aidl: { export_include_dirs: ["framework/aidl"], @@ -79,7 +79,7 @@ func GatherRequiredDepsForTest() string { java_library { name: "android.hidl.base-V1.0-java", srcs: ["a.java"], - no_standard_libs: true, + sdk_version: "none", system_modules: "core-platform-api-stubs-system-modules", installable: true, } @@ -87,7 +87,7 @@ func GatherRequiredDepsForTest() string { java_library { name: "android.hidl.manager-V1.0-java", srcs: ["a.java"], - no_standard_libs: true, + sdk_version: "none", system_modules: "core-platform-api-stubs-system-modules", installable: true, } @@ -95,7 +95,7 @@ func GatherRequiredDepsForTest() string { java_library { name: "org.apache.http.legacy", srcs: ["a.java"], - no_standard_libs: true, + sdk_version: "none", system_modules: "core-platform-api-stubs-system-modules", installable: true, } From 50c217c744901bf8a2408f110840bf22225ae361 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Wed, 12 Jun 2019 13:25:22 +0100 Subject: [PATCH 2/2] Add sdk_version:"core_platform" to replace no_framework_libs:true Where possible this duplicates any tests that use no_framework_libs:true with ones that use sdk_version:"core_platform". If not possible (e.g. in the default targets included in java/testing.go) it switches some to use sdk_version:"core_platform" to ensure that there is no regression in the behavior of no_framework_libs:true. Follow up changes will switch all usages of no_framework_libs:true over to use sdk_version:"core_platform" at which point no_framework_libs will be removed. Bug: 134566750 Test: m droid Change-Id: I42cb181f628b723c8f32a158ae4752b4c83365ae --- java/app_test.go | 159 ++++++++++++++++++++++++++++++++++++++++++++++- java/java.go | 11 +++- java/sdk.go | 13 +++- java/sdk_test.go | 8 +++ java/testing.go | 2 +- 5 files changed, 184 insertions(+), 9 deletions(-) diff --git a/java/app_test.go b/java/app_test.go index bb39c165c..27802cd21 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -546,7 +546,7 @@ func TestAppSdkVersion(t *testing.T) { } } -func TestJNIABI(t *testing.T) { +func TestJNIABI_no_framework_libs_true(t *testing.T) { ctx := testJava(t, cc.GatherRequiredDepsForTest(android.Android)+` cc_library { name: "libjni", @@ -619,7 +619,80 @@ func TestJNIABI(t *testing.T) { } } -func TestJNIPackaging(t *testing.T) { +func TestJNIABI(t *testing.T) { + ctx := testJava(t, cc.GatherRequiredDepsForTest(android.Android)+` + cc_library { + name: "libjni", + system_shared_libs: [], + stl: "none", + } + + android_test { + name: "test", + sdk_version: "core_platform", + jni_libs: ["libjni"], + } + + android_test { + name: "test_first", + sdk_version: "core_platform", + compile_multilib: "first", + jni_libs: ["libjni"], + } + + android_test { + name: "test_both", + sdk_version: "core_platform", + compile_multilib: "both", + jni_libs: ["libjni"], + } + + android_test { + name: "test_32", + sdk_version: "core_platform", + compile_multilib: "32", + jni_libs: ["libjni"], + } + + android_test { + name: "test_64", + sdk_version: "core_platform", + compile_multilib: "64", + jni_libs: ["libjni"], + } + `) + + testCases := []struct { + name string + abis []string + }{ + {"test", []string{"arm64-v8a"}}, + {"test_first", []string{"arm64-v8a"}}, + {"test_both", []string{"arm64-v8a", "armeabi-v7a"}}, + {"test_32", []string{"armeabi-v7a"}}, + {"test_64", []string{"arm64-v8a"}}, + } + + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + app := ctx.ModuleForTests(test.name, "android_common") + jniLibZip := app.Output("jnilibs.zip") + var abis []string + args := strings.Fields(jniLibZip.Args["jarArgs"]) + for i := 0; i < len(args); i++ { + if args[i] == "-P" { + abis = append(abis, filepath.Base(args[i+1])) + i++ + } + } + if !reflect.DeepEqual(abis, test.abis) { + t.Errorf("want abis %v, got %v", test.abis, abis) + } + }) + } +} + +func TestJNIPackaging_no_framework_libs_true(t *testing.T) { ctx := testJava(t, cc.GatherRequiredDepsForTest(android.Android)+` cc_library { name: "libjni", @@ -700,7 +773,89 @@ func TestJNIPackaging(t *testing.T) { } }) } +} +func TestJNIPackaging(t *testing.T) { + ctx := testJava(t, cc.GatherRequiredDepsForTest(android.Android)+` + cc_library { + name: "libjni", + system_shared_libs: [], + stl: "none", + } + + android_app { + name: "app", + jni_libs: ["libjni"], + } + + android_app { + name: "app_noembed", + jni_libs: ["libjni"], + use_embedded_native_libs: false, + } + + android_app { + name: "app_embed", + jni_libs: ["libjni"], + use_embedded_native_libs: true, + } + + android_test { + name: "test", + sdk_version: "core_platform", + jni_libs: ["libjni"], + } + + android_test { + name: "test_noembed", + sdk_version: "core_platform", + jni_libs: ["libjni"], + use_embedded_native_libs: false, + } + + android_test_helper_app { + name: "test_helper", + sdk_version: "core_platform", + jni_libs: ["libjni"], + } + + android_test_helper_app { + name: "test_helper_noembed", + sdk_version: "core_platform", + jni_libs: ["libjni"], + use_embedded_native_libs: false, + } + `) + + testCases := []struct { + name string + packaged bool + compressed bool + }{ + {"app", false, false}, + {"app_noembed", false, false}, + {"app_embed", true, false}, + {"test", true, false}, + {"test_noembed", true, true}, + {"test_helper", true, false}, + {"test_helper_noembed", true, true}, + } + + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + app := ctx.ModuleForTests(test.name, "android_common") + jniLibZip := app.MaybeOutput("jnilibs.zip") + if g, w := (jniLibZip.Rule != nil), test.packaged; g != w { + t.Errorf("expected jni packaged %v, got %v", w, g) + } + + if jniLibZip.Rule != nil { + if g, w := !strings.Contains(jniLibZip.Args["jarArgs"], "-L 0"), test.compressed; g != w { + t.Errorf("expected jni compressed %v, got %v", w, g) + } + } + }) + } } func TestCertificates(t *testing.T) { diff --git a/java/java.go b/java/java.go index 2a5c5d3ed..4b3845161 100644 --- a/java/java.go +++ b/java/java.go @@ -674,7 +674,7 @@ func getLinkType(m *Module, name string) (ret linkType, stubs bool) { return javaSdk, true case ver == "current": return javaSdk, false - case ver == "" || ver == "none": + case ver == "" || ver == "none" || ver == "core_platform": return javaPlatform, false default: if _, err := strconv.Atoi(ver); err != nil { @@ -852,7 +852,8 @@ func getJavaVersion(ctx android.ModuleContext, javaVersion string, sdkContext sd var ret string v := sdkContext.sdkVersion() // For PDK builds, use the latest SDK version instead of "current" - if ctx.Config().IsPdkBuild() && (v == "" || v == "none" || v == "current") { + if ctx.Config().IsPdkBuild() && + (v == "" || v == "none" || v == "core_platform" || v == "current") { sdkVersions := ctx.Config().Get(sdkVersionsKey).([]int) latestSdkVersion := 0 if len(sdkVersions) > 0 { @@ -871,7 +872,11 @@ func getJavaVersion(ctx android.ModuleContext, javaVersion string, sdkContext sd ret = "1.7" } else if ctx.Device() && sdk <= 29 || !ctx.Config().TargetOpenJDK9() { ret = "1.8" - } else if ctx.Device() && sdkContext.sdkVersion() != "" && sdkContext.sdkVersion() != "none" && sdk == android.FutureApiLevel { + } else if ctx.Device() && + sdkContext.sdkVersion() != "" && + sdkContext.sdkVersion() != "none" && + sdkContext.sdkVersion() != "core_platform" && + sdk == android.FutureApiLevel { // TODO(ccross): once we generate stubs we should be able to use 1.9 for sdk_version: "current" ret = "1.8" } else { diff --git a/java/sdk.go b/java/sdk.go index e01b730df..6ffe399fb 100644 --- a/java/sdk.go +++ b/java/sdk.go @@ -51,7 +51,7 @@ type sdkContext interface { func sdkVersionOrDefault(ctx android.BaseModuleContext, v string) string { switch v { - case "", "none", "current", "system_current", "test_current", "core_current": + case "", "none", "current", "test_current", "system_current", "core_current", "core_platform": return ctx.Config().DefaultAppTargetSdk() default: return v @@ -62,7 +62,7 @@ func sdkVersionOrDefault(ctx android.BaseModuleContext, v string) string { // it returns android.FutureApiLevel (10000). func sdkVersionToNumber(ctx android.BaseModuleContext, v string) (int, error) { switch v { - case "", "none", "current", "test_current", "system_current", "core_current": + case "", "none", "current", "test_current", "system_current", "core_current", "core_platform": return ctx.Config().DefaultAppTargetSdkInt(), nil default: n := android.GetNumericSdkVersion(v) @@ -182,7 +182,8 @@ func decodeSdkDep(ctx android.BaseModuleContext, sdkContext sdkContext) sdkDep { } } - if ctx.Config().UnbundledBuildUsePrebuiltSdks() && v != "" && v != "none" { + if ctx.Config().UnbundledBuildUsePrebuiltSdks() && + v != "" && v != "none" && v != "core_platform" { return toPrebuilt(v) } @@ -199,6 +200,12 @@ func decodeSdkDep(ctx android.BaseModuleContext, sdkContext sdkContext) sdkDep { return sdkDep{ noStandardLibs: true, } + case "core_platform": + return sdkDep{ + useDefaultLibs: true, + frameworkResModule: "framework-res", + noFrameworksLibs: true, + } case "current": return toModule("android_stubs_current", "framework-res", sdkFrameworkAidlPath(ctx)) case "system_current": diff --git a/java/sdk_test.go b/java/sdk_test.go index 32de8802a..915333ec9 100644 --- a/java/sdk_test.go +++ b/java/sdk_test.go @@ -54,6 +54,14 @@ func TestClasspath(t *testing.T) { classpath: []string{}, aidl: "", }, + { + name: `sdk_version:"core_platform"`, + properties: `sdk_version:"core_platform"`, + bootclasspath: config.DefaultBootclasspathLibraries, + system: config.DefaultSystemModules, + classpath: []string{}, + aidl: "", + }, { name: "blank sdk version", properties: `sdk_version: "",`, diff --git a/java/testing.go b/java/testing.go index 18fd30af2..5d116a787 100644 --- a/java/testing.go +++ b/java/testing.go @@ -73,7 +73,7 @@ func GatherRequiredDepsForTest() string { android_app { name: "framework-res", - no_framework_libs: true, + sdk_version: "core_platform", } java_library {