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.

Sign up for GitHub

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

Jump to bottom

Do not add rpath to swift-testing for CLT or custom toolchains #8295

Merged
merged 1 commit into from
Feb 18, 2025
Merged

Do not add rpath to swift-testing for CLT or custom toolchains #8295

merged 1 commit into from
Feb 18, 2025

Conversation

bnbarham
Copy link
Contributor

This was added so that test binaries could find swift-testing, but we should insert those paths into DYLD_*_PATH rather than add to the rpath. The current behavior adds an absolute rpath to all binaries, not even just test binaries.

@bnbarham
Copy link
Contributor Author

@swift-ci please test

dschaefer2
dschaefer2 approved these changes Feb 14, 2025
View reviewed changes
Copy link
Member

@dschaefer2 dschaefer2 left a comment

Choose a reason for hiding this comment

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

Oof, yes, adding this to the env makes sense. We'll have to keep an eye on this as we transition the build system to make sure it's doing what we need.

@bnbarham
Copy link
Contributor Author

@swift-ci please test macOS platform

@bnbarham
Do not add rpath to swift-testing for CLT or custom toolchains
a022c4a 
This was added so that test binaries could find swift-testing, but we
should insert those paths into `DYLD_*_PATH` rather than add to the
rpath. The current behavior adds an absolute rpath to *all* binaries,
not even just test binaries.
@bnbarham
Copy link
Contributor Author

@swift-ci please test macOS platform

@bnbarham
Copy link
Contributor Author

@swift-ci please test Linux platform

@bnbarham
Copy link
Contributor Author

@swift-ci please test Windows platform

@bnbarham
Copy link
Contributor Author

@swift-ci please test Windows platform

@bnbarham bnbarham merged commit 8493d0b into swiftlang:main Feb 18, 2025
5 checks passed
@bnbarham bnbarham deleted the move-rpath branch February 18, 2025 20:44
@stmontgomery stmontgomery mentioned this pull request Mar 12, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@plemarquand plemarquand plemarquand approved these changes

@jakepetroules jakepetroules jakepetroules approved these changes

@dschaefer2 dschaefer2 dschaefer2 approved these changes

@MaxDesiatov MaxDesiatov Awaiting requested review from MaxDesiatov MaxDesiatov is a code owner

@xedin xedin Awaiting requested review from xedin xedin is a code owner

@shawnhyam shawnhyam Awaiting requested review from shawnhyam shawnhyam is a code owner

@bripeticca bripeticca Awaiting requested review from bripeticca bripeticca is a code owner

@owenv owenv Awaiting requested review from owenv owenv is a code owner

@bkhouri bkhouri Awaiting requested review from bkhouri bkhouri is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bnbarham @plemarquand @jakepetroules @dschaefer2