Skip to content

Unix: Use llvm-ar by default #1244

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

finagolfin
Copy link
Member

@finagolfin finagolfin commented Jan 1, 2023

Now that llvm-ar is installed by default in the toolchain, swiftlang/swift#62510, and a recent SwiftPM 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.

This is the Swift translation of swiftlang/swift#62800.

@finagolfin
Copy link
Member Author

@artemcm, please run the CI on this.

@artemcm
Copy link
Contributor

artemcm commented Jan 3, 2023

@swift-ci test

@finagolfin
Copy link
Member Author

Windows CI failure is unrelated, all pulls are failing on the Windows CI.

@finagolfin
Copy link
Member Author

@artemcm, now that the CI is working, would you run it again?

@artemcm
Copy link
Contributor

artemcm commented Feb 8, 2023

@swift-ci test

@finagolfin
Copy link
Member Author

Rebased, need another CI run.

@rauhul
Copy link
Member

rauhul commented Jan 9, 2024

@swift-ci test

@finagolfin
Copy link
Member Author

@swift-ci test

@finagolfin
Copy link
Member Author

@swift-ci test macos

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.
@finagolfin
Copy link
Member Author

@swift-ci test

@finagolfin
Copy link
Member Author

@artemcm, can we go ahead and get this in before the upcoming branch? @al45tair once expressed interest in testing this more last summer in the linked Driver pull but hasn't responded since, and we have plenty of data that it is unlikely to break anything as it has been the default Unix archiver for SwiftPM for the last year and a half, swiftlang/swift-package-manager#6829.

If you're okay with merging this, which requires approval from someone like you, please approve and I'll go ahead and merge this and the Driver pull, which doesn't require anything else.

@al45tair
Copy link
Contributor

I'm honestly not sure this is a desirable change. As I observed previously, almost nobody uses ar via SwiftPM, because it's very rarely used to create static libraries, and it isn't true that the ar format is standardised. While most static library formats did indeed base their format on the original UNIX ar format, there are genuinely some significant differences.

I think we should be using the system librarian (ar or lib.exe). I don't think we should be using llvm-ar, except where that is the system librarian.

@compnerd, WDYT?

@finagolfin
Copy link
Member Author

finagolfin commented Apr 1, 2025

I think we should be using the system librarian (ar or lib.exe). I don't think we should be using llvm-ar, except where that is the system librarian.

@compnerd, WDYT?

I don't see why you think he'd agree with you, considering we switched SwiftPM to default to llvm-ar for non-Darwin Unix based on Saleem's plan and review: see the discussion at swiftlang/swift-package-manager#6829 where I made the change a year and a half ago.

I am skeptical that there are any worthwhile differences in what llvm-ar produces for major platforms today, but if you're so worried about it, what do you suggest we do instead, switch SwiftPM to default to ar then fall back to llvm-ar if no system ar is found, ie the opposite of what it does today?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants