Skip to content

Commit

Permalink
Add layering_check support for macOS
Browse files Browse the repository at this point in the history
There were 2 things with the previous implementation that needed to be improved here:

1. Apple Clang has a bug where it doesn't pass module compiler flags to the underlying -cc1 invocation, so we have to manually pass them directly to that invocation with -Xclang
2. The previous search script was too aggressive and slow for macOS. The macOS SDK has tons of files that aren't headers, and tons of symlinks pointing to other files within the SDK. This adds a fork in the script to run a version that works with Apple SDKs. The time difference on my machine is 41s->6s. 6s is still pretty long so if desired we can put this behavior behind an env var for users to opt in with.

I've added a hermetic version of this to the apple_support toolchain, but similar to the Linux setup here the modulemap file includes absolute paths.

Closes #22259.

PiperOrigin-RevId: 635735410
Change-Id: Icb3f30dd7319f147b0db54b3b42d4dc9032dc1a0
  • Loading branch information
keith authored and copybara-github committed May 21, 2024
1 parent fdaaa6a commit 0a65893
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 16 deletions.
5 changes: 0 additions & 5 deletions src/test/shell/bazel/bazel_layering_check_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,6 @@ EOF
}

function test_bazel_layering_check() {
if is_darwin; then
echo "This test doesn't run on Darwin. Skipping."
return
fi

local -r clang_tool=$(which clang)
if [[ ! -x ${clang_tool:-/usr/bin/clang_tool} ]]; then
echo "clang not installed. Skipping test."
Expand Down
19 changes: 14 additions & 5 deletions tools/cpp/generate_system_module_map.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/sh
#!/bin/bash
# Copyright 2020 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
Expand All @@ -17,10 +17,19 @@ set -eu

echo 'module "crosstool" [system] {'

for dir in $@; do
find -L "${dir}" -type f 2>/dev/null | LANG=C sort | uniq | while read header; do
echo " textual header \"${header}\""
if [[ "$OSTYPE" == darwin* ]]; then
for dir in $@; do
find "$dir" -type f \( -name "*.h" -o -name "*.def" -o -path "*/c++/*" \) \
| LANG=C sort -u | while read -r header; do
echo " textual header \"${header}\""
done
done
done
else
for dir in $@; do
find -L "${dir}" -type f 2>/dev/null | LANG=C sort -u | while read -r header; do
echo " textual header \"${header}\""
done
done
fi

echo "}"
2 changes: 1 addition & 1 deletion tools/cpp/unix_cc_configure.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ def configure_unix_toolchain(repository_ctx, cpu_value, overriden_tools):
["/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk"],
)

generate_modulemap = is_clang and not darwin
generate_modulemap = is_clang
if generate_modulemap:
repository_ctx.file("module.modulemap", _generate_system_module_map(
repository_ctx,
Expand Down
12 changes: 7 additions & 5 deletions tools/cpp/unix_cc_toolchain_config.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def _target_os_version(ctx):
xcode_config = ctx.attr._xcode_config[apple_common.XcodeVersionConfig]
return xcode_config.minimum_os_for_platform_type(platform_type)

def layering_check_features(compiler):
def layering_check_features(compiler, is_macos):
if compiler != "clang":
return []
return [
Expand All @@ -53,8 +53,10 @@ def layering_check_features(compiler):
],
flag_groups = [
flag_group(
flags = [
# macOS requires -Xclang because of a bug in Apple Clang
flags = (["-Xclang"] if is_macos else []) + [
"-fmodule-name=%{module_name}",
] + (["-Xclang"] if is_macos else []) + [
"-fmodule-map-file=%{module_map_file}",
],
),
Expand Down Expand Up @@ -86,7 +88,7 @@ def layering_check_features(compiler):
]),
flag_group(
iterate_over = "dependent_module_map_files",
flags = [
flags = (["-Xclang"] if is_macos else []) + [
"-fmodule-map-file=%{dependent_module_map_files}",
],
),
Expand Down Expand Up @@ -1487,7 +1489,7 @@ def _impl(ctx):
unfiltered_compile_flags_feature,
treat_warnings_as_errors_feature,
archive_param_file_feature,
] + layering_check_features(ctx.attr.compiler)
] + layering_check_features(ctx.attr.compiler, is_macos = False)
else:
# macOS artifact name patterns differ from the defaults only for dynamic
# libraries.
Expand Down Expand Up @@ -1528,7 +1530,7 @@ def _impl(ctx):
treat_warnings_as_errors_feature,
archive_param_file_feature,
generate_linkmap_feature,
]
] + layering_check_features(ctx.attr.compiler, is_macos = True)

parse_headers_action_configs, parse_headers_features = parse_headers_support(
parse_headers_tool_path = ctx.attr.tool_paths.get("parse_headers"),
Expand Down

0 comments on commit 0a65893

Please sign in to comment.