llvm: Support all `-macosx` target triples

madsmtm commented 2 days ago

`brew gist-logs <formula>` link OR `brew config` AND `brew doctor` output

% brew gist-logs llvm@19
Error: No logs.

% brew config
HOMEBREW_VERSION: 4.4.4
ORIGIN: https://github.com/Homebrew/brew
HEAD: 824efa8836dc226aa92dbbf7404c1b4a66707cca
Last commit: 7 days ago
Core tap JSON: 10 Nov 22:51 UTC
Core cask tap JSON: 10 Nov 22:51 UTC
HOMEBREW_PREFIX: /opt/homebrew
HOMEBREW_CASK_OPTS: []
HOMEBREW_MAKE_JOBS: 8
Homebrew Ruby: 3.3.5 => /opt/homebrew/Library/Homebrew/vendor/portable-ruby/3.3.5/bin/ruby
CPU: octa-core 64-bit dunno
Clang: 16.0.0 build 1600
Git: 2.39.5 => /Library/Developer/CommandLineTools/usr/bin/git
Curl: 8.7.1 => /usr/bin/curl
macOS: 14.6.1-arm64
CLT: 16.1.0.0.1.1729049160
Xcode: N/A
Rosetta 2: false

% brew doctor
Your system is ready to brew.

Note that the system is a clean virtual machine with a newly installed homebrew.

Verification

[X] My brew doctor output says Your system is ready to brew. and am still able to reproduce my issue.
[X] I ran brew update and am still able to reproduce my issue.
[X] I have resolved all warnings from brew doctor and that did not fix my problem.
[X] I searched for recent similar issues at https://github.com/Homebrew/homebrew-core/issues?q=is%3Aissue and found no duplicates.

What were you trying to do (and why)?

https://github.com/Homebrew/homebrew-core/pull/196094 introduced new logic for finding the SDK root.

There are a few problems with it, however, including:

It conflates SDK root and deployment target (the version in the target triples is the deployment target, and doesn't have much to do with SDK versions).
It does not support "macosx" target triples (such as arm64-apple-macosx).

Discovered originally in https://github.com/rust-lang/cc-rs/issues/1278.

What happened (include all command output)?

Using Clang from the llvm package with the -target arm64-apple-macosx10.7 flag fails to link (or compile), because the logic for using a different SDK is not correct.

ld: library 'System' not found
clang: error: linker command failed with exit code 1 (use -v to see invocation)

What did you expect to happen?

Using any macOS target triple supported by Clang compiles, as long as there's at least one installed macOS SDK.

Step-by-step reproduction instructions (by running `brew` commands)

brew install llvm@19
echo "int main() { return 0; }" > foo.c
/opt/homebrew/opt/llvm@19/bin/clang -target arm64-apple-macosx10.7 foo.c

madsmtm commented 2 days ago

CC @carlocab @Bo98

Instead of using config files, perhaps it'd be possible to create a symlink to the SDK somewhere inside /opt/homebrew/opt/llvm/, and tell Clang to use that with the -DDEFAULT_SYSROOT instead? That way, you can update the default SDK without having to recompile.

Also, remember that macOS SDKs are "backwards compatible", so -target arm64-apple-macosx14.0 should work with newer SDK versions (so you should only ever need to link the newest SDK).

carlocab commented 2 days ago

As a workaround, do:

mkdir -p ~/.config/clang
echo "--sysroot=\"$(xcrun -sdk macosx --show-sdk-path)\"" >~/.config/arm64-apple-macosx.cfg

If you only want to set this temporarily:

export HOME="$(mktemp -d)"
mkdir -p "$HOME/.config/clang"
echo "--sysroot=\"$(xcrun -sdk macosx --show-sdk-path)\"" >"$HOME/.config/arm64-apple-macosx.cfg"

Instead of using config files, perhaps it'd be possible to create a symlink to the SDK somewhere inside /opt/homebrew/opt/llvm/, and tell Clang to use that with the -DDEFAULT_SYSROOT instead? That way, you can update the default SDK without having to recompile.

That's not a good solution as-is for three reasons:

this will break relocatability of LLVM, so that it will now only work if Homebrew is in the default prefix
this still requires action on users to update the symlink, which we should avoid
setting DEFAULT_SYSROOT affects clang-cl too, which shouldn't be looking in the macOS SDK

Why is providing a configuration file for *-apple-macosx not sufficient?

madsmtm commented 2 days ago

As a workaround, do:

I assume you meant:

mkdir -p ~/.config/clang
echo "--sysroot \"$(xcrun -sdk macosx --show-sdk-path)\"" > ~/.config/clang/arm64-apple-macosx.cfg

But yeah, that works.

this will break relocatability of LLVM, so that it will now only work if Homebrew is in the default prefix

Perhaps -DDEFAULT_SYSROOT could be updated to support installation-relative paths, then?

this still requires action on users to update the symlink, which we should avoid

My understanding was that the issue is that the SDK root from the currently installed Command Line Tools is embedded into the binary, which is indeed problematic - then a symlink that Homebrew kept up to date would be nicer?

In any case, why are you not just using /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk, that's already a symlink to the latest SDK root?

setting DEFAULT_SYSROOT affects clang-cl too, which shouldn't be looking in the macOS SDK

Don't know anything about that, so probably true.

Why is providing a configuration file for *-apple-macosx not sufficient?

I was confused, because I hadn't seen that https://github.com/llvm/llvm-project/pull/111387 was patched in, and thought that you'd need a bunch of different .cfg for each possible OS version.

So yeah, providing *-apple-macosx.cfg is probably enough.

carlocab commented 2 days ago

I assume you meant:

Yes, thanks. I'll update my comment to avoid misleading anyone else reading.

Perhaps -DDEFAULT_SYSROOT could be updated to support installation-relative paths, then?

Perhaps -- this would need to be upstreamed though.

My understanding was that the issue is that the SDK root from the currently installed Command Line Tools is embedded into the binary, which is indeed problematic - then a symlink that Homebrew kept up to date would be nicer?

Yes, this was discussed as a possibility in #196094. We could probably try it.

In any case, why are you not just using /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk, that's already a symlink to the latest SDK root?

We used to briefly, but @Bo98 maintains that this is wrong and causes problems. It certainly causes problems for gcc, but I suspect we may be able to do it for llvm. But we'll need @Bo98 to confirm.

Why is providing a configuration file for *-apple-macosx not sufficient?

I was confused, because I hadn't seen that llvm/llvm-project#111387 was patched in, and thought that you'd need a bunch of different .cfg for each possible OS version.

So yeah, providing *-apple-macosx.cfg is probably enough.

Well, we could maybe also do versioned config files in case we need to retain versions pending above discussion about using MacOSX.sdk.

carlocab commented 7 hours ago

This should be fixed now. You may need to do

brew update
brew reinstall llvm

to pick up the fix.

madsmtm commented 5 hours ago

Thanks!

Wish I had seen the PR before it merged though, then I'd have commented then, there is still an issue with the solution. I've opened https://github.com/Homebrew/homebrew-core/issues/197532 instead to track that.

Homebrew / homebrew-core