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

On ELF platforms, only add runpaths as needed #6321

Merged
merged 1 commit into from Apr 10, 2023
Merged

Conversation

finagolfin
Copy link
Contributor

As long as I'm modifying the Python bootstrap script, clean up some incorrect checks of the host platform.system() and replace some unnecessary regexes.

It has always bothered me that since I suggested this hack of adding this second runpath for the Package libraries a couple years ago, all installed ELF binaries incorrectly have both runpaths:

> readelf -d swift-DEVELOPMENT-SNAPSHOT-2023-03-17-a-ubuntu20.04/usr/lib/swift/pm/*I/lib*so swift-DEVELOPMENT-SNAPSHOT-2023-03-17-a-ubuntu20.04/usr/bin/swift-package|ag "File:|runpath"
File: swift-DEVELOPMENT-SNAPSHOT-2023-03-17-a-ubuntu20.04/usr/lib/swift/pm/ManifestAPI/libPackageDescription.so
 0x000000000000001d (RUNPATH)            Library runpath: [$ORIGIN:$ORIGIN/../lib/swift/linux:$ORIGIN/../../linux]
File: swift-DEVELOPMENT-SNAPSHOT-2023-03-17-a-ubuntu20.04/usr/lib/swift/pm/PluginAPI/libPackagePlugin.so
 0x000000000000001d (RUNPATH)            Library runpath: [$ORIGIN:$ORIGIN/../lib/swift/linux:$ORIGIN/../../linux]
File: swift-DEVELOPMENT-SNAPSHOT-2023-03-17-a-ubuntu20.04/usr/bin/swift-package
 0x000000000000001d (RUNPATH)            Library runpath: [$ORIGIN:$ORIGIN/../lib/swift/linux:$ORIGIN/../../linux]

This adds the single correct relative runpath to each, in such a way so that it is only done when installing on the CI, as @ahoppen suggested in apple/sourcekit-lsp#715 and including his recommendation of removing regexes.

As noted there, this also speeds up the SPM build. I tested this natively on Android AArch64 without a problem.

I considered moving the macOS rpath flags to the package manifest too, then found that it may install all the SPM libraries (I'm assuming those flags are for macOS), so left that as a command-line flag applied to all libraries.

As long as I'm modifying the Python bootstrap script, clean up some incorrect
checks of the host `platform.system()` and replace some unnecessary regexes.
@neonichu
Copy link
Member

@swift-ci please smoke test

@neonichu
Copy link
Member

We probably also want a toolchain build for this.

@finagolfin
Copy link
Contributor Author

Ran a toolchain build, downloaded the linux toolchain built, and checked that the rpaths are correct and that it works well.

The mac toolchain build failed only because the CI ran out of disk space:

86%: Link libSwiftPMDataModel.dylib (x86_64)
fatal error: lipo: can't write to output file: /Users/ec2-user/jenkins/workspace/swift-PR-toolchain-macos/branch-main/buildbot_osx/swiftpm-macosx-x86_64/apple/Products/Release/swift-bootstrap.dSYM/Contents/Resources/DWARF/swift-bootstrap.lipo
 (No space left on device)

@finagolfin
Copy link
Contributor Author

Ping, ready to go in? I will submit for 5.9 next.

@neonichu neonichu merged commit a801f39 into apple:main Apr 10, 2023
4 checks passed
@finagolfin finagolfin deleted the rpath branch April 10, 2023 20:04
finagolfin added a commit to finagolfin/swift-package-manager that referenced this pull request Apr 11, 2023
As long as I'm modifying the Python bootstrap script, clean up some incorrect
checks of the host `platform.system()` and replace some unnecessary regexes.
tomerd pushed a commit that referenced this pull request Apr 12, 2023
As long as I'm modifying the Python bootstrap script, clean up some incorrect
checks of the host `platform.system()` and replace some unnecessary regexes.
finagolfin added a commit to finagolfin/swift-package-manager that referenced this pull request Apr 13, 2023
As long as I'm modifying the Python bootstrap script, clean up some incorrect
checks of the host `platform.system()` and replace some unnecessary regexes.
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.

None yet

2 participants