Skip to content

Commit

Permalink
Automated rollback of commit 0a65893.
Browse files Browse the repository at this point in the history
*** Reason for rollback ***

Broke Bazel postsubmit: https://buildkite.com/bazel/bazel-bazel/builds/28021#018f9a8b-8711-4f02-af8a-c29a1e7c6255

#22259 (comment)

*** Original change description ***

Add layering_check support for macOS

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 symlin...

***

RELNOTES:
PiperOrigin-RevId: 635852793
Change-Id: I03b0831706a555bbcf141742aefa3bf48ed0caf3
  • Loading branch information
meteorcloudy authored and copybara-github committed May 21, 2024
1 parent c186760 commit 1f1b4fd
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 22 deletions.
5 changes: 5 additions & 0 deletions src/test/shell/bazel/bazel_layering_check_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,11 @@ 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: 5 additions & 14 deletions tools/cpp/generate_system_module_map.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/bash
#!/bin/sh
# Copyright 2020 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
Expand All @@ -17,19 +17,10 @@ set -eu

echo 'module "crosstool" [system] {'

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
for dir in $@; do
find -L "${dir}" -type f 2>/dev/null | LANG=C sort | uniq | while read header; do
echo " textual header \"${header}\""
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
done

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
generate_modulemap = is_clang and not darwin
if generate_modulemap:
repository_ctx.file("module.modulemap", _generate_system_module_map(
repository_ctx,
Expand Down
12 changes: 5 additions & 7 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, is_macos):
def layering_check_features(compiler):
if compiler != "clang":
return []
return [
Expand All @@ -53,10 +53,8 @@ def layering_check_features(compiler, is_macos):
],
flag_groups = [
flag_group(
# macOS requires -Xclang because of a bug in Apple Clang
flags = (["-Xclang"] if is_macos else []) + [
flags = [
"-fmodule-name=%{module_name}",
] + (["-Xclang"] if is_macos else []) + [
"-fmodule-map-file=%{module_map_file}",
],
),
Expand Down Expand Up @@ -88,7 +86,7 @@ def layering_check_features(compiler, is_macos):
]),
flag_group(
iterate_over = "dependent_module_map_files",
flags = (["-Xclang"] if is_macos else []) + [
flags = [
"-fmodule-map-file=%{dependent_module_map_files}",
],
),
Expand Down Expand Up @@ -1489,7 +1487,7 @@ def _impl(ctx):
unfiltered_compile_flags_feature,
treat_warnings_as_errors_feature,
archive_param_file_feature,
] + layering_check_features(ctx.attr.compiler, is_macos = False)
] + layering_check_features(ctx.attr.compiler)
else:
# macOS artifact name patterns differ from the defaults only for dynamic
# libraries.
Expand Down Expand Up @@ -1530,7 +1528,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 1f1b4fd

Please sign in to comment.