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

Add swiftpm-xctest-helper rpath on macOS. #2785

Merged
merged 1 commit into from Jun 23, 2020

Conversation

dan-zheng
Copy link
Contributor

@dan-zheng dan-zheng commented Jun 13, 2020

Add an extra rpath for swiftpm-xctest-helper on macOS:
@executable_path/../../../lib/swift/macosx.

This fixes SR-12600:

$ swift test --filter a
error: signalled(6): /Library/Developer/Toolchains/swift-DEVELOPMENT-SNAPSHOT-2020-06-10-a.xctoolchain/usr/libexec/swift/pm/swiftpm-xctest-helper /Users/danielzheng/TestPkg/.build/x86_64-apple-macosx/debug/TestPkgPackageTests.xctest /var/folders/m_/6f7q8zfs3n9fr0c_4gy8840m00hc_q/T/TemporaryFile.3eFu0u output:
    dyld: Library not loaded: @rpath/XCTest.framework/Versions/A/XCTest
      Referenced from: /Library/Developer/Toolchains/swift-DEVELOPMENT-SNAPSHOT-2020-06-10-a.xctoolchain/usr/libexec/swift/pm/swiftpm-xctest-helper
      Reason: image not found

Fix found by @abertelrud in #2694 (comment).

Add an extra rpath for swiftpm-xctest-helper on macOS:
`@executable_path/../../../lib/swift/macosx`.

This fixes SR-12600: `swift test --filter`.

Co-authored-by: Anders Bertelrud <anders@apple.com>
@dan-zheng
Copy link
Contributor Author

Just triggering tests.
@swift-ci Please test

Copy link
Contributor

@abertelrud abertelrud left a comment

Choose a reason for hiding this comment

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

This looks good to me, but I am not as familiar with how SwiftPM is built as some others are. Would also appreciate review by @aciidb0mb3r and @neonichu

@neonichu
Copy link
Member

I don't think we should be using unsafeFlags here and instead do this in the bootstrap script.

@dan-zheng
Copy link
Contributor Author

I don't think we should be using unsafeFlags here and instead do this in the bootstrap script.

Here's a PR (with more context) that edits the bootstrap script: #2692.

The change is more invasive though, adding the rpath to all tools instead of just swiftpm-xctest-helper.

Any suggestions would be appreciated for moving forward with this PR or that one!

@neonichu
Copy link
Member

Oh, I didn't realise this change had so much history. It seems like Ankit and Anders think it is fine to do this with unsafeFlags so let's go ahead with this.

@neonichu
Copy link
Member

@swift-ci please smoke test

@dan-zheng
Copy link
Contributor Author

Could someone please help merge this PR, if everything's okay? @neonichu

Copy link
Contributor

@abertelrud abertelrud left a comment

Choose a reason for hiding this comment

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

Thanks! Yes, I think this is preferable to adding this flag to all the binaries.

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

3 participants