From 2807f0047b213d6725bed98082d45d7445f69699 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 2 Mar 2021 10:15:29 -0800 Subject: [PATCH 1/2] Remove global state from VNDK apexes Use the name of the apex instead of a global map to find the right VNDK apex for each VNDK version. Bug: 181689854 Test: apex tests Change-Id: If9f8fb10d09e125c9e7d44228e1aa746bf53c082 --- apex/apex_test.go | 145 ++++++++++++++++------------------------------ apex/vndk.go | 34 ++++------- apex/vndk_test.go | 24 ++++---- 3 files changed, 71 insertions(+), 132 deletions(-) diff --git a/apex/apex_test.go b/apex/apex_test.go index 3f5604741..c27eed62e 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -143,17 +143,18 @@ func testApexContext(_ *testing.T, bp string, handlers ...testCustomizer) (*andr bp = bp + java.GatherRequiredDepsForTest() fs := map[string][]byte{ - "a.java": nil, - "PrebuiltAppFoo.apk": nil, - "PrebuiltAppFooPriv.apk": nil, - "build/make/target/product/security": nil, - "apex_manifest.json": nil, - "AndroidManifest.xml": nil, - "system/sepolicy/apex/myapex-file_contexts": nil, - "system/sepolicy/apex/myapex.updatable-file_contexts": nil, - "system/sepolicy/apex/myapex2-file_contexts": nil, - "system/sepolicy/apex/otherapex-file_contexts": nil, - "system/sepolicy/apex/com.android.vndk-file_contexts": nil, + "a.java": nil, + "PrebuiltAppFoo.apk": nil, + "PrebuiltAppFooPriv.apk": nil, + "build/make/target/product/security": nil, + "apex_manifest.json": nil, + "AndroidManifest.xml": nil, + "system/sepolicy/apex/myapex-file_contexts": nil, + "system/sepolicy/apex/myapex.updatable-file_contexts": nil, + "system/sepolicy/apex/myapex2-file_contexts": nil, + "system/sepolicy/apex/otherapex-file_contexts": nil, + "system/sepolicy/apex/com.android.vndk-file_contexts": nil, + "system/sepolicy/apex/com.android.vndk.current-file_contexts": nil, "mylib.cpp": nil, "mytest.cpp": nil, "mytest1.cpp": nil, @@ -3237,12 +3238,12 @@ func ensureExactContents(t *testing.T, ctx *android.TestContext, moduleName, var func TestVndkApexCurrent(t *testing.T) { ctx, _ := testApex(t, ` apex_vndk { - name: "myapex", - key: "myapex.key", + name: "com.android.vndk.current", + key: "com.android.vndk.current.key", } apex_key { - name: "myapex.key", + name: "com.android.vndk.current.key", public_key: "testkey.avbpubkey", private_key: "testkey.pem", } @@ -3257,7 +3258,7 @@ func TestVndkApexCurrent(t *testing.T) { }, system_shared_libs: [], stl: "none", - apex_available: [ "myapex" ], + apex_available: [ "com.android.vndk.current" ], } cc_library { @@ -3271,11 +3272,11 @@ func TestVndkApexCurrent(t *testing.T) { }, system_shared_libs: [], stl: "none", - apex_available: [ "myapex" ], + apex_available: [ "com.android.vndk.current" ], } `+vndkLibrariesTxtFiles("current")) - ensureExactContents(t, ctx, "myapex", "android_common_image", []string{ + ensureExactContents(t, ctx, "com.android.vndk.current", "android_common_image", []string{ "lib/libvndk.so", "lib/libvndksp.so", "lib/libc++.so", @@ -3293,12 +3294,12 @@ func TestVndkApexCurrent(t *testing.T) { func TestVndkApexWithPrebuilt(t *testing.T) { ctx, _ := testApex(t, ` apex_vndk { - name: "myapex", - key: "myapex.key", + name: "com.android.vndk.current", + key: "com.android.vndk.current.key", } apex_key { - name: "myapex.key", + name: "com.android.vndk.current.key", public_key: "testkey.avbpubkey", private_key: "testkey.pem", } @@ -3313,7 +3314,7 @@ func TestVndkApexWithPrebuilt(t *testing.T) { }, system_shared_libs: [], stl: "none", - apex_available: [ "myapex" ], + apex_available: [ "com.android.vndk.current" ], } cc_prebuilt_library_shared { @@ -3332,15 +3333,14 @@ func TestVndkApexWithPrebuilt(t *testing.T) { }, system_shared_libs: [], stl: "none", - apex_available: [ "myapex" ], + apex_available: [ "com.android.vndk.current" ], } `+vndkLibrariesTxtFiles("current"), withFiles(map[string][]byte{ "libvndk.so": nil, "libvndk.arm.so": nil, })) - - ensureExactContents(t, ctx, "myapex", "android_common_image", []string{ + ensureExactContents(t, ctx, "com.android.vndk.current", "android_common_image", []string{ "lib/libvndk.so", "lib/libvndk.arm.so", "lib64/libvndk.so", @@ -3377,7 +3377,7 @@ func vndkLibrariesTxtFiles(vers ...string) (result string) { func TestVndkApexVersion(t *testing.T) { ctx, _ := testApex(t, ` apex_vndk { - name: "myapex_v27", + name: "com.android.vndk.v27", key: "myapex.key", file_contexts: ":myapex-file_contexts", vndk_version: "27", @@ -3406,7 +3406,7 @@ func TestVndkApexVersion(t *testing.T) { srcs: ["libvndk27_arm64.so"], }, }, - apex_available: [ "myapex_v27" ], + apex_available: [ "com.android.vndk.v27" ], } vndk_prebuilt_shared { @@ -3435,70 +3435,22 @@ func TestVndkApexVersion(t *testing.T) { "libvndk27_x86_64.so": nil, })) - ensureExactContents(t, ctx, "myapex_v27", "android_common_image", []string{ + ensureExactContents(t, ctx, "com.android.vndk.v27", "android_common_image", []string{ "lib/libvndk27_arm.so", "lib64/libvndk27_arm64.so", "etc/*", }) } -func TestVndkApexErrorWithDuplicateVersion(t *testing.T) { - testApexError(t, `module "myapex_v27.*" .*: vndk_version: 27 is already defined in "myapex_v27.*"`, ` - apex_vndk { - name: "myapex_v27", - key: "myapex.key", - file_contexts: ":myapex-file_contexts", - vndk_version: "27", - } - apex_vndk { - name: "myapex_v27_other", - key: "myapex.key", - file_contexts: ":myapex-file_contexts", - vndk_version: "27", - } - - apex_key { - name: "myapex.key", - public_key: "testkey.avbpubkey", - private_key: "testkey.pem", - } - - cc_library { - name: "libvndk", - srcs: ["mylib.cpp"], - vendor_available: true, - product_available: true, - vndk: { - enabled: true, - }, - system_shared_libs: [], - stl: "none", - } - - vndk_prebuilt_shared { - name: "libvndk", - version: "27", - vendor_available: true, - product_available: true, - vndk: { - enabled: true, - }, - srcs: ["libvndk.so"], - } - `, withFiles(map[string][]byte{ - "libvndk.so": nil, - })) -} - func TestVndkApexNameRule(t *testing.T) { ctx, _ := testApex(t, ` apex_vndk { - name: "myapex", + name: "com.android.vndk.current", key: "myapex.key", file_contexts: ":myapex-file_contexts", } apex_vndk { - name: "myapex_v28", + name: "com.android.vndk.v28", key: "myapex.key", file_contexts: ":myapex-file_contexts", vndk_version: "28", @@ -3517,20 +3469,20 @@ func TestVndkApexNameRule(t *testing.T) { } } - assertApexName("com.android.vndk.vVER", "myapex") - assertApexName("com.android.vndk.v28", "myapex_v28") + assertApexName("com.android.vndk.vVER", "com.android.vndk.current") + assertApexName("com.android.vndk.v28", "com.android.vndk.v28") } func TestVndkApexSkipsNativeBridgeSupportedModules(t *testing.T) { ctx, _ := testApex(t, ` apex_vndk { - name: "myapex", - key: "myapex.key", + name: "com.android.vndk.current", + key: "com.android.vndk.current.key", file_contexts: ":myapex-file_contexts", } apex_key { - name: "myapex.key", + name: "com.android.vndk.current.key", public_key: "testkey.avbpubkey", private_key: "testkey.pem", } @@ -3547,11 +3499,12 @@ func TestVndkApexSkipsNativeBridgeSupportedModules(t *testing.T) { }, system_shared_libs: [], stl: "none", - apex_available: [ "myapex" ], + apex_available: [ "com.android.vndk.current" ], } - `+vndkLibrariesTxtFiles("current"), withNativeBridgeEnabled) + `+vndkLibrariesTxtFiles("current"), + withNativeBridgeEnabled) - ensureExactContents(t, ctx, "myapex", "android_common_image", []string{ + ensureExactContents(t, ctx, "com.android.vndk.current", "android_common_image", []string{ "lib/libvndk.so", "lib64/libvndk.so", "lib/libc++.so", @@ -3561,16 +3514,16 @@ func TestVndkApexSkipsNativeBridgeSupportedModules(t *testing.T) { } func TestVndkApexDoesntSupportNativeBridgeSupported(t *testing.T) { - testApexError(t, `module "myapex" .*: native_bridge_supported: .* doesn't support native bridge binary`, ` + testApexError(t, `module "com.android.vndk.current" .*: native_bridge_supported: .* doesn't support native bridge binary`, ` apex_vndk { - name: "myapex", - key: "myapex.key", + name: "com.android.vndk.current", + key: "com.android.vndk.current.key", file_contexts: ":myapex-file_contexts", native_bridge_supported: true, } apex_key { - name: "myapex.key", + name: "com.android.vndk.current.key", public_key: "testkey.avbpubkey", private_key: "testkey.pem", } @@ -3594,7 +3547,7 @@ func TestVndkApexDoesntSupportNativeBridgeSupported(t *testing.T) { func TestVndkApexWithBinder32(t *testing.T) { ctx, _ := testApex(t, ` apex_vndk { - name: "myapex_v27", + name: "com.android.vndk.v27", key: "myapex.key", file_contexts: ":myapex-file_contexts", vndk_version: "27", @@ -3637,7 +3590,7 @@ func TestVndkApexWithBinder32(t *testing.T) { srcs: ["libvndk27binder32.so"], } }, - apex_available: [ "myapex_v27" ], + apex_available: [ "com.android.vndk.v27" ], } `+vndkLibrariesTxtFiles("27"), withFiles(map[string][]byte{ @@ -3653,7 +3606,7 @@ func TestVndkApexWithBinder32(t *testing.T) { }), ) - ensureExactContents(t, ctx, "myapex_v27", "android_common_image", []string{ + ensureExactContents(t, ctx, "com.android.vndk.v27", "android_common_image", []string{ "lib/libvndk27binder32.so", "etc/*", }) @@ -3662,13 +3615,13 @@ func TestVndkApexWithBinder32(t *testing.T) { func TestVndkApexShouldNotProvideNativeLibs(t *testing.T) { ctx, _ := testApex(t, ` apex_vndk { - name: "myapex", - key: "myapex.key", + name: "com.android.vndk.current", + key: "com.android.vndk.current.key", file_contexts: ":myapex-file_contexts", } apex_key { - name: "myapex.key", + name: "com.android.vndk.current.key", public_key: "testkey.avbpubkey", private_key: "testkey.pem", } @@ -3689,7 +3642,7 @@ func TestVndkApexShouldNotProvideNativeLibs(t *testing.T) { "libz.map.txt": nil, })) - apexManifestRule := ctx.ModuleForTests("myapex", "android_common_image").Rule("apexManifestRule") + apexManifestRule := ctx.ModuleForTests("com.android.vndk.current", "android_common_image").Rule("apexManifestRule") provideNativeLibs := names(apexManifestRule.Args["provideNativeLibs"]) ensureListEmpty(t, provideNativeLibs) } diff --git a/apex/vndk.go b/apex/vndk.go index f4b12b564..75c0fb01b 100644 --- a/apex/vndk.go +++ b/apex/vndk.go @@ -17,7 +17,6 @@ package apex import ( "path/filepath" "strings" - "sync" "android/soong/android" "android/soong/cc" @@ -60,17 +59,6 @@ type apexVndkProperties struct { Vndk_version *string } -var ( - vndkApexListKey = android.NewOnceKey("vndkApexList") - vndkApexListMutex sync.Mutex -) - -func vndkApexList(config android.Config) map[string]string { - return config.Once(vndkApexListKey, func() interface{} { - return map[string]string{} - }).(map[string]string) -} - func apexVndkMutator(mctx android.TopDownMutatorContext) { if ab, ok := mctx.Module().(*apexBundle); ok && ab.vndkApex { if ab.IsNativeBridgeSupported() { @@ -80,15 +68,6 @@ func apexVndkMutator(mctx android.TopDownMutatorContext) { vndkVersion := ab.vndkVersion(mctx.DeviceConfig()) // Ensure VNDK APEX mount point is formatted as com.android.vndk.v### ab.properties.Apex_name = proptools.StringPtr(vndkApexNamePrefix + vndkVersion) - - // vndk_version should be unique - vndkApexListMutex.Lock() - defer vndkApexListMutex.Unlock() - vndkApexList := vndkApexList(mctx.Config()) - if other, ok := vndkApexList[vndkVersion]; ok { - mctx.PropertyErrorf("vndk_version", "%v is already defined in %q", vndkVersion, other) - } - vndkApexList[vndkVersion] = mctx.ModuleName() } } @@ -99,9 +78,16 @@ func apexVndkDepsMutator(mctx android.BottomUpMutatorContext) { if vndkVersion == "" { vndkVersion = mctx.DeviceConfig().PlatformVndkVersion() } - vndkApexList := vndkApexList(mctx.Config()) - if vndkApex, ok := vndkApexList[vndkVersion]; ok { - mctx.AddReverseDependency(mctx.Module(), sharedLibTag, vndkApex) + if vndkVersion == mctx.DeviceConfig().PlatformVndkVersion() { + vndkVersion = "current" + } else { + vndkVersion = "v" + vndkVersion + } + + vndkApexName := "com.android.vndk." + vndkVersion + + if mctx.OtherModuleExists(vndkApexName) { + mctx.AddReverseDependency(mctx.Module(), sharedLibTag, vndkApexName) } } else if a, ok := mctx.Module().(*apexBundle); ok && a.vndkApex { vndkVersion := proptools.StringDefault(a.vndkProperties.Vndk_version, "current") diff --git a/apex/vndk_test.go b/apex/vndk_test.go index ccf4e57e6..74a79e1c5 100644 --- a/apex/vndk_test.go +++ b/apex/vndk_test.go @@ -11,12 +11,12 @@ import ( func TestVndkApexForVndkLite(t *testing.T) { ctx, _ := testApex(t, ` apex_vndk { - name: "myapex", - key: "myapex.key", + name: "com.android.vndk.current", + key: "com.android.vndk.current.key", } apex_key { - name: "myapex.key", + name: "com.android.vndk.current.key", public_key: "testkey.avbpubkey", private_key: "testkey.pem", } @@ -31,7 +31,7 @@ func TestVndkApexForVndkLite(t *testing.T) { }, system_shared_libs: [], stl: "none", - apex_available: [ "myapex" ], + apex_available: [ "com.android.vndk.current" ], } cc_library { @@ -45,13 +45,13 @@ func TestVndkApexForVndkLite(t *testing.T) { }, system_shared_libs: [], stl: "none", - apex_available: [ "myapex" ], + apex_available: [ "com.android.vndk.current" ], } `+vndkLibrariesTxtFiles("current"), func(fs map[string][]byte, config android.Config) { config.TestProductVariables.DeviceVndkVersion = proptools.StringPtr("") }) // VNDK-Lite contains only core variants of VNDK-Sp libraries - ensureExactContents(t, ctx, "myapex", "android_common_image", []string{ + ensureExactContents(t, ctx, "com.android.vndk.current", "android_common_image", []string{ "lib/libvndksp.so", "lib/libc++.so", "lib64/libvndksp.so", @@ -67,7 +67,7 @@ func TestVndkApexForVndkLite(t *testing.T) { func TestVndkApexUsesVendorVariant(t *testing.T) { bp := ` apex_vndk { - name: "myapex", + name: "com.android.vndk.current", key: "mykey", } apex_key { @@ -94,7 +94,7 @@ func TestVndkApexUsesVendorVariant(t *testing.T) { return } } - t.Fail() + t.Errorf("expected path %q not found", path) } t.Run("VNDK lib doesn't have an apex variant", func(t *testing.T) { @@ -106,7 +106,7 @@ func TestVndkApexUsesVendorVariant(t *testing.T) { } // VNDK APEX doesn't create apex variant - files := getFiles(t, ctx, "myapex", "android_common_image") + files := getFiles(t, ctx, "com.android.vndk.current", "android_common_image") ensureFileSrc(t, files, "lib/libfoo.so", "libfoo/android_vendor.VER_arm_armv7-a-neon_shared/libfoo.so") }) @@ -116,7 +116,7 @@ func TestVndkApexUsesVendorVariant(t *testing.T) { config.TestProductVariables.ProductVndkVersion = proptools.StringPtr("current") }) - files := getFiles(t, ctx, "myapex", "android_common_image") + files := getFiles(t, ctx, "com.android.vndk.current", "android_common_image") ensureFileSrc(t, files, "lib/libfoo.so", "libfoo/android_vendor.VER_arm_armv7-a-neon_shared/libfoo.so") }) @@ -126,10 +126,10 @@ func TestVndkApexUsesVendorVariant(t *testing.T) { config.TestProductVariables.Native_coverage = proptools.BoolPtr(true) }) - files := getFiles(t, ctx, "myapex", "android_common_image") + files := getFiles(t, ctx, "com.android.vndk.current", "android_common_image") ensureFileSrc(t, files, "lib/libfoo.so", "libfoo/android_vendor.VER_arm_armv7-a-neon_shared/libfoo.so") - files = getFiles(t, ctx, "myapex", "android_common_cov_image") + files = getFiles(t, ctx, "com.android.vndk.current", "android_common_cov_image") ensureFileSrc(t, files, "lib/libfoo.so", "libfoo/android_vendor.VER_arm_armv7-a-neon_shared_cov/libfoo.so") }) } From 6cb1128b541240a2320608e7bdf96a81621d1944 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 2 Mar 2021 10:16:26 -0800 Subject: [PATCH 2/2] Disable TestSendLog on the buildbots TestSendLog sometimes hangs on the buildbot, although it has never been reproduced locally. The cause appears to be a unix domain packet that was sent but never received. Skip the test in short mode for now, which will leave it manually runnable but not run it as part of the build. Fixes: 120596545 Test: m nothing Change-Id: I3e6bc9b5f1d9c15dc9a7294d98cbd917d0637c44 --- ui/build/paths/logs_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ui/build/paths/logs_test.go b/ui/build/paths/logs_test.go index 3b1005fb5..067f3f3fe 100644 --- a/ui/build/paths/logs_test.go +++ b/ui/build/paths/logs_test.go @@ -26,6 +26,9 @@ import ( ) func TestSendLog(t *testing.T) { + if testing.Short() { + t.Skip("skipping in short mode, sometimes hangs") + } t.Run("Short name", func(t *testing.T) { d, err := ioutil.TempDir("", "s") if err != nil {