From b4cb0c857ffe2f716dfc899fec1cedd952e6ee70 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Thu, 14 Sep 2023 15:16:58 -0700 Subject: [PATCH] Move the android_platform next to it's entrypoint product config file Because we're going to start generating partition images for the product, and the partitions will eventually be checked in, we want them to be in sensible locations. And the platform should be there as well so all the targets for a product are co-located. Bug: 297269187 Test: m nothing && b build --config=android //build/bazel/examples/apex/minimal:build.bazel.examples.apex.minimal Change-Id: Iaa25c44aa00295ada279d5fd49b5498bbafb89d5 --- android/variable.go | 6 ++ bp2build/bp2build_product_config.go | 101 ++++++++++++++------------ bp2build/build_conversion.go | 109 ++++++++++++++++++++-------- bp2build/build_conversion_test.go | 92 +++++++++++++++-------- 4 files changed, 200 insertions(+), 108 deletions(-) diff --git a/android/variable.go b/android/variable.go index 6d9cc4b8d..a495544b8 100644 --- a/android/variable.go +++ b/android/variable.go @@ -484,6 +484,12 @@ type ProductVariables struct { KeepVndk *bool `json:",omitempty"` CheckVendorSeappViolations *bool `json:",omitempty"` + + // PartitionsVars are extra variables that are used to define the partition images. They should + // not be read from soong modules. + PartitionVars struct { + ProductDirectory string `json:",omitempty"` + } `json:",omitempty"` } func boolPtr(v bool) *bool { diff --git a/bp2build/bp2build_product_config.go b/bp2build/bp2build_product_config.go index 3622e6703..3d9cae069 100644 --- a/bp2build/bp2build_product_config.go +++ b/bp2build/bp2build_product_config.go @@ -53,9 +53,10 @@ func createProductConfigFiles( return res, err } - // TODO(b/249685973): the name is product_config_platforms because product_config - // was already used for other files. Deduplicate them. - currentProductFolder := fmt.Sprintf("product_config_platforms/products/%s-%s", targetProduct, targetBuildVariant) + currentProductFolder := fmt.Sprintf("build/bazel/products/%s-%s", targetProduct, targetBuildVariant) + if len(productVariables.PartitionVars.ProductDirectory) > 0 { + currentProductFolder = fmt.Sprintf("%s%s-%s", productVariables.PartitionVars.ProductDirectory, targetProduct, targetBuildVariant) + } productReplacer := strings.NewReplacer( "{PRODUCT}", targetProduct, @@ -72,7 +73,7 @@ func createProductConfigFiles( } productLabelsToVariables := make(map[string]*android.ProductVariables) - productLabelsToVariables[productReplacer.Replace("@soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}")] = &productVariables + productLabelsToVariables[productReplacer.Replace("@//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}")] = &productVariables for product, productVariablesStarlark := range productsForTestingMap { productVariables, err := starlarkMapToProductVariables(productVariablesStarlark) if err != nil { @@ -81,7 +82,30 @@ func createProductConfigFiles( productLabelsToVariables["@//build/bazel/tests/products:"+product] = &productVariables } - res.bp2buildTargets = createTargets(productLabelsToVariables) + res.bp2buildTargets = make(map[string]BazelTargets) + res.bp2buildTargets[currentProductFolder] = append(res.bp2buildTargets[currentProductFolder], BazelTarget{ + name: productReplacer.Replace("{PRODUCT}-{VARIANT}"), + packageName: currentProductFolder, + content: productReplacer.Replace(`android_product( + name = "{PRODUCT}-{VARIANT}", + soong_variables = _soong_variables, +)`), + ruleClass: "android_product", + loads: []BazelLoad{ + { + file: ":soong.variables.bzl", + symbols: []BazelLoadSymbol{{ + symbol: "variables", + alias: "_soong_variables", + }}, + }, + { + file: "//build/bazel/product_config:android_product.bzl", + symbols: []BazelLoadSymbol{{symbol: "android_product"}}, + }, + }, + }) + createTargets(productLabelsToVariables, res.bp2buildTargets) platformMappingContent, err := platformMappingContent( productLabelsToVariables, @@ -92,26 +116,6 @@ func createProductConfigFiles( } res.injectionFiles = []BazelFile{ - newFile( - currentProductFolder, - "soong.variables.bzl", - `variables = json.decode("""`+strings.ReplaceAll(string(productVariablesBytes), "\\", "\\\\")+`""")`), - newFile( - currentProductFolder, - "BUILD", - productReplacer.Replace(` -package(default_visibility=[ - "@soong_injection//product_config_platforms:__subpackages__", - "@//build/bazel/product_config:__subpackages__", -]) -load(":soong.variables.bzl", _soong_variables = "variables") -load("@//build/bazel/product_config:android_product.bzl", "android_product") - -android_product( - name = "{PRODUCT}-{VARIANT}", - soong_variables = _soong_variables, -) -`)), newFile( "product_config_platforms", "BUILD.bazel", @@ -121,7 +125,7 @@ package(default_visibility = [ "@soong_injection//product_config_platforms:__subpackages__", ]) -load("//{PRODUCT_FOLDER}:soong.variables.bzl", _soong_variables = "variables") +load("@//{PRODUCT_FOLDER}:soong.variables.bzl", _soong_variables = "variables") load("@//build/bazel/product_config:android_product.bzl", "android_product") # Bazel will qualify its outputs by the platform name. When switching between products, this @@ -145,33 +149,33 @@ android_product( # currently lunched product, they should all be listed here product_labels = [ "@soong_injection//product_config_platforms:mixed_builds_product-{VARIANT}", - "@soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}", + "@//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}", `)+strings.Join(productsForTesting, "\n")+"\n]\n"), newFile( "product_config_platforms", "common.bazelrc", productReplacer.Replace(` build --platform_mappings=platform_mappings -build --platforms @soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_linux_x86_64 +build --platforms @//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_linux_x86_64 -build:android --platforms=@soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT} -build:linux_x86 --platforms=@soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_linux_x86 -build:linux_x86_64 --platforms=@soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_linux_x86_64 -build:linux_bionic_x86_64 --platforms=@soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_linux_bionic_x86_64 -build:linux_musl_x86 --platforms=@soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_linux_musl_x86 -build:linux_musl_x86_64 --platforms=@soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_linux_musl_x86_64 +build:android --platforms=@//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT} +build:linux_x86 --platforms=@//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_linux_x86 +build:linux_x86_64 --platforms=@//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_linux_x86_64 +build:linux_bionic_x86_64 --platforms=@//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_linux_bionic_x86_64 +build:linux_musl_x86 --platforms=@//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_linux_musl_x86 +build:linux_musl_x86_64 --platforms=@//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_linux_musl_x86_64 `)), newFile( "product_config_platforms", "linux.bazelrc", productReplacer.Replace(` -build --host_platform @soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_linux_x86_64 +build --host_platform @//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_linux_x86_64 `)), newFile( "product_config_platforms", "darwin.bazelrc", productReplacer.Replace(` -build --host_platform @soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_darwin_x86_64 +build --host_platform @//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_darwin_x86_64 `)), } res.bp2buildFiles = []BazelFile{ @@ -179,6 +183,10 @@ build --host_platform @soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_dar "", "platform_mappings", platformMappingContent), + newFile( + currentProductFolder, + "soong.variables.bzl", + `variables = json.decode("""`+strings.ReplaceAll(string(productVariablesBytes), "\\", "\\\\")+`""")`), } return res, nil @@ -421,8 +429,11 @@ func starlarkMapToProductVariables(in map[string]starlark.Value) (android.Produc return result, nil } -func createTargets(productLabelsToVariables map[string]*android.ProductVariables) map[string]BazelTargets { - res := make(map[string]BazelTargets) +func createTargets(productLabelsToVariables map[string]*android.ProductVariables, res map[string]BazelTargets) { + createGeneratedAndroidCertificateDirectories(productLabelsToVariables, res) +} + +func createGeneratedAndroidCertificateDirectories(productLabelsToVariables map[string]*android.ProductVariables, targets map[string]BazelTargets) { var allDefaultAppCertificateDirs []string for _, productVariables := range productLabelsToVariables { if proptools.String(productVariables.DefaultAppCertificate) != "" { @@ -433,20 +444,20 @@ func createTargets(productLabelsToVariables map[string]*android.ProductVariables } } for _, dir := range allDefaultAppCertificateDirs { - content := fmt.Sprintf(ruleTargetTemplate, "filegroup", "generated_android_certificate_directory", propsToAttributes(map[string]string{ - "srcs": `glob([ + content := `filegroup( + name = "generated_android_certificate_directory", + srcs = glob([ "*.pk8", "*.pem", "*.avbpubkey", - ])`, - "visibility": `["//visibility:public"]`, - })) - res[dir] = append(res[dir], BazelTarget{ + ]), + visibility = ["//visibility:public"], +)` + targets[dir] = append(targets[dir], BazelTarget{ name: "generated_android_certificate_directory", packageName: dir, content: content, ruleClass: "filegroup", }) } - return res } diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index 906036330..f332f2be0 100644 --- a/bp2build/build_conversion.go +++ b/bp2build/build_conversion.go @@ -39,18 +39,24 @@ type BazelAttributes struct { Attrs map[string]string } -type BazelTarget struct { - name string - packageName string - content string - ruleClass string - bzlLoadLocation string +type BazelLoadSymbol struct { + // The name of the symbol in the file being loaded + symbol string + // The name the symbol wil have in this file. Can be left blank to use the same name as symbol. + alias string } -// IsLoadedFromStarlark determines if the BazelTarget's rule class is loaded from a .bzl file, -// as opposed to a native rule built into Bazel. -func (t BazelTarget) IsLoadedFromStarlark() bool { - return t.bzlLoadLocation != "" +type BazelLoad struct { + file string + symbols []BazelLoadSymbol +} + +type BazelTarget struct { + name string + packageName string + content string + ruleClass string + loads []BazelLoad } // Label is the fully qualified Bazel label constructed from the BazelTarget's @@ -110,30 +116,62 @@ func (targets BazelTargets) String() string { // LoadStatements return the string representation of the sorted and deduplicated // Starlark rule load statements needed by a group of BazelTargets. func (targets BazelTargets) LoadStatements() string { - bzlToLoadedSymbols := map[string][]string{} + // First, merge all the load statements from all the targets onto one list + bzlToLoadedSymbols := map[string][]BazelLoadSymbol{} for _, target := range targets { - if target.IsLoadedFromStarlark() { - bzlToLoadedSymbols[target.bzlLoadLocation] = - append(bzlToLoadedSymbols[target.bzlLoadLocation], target.ruleClass) + for _, load := range target.loads { + outer: + for _, symbol := range load.symbols { + alias := symbol.alias + if alias == "" { + alias = symbol.symbol + } + for _, otherSymbol := range bzlToLoadedSymbols[load.file] { + otherAlias := otherSymbol.alias + if otherAlias == "" { + otherAlias = otherSymbol.symbol + } + if symbol.symbol == otherSymbol.symbol && alias == otherAlias { + continue outer + } else if alias == otherAlias { + panic(fmt.Sprintf("Conflicting destination (%s) for loads of %s and %s", alias, symbol.symbol, otherSymbol.symbol)) + } + } + bzlToLoadedSymbols[load.file] = append(bzlToLoadedSymbols[load.file], symbol) + } } } - var loadStatements []string - for bzl, ruleClasses := range bzlToLoadedSymbols { - loadStatement := "load(\"" - loadStatement += bzl - loadStatement += "\", " - ruleClasses = android.SortedUniqueStrings(ruleClasses) - for i, ruleClass := range ruleClasses { - loadStatement += "\"" + ruleClass + "\"" - if i != len(ruleClasses)-1 { - loadStatement += ", " + var loadStatements strings.Builder + for i, bzl := range android.SortedKeys(bzlToLoadedSymbols) { + symbols := bzlToLoadedSymbols[bzl] + loadStatements.WriteString("load(\"") + loadStatements.WriteString(bzl) + loadStatements.WriteString("\", ") + sort.Slice(symbols, func(i, j int) bool { + if symbols[i].symbol < symbols[j].symbol { + return true + } + return symbols[i].alias < symbols[j].alias + }) + for j, symbol := range symbols { + if symbol.alias != "" && symbol.alias != symbol.symbol { + loadStatements.WriteString(symbol.alias) + loadStatements.WriteString(" = ") + } + loadStatements.WriteString("\"") + loadStatements.WriteString(symbol.symbol) + loadStatements.WriteString("\"") + if j != len(symbols)-1 { + loadStatements.WriteString(", ") } } - loadStatement += ")" - loadStatements = append(loadStatements, loadStatement) + loadStatements.WriteString(")") + if i != len(bzlToLoadedSymbols)-1 { + loadStatements.WriteString("\n") + } } - return strings.Join(android.SortedUniqueStrings(loadStatements), "\n") + return loadStatements.String() } type bpToBuildContext interface { @@ -857,12 +895,19 @@ func generateBazelTarget(ctx bpToBuildContext, m bp2buildModule) (BazelTarget, e } else { content = fmt.Sprintf(unnamedRuleTargetTemplate, ruleClass, attributes) } + var loads []BazelLoad + if bzlLoadLocation != "" { + loads = append(loads, BazelLoad{ + file: bzlLoadLocation, + symbols: []BazelLoadSymbol{{symbol: ruleClass}}, + }) + } return BazelTarget{ - name: targetName, - packageName: m.TargetPackage(), - ruleClass: ruleClass, - bzlLoadLocation: bzlLoadLocation, - content: content, + name: targetName, + packageName: m.TargetPackage(), + ruleClass: ruleClass, + loads: loads, + content: content, }, nil } diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go index cefa17158..1fe01591e 100644 --- a/bp2build/build_conversion_test.go +++ b/bp2build/build_conversion_test.go @@ -773,9 +773,12 @@ func TestLoadStatements(t *testing.T) { { bazelTargets: BazelTargets{ BazelTarget{ - name: "foo", - ruleClass: "cc_library", - bzlLoadLocation: "//build/bazel/rules:cc.bzl", + name: "foo", + ruleClass: "cc_library", + loads: []BazelLoad{{ + file: "//build/bazel/rules:cc.bzl", + symbols: []BazelLoadSymbol{{symbol: "cc_library"}}, + }}, }, }, expectedLoadStatements: `load("//build/bazel/rules:cc.bzl", "cc_library")`, @@ -783,14 +786,20 @@ func TestLoadStatements(t *testing.T) { { bazelTargets: BazelTargets{ BazelTarget{ - name: "foo", - ruleClass: "cc_library", - bzlLoadLocation: "//build/bazel/rules:cc.bzl", + name: "foo", + ruleClass: "cc_library", + loads: []BazelLoad{{ + file: "//build/bazel/rules:cc.bzl", + symbols: []BazelLoadSymbol{{symbol: "cc_library"}}, + }}, }, BazelTarget{ - name: "bar", - ruleClass: "cc_library", - bzlLoadLocation: "//build/bazel/rules:cc.bzl", + name: "bar", + ruleClass: "cc_library", + loads: []BazelLoad{{ + file: "//build/bazel/rules:cc.bzl", + symbols: []BazelLoadSymbol{{symbol: "cc_library"}}, + }}, }, }, expectedLoadStatements: `load("//build/bazel/rules:cc.bzl", "cc_library")`, @@ -798,14 +807,20 @@ func TestLoadStatements(t *testing.T) { { bazelTargets: BazelTargets{ BazelTarget{ - name: "foo", - ruleClass: "cc_library", - bzlLoadLocation: "//build/bazel/rules:cc.bzl", + name: "foo", + ruleClass: "cc_library", + loads: []BazelLoad{{ + file: "//build/bazel/rules:cc.bzl", + symbols: []BazelLoadSymbol{{symbol: "cc_library"}}, + }}, }, BazelTarget{ - name: "bar", - ruleClass: "cc_binary", - bzlLoadLocation: "//build/bazel/rules:cc.bzl", + name: "bar", + ruleClass: "cc_binary", + loads: []BazelLoad{{ + file: "//build/bazel/rules:cc.bzl", + symbols: []BazelLoadSymbol{{symbol: "cc_binary"}}, + }}, }, }, expectedLoadStatements: `load("//build/bazel/rules:cc.bzl", "cc_binary", "cc_library")`, @@ -813,19 +828,28 @@ func TestLoadStatements(t *testing.T) { { bazelTargets: BazelTargets{ BazelTarget{ - name: "foo", - ruleClass: "cc_library", - bzlLoadLocation: "//build/bazel/rules:cc.bzl", + name: "foo", + ruleClass: "cc_library", + loads: []BazelLoad{{ + file: "//build/bazel/rules:cc.bzl", + symbols: []BazelLoadSymbol{{symbol: "cc_library"}}, + }}, }, BazelTarget{ - name: "bar", - ruleClass: "cc_binary", - bzlLoadLocation: "//build/bazel/rules:cc.bzl", + name: "bar", + ruleClass: "cc_binary", + loads: []BazelLoad{{ + file: "//build/bazel/rules:cc.bzl", + symbols: []BazelLoadSymbol{{symbol: "cc_binary"}}, + }}, }, BazelTarget{ - name: "baz", - ruleClass: "java_binary", - bzlLoadLocation: "//build/bazel/rules:java.bzl", + name: "baz", + ruleClass: "java_binary", + loads: []BazelLoad{{ + file: "//build/bazel/rules:java.bzl", + symbols: []BazelLoadSymbol{{symbol: "java_binary"}}, + }}, }, }, expectedLoadStatements: `load("//build/bazel/rules:cc.bzl", "cc_binary", "cc_library") @@ -834,19 +858,25 @@ load("//build/bazel/rules:java.bzl", "java_binary")`, { bazelTargets: BazelTargets{ BazelTarget{ - name: "foo", - ruleClass: "cc_binary", - bzlLoadLocation: "//build/bazel/rules:cc.bzl", + name: "foo", + ruleClass: "cc_binary", + loads: []BazelLoad{{ + file: "//build/bazel/rules:cc.bzl", + symbols: []BazelLoadSymbol{{symbol: "cc_binary"}}, + }}, }, BazelTarget{ - name: "bar", - ruleClass: "java_binary", - bzlLoadLocation: "//build/bazel/rules:java.bzl", + name: "bar", + ruleClass: "java_binary", + loads: []BazelLoad{{ + file: "//build/bazel/rules:java.bzl", + symbols: []BazelLoadSymbol{{symbol: "java_binary"}}, + }}, }, BazelTarget{ name: "baz", ruleClass: "genrule", - // Note: no bzlLoadLocation for native rules + // Note: no loads for native rules }, }, expectedLoadStatements: `load("//build/bazel/rules:cc.bzl", "cc_binary")