From bea58093b4f2cea35665b24003ec094f133aedab Mon Sep 17 00:00:00 2001 From: Jingwen Chen Date: Thu, 29 Sep 2022 16:56:02 +0000 Subject: [PATCH] Share certificate bp2build between android_app and apex. The certificate module is handled the same in Soong between android apps and apexes, so share the bp2build code as well. There are a few changes in this CL: - If override_apex.certificate is unset, the generated apex also unsets it. This prevents the generated apex from using the base apex's certificate, which is most likely incorrect (e.g. google variant using the cert for the aosp variant). Instead, rely on the default certificate handling in the macro. - If the certificate prop is a string, then it gets generated into certificate_name in order to disambiguate. This behavior is identical to android_app. Test: added various unit tests. Bug: 249089160 Fixes: 249089160 Change-Id: I99e18964ff546429a985d0f64dc21e2c69d35d9d --- apex/apex.go | 21 ++-- bp2build/apex_conversion_test.go | 196 ++++++++++++++++++++++++++++++- java/app.go | 26 ++-- 3 files changed, 221 insertions(+), 22 deletions(-) diff --git a/apex/apex.go b/apex/apex.go index 2e54e7e6a..498c8c0ff 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -2659,9 +2659,13 @@ func (o *OverrideApex) ConvertWithBp2build(ctx android.TopDownMutatorContext) { } // Certificate - if overridableProperties.Certificate != nil { - attrs.Certificate = bazel.LabelAttribute{} - attrs.Certificate.SetValue(android.BazelLabelForModuleDepSingle(ctx, *overridableProperties.Certificate)) + if overridableProperties.Certificate == nil { + // delegated to the rule attr default + attrs.Certificate = nil + } else { + certificateName, certificate := java.ParseCertificateToAttribute(ctx, overridableProperties.Certificate) + attrs.Certificate_name = certificateName + attrs.Certificate = certificate } // Prebuilts @@ -3335,7 +3339,8 @@ type bazelApexBundleAttributes struct { Android_manifest bazel.LabelAttribute File_contexts bazel.LabelAttribute Key bazel.LabelAttribute - Certificate bazel.LabelAttribute + Certificate *bazel.Label // used when the certificate prop is a module + Certificate_name *string // used when the certificate prop is a string Min_sdk_version *string Updatable bazel.BoolAttribute Installable bazel.BoolAttribute @@ -3397,10 +3402,7 @@ func convertWithBp2build(a *apexBundle, ctx android.TopDownMutatorContext) (baze keyLabelAttribute.SetValue(android.BazelLabelForModuleDepSingle(ctx, *a.overridableProperties.Key)) } - var certificateLabelAttribute bazel.LabelAttribute - if a.overridableProperties.Certificate != nil { - certificateLabelAttribute.SetValue(android.BazelLabelForModuleDepSingle(ctx, *a.overridableProperties.Certificate)) - } + certificateName, certificate := java.ParseCertificateToAttribute(ctx, a.overridableProperties.Certificate) nativeSharedLibs := &convertedNativeSharedLibs{ Native_shared_libs_32: bazel.LabelListAttribute{}, @@ -3456,7 +3458,8 @@ func convertWithBp2build(a *apexBundle, ctx android.TopDownMutatorContext) (baze File_contexts: fileContextsLabelAttribute, Min_sdk_version: minSdkVersion, Key: keyLabelAttribute, - Certificate: certificateLabelAttribute, + Certificate: certificate, + Certificate_name: certificateName, Updatable: updatableAttribute, Installable: installableAttribute, Native_shared_libs_32: nativeSharedLibs.Native_shared_libs_32, diff --git a/bp2build/apex_conversion_test.go b/bp2build/apex_conversion_test.go index b0a2966b3..233fce4c0 100644 --- a/bp2build/apex_conversion_test.go +++ b/bp2build/apex_conversion_test.go @@ -120,7 +120,7 @@ apex { file_contexts: ":com.android.apogee-file_contexts", min_sdk_version: "29", key: "com.android.apogee.key", - certificate: "com.android.apogee.certificate", + certificate: ":com.android.apogee.certificate", updatable: false, installable: false, compressible: false, @@ -582,7 +582,7 @@ apex { file_contexts: ":com.android.apogee-file_contexts", min_sdk_version: "29", key: "com.android.apogee.key", - certificate: "com.android.apogee.certificate", + certificate: ":com.android.apogee.certificate", updatable: false, installable: false, compressible: false, @@ -618,7 +618,7 @@ override_apex { name: "com.google.android.apogee", base: ":com.android.apogee", key: "com.google.android.apogee.key", - certificate: "com.google.android.apogee.certificate", + certificate: ":com.google.android.apogee.certificate", prebuilts: [], compressible: true, } @@ -1016,3 +1016,193 @@ override_apex { }), }}) } + +func TestBp2BuildOverrideApex_CertificateNil(t *testing.T) { + runOverrideApexTestCase(t, Bp2buildTestCase{ + Description: "override_apex - don't set default certificate", + ModuleTypeUnderTest: "override_apex", + ModuleTypeUnderTestFactory: apex.OverrideApexFactory, + Filesystem: map[string]string{}, + Blueprint: ` +android_app_certificate { + name: "com.android.apogee.certificate", + certificate: "com.android.apogee", + bazel_module: { bp2build_available: false }, +} + +filegroup { + name: "com.android.apogee-file_contexts", + srcs: [ + "com.android.apogee-file_contexts", + ], + bazel_module: { bp2build_available: false }, +} + +apex { + name: "com.android.apogee", + manifest: "apogee_manifest.json", + file_contexts: ":com.android.apogee-file_contexts", + certificate: ":com.android.apogee.certificate", + bazel_module: { bp2build_available: false }, +} + +override_apex { + name: "com.google.android.apogee", + base: ":com.android.apogee", + // certificate is deliberately omitted, and not converted to bazel, + // because the overridden apex shouldn't be using the base apex's cert. +} +`, + ExpectedBazelTargets: []string{ + MakeBazelTarget("apex", "com.google.android.apogee", AttrNameToString{ + "file_contexts": `":com.android.apogee-file_contexts"`, + "manifest": `"apogee_manifest.json"`, + }), + }}) +} + +func TestApexCertificateIsModule(t *testing.T) { + runApexTestCase(t, Bp2buildTestCase{ + Description: "apex - certificate is module", + ModuleTypeUnderTest: "apex", + ModuleTypeUnderTestFactory: apex.BundleFactory, + Filesystem: map[string]string{}, + Blueprint: ` +android_app_certificate { + name: "com.android.apogee.certificate", + certificate: "com.android.apogee", + bazel_module: { bp2build_available: false }, +} + +apex { + name: "com.android.apogee", + manifest: "apogee_manifest.json", + file_contexts: ":com.android.apogee-file_contexts", + certificate: ":com.android.apogee.certificate", +} +` + simpleModuleDoNotConvertBp2build("filegroup", "com.android.apogee-file_contexts"), + ExpectedBazelTargets: []string{ + MakeBazelTarget("apex", "com.android.apogee", AttrNameToString{ + "certificate": `":com.android.apogee.certificate"`, + "file_contexts": `":com.android.apogee-file_contexts"`, + "manifest": `"apogee_manifest.json"`, + }), + }}) +} + +func TestApexCertificateIsSrc(t *testing.T) { + runApexTestCase(t, Bp2buildTestCase{ + Description: "apex - certificate is src", + ModuleTypeUnderTest: "apex", + ModuleTypeUnderTestFactory: apex.BundleFactory, + Filesystem: map[string]string{}, + Blueprint: ` +apex { + name: "com.android.apogee", + manifest: "apogee_manifest.json", + file_contexts: ":com.android.apogee-file_contexts", + certificate: "com.android.apogee.certificate", +} +` + simpleModuleDoNotConvertBp2build("filegroup", "com.android.apogee-file_contexts"), + ExpectedBazelTargets: []string{ + MakeBazelTarget("apex", "com.android.apogee", AttrNameToString{ + "certificate_name": `"com.android.apogee.certificate"`, + "file_contexts": `":com.android.apogee-file_contexts"`, + "manifest": `"apogee_manifest.json"`, + }), + }}) +} + +func TestBp2BuildOverrideApex_CertificateIsModule(t *testing.T) { + runOverrideApexTestCase(t, Bp2buildTestCase{ + Description: "override_apex - certificate is module", + ModuleTypeUnderTest: "override_apex", + ModuleTypeUnderTestFactory: apex.OverrideApexFactory, + Filesystem: map[string]string{}, + Blueprint: ` +android_app_certificate { + name: "com.android.apogee.certificate", + certificate: "com.android.apogee", + bazel_module: { bp2build_available: false }, +} + +filegroup { + name: "com.android.apogee-file_contexts", + srcs: [ + "com.android.apogee-file_contexts", + ], + bazel_module: { bp2build_available: false }, +} + +apex { + name: "com.android.apogee", + manifest: "apogee_manifest.json", + file_contexts: ":com.android.apogee-file_contexts", + certificate: ":com.android.apogee.certificate", + bazel_module: { bp2build_available: false }, +} + +android_app_certificate { + name: "com.google.android.apogee.certificate", + certificate: "com.google.android.apogee", + bazel_module: { bp2build_available: false }, +} + +override_apex { + name: "com.google.android.apogee", + base: ":com.android.apogee", + certificate: ":com.google.android.apogee.certificate", +} +`, + ExpectedBazelTargets: []string{ + MakeBazelTarget("apex", "com.google.android.apogee", AttrNameToString{ + "file_contexts": `":com.android.apogee-file_contexts"`, + "certificate": `":com.google.android.apogee.certificate"`, + "manifest": `"apogee_manifest.json"`, + }), + }}) +} + +func TestBp2BuildOverrideApex_CertificateIsSrc(t *testing.T) { + runOverrideApexTestCase(t, Bp2buildTestCase{ + Description: "override_apex - certificate is src", + ModuleTypeUnderTest: "override_apex", + ModuleTypeUnderTestFactory: apex.OverrideApexFactory, + Filesystem: map[string]string{}, + Blueprint: ` +android_app_certificate { + name: "com.android.apogee.certificate", + certificate: "com.android.apogee", + bazel_module: { bp2build_available: false }, +} + +filegroup { + name: "com.android.apogee-file_contexts", + srcs: [ + "com.android.apogee-file_contexts", + ], + bazel_module: { bp2build_available: false }, +} + +apex { + name: "com.android.apogee", + manifest: "apogee_manifest.json", + file_contexts: ":com.android.apogee-file_contexts", + certificate: ":com.android.apogee.certificate", + bazel_module: { bp2build_available: false }, +} + +override_apex { + name: "com.google.android.apogee", + base: ":com.android.apogee", + certificate: "com.google.android.apogee.certificate", +} +`, + ExpectedBazelTargets: []string{ + MakeBazelTarget("apex", "com.google.android.apogee", AttrNameToString{ + "file_contexts": `":com.android.apogee-file_contexts"`, + "certificate_name": `"com.google.android.apogee.certificate"`, + "manifest": `"apogee_manifest.json"`, + }), + }}) +} diff --git a/java/app.go b/java/app.go index 1955e2a73..a42816593 100755 --- a/java/app.go +++ b/java/app.go @@ -1489,6 +1489,20 @@ type bazelAndroidAppAttributes struct { Certificate_name *string } +// ParseCertificateToAttribute splits the certificate prop into a certificate +// label attribute or a certificate_name string attribute. +func ParseCertificateToAttribute(ctx android.TopDownMutatorContext, certificate *string) (*string, *bazel.Label) { + var certificateLabel *bazel.Label + certificateName := proptools.StringDefault(certificate, "") + certModule := android.SrcIsModule(certificateName) + if certModule != "" { + c := android.BazelLabelForModuleDepSingle(ctx, certificateName) + certificateLabel = &c + certificate = nil + } + return certificate, certificateLabel +} + // ConvertWithBp2build is used to convert android_app to Bazel. func (a *AndroidApp) ConvertWithBp2build(ctx android.TopDownMutatorContext) { commonAttrs, depLabels := a.convertLibraryAttrsBp2Build(ctx) @@ -1498,15 +1512,7 @@ func (a *AndroidApp) ConvertWithBp2build(ctx android.TopDownMutatorContext) { aapt := a.convertAaptAttrsWithBp2Build(ctx) - var certificate *bazel.Label - certificateNamePtr := a.overridableAppProperties.Certificate - certificateName := proptools.StringDefault(certificateNamePtr, "") - certModule := android.SrcIsModule(certificateName) - if certModule != "" { - c := android.BazelLabelForModuleDepSingle(ctx, certificateName) - certificate = &c - certificateNamePtr = nil - } + certificateName, certificate := ParseCertificateToAttribute(ctx, a.overridableAppProperties.Certificate) attrs := &bazelAndroidAppAttributes{ commonAttrs, aapt, @@ -1514,7 +1520,7 @@ func (a *AndroidApp) ConvertWithBp2build(ctx android.TopDownMutatorContext) { // TODO(b/209576404): handle package name override by product variable PRODUCT_MANIFEST_PACKAGE_NAME_OVERRIDES a.overridableAppProperties.Package_name, certificate, - certificateNamePtr, + certificateName, } props := bazel.BazelTargetModuleProperties{