From 4cfa2c692b9c901afd3265ad34af3dd7b38576c7 Mon Sep 17 00:00:00 2001 From: Martin Stjernholm Date: Fri, 10 Jul 2020 19:55:36 +0100 Subject: [PATCH] Set compile_multilib for SDK snapshots for device. Previously compile_multilib was ignored for device and defaulted to "both", which could lead to an internal inconsistency in the snapshot if a member was 32- or 64-only. This also implements common property optimisation for compile_multilib, by adding a struct that can be extended to other properties in the future. Test: m nothing Bug: 151303681 Change-Id: I3e96222858e5e755173aca03508a23c431977e2a --- sdk/cc_sdk_test.go | 38 ++++++++++++++++---------------------- sdk/update.go | 32 +++++++++++++++++++++++++------- 2 files changed, 41 insertions(+), 29 deletions(-) diff --git a/sdk/cc_sdk_test.go b/sdk/cc_sdk_test.go index 497f14bde..17afdb804 100644 --- a/sdk/cc_sdk_test.go +++ b/sdk/cc_sdk_test.go @@ -73,7 +73,6 @@ func TestSdkCompileMultilibOverride(t *testing.T) { result := testSdkWithCc(t, ` sdk { name: "mysdk", - device_supported: false, host_supported: true, native_shared_libs: ["sdkmember"], compile_multilib: "64", @@ -81,7 +80,6 @@ func TestSdkCompileMultilibOverride(t *testing.T) { cc_library_shared { name: "sdkmember", - device_supported: false, host_supported: true, srcs: ["Test.cpp"], stl: "none", @@ -96,14 +94,16 @@ func TestSdkCompileMultilibOverride(t *testing.T) { cc_prebuilt_library_shared { name: "mysdk_sdkmember@current", sdk_member_name: "sdkmember", - device_supported: false, host_supported: true, installable: false, stl: "none", compile_multilib: "64", - arch: { - x86_64: { - srcs: ["x86_64/lib/sdkmember.so"], + target: { + android_arm64: { + srcs: ["android/arm64/lib/sdkmember.so"], + }, + linux_glibc_x86_64: { + srcs: ["linux_glibc/x86_64/lib/sdkmember.so"], }, }, } @@ -111,31 +111,29 @@ cc_prebuilt_library_shared { cc_prebuilt_library_shared { name: "sdkmember", prefer: false, - device_supported: false, host_supported: true, stl: "none", compile_multilib: "64", - arch: { - x86_64: { - srcs: ["x86_64/lib/sdkmember.so"], + target: { + android_arm64: { + srcs: ["android/arm64/lib/sdkmember.so"], + }, + linux_glibc_x86_64: { + srcs: ["linux_glibc/x86_64/lib/sdkmember.so"], }, }, } sdk_snapshot { name: "mysdk@current", - device_supported: false, host_supported: true, native_shared_libs: ["mysdk_sdkmember@current"], - target: { - linux_glibc: { - compile_multilib: "64", - }, - }, + compile_multilib: "64", } `), checkAllCopyRules(` -.intermediates/sdkmember/linux_glibc_x86_64_shared/sdkmember.so -> x86_64/lib/sdkmember.so +.intermediates/sdkmember/android_arm64_armv8-a_shared/sdkmember.so -> android/arm64/lib/sdkmember.so +.intermediates/sdkmember/linux_glibc_x86_64_shared/sdkmember.so -> linux_glibc/x86_64/lib/sdkmember.so `)) } @@ -1527,11 +1525,7 @@ module_exports_snapshot { device_supported: false, host_supported: true, native_static_libs: ["myexports_mynativelib@current"], - target: { - linux_glibc: { - compile_multilib: "64", - }, - }, + compile_multilib: "64", }`), checkAllCopyRules(` include/Test.h -> include/include/Test.h diff --git a/sdk/update.go b/sdk/update.go index b8d73c63f..25d50d217 100644 --- a/sdk/update.go +++ b/sdk/update.go @@ -323,19 +323,37 @@ func (s *sdk) buildSnapshot(ctx android.ModuleContext, sdkVariants []*sdk) andro // Add properties common to all os types. s.addMemberPropertiesToPropertySet(builder, snapshotModule, commonDynamicMemberProperties) + // Optimize other per-variant properties, besides the dynamic member lists. + type variantProperties struct { + Compile_multilib string `android:"arch_variant"` + } + var variantPropertiesContainers []propertiesContainer + variantToProperties := make(map[*sdk]*variantProperties) + for _, sdkVariant := range sdkVariants { + props := &variantProperties{ + Compile_multilib: sdkVariant.multilibUsages.String(), + } + variantPropertiesContainers = append(variantPropertiesContainers, &dynamicMemberPropertiesContainer{sdkVariant, props}) + variantToProperties[sdkVariant] = props + } + commonVariantProperties := variantProperties{} + extractor = newCommonValueExtractor(commonVariantProperties) + extractCommonProperties(ctx, extractor, &commonVariantProperties, variantPropertiesContainers) + if commonVariantProperties.Compile_multilib != "" && commonVariantProperties.Compile_multilib != "both" { + // Compile_multilib defaults to both so only needs to be set when it's + // specified and not both. + snapshotModule.AddProperty("compile_multilib", commonVariantProperties.Compile_multilib) + } + // Iterate over the os types in a fixed order. targetPropertySet := snapshotModule.AddPropertySet("target") for _, osType := range s.getPossibleOsTypes() { if sdkVariant, ok := osTypeToMemberProperties[osType]; ok { osPropertySet := targetPropertySet.AddPropertySet(sdkVariant.Target().Os.Name) - // Compile_multilib defaults to both and must always be set to both on the - // device and so only needs to be set when targeted at the host and is neither - // unspecified or both. - multilib := sdkVariant.multilibUsages - if (osType.Class == android.Host || osType.Class == android.HostCross) && - multilib != multilibNone && multilib != multilibBoth { - osPropertySet.AddProperty("compile_multilib", multilib.String()) + variantProps := variantToProperties[sdkVariant] + if variantProps.Compile_multilib != "" && variantProps.Compile_multilib != "both" { + osPropertySet.AddProperty("compile_multilib", variantProps.Compile_multilib) } s.addMemberPropertiesToPropertySet(builder, osPropertySet, sdkVariant.dynamicMemberTypeListProperties)