Skip to content

[5.9.0] Remove extra paths added to LD_LIBRARY_PATH on Linux when running tests #6822

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

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. Manually tested that swift test works on a SwiftPM project using a toolchain with this change.
  • Original PR: Remove extra paths added to LD_LIBRARY_PATH on Linux when running tests #6684

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)
(cherry picked from commit 07952dc)
@bnbarham
Copy link
Contributor Author

Risk: Low, our tests would fail if this was still needed.

Note that in the 5.9 cherry-pick @neonichu mentioned:

I'm not sure that is actually true. When I introduced the workaround, the shipped nightly toolchain was broken which I think implies that the tests were passing despite that being the case.

So I did additionally test that the toolchain with this change is still able to swift test.

@bnbarham
Copy link
Contributor Author

@swift-ci please smoke test

@bnbarham bnbarham merged commit 2dbd2ee into swiftlang:release/5.9.0 Aug 17, 2023
@bnbarham bnbarham deleted the cherry-590-remove-linux-test-ld-path branch August 17, 2023 00:39
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.

3 participants