From f8231dd0ea2aa0c2ba99e404a16d47db1322b223 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Fri, 21 Apr 2023 17:37:11 -0700 Subject: [PATCH] Platform mapping-based product config This allows us to set product variables as build settings instead of loading them from a target's provider, which further allows us to read product config variables in transitions. Bug: 287539062 Bug: 269577299 Test: Presubmits Change-Id: I8497703f706162572ceb3486240e1eb02a37f5f6 --- android/config.go | 10 ++--- android/config_test.go | 10 ++--- android/configured_jars_test.go | 2 +- android/fixture.go | 2 +- android/sdk_version_test.go | 2 +- android/test_config.go | 2 +- android/variable.go | 6 +-- bp2build/bp2build.go | 25 ++++++----- bp2build/bp2build_product_config.go | 69 ++++++++++++++++++++++++++--- cmd/soong_build/main.go | 5 ++- starlark_import/unmarshal.go | 20 ++++++++- starlark_import/unmarshal_test.go | 21 +++++++-- 12 files changed, 134 insertions(+), 40 deletions(-) diff --git a/android/config.go b/android/config.go index fa439625d..f95010e05 100644 --- a/android/config.go +++ b/android/config.go @@ -202,10 +202,10 @@ type VendorConfig soongconfig.SoongConfig // product configuration values are read from Kati-generated soong.variables. type config struct { // Options configurable with soong.variables - productVariables productVariables + productVariables ProductVariables // Only available on configs created by TestConfig - TestProductVariables *productVariables + TestProductVariables *ProductVariables // A specialized context object for Bazel/Soong mixed builds and migration // purposes. @@ -321,7 +321,7 @@ func loadConfig(config *config) error { // loadFromConfigFile loads and decodes configuration options from a JSON file // in the current working directory. -func loadFromConfigFile(configurable *productVariables, filename string) error { +func loadFromConfigFile(configurable *ProductVariables, filename string) error { // Try to open the file configFileReader, err := os.Open(filename) defer configFileReader.Close() @@ -373,7 +373,7 @@ func loadFromConfigFile(configurable *productVariables, filename string) error { // atomically writes the config file in case two copies of soong_build are running simultaneously // (for example, docs generation and ninja manifest generation) -func saveToConfigFile(config *productVariables, filename string) error { +func saveToConfigFile(config *ProductVariables, filename string) error { data, err := json.MarshalIndent(&config, "", " ") if err != nil { return fmt.Errorf("cannot marshal config data: %s", err.Error()) @@ -402,7 +402,7 @@ func saveToConfigFile(config *productVariables, filename string) error { return nil } -func saveToBazelConfigFile(config *productVariables, outDir string) error { +func saveToBazelConfigFile(config *ProductVariables, outDir string) error { dir := filepath.Join(outDir, bazel.SoongInjectionDirName, "product_config") err := createDirIfNonexistent(dir, os.ModePerm) if err != nil { diff --git a/android/config_test.go b/android/config_test.go index 9df5288a1..7d327a27e 100644 --- a/android/config_test.go +++ b/android/config_test.go @@ -75,7 +75,7 @@ Did you mean to use an annotation of ",omitempty"? // run validateConfigAnnotations against each type that might have json annotations func TestProductConfigAnnotations(t *testing.T) { - err := validateConfigAnnotations(&productVariables{}) + err := validateConfigAnnotations(&ProductVariables{}) if err != nil { t.Errorf(err.Error()) } @@ -88,7 +88,7 @@ func TestMissingVendorConfig(t *testing.T) { } } -func verifyProductVariableMarshaling(t *testing.T, v productVariables) { +func verifyProductVariableMarshaling(t *testing.T, v ProductVariables) { dir := t.TempDir() path := filepath.Join(dir, "test.variables") err := saveToConfigFile(&v, path) @@ -96,20 +96,20 @@ func verifyProductVariableMarshaling(t *testing.T, v productVariables) { t.Errorf("Couldn't save default product config: %q", err) } - var v2 productVariables + var v2 ProductVariables err = loadFromConfigFile(&v2, path) if err != nil { t.Errorf("Couldn't load default product config: %q", err) } } func TestDefaultProductVariableMarshaling(t *testing.T) { - v := productVariables{} + v := ProductVariables{} v.SetDefaultConfig() verifyProductVariableMarshaling(t, v) } func TestBootJarsMarshaling(t *testing.T) { - v := productVariables{} + v := ProductVariables{} v.SetDefaultConfig() v.BootJars = ConfiguredJarList{ apexes: []string{"apex"}, diff --git a/android/configured_jars_test.go b/android/configured_jars_test.go index 32c3613f7..4b586e4d7 100644 --- a/android/configured_jars_test.go +++ b/android/configured_jars_test.go @@ -21,7 +21,7 @@ import ( func TestOverrideConfiguredJarLocationFor(t *testing.T) { cfg := NullConfig("", "") - cfg.productVariables = productVariables{ + cfg.productVariables = ProductVariables{ ConfiguredJarLocationOverrides: []string{ "platform:libfoo-old:com.android.foo:libfoo-new", "com.android.bar:libbar-old:platform:libbar-new", diff --git a/android/fixture.go b/android/fixture.go index dbc3bc5e0..6660afd65 100644 --- a/android/fixture.go +++ b/android/fixture.go @@ -369,7 +369,7 @@ func FixtureModifyEnv(mutator func(env map[string]string)) FixturePreparer { // Allow access to the product variables when preparing the fixture. type FixtureProductVariables struct { - *productVariables + *ProductVariables } // Modify product variables. diff --git a/android/sdk_version_test.go b/android/sdk_version_test.go index ea99c4d62..30bd002c2 100644 --- a/android/sdk_version_test.go +++ b/android/sdk_version_test.go @@ -75,7 +75,7 @@ func TestSdkSpecFrom(t *testing.T) { config := NullConfig("", "") - config.productVariables = productVariables{ + config.productVariables = ProductVariables{ Platform_sdk_version: intPtr(31), Platform_sdk_codename: stringPtr("Tiramisu"), Platform_version_active_codenames: []string{"Tiramisu"}, diff --git a/android/test_config.go b/android/test_config.go index 28d9ec403..2a59d9228 100644 --- a/android/test_config.go +++ b/android/test_config.go @@ -35,7 +35,7 @@ func TestConfig(buildDir string, env map[string]string, bp string, fs map[string envCopy["PATH"] = os.Getenv("PATH") config := &config{ - productVariables: productVariables{ + productVariables: ProductVariables{ DeviceName: stringPtr("test_device"), DeviceProduct: stringPtr("test_product"), Platform_sdk_version: intPtr(30), diff --git a/android/variable.go b/android/variable.go index 4442a09eb..f5dddafd8 100644 --- a/android/variable.go +++ b/android/variable.go @@ -185,7 +185,7 @@ type variableProperties struct { var defaultProductVariables interface{} = variableProperties{} -type productVariables struct { +type ProductVariables struct { // Suffix to add to generated Makefiles Make_suffix *string `json:",omitempty"` @@ -489,8 +489,8 @@ func stringPtr(v string) *string { return &v } -func (v *productVariables) SetDefaultConfig() { - *v = productVariables{ +func (v *ProductVariables) SetDefaultConfig() { + *v = ProductVariables{ BuildNumberFile: stringPtr("build_number.txt"), Platform_version_name: stringPtr("S"), diff --git a/bp2build/bp2build.go b/bp2build/bp2build.go index b22cb2861..cfe52db47 100644 --- a/bp2build/bp2build.go +++ b/bp2build/bp2build.go @@ -80,6 +80,12 @@ func Codegen(ctx *CodegenContext) *CodegenMetrics { os.Exit(1) } bp2buildFiles := CreateBazelFiles(ctx.Config(), nil, res.buildFileToTargets, ctx.mode) + injectionFiles, additionalBp2buildFiles, err := CreateSoongInjectionDirFiles(ctx, res.metrics) + if err != nil { + fmt.Printf("%s\n", err.Error()) + os.Exit(1) + } + bp2buildFiles = append(bp2buildFiles, additionalBp2buildFiles...) writeFiles(ctx, bp2buildDir, bp2buildFiles) // Delete files under the bp2build root which weren't just written. An // alternative would have been to delete the whole directory and write these @@ -88,11 +94,6 @@ func Codegen(ctx *CodegenContext) *CodegenMetrics { // performance implications. deleteFilesExcept(ctx, bp2buildDir, bp2buildFiles) - injectionFiles, err := CreateSoongInjectionDirFiles(ctx, res.metrics) - if err != nil { - fmt.Printf("%s\n", err.Error()) - os.Exit(1) - } writeFiles(ctx, android.PathForOutput(ctx, bazel.SoongInjectionDirName), injectionFiles) starlarkDeps, err := starlark_import.GetNinjaDeps() if err != nil { @@ -107,20 +108,20 @@ func Codegen(ctx *CodegenContext) *CodegenMetrics { // This includes // 1. config value(s) that are hardcoded in Soong // 2. product_config variables -func CreateSoongInjectionDirFiles(ctx *CodegenContext, metrics CodegenMetrics) ([]BazelFile, error) { +func CreateSoongInjectionDirFiles(ctx *CodegenContext, metrics CodegenMetrics) ([]BazelFile, []BazelFile, error) { var ret []BazelFile - productConfigFiles, err := CreateProductConfigFiles(ctx) + productConfigInjectionFiles, productConfigBp2BuildDirFiles, err := CreateProductConfigFiles(ctx) if err != nil { - return nil, err + return nil, nil, err } - ret = append(ret, productConfigFiles...) + ret = append(ret, productConfigInjectionFiles...) injectionFiles, err := soongInjectionFiles(ctx.Config(), metrics) if err != nil { - return nil, err + return nil, nil, err } - ret = append(ret, injectionFiles...) - return ret, nil + ret = append(injectionFiles, ret...) + return ret, productConfigBp2BuildDirFiles, nil } // Get the output directory and create it if it doesn't exist. diff --git a/bp2build/bp2build_product_config.go b/bp2build/bp2build_product_config.go index 72244961d..8d5c99c28 100644 --- a/bp2build/bp2build_product_config.go +++ b/bp2build/bp2build_product_config.go @@ -1,14 +1,20 @@ package bp2build import ( + "android/soong/android" + "android/soong/starlark_import" + "encoding/json" "fmt" "os" "path/filepath" "strings" + + "github.com/google/blueprint/proptools" + "go.starlark.net/starlark" ) func CreateProductConfigFiles( - ctx *CodegenContext) ([]BazelFile, error) { + ctx *CodegenContext) ([]BazelFile, []BazelFile, error) { cfg := &ctx.config targetProduct := "unknown" if cfg.HasDeviceProduct() { @@ -25,9 +31,14 @@ func CreateProductConfigFiles( if !strings.HasPrefix(productVariablesFileName, "/") { productVariablesFileName = filepath.Join(ctx.topDir, productVariablesFileName) } - bytes, err := os.ReadFile(productVariablesFileName) + productVariablesBytes, err := os.ReadFile(productVariablesFileName) if err != nil { - return nil, err + return nil, nil, err + } + productVariables := android.ProductVariables{} + err = json.Unmarshal(productVariablesBytes, &productVariables) + if err != nil { + return nil, nil, err } // TODO(b/249685973): the name is product_config_platforms because product_config @@ -39,11 +50,16 @@ func CreateProductConfigFiles( "{VARIANT}", targetBuildVariant, "{PRODUCT_FOLDER}", currentProductFolder) - result := []BazelFile{ + platformMappingContent, err := platformMappingContent(productReplacer.Replace("@soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}"), &productVariables) + if err != nil { + return nil, nil, err + } + + injectionDirFiles := []BazelFile{ newFile( currentProductFolder, "soong.variables.bzl", - `variables = json.decode("""`+strings.ReplaceAll(string(bytes), "\\", "\\\\")+`""")`), + `variables = json.decode("""`+strings.ReplaceAll(string(productVariablesBytes), "\\", "\\\\")+`""")`), newFile( currentProductFolder, "BUILD", @@ -99,6 +115,7 @@ product_labels = [ "product_config_platforms", "common.bazelrc", productReplacer.Replace(` +build --platform_mappings=platform_mappings build --platforms @soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_linux_x86_64 build:android --platforms=@soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT} @@ -120,6 +137,48 @@ build --host_platform @soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_lin build --host_platform @soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_darwin_x86_64 `)), } + bp2buildDirFiles := []BazelFile{ + newFile( + "", + "platform_mappings", + platformMappingContent), + } + return injectionDirFiles, bp2buildDirFiles, nil +} +func platformMappingContent(mainProductLabel string, mainProductVariables *android.ProductVariables) (string, error) { + productsForTesting, err := starlark_import.GetStarlarkValue[map[string]map[string]starlark.Value]("products_for_testing") + if err != nil { + return "", err + } + result := "platforms:\n" + result += platformMappingSingleProduct(mainProductLabel, mainProductVariables) + for product, productVariablesStarlark := range productsForTesting { + productVariables, err := starlarkMapToProductVariables(productVariablesStarlark) + if err != nil { + return "", err + } + result += platformMappingSingleProduct("@//build/bazel/tests/products:"+product, &productVariables) + } + return result, nil +} + +func platformMappingSingleProduct(label string, productVariables *android.ProductVariables) string { + buildSettings := "" + buildSettings += fmt.Sprintf(" --//build/bazel/product_config:apex_global_min_sdk_version_override=%s\n", proptools.String(productVariables.ApexGlobalMinSdkVersionOverride)) + result := "" + for _, extension := range []string{"", "_linux_x86_64", "_linux_bionic_x86_64", "_linux_musl_x86", "_linux_musl_x86_64"} { + result += " " + label + extension + "\n" + buildSettings + } + return result +} + +func starlarkMapToProductVariables(in map[string]starlark.Value) (android.ProductVariables, error) { + var err error + result := android.ProductVariables{} + result.ApexGlobalMinSdkVersionOverride, err = starlark_import.NoneableString(in["ApexGlobalMinSdkVersionOverride"]) + if err != nil { + return result, err + } return result, nil } diff --git a/cmd/soong_build/main.go b/cmd/soong_build/main.go index 22d64a2ed..5ea84bcba 100644 --- a/cmd/soong_build/main.go +++ b/cmd/soong_build/main.go @@ -225,7 +225,7 @@ func runApiBp2build(ctx *android.Context, extraNinjaDeps []string) string { ninjaDeps = append(ninjaDeps, codegenContext.AdditionalNinjaDeps()...) // Create soong_injection repository - soongInjectionFiles, err := bp2build.CreateSoongInjectionDirFiles(codegenContext, bp2build.CreateCodegenMetrics()) + soongInjectionFiles, workspaceFiles, err := bp2build.CreateSoongInjectionDirFiles(codegenContext, bp2build.CreateCodegenMetrics()) maybeQuit(err, "") absoluteSoongInjectionDir := shared.JoinPath(topDir, ctx.Config().SoongOutDir(), bazel.SoongInjectionDirName) for _, file := range soongInjectionFiles { @@ -236,6 +236,9 @@ func runApiBp2build(ctx *android.Context, extraNinjaDeps []string) string { // to allow users to edit/experiment in the synthetic workspace. writeReadWriteFile(absoluteSoongInjectionDir, file) } + for _, file := range workspaceFiles { + writeReadWriteFile(absoluteApiBp2buildDir, file) + } workspace := shared.JoinPath(ctx.Config().SoongOutDir(), "api_bp2build") // Create the symlink forest diff --git a/starlark_import/unmarshal.go b/starlark_import/unmarshal.go index 1b5443782..0e6f13030 100644 --- a/starlark_import/unmarshal.go +++ b/starlark_import/unmarshal.go @@ -25,12 +25,14 @@ import ( ) func Unmarshal[T any](value starlark.Value) (T, error) { - var zero T - x, err := UnmarshalReflect(value, reflect.TypeOf(zero)) + x, err := UnmarshalReflect(value, reflect.TypeOf((*T)(nil)).Elem()) return x.Interface().(T), err } func UnmarshalReflect(value starlark.Value, ty reflect.Type) (reflect.Value, error) { + if ty == reflect.TypeOf((*starlark.Value)(nil)).Elem() { + return reflect.ValueOf(value), nil + } zero := reflect.Zero(ty) var result reflect.Value if ty.Kind() == reflect.Interface { @@ -286,3 +288,17 @@ func typeOfStarlarkValue(value starlark.Value) (reflect.Type, error) { return nil, fmt.Errorf("unimplemented starlark type: %s", value.Type()) } } + +// NoneableString converts a starlark.Value to a string pointer. If the starlark.Value is NoneType, +// a nil pointer will be returned instead. All other types of starlark values are errors. +func NoneableString(value starlark.Value) (*string, error) { + switch v := value.(type) { + case starlark.String: + result := v.GoString() + return &result, nil + case starlark.NoneType: + return nil, nil + default: + return nil, fmt.Errorf("expected string or none, got %q", value.Type()) + } +} diff --git a/starlark_import/unmarshal_test.go b/starlark_import/unmarshal_test.go index ee7a9e340..bc0ea4c49 100644 --- a/starlark_import/unmarshal_test.go +++ b/starlark_import/unmarshal_test.go @@ -30,7 +30,7 @@ func createStarlarkValue(t *testing.T, code string) starlark.Value { return result["x"] } -func TestUnmarshallConcreteType(t *testing.T) { +func TestUnmarshalConcreteType(t *testing.T) { x, err := Unmarshal[string](createStarlarkValue(t, `"foo"`)) if err != nil { t.Error(err) @@ -41,7 +41,7 @@ func TestUnmarshallConcreteType(t *testing.T) { } } -func TestUnmarshallConcreteTypeWithInterfaces(t *testing.T) { +func TestUnmarshalConcreteTypeWithInterfaces(t *testing.T) { x, err := Unmarshal[map[string]map[string]interface{}](createStarlarkValue(t, `{"foo": {"foo2": "foo3"}, "bar": {"bar2": ["bar3"]}}`)) if err != nil { @@ -57,7 +57,22 @@ func TestUnmarshallConcreteTypeWithInterfaces(t *testing.T) { } } -func TestUnmarshall(t *testing.T) { +func TestUnmarshalToStarlarkValue(t *testing.T) { + x, err := Unmarshal[map[string]starlark.Value](createStarlarkValue(t, + `{"foo": "Hi", "bar": None}`)) + if err != nil { + t.Error(err) + return + } + if x["foo"].(starlark.String).GoString() != "Hi" { + t.Errorf("Expected \"Hi\", got: %q", x["foo"].(starlark.String).GoString()) + } + if x["bar"].Type() != "NoneType" { + t.Errorf("Expected \"NoneType\", got: %q", x["bar"].Type()) + } +} + +func TestUnmarshal(t *testing.T) { testCases := []struct { input string expected interface{}