From 0a658930a9fe39f98206f32fac068c97ebd1f09f Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Tue, 21 May 2024 02:42:52 -0700 Subject: [PATCH] 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 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 --- .../shell/bazel/bazel_layering_check_test.sh | 5 ----- tools/cpp/generate_system_module_map.sh | 19 ++++++++++++++----- tools/cpp/unix_cc_configure.bzl | 2 +- tools/cpp/unix_cc_toolchain_config.bzl | 12 +++++++----- 4 files changed, 22 insertions(+), 16 deletions(-) diff --git a/src/test/shell/bazel/bazel_layering_check_test.sh b/src/test/shell/bazel/bazel_layering_check_test.sh index 4ccf75df384499..720a698ae30949 100755 --- a/src/test/shell/bazel/bazel_layering_check_test.sh +++ b/src/test/shell/bazel/bazel_layering_check_test.sh @@ -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." diff --git a/tools/cpp/generate_system_module_map.sh b/tools/cpp/generate_system_module_map.sh index abbaec2e882328..deb52c2f6b6548 100755 --- a/tools/cpp/generate_system_module_map.sh +++ b/tools/cpp/generate_system_module_map.sh @@ -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"); @@ -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 "}" diff --git a/tools/cpp/unix_cc_configure.bzl b/tools/cpp/unix_cc_configure.bzl index 3f162bdeb12551..ef8b8b49fb6fc8 100644 --- a/tools/cpp/unix_cc_configure.bzl +++ b/tools/cpp/unix_cc_configure.bzl @@ -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, diff --git a/tools/cpp/unix_cc_toolchain_config.bzl b/tools/cpp/unix_cc_toolchain_config.bzl index a361e7b02e2ef6..5a0cd710575076 100644 --- a/tools/cpp/unix_cc_toolchain_config.bzl +++ b/tools/cpp/unix_cc_toolchain_config.bzl @@ -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 [ @@ -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}", ], ), @@ -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}", ], ), @@ -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. @@ -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"),