From 319abae1c7200e2c297cafa85592951e2cda8c07 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Tue, 6 Jun 2023 15:12:49 -0700 Subject: [PATCH] Remove --noexperimental_platform_in_output_dir This is a followup to aosp/2606989. This flag is not necessary now that we're using one platform name for all of mixed builds. Also rename current_product to mixed_builds_product so that it's clear that that this platform should only be used for mixed builds. In addition, make the bazelrc files point to the named products again instead of the mixed build product so that b builds will still have qualified outputs, but mixed builds won't. Test: Presubmit and kernel build tools abtd run Change-Id: I7f764cf42cd1323f4b495d1320931f59a076ac63 --- android/bazel_handler.go | 34 +++++++++++--------------- bp2build/bp2build_product_config.go | 38 ++++++++++++----------------- tests/bp2build_bazel_test.sh | 35 ++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 43 deletions(-) diff --git a/android/bazel_handler.go b/android/bazel_handler.go index 28bcaced5..4645e6be5 100644 --- a/android/bazel_handler.go +++ b/android/bazel_handler.go @@ -689,26 +689,20 @@ func (r *builtinBazelRunner) issueBazelCommand(cmdRequest bazel.CmdRequest, path func (context *mixedBuildBazelContext) createBazelCommand(config Config, runName bazel.RunName, command bazelCommand, extraFlags ...string) bazel.CmdRequest { + if runtime.GOOS != "linux" && runtime.GOOS != "darwin" { + panic("Unknown GOOS: " + runtime.GOOS) + } cmdFlags := []string{ "--output_base=" + absolutePath(context.paths.outputBase), command.command, command.expression, "--profile=" + shared.BazelMetricsFilename(context.paths, runName), - // We don't need to set --host_platforms because it's set in bazelrc files - // that the bazel shell script wrapper passes - - // Optimize Ninja rebuilds by ensuring Bazel write into product-agnostic - // output paths for the configured targets that shouldn't be affected by - // TARGET_PRODUCT. Otherwise product agnostic modules will be rebuilt by - // Ninja when the product changes, unconditionally. - // - // For example, Mainline APEXes should be identical regardless of the - // product (modulo arch/cpu). - // - // This flag forcibly disables the platform prefix in the intermediate - // outputs during a mixed build. - "--noexperimental_platform_in_output_dir", + "--host_platform=@soong_injection//product_config_platforms:mixed_builds_product-" + context.targetBuildVariant + "_" + runtime.GOOS + "_x86_64", + // Don't specify --platforms, because on some products/branches (like kernel-build-tools) + // the main platform for mixed_builds_product-variant doesn't exist because an arch isn't + // specified in product config. The derivative platforms that config_node transitions into + // will still work. // Suppress noise "--ui_event_filters=-INFO", @@ -755,9 +749,9 @@ func (context *mixedBuildBazelContext) mainBzlFileContents() []byte { ##################################################### def _config_node_transition_impl(settings, attr): if attr.os == "android" and attr.arch == "target": - target = "current_product-{VARIANT}" + target = "mixed_builds_product-{VARIANT}" else: - target = "current_product-{VARIANT}_%s_%s" % (attr.os, attr.arch) + target = "mixed_builds_product-{VARIANT}_%s_%s" % (attr.os, attr.arch) apex_name = "" if attr.within_apex: # //build/bazel/rules/apex:apex_name has to be set to a non_empty value, @@ -994,9 +988,9 @@ def get_arch(target): platform_name = platforms[0].name if platform_name == "host": return "HOST" - if not platform_name.startswith("current_product-{TARGET_BUILD_VARIANT}"): - fail("expected platform name of the form 'current_product-{TARGET_BUILD_VARIANT}_android_' or 'current_product-{TARGET_BUILD_VARIANT}_linux_', but was " + str(platforms)) - platform_name = platform_name.removeprefix("current_product-{TARGET_BUILD_VARIANT}").removeprefix("_") + if not platform_name.startswith("mixed_builds_product-{TARGET_BUILD_VARIANT}"): + fail("expected platform name of the form 'mixed_builds_product-{TARGET_BUILD_VARIANT}_android_' or 'mixed_builds_product-{TARGET_BUILD_VARIANT}_linux_', but was " + str(platforms)) + platform_name = platform_name.removeprefix("mixed_builds_product-{TARGET_BUILD_VARIANT}").removeprefix("_") config_key = "" if not platform_name: config_key = "target|android" @@ -1005,7 +999,7 @@ def get_arch(target): elif platform_name.startswith("linux_"): config_key = platform_name.removeprefix("linux_") + "|linux" else: - fail("expected platform name of the form 'current_product-{TARGET_BUILD_VARIANT}_android_' or 'current_product-{TARGET_BUILD_VARIANT}_linux_', but was " + str(platforms)) + fail("expected platform name of the form 'mixed_builds_product-{TARGET_BUILD_VARIANT}_android_' or 'mixed_builds_product-{TARGET_BUILD_VARIANT}_linux_', but was " + str(platforms)) within_apex = buildoptions.get("//build/bazel/rules/apex:within_apex") apex_sdk_version = buildoptions.get("//build/bazel/rules/apex:min_sdk_version") diff --git a/bp2build/bp2build_product_config.go b/bp2build/bp2build_product_config.go index 66d0cc554..72244961d 100644 --- a/bp2build/bp2build_product_config.go +++ b/bp2build/bp2build_product_config.go @@ -72,8 +72,13 @@ package(default_visibility = [ 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 +# means that soong-built files that depend on bazel-built files will suddenly get different +# dependency files, because the path changes, and they will be rebuilt. In order to avoid this +# extra rebuilding, make mixed builds always use a single platform so that the bazel artifacts +# are always under the same path. android_product( - name = "current_product-{VARIANT}", + name = "mixed_builds_product-{VARIANT}", soong_variables = _soong_variables, ) `)), @@ -86,7 +91,7 @@ android_product( # TODO: When we start generating the platforms for more than just the # currently lunched product, they should all be listed here product_labels = [ - "@soong_injection//product_config_platforms:current_product-{VARIANT}", + "@soong_injection//product_config_platforms:mixed_builds_product-{VARIANT}", "@soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}" ] `)), @@ -94,38 +99,25 @@ product_labels = [ "product_config_platforms", "common.bazelrc", productReplacer.Replace(` -# current_product refers to the current TARGET_PRODUCT set, usually through -# 'lunch' or 'banchan'. Every build will have a primary TARGET_PRODUCT, but -# bazel supports using other products in tests or configuration transitions. The -# other products can be found in -# @soong_injection//product_config_platforms/products/... -build --platforms @soong_injection//product_config_platforms:current_product-{VARIANT}_linux_x86_64 +build --platforms @soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_linux_x86_64 -build:android --platforms=@soong_injection//product_config_platforms:current_product-{VARIANT} -build:linux_x86_64 --platforms=@soong_injection//product_config_platforms:current_product-{VARIANT}_linux_x86_64 -build:linux_bionic_x86_64 --platforms=@soong_injection//product_config_platforms:current_product-{VARIANT}_linux_bionic_x86_64 -build:linux_musl_x86 --platforms=@soong_injection//product_config_platforms:current_product-{VARIANT}_linux_musl_x86 -build:linux_musl_x86_64 --platforms=@soong_injection//product_config_platforms:current_product-{VARIANT}_linux_musl_x86_64 +build:android --platforms=@soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT} +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 `)), newFile( "product_config_platforms", "linux.bazelrc", productReplacer.Replace(` -build --host_platform @soong_injection//product_config_platforms:current_product-{VARIANT}_linux_x86_64 +build --host_platform @soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_linux_x86_64 `)), newFile( "product_config_platforms", "darwin.bazelrc", productReplacer.Replace(` -build --host_platform product_config_platforms:current_product-{VARIANT}_darwin_x86_64 -`)), - newFile( - "product_config_platforms", - "platform_mappings", - productReplacer.Replace(` -flags: - --cpu=k8 - @soong_injection//product_config_platforms:current_product-{VARIANT} +build --host_platform @soong_injection//{PRODUCT_FOLDER}:{PRODUCT}-{VARIANT}_darwin_x86_64 `)), } diff --git a/tests/bp2build_bazel_test.sh b/tests/bp2build_bazel_test.sh index 71e6af009..7878be65f 100755 --- a/tests/bp2build_bazel_test.sh +++ b/tests/bp2build_bazel_test.sh @@ -382,4 +382,39 @@ EOF run_bazel build --config=android --config=api_bp2build //foo:libfoo.contribution } +function test_bazel_standalone_output_paths_contain_product_name { + setup + mkdir -p a + cat > a/Android.bp < a/qq.cc < a/qq.h <