Skip to content

[5.8] Remove extra paths added to LD_LIBRARY_PATH on Linux when running tests #6692

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
Merged
merged 1 commit into from
Aug 16, 2023
Merged

[5.8] Remove extra paths added to LD_LIBRARY_PATH on Linux when running tests #6692

merged 1 commit into from
Aug 16, 2023

Conversation

bnbarham
Copy link
Contributor

  • Explanation: Removes an old workaround that breaks running tests when there's already a swift on PATH that isn't the swift currently being run.
  • Scope: swift test on Linux platforms
  • Risk: Low, our tests would fail if this was still needed.
  • Testing: Toolchain build and test on all Linux platforms.
  • Original PR: Remove extra paths added to LD_LIBRARY_PATH on Linux when running tests #6684

@bnbarham
Remove extra paths added to LD_LIBRARY_PATH on Linux when running tests
2bc5b6c 
These break running tests when there's already a swift on PATH that
isn't the swift currently being run. There shouldn't be a need for these
as the run path of the compiled binary is already the correct path (ie.
the library path of the swift that built it and `$ORIGIN`).

(cherry picked from commit 5c7db58)
@bnbarham
Copy link
Contributor Author

I suspect this may fail, building/testing toolchains now.

@tomerd tomerd added the swift 5.8 This PR targets the 5.8 branch label Jul 25, 2023
@tomerd
Copy link
Contributor

tomerd commented Jul 25, 2023

@bnbarham does this need to be merged?

@bnbarham
Copy link
Contributor Author

Same comment as the other one. Yes, but we need to check it first.

@bnbarham bnbarham mentioned this pull request Aug 10, 2023
Closed
@bnbarham
Copy link
Contributor Author

Same results as 5.9 - the test executable has eg. /home/build-user/ws/tmp/pmtest58/usr/lib/swift/linux:$ORIGIN (where pmtest58 is the toolchain downloaded from the linked PR) in its RPATH and works fine without the extra LD_LIBRARY_PATH.

@bnbarham
Copy link
Contributor Author

@swift-ci please test

@MaxDesiatov
Copy link
Contributor

@swift-ci smoke test

@bnbarham bnbarham merged commit 85426bd into swiftlang:release/5.8 Aug 16, 2023
@bnbarham bnbarham deleted the cherry-5.8-remove-linux-test-ld-path branch August 16, 2023 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neonichu neonichu neonichu approved these changes

@DougGregor DougGregor DougGregor approved these changes

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

@tomerd tomerd Awaiting requested review from tomerd

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

Assignees
No one assigned
Labels
swift 5.8 This PR targets the 5.8 branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@bnbarham @tomerd @MaxDesiatov @neonichu @DougGregor