Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not depend on system tools on Darwin #2136

Merged
merged 3 commits into from
Jul 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions haskell/private/cc_wrapper.bzl
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
load("@bazel_skylib//lib:paths.bzl", "paths")
load("@rules_cc//cc:action_names.bzl", "ACTION_NAMES")
load("@rules_cc//cc:find_cc_toolchain.bzl", "find_cc_toolchain", "use_cc_toolchain")
load("@rules_python//python:defs.bzl", "py_binary")
Expand Down Expand Up @@ -46,6 +47,10 @@ def _cc_wrapper_impl(ctx):
feature_configuration = feature_configuration,
action_name = ACTION_NAMES.c_compile,
)
ar = cc_common.get_tool_for_action(
feature_configuration = feature_configuration,
action_name = ACTION_NAMES.cpp_link_static_library,
)
cc_wrapper = ctx.actions.declare_file(ctx.label.name)
ctx.actions.expand_template(
template = ctx.file.template,
Expand All @@ -56,6 +61,7 @@ def _cc_wrapper_impl(ctx):
"{:cpu:}": cc_toolchain.cpu,
"{:workspace:}": ctx.workspace_name,
"{:platform:}": ctx.attr.platform,
"{:bindir:}": paths.dirname(ar),
},
)
return [DefaultInfo(
Expand Down
15 changes: 11 additions & 4 deletions haskell/private/cc_wrapper.py.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,10 @@ WORKSPACE = "{:workspace:}"
CC = os.environ.get("CC_WRAPPER_CC_PATH", "{:cc:}")
PLATFORM = os.environ.get("CC_WRAPPER_PLATFORM", "{:platform:}")
CPU = os.environ.get("CC_WRAPPER_CPU", "{:cpu:}")
INSTALL_NAME_TOOL = "/usr/bin/install_name_tool"
CODESIGN = "/usr/bin/codesign"
OTOOL = "/usr/bin/otool"
INSTALL_NAME_TOOL = "{:bindir:}/install_name_tool"
CODESIGN = "{:bindir:}/codesign"
CODESIGN_ALLOCATE = "{:bindir:}/codesign_allocate"
OTOOL = "{:bindir:}/otool"


def main():
Expand Down Expand Up @@ -930,12 +931,18 @@ def darwin_rewrite_load_commands(rewrites, output):
if args:
subprocess.check_call([INSTALL_NAME_TOOL] + args + [output])
# Resign the binary after patching it.
# Fall back to /usr/bin/codesign if the `CODESIGN` executable is not available
# (this might happen when using a default cc toolchain from a nix shell on Darwin instead
# of using a nixpkgs_cc_configure'd toolchain).
# Do the same for codesign_allocate.
codesign = CODESIGN if os.access(CODESIGN, os.X_OK) else "/usr/bin/codesign"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we do similar checks for INSTALL_NAME_TOOL and OTOOL ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these tools really should be available from the same place as the ar tool. Otherwise mixing tools from different toolchains is just calling for trouble (as we've seen previously: tweag/rules_nixpkgs#479).

Also, the difference between the codesign and any of these tools from /usr/bin is that codesign does not need an installation of the Xcode command line tools, the latter do.

So I'd like to only do the fallback for codesign until there is a real problem that we need to solve.

codesign_allocate = CODESIGN_ALLOCATE if os.access(CODESIGN_ALLOCATE, os.X_OK) else "/usr/bin/codesign_allocate"
# This is necessary on MacOS Monterey on M1.
# The moving back and forth is necessary because the OS caches the signature.
# See this note from nixpkgs for reference:
# https://github.com/NixOS/nixpkgs/blob/5855ff74f511423e3e2646248598b3ffff229223/pkgs/os-specific/darwin/signing-utils/utils.sh#L1-L6
os.rename(output, f"{output}.resign")
subprocess.check_call([CODESIGN] + ["-f", "-s", "-"] + [f"{output}.resign"])
subprocess.check_call([codesign] + ["-f", "-s", "-"] + [f"{output}.resign"], env = {'CODESIGN_ALLOCATE': codesign_allocate})
os.rename(f"{output}.resign", output)


Expand Down
Loading