-
Notifications
You must be signed in to change notification settings - Fork 304
Remove deleting toolchain rpath step #468
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
Remove deleting toolchain rpath step #468
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as duplicate.
This comment was marked as duplicate.
31644aa
to
7d5db6d
Compare
@swift-ci Please test |
I'm fine with this, but maybe we should consider just removing this step if it's no longer needed with the latest swiftpm. Is anyone running build_script_helper.py to install sourcekit-lsp manually without building the rest of the toolchain? |
the install script has been broken since swiftlang/swift-package-manager@8876f87 which stops adding toolchain rpath for older OSes without system swift libraries.
7d5db6d
to
707608c
Compare
@benlangmuir It seems reasonable to just remove the step 👍 |
@swift-ci please build toolchain |
@swift-ci please test |
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.
Looks good to me, thanks for fixing it. I just triggered a toolchain build in swiftlang/swift#37710 and would like to see that pass before merging it.
I'm still seeing the stdlib rpath (via the toolchain built at swiftlang/swift#41888):
|
This is the swift-5.5 back deploy rpath, which has always been in there (at least it’s also in there for |
Ah, good catch. I guess for 5.5 we would want to keep it but fix it to be relative to |
the install script has been broken since swiftlang/swift-package-manager@8876f87
which stops adding toolchain rpath for older OSes without system
swift libraries.