-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
New LLVM 16 release to fix Homebrew installing zstd in nonstandard location #25002
base: master
Are you sure you want to change the base?
Conversation
How do I test this? |
Can you try |
It installs with no errors. |
The problem which this release fixes involves the interaction of multiple factors between Apple, Homebrew, and LLVM: Apple's previous Macs used Intel, but its newest Macs use ARM (the Mac M1 and onward). To ease the transition, Apple has an emulation layer called Rosetta that can execute Intel binaries on ARM. Therefore, Intel and ARM binaries may for now coexist on the new ARM-based Macs. The older versions of Homebrew installed packages in the "standard" location, As a security feature on Apple, child processes ignore the The The Homebrew "recipe" for zstd alters the path in the So, the causes of the issue are as follows:
This change is now tripping up multiple people who try to use the zstd package, as seen at https://stackoverflow.com/questions/67840691/ld-library-not-found-for-lzstd-while-bundle-install-for-mysql2-gem-ruby-on-mac and https://discourse.llvm.org/t/kaleidoscope-on-m1-mac-help/69010. It's not clear to me which party is ultimately "responsible" for fixing this issue, but I'm going ahead and fixing the issue in this package by explicitly looking up the zstd package using I feel that opam packages generally expect Homebrew to work out of the box, treating it as an ordinary package manager, as Please let me know if my explanation is unclear or incorrect. |
It's been several months since I opened this PR and there has been no action from the opam-repository maintainers. I want to understand what they think of this PR, and of the changes on Mac and Homebrew that broke stuff. If this PR shouldn't be merged that's fine, but I would like to be told that it shouldn't be merged rather than have a confusing silence. |
Apologies - this is a tricky package, and I suppose none of the maintainers have time to understand the various issues at play. I think that would help if you could review the CI failures (in https://opam.ci.ocaml.org/github/ocaml/opam-repository/commit/4d9fad21a5bc93fd3b1cd19733ce12ee7f259505) and report if these errors are expected or not. |
@samoht The compile errors for the binsec package are because of the change from the typed to opaque pointer API. Its LLVM dependency needs to have a version bound of < 16. The other errors relate to failures to install the system LLVM package in some way or another. |
The binsec issue is addressed in #25667. That PR should be merged first, because it's straightforward while this one may involve some consideration. |
Hello @alan-j-hu! I'm just circling around to see if you think we may be in a position to land this, or at least whether we should put this pack in the queue of PRs needing maintainer attention, now that the PR you noted in #25002 (comment) seems to be merged. Let me know what you think! PS: I re ran the CI so we could see how things stand now. |
Should fix the issue at alan-j-hu/llvm-dune#18. @jordydickinson do you think you could test this out and make sure it works for you?