bazel icon indicating copy to clipboard operation
bazel copied to clipboard

Add layering_check support for macOS

Open keith opened this issue 9 months ago • 5 comments

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.

keith avatar May 06 '24 20:05 keith

@pzembrod could you take a look? 🙏🏻

keith avatar May 14 '24 20:05 keith

@keith I left a few comments on your push, because apparently GitHub's UI succeeded in confusing me. ;-)

pzembrod avatar May 15 '24 10:05 pzembrod

@keith I left a few comments on your push, because apparently GitHub's UI succeeded in confusing me. ;-)

I can't find the comments, but I'm ok if @keith can.

comius avatar May 15 '24 13:05 comius

@comius here: https://github.com/bazelbuild/bazel/commit/e59f37064f6fb3924083c81e8a79952e34765cf7

brentleyjones avatar May 15 '24 13:05 brentleyjones

I moved my comments over to the PR

pzembrod avatar May 15 '24 13:05 pzembrod

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

//src/test/shell/bazel:bazel_bootstrap_distfile_tar_test

# Configuration: 7c706e29d44fb244031cd9f1b5563076a031b073c9a89bd361b0680404c6068b
# Execution platform: //:default_host_platform
[1mexternal/grpc~/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc:42:10: [0m[0;1;31merror: [0m[1mmodule grpc~//:grpc_resolver_dns_ares does not depend on a module exporting 'ares.h'[0m
#include <ares.h>
[0;1;32m         ^
[0m1 error generated.
[32m[1,873 / 4,221][0m 36 actions running

meteorcloudy avatar May 21 '24 15:05 meteorcloudy

I think we can either patch gRPC to disable the layering check for that target, or we can try and update it (not sure how hard that is for bazel?) wdyt?

keith avatar May 21 '24 16:05 keith

submitted https://github.com/bazelbuild/bazel/pull/22468 because the upgrade is non-trivial and requires https://github.com/protocolbuffers/protobuf/pull/16884

keith avatar May 21 '24 17:05 keith

@bazel-io flag

keith avatar May 21 '24 17:05 keith

@bazel-io fork 7.2.0

iancha1992 avatar May 21 '24 18:05 iancha1992

got reverted, new version here https://github.com/bazelbuild/bazel/pull/22475

keith avatar May 21 '24 18:05 keith