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

DNM: On ELF platforms, look for the runtime libraries in an architecture-specific directory. #6179

Closed
wants to merge 1 commit into from

Conversation

finagolfin
Copy link
Member

This is needed for swiftlang/swift#63782, which changes the Unix toolchain to look for libraries in architecture-specific directories.

…pecific directory.

This is needed for swiftlang/swift#63782, which changes the Unix toolchain to look
for libraries in architecture-specific directories.
@tomerd
Copy link
Contributor

tomerd commented Feb 23, 2023

cc @MaxDesiatov @etcwilde @compnerd

@tomerd
Copy link
Contributor

tomerd commented Feb 23, 2023

@swift-ci smoke test

Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me (other than Utilities/bootstrap which I didn't look at). Not approving to give others a chance to look at it as well.

@MaxDesiatov
Copy link
Contributor

Could we use this as an opportunity to encode more triple components in that path, such as ABI/environment, which would help with Musl support, or this is meant to be a purely incremental change?

@finagolfin
Copy link
Member Author

@MaxDesiatov, we could do it together, what scheme do you have in mind? Alternatively, if we simply name the platform linux-musl instead, ie store the libraries in lib/swift/linux-musl/aarch64/, it would require no changes to where the compiler looks for these libraries.

@MaxDesiatov
Copy link
Contributor

Alternatively, if we simply name the platform linux-musl instead, ie store the libraries in lib/swift/linux-musl/aarch64/, it would require no changes to where the compiler looks for these libraries.

I already tried that route and it doesn't seem to be practical. There is enough code in the build system that identifies Musl-based distros as plain linux, and that gets propagated into multiple places that reconstitute these paths from scratch, specifically ClangImporter, but I'm pretty sure there are more. I hope we can just stick to using LLVM/Clang triples instead of coming up with our own directory layout and naming.

@finagolfin
Copy link
Member Author

There is enough code in the build system that identifies Musl-based distros as plain linux, and that gets propagated into multiple places that reconstitute these paths from scratch, specifically ClangImporter, but I'm pretty sure there are more.

Well, we will have to change the build system either way, but it would be easier to make these changes you list here than move the Swift resource directory to a whole new triple-based naming scheme.

I hope we can just stick to using LLVM/Clang triples instead of coming up with our own directory layout and naming.

As in we move to a whole new scheme like lib/swift/aarch64-linux-musl/ or change the current layout to handle lib/swift/linux/aarch64-musl too? Any of these alternatives to the simpler change I suggested will require much more changes to the build system and changes to the compiler too, the latter of which my suggestion wouldn't require.

I think you raise an important point that Swift should start supporting alternate C libraries and ABIs, particularly for linux, but the question is how best to do so.

@tomerd
Copy link
Contributor

tomerd commented Feb 28, 2023

@MaxDesiatov @buttaface is this still pending, or the discussion impact this PR?

@finagolfin
Copy link
Member Author

@tomerd, this is under active testing with the linked compiler pull swiftlang/swift#63782 still: once it passes the CI, I will ask for all 8 pulls to be merged simultaneously.

@tomerd tomerd added the WIP Work in progress label Feb 28, 2023
@tomerd tomerd changed the title On ELF platforms, look for the runtime libraries in an architecture-specific directory. DNM: On ELF platforms, look for the runtime libraries in an architecture-specific directory. Feb 28, 2023
@finagolfin
Copy link
Member Author

Alright, the CI passed and the Swift compiler pull was approved, the draft/WIP tags can be removed, @tomerd.

Please don't merge yet though, as all 11 pulls should be merged together.

@finagolfin
Copy link
Member Author

@finagolfin finagolfin closed this Apr 12, 2023
@finagolfin finagolfin deleted the arch branch April 12, 2023 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants