From 21ff46a639a83084975251ca7c21cfc8d74763eb Mon Sep 17 00:00:00 2001 From: Brentley Jones Date: Wed, 9 Feb 2022 07:41:41 -0600 Subject: [PATCH] Fix `ctx.fragments.apple.single_arch_cpu` returning incorrect cpu for tools when host cpu and exec cpu are different (#14751) Fixes https://github.com/bazelbuild/bazel/issues/14291 Closes #14665. PiperOrigin-RevId: 425886938 (cherry picked from commit fce7ea8d5e0facfc125ae7c37bfb4b9a7c586e40) Co-authored-by: Thi Doan --- .../lib/rules/apple/AppleConfiguration.java | 15 ++- .../lib/rules/apple/AppleFragmentTest.java | 101 ++++++++++++++++++ .../devtools/build/lib/rules/apple/BUILD | 1 + 3 files changed, 115 insertions(+), 2 deletions(-) create mode 100644 src/test/java/com/google/devtools/build/lib/rules/apple/AppleFragmentTest.java diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java index a7a8a3e7619907..886677e1f91a40 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java @@ -73,6 +73,9 @@ public class AppleConfiguration extends Fragment implements AppleConfigurationAp /** Prefix for iOS cpu values */ public static final String IOS_CPU_PREFIX = "ios_"; + /** Prefix for macOS cpu values */ + private static final String MACOS_CPU_PREFIX = "darwin_"; + // TODO(b/180572694): Remove after platforms based toolchain resolution supported. /** Prefix for forced iOS simulator cpu values */ public static final String IOS_FORCED_SIMULATOR_CPU_PREFIX = "sim_"; @@ -122,7 +125,7 @@ public static AppleCpus create(AppleCommandLineOptions options, CoreOptions core : ImmutableList.copyOf(options.tvosCpus); ImmutableList macosCpus = (options.macosCpus == null || options.macosCpus.isEmpty()) - ? ImmutableList.of(AppleCommandLineOptions.DEFAULT_MACOS_CPU) + ? ImmutableList.of(macosCpuFromCpu(coreOptions.cpu)) : ImmutableList.copyOf(options.macosCpus); ImmutableList catalystCpus = (options.catalystCpus == null || options.catalystCpus.isEmpty()) @@ -148,7 +151,7 @@ public static AppleCpus create(AppleCommandLineOptions options, CoreOptions core abstract ImmutableList catalystCpus(); } - /** Determines cpu value from apple-specific toolchain identifier. */ + /** Determines iOS cpu value from apple-specific toolchain identifier. */ public static String iosCpuFromCpu(String cpu) { if (cpu.startsWith(IOS_CPU_PREFIX)) { return cpu.substring(IOS_CPU_PREFIX.length()); @@ -157,6 +160,14 @@ public static String iosCpuFromCpu(String cpu) { } } + /** Determines macOS cpu value from apple-specific toolchain identifier. */ + private static String macosCpuFromCpu(String cpu) { + if (cpu.startsWith(MACOS_CPU_PREFIX)) { + return cpu.substring(MACOS_CPU_PREFIX.length()); + } + return AppleCommandLineOptions.DEFAULT_MACOS_CPU; + } + public AppleCommandLineOptions getOptions() { return options; } diff --git a/src/test/java/com/google/devtools/build/lib/rules/apple/AppleFragmentTest.java b/src/test/java/com/google/devtools/build/lib/rules/apple/AppleFragmentTest.java new file mode 100644 index 00000000000000..b126b6a0eba0ea --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/rules/apple/AppleFragmentTest.java @@ -0,0 +1,101 @@ +// Copyright 2022 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.rules.apple; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.packages.Provider; +import com.google.devtools.build.lib.packages.StarlarkProvider; +import com.google.devtools.build.lib.packages.StructImpl; +import com.google.devtools.build.lib.testutil.TestConstants; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for the Starlark interface of Apple fragment. */ +@RunWith(JUnit4.class) +public class AppleFragmentTest extends BuildViewTestCase { + + @Before + public void setup() throws Exception { + scratch.file( + "rules.bzl", + "MyInfo = provider()", + "def _my_binary_impl(ctx):", + " out = ctx.actions.declare_file(ctx.label.name)", + " ctx.actions.write(out, '')", + " return [", + " DefaultInfo(executable = out),", + " MyInfo(", + " exec_cpu = ctx.fragments.apple.single_arch_cpu,", + " ),", + " ]", + "my_binary = rule(", + " fragments = ['apple'],", + " implementation = _my_binary_impl,", + ")", + "def _my_rule_impl(ctx):", + " return ctx.attr._tool[MyInfo]", + "my_rule = rule(", + " _my_rule_impl,", + " attrs = {", + " '_tool': attr.label(", + " cfg = 'exec',", + " executable = True,", + " default = ('//:bin'),", + " ),", + " },", + ")"); + scratch.file( + "BUILD", + "load(':rules.bzl', 'my_binary', 'my_rule')", + "my_binary(name = 'bin')", + "my_rule(name = 'a')", + "platform(", + " name = 'macos_arm64',", + " constraint_values = [", + " '" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "cpu:aarch64',", + " '" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "os:osx',", + " ],", + ")"); + scratch.file( + "/workspace/platform_mappings", + "platforms:", + " //:macos_arm64", + " --cpu=darwin_arm64", + "flags:", + " --cpu=darwin_arm64", + " --apple_platform_type=macos", + " //:macos_arm64"); + invalidatePackages(false); + } + + @Test + public void appleFragmentSingleArchCpuOnExtraExecPlatform() throws Exception { + // Test that ctx.fragments.apple.single_arch_cpu returns the execution + // platform's cpu in a tool's rule context. + useConfiguration("--extra_execution_platforms=//:macos_arm64"); + ConfiguredTarget configuredTarget = getConfiguredTarget("//:a"); + Provider.Key key = + new StarlarkProvider.Key(Label.parseAbsolute("//:rules.bzl", ImmutableMap.of()), "MyInfo"); + StructImpl myInfo = (StructImpl) configuredTarget.get(key); + assertThat((String) myInfo.getValue("exec_cpu")).isEqualTo("arm64"); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/rules/apple/BUILD b/src/test/java/com/google/devtools/build/lib/rules/apple/BUILD index 1906a5289bb977..6cca6cff3b9d36 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/apple/BUILD +++ b/src/test/java/com/google/devtools/build/lib/rules/apple/BUILD @@ -32,6 +32,7 @@ java_library( "//src/test/java/com/google/devtools/build/lib/analysis/util", "//src/test/java/com/google/devtools/build/lib/packages:testutil", "//src/test/java/com/google/devtools/build/lib/testutil", + "//src/test/java/com/google/devtools/build/lib/testutil:TestConstants", "//third_party:guava", "//third_party:guava-testlib", "//third_party:jsr305",