From c668b780857a622c78e4502a17f6cd98c58357f0 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Tue, 8 Jan 2019 15:38:02 -0800 Subject: [PATCH 1/3] Include CPU in darwin platform string Bazel uses the CPU to determine the --apple_platform_type to build for in some cases. Previously on a macOS machine the CPU would be returned as just `darwin`, which isn't considered a valid macOS CPU, instead we should return what we support and include the architecture. --- .../build/lib/analysis/config/AutoCpuConverter.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/AutoCpuConverter.java b/src/main/java/com/google/devtools/build/lib/analysis/config/AutoCpuConverter.java index 32d25c8309f0a4..4d798805cb701f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/AutoCpuConverter.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/AutoCpuConverter.java @@ -30,11 +30,15 @@ public class AutoCpuConverter implements Converter { @Override public String convert(String input) throws OptionsParsingException { if (input.isEmpty()) { - // TODO(philwo) - replace these deprecated names with more logical ones (e.g. k8 becomes - // linux-x86_64, darwin includes the CPU architecture, ...). + // TODO(philwo) - replace these deprecated names with more logical ones (e.g. k8 becomes linux-x86_64, ...). switch (OS.getCurrent()) { case DARWIN: - return "darwin"; + switch (CPU.getCurrent()) { + case X86_64: + return "darwin_x86_64"; + default: + return "darwin"; + } case FREEBSD: return "freebsd"; case WINDOWS: From 9bfe8eb5c771cb0e4b492c652b10dda047726373 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Mon, 14 Jan 2019 12:03:35 -0800 Subject: [PATCH 2/3] Change to only support darwin_x86_64 --- .../lib/analysis/config/AutoCpuConverter.java | 7 +----- .../build/lib/analysis/mock/MOCK_CROSSTOOL | 4 ++-- .../lib/packages/util/BazelMockCcSupport.java | 22 +++++++++---------- .../cpp/CrosstoolConfigurationHelper.java | 2 +- tools/cpp/BUILD | 14 ++++++------ tools/cpp/CROSSTOOL | 4 ++-- tools/cpp/cc_configure.bzl | 2 +- tools/cpp/crosstool_lib.bzl | 2 +- tools/cpp/lib_cc_configure.bzl | 2 +- tools/cpp/osx_cc_configure.bzl | 2 +- tools/cpp/unix_cc_configure.bzl | 2 +- tools/osx/crosstool/BUILD.tpl | 4 ++-- 12 files changed, 31 insertions(+), 36 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/AutoCpuConverter.java b/src/main/java/com/google/devtools/build/lib/analysis/config/AutoCpuConverter.java index 4d798805cb701f..2b0b5b3e35b6e0 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/AutoCpuConverter.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/AutoCpuConverter.java @@ -33,12 +33,7 @@ public String convert(String input) throws OptionsParsingException { // TODO(philwo) - replace these deprecated names with more logical ones (e.g. k8 becomes linux-x86_64, ...). switch (OS.getCurrent()) { case DARWIN: - switch (CPU.getCurrent()) { - case X86_64: - return "darwin_x86_64"; - default: - return "darwin"; - } + return "darwin_x86_64"; case FREEBSD: return "freebsd"; case WINDOWS: diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/MOCK_CROSSTOOL b/src/test/java/com/google/devtools/build/lib/analysis/mock/MOCK_CROSSTOOL index a8bee8897fc78b..22c00cadce7854 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/mock/MOCK_CROSSTOOL +++ b/src/test/java/com/google/devtools/build/lib/analysis/mock/MOCK_CROSSTOOL @@ -367,7 +367,7 @@ toolchain { host_system_name: "local" target_system_name: "local" - target_cpu: "darwin" + target_cpu: "darwin_x86_64" target_libc: "macosx" compiler: "compiler" linking_mode_flags { mode: DYNAMIC } @@ -407,7 +407,7 @@ toolchain { toolchain_identifier: "darwin-no-dyn-linker" host_system_name: "local" target_system_name: "local" - target_cpu: "darwin" + target_cpu: "darwin_x86_64" target_libc: "macosx" compiler: "compiler_no_dyn_linker" # No linking_mode_flags here diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockCcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockCcSupport.java index 317dce7dbb1873..080ce690a137af 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockCcSupport.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockCcSupport.java @@ -86,7 +86,7 @@ public void setup(MockToolsConfig config) throws IOException { " 'local': ':cc-compiler-local',", " 'k8': ':cc-compiler-k8',", " 'piii': ':cc-compiler-piii-gcc-4.4.0',", - " 'darwin': ':cc-compiler-darwin',", + " 'darwin_x86_64': ':cc-compiler-darwin_x86_64',", " 'ios_x86_64': ':cc-compiler-ios_x86_64',", " 'armeabi-v7a': ':cc-compiler-armeabi-v7a',", " 'x64_windows': ':cc-compiler-x64_windows',", @@ -95,8 +95,8 @@ public void setup(MockToolsConfig config) throws IOException { " 'k8|compiler': ':cc-compiler-k8',", " 'k8|compiler_no_dyn_linker': ':cc-no-dyn-linker-k8',", " 'piii|compiler': ':cc-compiler-piii-gcc-4.4.0',", - " 'darwin|compiler': ':cc-compiler-darwin',", - " 'darwin|compiler_no_dyn_linker': ':cc-no-dyn-linker-darwin',", + " 'darwin_x86_64|compiler': ':cc-compiler-darwin_x86_64',", + " 'darwin_x86_64|compiler_no_dyn_linker': ':cc-no-dyn-linker-darwin_x86_64',", " 'ios_x86_64|compiler': ':cc-compiler-ios_x86_64',", " 'armeabi-v7a|compiler': ':cc-compiler-armeabi-v7a',", " 'x64_windows|windows_msys64': ':cc-compiler-x64_windows',", @@ -200,9 +200,9 @@ public void setup(MockToolsConfig config) throws IOException { " toolchain = ':cc-compiler-piii-gcc-4.4.0',", " toolchain_type = ':toolchain_type',", ")", - "cc_toolchain(name = 'cc-compiler-darwin', all_files = ':empty', ", + "cc_toolchain(name = 'cc-compiler-darwin_x86_64', all_files = ':empty', ", " compiler_files = ':empty',", - " cpu = 'darwin',", + " cpu = 'darwin_x86_64',", " compiler = 'compiler',", " dwp_files = ':empty',", " dynamic_runtime_lib = ':empty', ", @@ -210,33 +210,33 @@ public void setup(MockToolsConfig config) throws IOException { " module_map = 'crosstool.cppmap', supports_header_parsing = 1,", " objcopy_files = ':empty', static_runtime_lib = ':empty', strip_files = ':empty',", ")", - "toolchain(name = 'cc-toolchain-darwin',", + "toolchain(name = 'cc-toolchain-darwin_x86_64',", // Needs to be compatible with all execution environments for tests to work properly. " exec_compatible_with = [],", " target_compatible_with = [", " '@bazel_tools//platforms:x86_64',", " '@bazel_tools//platforms:osx',", " ],", - " toolchain = ':cc-compiler-darwin',", + " toolchain = ':cc-compiler-darwin_x86_64',", " toolchain_type = ':toolchain_type',", ")", - "cc_toolchain(name = 'cc-no-dyn-linker-darwin', all_files = ':empty', ", + "cc_toolchain(name = 'cc-no-dyn-linker-darwin_x86_64', all_files = ':empty', ", " compiler_files = ':empty',", - " cpu = 'darwin',", + " cpu = 'darwin_x86_64',", " compiler = 'compiler_no_dyn_linker',", " dwp_files = ':empty', dynamic_runtime_lib = ':empty', ", " ar_files = ':empty', as_files = ':empty', linker_files = ':empty',", " module_map = 'crosstool.cppmap', supports_header_parsing = 1,", " objcopy_files = ':empty', static_runtime_lib = ':empty', strip_files = ':empty',", ")", - "toolchain(name = 'cc-toolchain-no-dyn-linker-darwin',", + "toolchain(name = 'cc-toolchain-no-dyn-linker-darwin_x86_64',", // Needs to be compatible with all execution environments for tests to work properly. " exec_compatible_with = [],", " target_compatible_with = [", " '@bazel_tools//platforms:x86_64',", " '@bazel_tools//platforms:osx',", " ],", - " toolchain = ':cc-no-dyn-linker-darwin',", + " toolchain = ':cc-no-dyn-linker-darwin_x86_64',", " toolchain_type = ':toolchain_type',", ")", "cc_toolchain(name = 'cc-compiler-ios_x86_64', all_files = ':empty', ", diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationHelper.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationHelper.java index 5c43c3f7286c79..55e26aede83a1c 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationHelper.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationHelper.java @@ -74,7 +74,7 @@ public static String defaultCpu() { return "unknown"; } } - return OS.getCurrent() == OS.DARWIN ? "darwin" : "k8"; + return OS.getCurrent() == OS.DARWIN ? "darwin_x86_64" : "k8"; } public static CrosstoolConfig.CrosstoolRelease simpleCompleteToolchainProto() { diff --git a/tools/cpp/BUILD b/tools/cpp/BUILD index abad32cff94420..1f818e855df3e2 100644 --- a/tools/cpp/BUILD +++ b/tools/cpp/BUILD @@ -89,7 +89,7 @@ cc_toolchain_suite( name = "default-toolchain", toolchains = { "armeabi-v7a|compiler": ":cc-compiler-armeabi-v7a", - "darwin|compiler": ":cc-compiler-darwin", + "darwin_x86_64|compiler": ":cc-compiler-darwin", "freebsd|compiler": ":cc-compiler-freebsd", "local|compiler": ":cc-compiler-local", "x64_windows|compiler": ":cc-compiler-x64_windows", @@ -102,7 +102,7 @@ cc_toolchain_suite( "s390x": ":cc-compiler-local", "ppc": ":cc-compiler-local", "ppc64": ":cc-compiler-local", - "darwin": ":cc-compiler-darwin", + "darwin_x86_64": ":cc-compiler-darwin", "freebsd": ":cc-compiler-freebsd", "armeabi-v7a": ":cc-compiler-armeabi-v7a", "x64_windows": ":cc-compiler-x64_windows", @@ -227,12 +227,12 @@ toolchain( ) cc_toolchain( - name = "cc-compiler-darwin", + name = "cc-compiler-darwin_x86_64", all_files = ":osx_wrapper", ar_files = ":empty", as_files = ":empty", compiler_files = ":osx_wrapper", - cpu = "darwin", + cpu = "darwin_x86_64", dwp_files = ":empty", dynamic_runtime_libs = [":empty"], linker_files = ":osx_wrapper", @@ -240,11 +240,11 @@ cc_toolchain( static_runtime_libs = [":empty"], strip_files = ":empty", supports_param_files = 0, - toolchain_identifier = "local_darwin", + toolchain_identifier = "local_darwin_x86_64", ) toolchain( - name = "cc-toolchain-darwin", + name = "cc-toolchain-darwin_x86_64", exec_compatible_with = [ "@bazel_tools//platforms:x86_64", "@bazel_tools//platforms:osx", @@ -253,7 +253,7 @@ toolchain( "@bazel_tools//platforms:x86_64", "@bazel_tools//platforms:osx", ], - toolchain = ":cc-compiler-darwin", + toolchain = ":cc-compiler-darwin_x86_64", toolchain_type = ":toolchain_type", ) diff --git a/tools/cpp/CROSSTOOL b/tools/cpp/CROSSTOOL index 1eef9de16385b9..3afa8858c7163c 100644 --- a/tools/cpp/CROSSTOOL +++ b/tools/cpp/CROSSTOOL @@ -159,9 +159,9 @@ toolchain { host_system_name: "local" needsPic: true target_libc: "macosx" - target_cpu: "darwin" + target_cpu: "darwin_x86_64" target_system_name: "local" - toolchain_identifier: "local_darwin" + toolchain_identifier: "local_darwin_x86_64" tool_path { name: "ar" path: "/usr/bin/libtool" } tool_path { name: "compat-ld" path: "/usr/bin/ld" } diff --git a/tools/cpp/cc_configure.bzl b/tools/cpp/cc_configure.bzl index 699c3489b3758a..a89be3ebbed99c 100644 --- a/tools/cpp/cc_configure.bzl +++ b/tools/cpp/cc_configure.bzl @@ -49,7 +49,7 @@ def cc_autoconf_impl(repository_ctx, overriden_tools = dict()): # TODO(ibiryukov): overriden_tools are only supported in configure_unix_toolchain. # We might want to add that to Windows too(at least for msys toolchain). configure_windows_toolchain(repository_ctx) - elif (cpu_value == "darwin" and + elif (cpu_value == "darwin_x86_64" and ("BAZEL_USE_CPP_ONLY_TOOLCHAIN" not in env or env["BAZEL_USE_CPP_ONLY_TOOLCHAIN"] != "1")): configure_osx_toolchain(repository_ctx, overriden_tools) else: diff --git a/tools/cpp/crosstool_lib.bzl b/tools/cpp/crosstool_lib.bzl index 8d2ce143177a3e..e59ac2a5c4ae75 100644 --- a/tools/cpp/crosstool_lib.bzl +++ b/tools/cpp/crosstool_lib.bzl @@ -284,7 +284,7 @@ def _is_linux(platform): return platform == "k8" def _is_darwin(platform): - return platform == "darwin" + return platform.startswith("darwin") def _is_msvc(platform): return platform == "msvc" diff --git a/tools/cpp/lib_cc_configure.bzl b/tools/cpp/lib_cc_configure.bzl index ebb0a05e5017ab..0d2fb25d8b83c0 100644 --- a/tools/cpp/lib_cc_configure.bzl +++ b/tools/cpp/lib_cc_configure.bzl @@ -178,7 +178,7 @@ def get_cpu_value(repository_ctx): """Compute the cpu_value based on the OS name. Doesn't %-escape the result!""" os_name = repository_ctx.os.name.lower() if os_name.startswith("mac os"): - return "darwin" + return "darwin_x86_64" if os_name.find("freebsd") != -1: return "freebsd" if os_name.find("windows") != -1: diff --git a/tools/cpp/osx_cc_configure.bzl b/tools/cpp/osx_cc_configure.bzl index aef9e34776b60b..9f59f39532451f 100644 --- a/tools/cpp/osx_cc_configure.bzl +++ b/tools/cpp/osx_cc_configure.bzl @@ -157,4 +157,4 @@ def configure_osx_toolchain(repository_ctx, overriden_tools): {"%{cxx_builtin_include_directory}": "\n".join(escaped_cxx_include_directories)}, ) else: - configure_unix_toolchain(repository_ctx, cpu_value = "darwin", overriden_tools = overriden_tools) + configure_unix_toolchain(repository_ctx, cpu_value = "darwin_x86_64", overriden_tools = overriden_tools) diff --git a/tools/cpp/unix_cc_configure.bzl b/tools/cpp/unix_cc_configure.bzl index 4ef6e01722391e..bb62d7186c3324 100644 --- a/tools/cpp/unix_cc_configure.bzl +++ b/tools/cpp/unix_cc_configure.bzl @@ -473,7 +473,7 @@ def configure_unix_toolchain(repository_ctx, cpu_value, overriden_tools): ]) repository_ctx.file("tools/cpp/empty.cc", "int main() {}") - darwin = cpu_value == "darwin" + darwin = cpu_value == "darwin_x86_64" cc = _find_generic(repository_ctx, "gcc", "CC", overriden_tools) overriden_tools = dict(overriden_tools) diff --git a/tools/osx/crosstool/BUILD.tpl b/tools/osx/crosstool/BUILD.tpl index 903c701c02ba13..ba596d6c5c6449 100644 --- a/tools/osx/crosstool/BUILD.tpl +++ b/tools/osx/crosstool/BUILD.tpl @@ -10,9 +10,9 @@ CC_TOOLCHAINS = [( ":cc-compiler-" + cpu, ) for cpu in OSX_TOOLS_ARCHS] + [ ("k8|compiler", ":cc-compiler-darwin_x86_64", ), - ("darwin|compiler", ":cc-compiler-darwin_x86_64", ), + ("darwin_x86_64|compiler", ":cc-compiler-darwin_x86_64", ), ("k8", ":cc-compiler-darwin_x86_64", ), - ("darwin", ":cc-compiler-darwin_x86_64", ), + ("darwin_x86_64", ":cc-compiler-darwin_x86_64", ), ] cc_library( From 7c168440b4184b25647d96ba44e424ccf167f869 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Mon, 14 Jan 2019 13:34:13 -0800 Subject: [PATCH 3/3] Separate static cc_library test for darwin --- .../build/lib/rules/cpp/CcCommonTest.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java index 9b76995d817817..003de1a54927a8 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java @@ -17,6 +17,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.baseArtifactNames; import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.baseNamesOf; +import static org.junit.Assume.assumeTrue; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -47,6 +48,7 @@ import com.google.devtools.build.lib.rules.platform.PlatformRules; import com.google.devtools.build.lib.rules.repository.CoreWorkspaceRules; import com.google.devtools.build.lib.util.FileType; +import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.ModifiedFileSet; import com.google.devtools.build.lib.vfs.PathFragment; @@ -689,6 +691,7 @@ public void testExpandLabelInLinkoptsAgainstSrc() throws Exception { @Test public void testCcLibraryWithDashStatic() throws Exception { + assumeTrue(OS.getCurrent() != OS.DARWIN); checkWarning( "badlib", "lib_with_dash_static", @@ -701,6 +704,21 @@ public void testCcLibraryWithDashStatic() throws Exception { " linkopts = [ '-static' ])"); } + @Test + public void testCcLibraryWithDashStaticOnDarwin() throws Exception { + assumeTrue(OS.getCurrent() == OS.DARWIN); + checkError( + "badlib", + "lib_with_dash_static", + // message: + "in linkopts attribute of cc_library rule //badlib:lib_with_dash_static: " + + "Apple builds do not support statically linked binaries", + // build file: + "cc_library(name = 'lib_with_dash_static',", + " srcs = [ 'ok.cc' ],", + " linkopts = [ '-static' ])"); + } + @Test public void testStampTests() throws Exception { scratch.file(