-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Install llvm-ar in the toolchain #62510
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that the build and packaging is good. Changing the default I'm less certain about. Arguably, we already have to do that with LTO, so this does simplify things, but honouring AR
from the environment seems reasonable as that mirrors gcc and clang IIRC.
This simply changes the name of the default archiver looked for by the Swift compiler on Unix, it changes nothing about where it's looked for, ie you can still configure that by passing in a |
@MaxDesiatov, I see you're up, would you run the CI on this? |
I'd like to remind that as a frequent contributor you can get CI access, as described in the corresponding docs. |
@swift-ci please smoke test |
@MaxDesiatov, as I mentioned to you the last time you reminded me, I'm winding down my port of Swift to Android and plan to submit only maybe 5-10 maintenance pulls in the coming year. With such a low frequency, I think it's better for security reasons that I don't have commit access, so I prefer to ask others to run the CI. If I'm bothering you in any way, please just say so and I will ask other committers from now on. |
Building CMark on Linux and macOS failed because I changed the host archiver to llvm-ar too. I wasn't sure about that one as it depends on what is pre-installed on the CI host. Since it appears llvm-ar is not installed- Windows passed as it doesn't build CMark for some reason- should be fine to use the installed |
@egorzhdan, please run the CI again. |
@swift-ci please smoke test |
A recent SPM change made it mandatory for there to be an archiver in the toolchain/PATH, swiftlang/swift-package-manager#5761, so make sure llvm-ar is bundled in the toolchain.
Linux CI failed because I changed the default archiver name on Unix to @egorzhdan, one final CI run and this can go in. |
@swift-ci please smoke test |
Alright, @compnerd, this is ready for review. |
@keith, mind getting this in? I submitted this so we can get this mitigation in before the 5.8 branch tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Building and packaging the LLVM archiver is a good idea in general.
Yeah, merging this before the 5.8 branch sounds reasonable. Thanks @buttaface! |
…ld directory Now that llvm-ar is installed by default in the toolchain, swiftlang#62510, and a recent SPM change requires there to be an archiver in the toolchain/PATH, swiftlang/swift-package-manager#5761, use that bundled llvm-ar for all Unix platforms, which requires copying it over into the build directory too before building the corelibs.
…ld directory Now that llvm-ar is installed by default in the toolchain, swiftlang#62510, and a recent SPM change requires there to be an archiver in the toolchain/PATH, swiftlang/swift-package-manager#5761, use that bundled llvm-ar for all Unix platforms, which requires copying it over into the build directory too before building the corelibs.
…ld directory Now that llvm-ar is installed by default in the toolchain, swiftlang#62510, and a recent SPM change requires there to be an archiver in the toolchain/PATH, swiftlang/swift-package-manager#5761, use that bundled llvm-ar for all Unix platforms, which requires copying it over into the build directory too before building the corelibs.
Now that llvm-ar is installed by default in the toolchain, swiftlang/swift#62510, and a recent SPM change requires there to be an archiver in the toolchain/PATH, swiftlang/swift-package-manager#5761, use that bundled llvm-ar for all Unix platforms.
…ld directory Now that llvm-ar is installed by default in the toolchain, swiftlang#62510, and a recent SPM change requires there to be an archiver in the toolchain/PATH, swiftlang/swift-package-manager#5761, use that bundled llvm-ar for all Unix platforms, which requires copying it over into the build directory too before building the corelibs.
Now that llvm-ar is installed by default in the toolchain, swiftlang/swift#62510, and a recent SPM change requires there to be an archiver in the toolchain/PATH, swiftlang/swift-package-manager#5761, use that bundled llvm-ar for all Unix platforms.
A recent SPM change made it mandatory for there to be an archiver in the toolchain/PATH, swiftlang/swift-package-manager#5761, so make sure llvm-ar is bundled in the toolchain.
@compnerd and @tomerd, let me know what you think.