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

llvm: Support all -macosx target triples #197278

Closed
4 tasks done
madsmtm opened this issue Nov 11, 2024 · 6 comments · Fixed by #197410
Closed
4 tasks done

llvm: Support all -macosx target triples #197278

madsmtm opened this issue Nov 11, 2024 · 6 comments · Fixed by #197410
Labels
bug Reproducible Homebrew/homebrew-core bug

Comments

@madsmtm
Copy link

madsmtm commented Nov 11, 2024

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

What were you trying to do (and why)?

#196094 introduced new logic for finding the SDK root.

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

  1. 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).
  2. It does not support "macosx" target triples (such as arm64-apple-macosx).

Discovered originally in rust-lang/cc-rs#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 madsmtm added the bug Reproducible Homebrew/homebrew-core bug label Nov 11, 2024
@madsmtm
Copy link
Author

madsmtm commented Nov 11, 2024

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
Copy link
Member

carlocab commented Nov 11, 2024

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
Copy link
Author

madsmtm commented Nov 11, 2024

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

@carlocab
Copy link
Member

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 added a commit that referenced this issue Nov 12, 2024
Fixes #197278.

Also, remove the CLT requirement for pouring bottles. Many users use
this for the libraries, which don't need the CLT. We can add it back if
users report getting tripped up by this.
carlocab added a commit that referenced this issue Nov 12, 2024
Fixes #197278.

Also, remove the CLT requirement for pouring bottles. Many users use
this for the libraries, which don't need the CLT. We can add it back if
users report getting tripped up by this.

Co-authored-by: Bo Anderson <[email protected]>
carlocab added a commit that referenced this issue Nov 12, 2024
Fixes #197278.

Also, remove the CLT requirement for pouring bottles. Many users use
this for the libraries, which don't need the CLT. We can add it back if
users report getting tripped up by this.

Co-authored-by: Bo Anderson <[email protected]>
carlocab added a commit that referenced this issue Nov 12, 2024
Fixes #197278.

Also, remove the CLT requirement for pouring bottles. Many users use
this for the libraries, which don't need the CLT. We can add it back if
users report getting tripped up by this.

Co-authored-by: Bo Anderson <[email protected]>
carlocab added a commit that referenced this issue Nov 12, 2024
Fixes #197278.

Also, remove the CLT requirement for pouring bottles. Many users use
this for the libraries, which don't need the CLT. We can add it back if
users report getting tripped up by this.

Co-authored-by: Bo Anderson <[email protected]>
carlocab added a commit that referenced this issue Nov 12, 2024
Fixes #197278.

Also, remove the CLT requirement for pouring bottles. Many users use
this for the libraries, which don't need the CLT. We can add it back if
users report getting tripped up by this.

Co-authored-by: Bo Anderson <[email protected]>
@carlocab
Copy link
Member

This should be fixed now. You may need to do

brew update
brew reinstall llvm

to pick up the fix.

@madsmtm
Copy link
Author

madsmtm commented Nov 13, 2024

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 #197532 instead to track that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Reproducible Homebrew/homebrew-core bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants