Skip to content

Look up runtime libraries in SDK #25740

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

Merged
merged 5 commits into from
Jun 25, 2019

Conversation

beccadax
Copy link
Contributor

@beccadax beccadax commented Jun 25, 2019

In #23175, we started looking in the SDK for swiftmodules, but we want to look for the libraries there too while linking. Fixes rdar://problem/52059706.

(This PR is not quite fully baked, but I want to kick off Linux tests while I read over the code and check a few details about how this should behave.)

This PR ought to be ready to go, but I could use confirmation that it's actually implementing the desired behavior.

In swiftlang#23175, we started looking in the SDK for swiftmodules, but we want to look for the dylibs there too. Fixes <rdar://problem/52059706>.
@beccadax
Copy link
Contributor Author

@swift-ci please smoke test

beccadax added 2 commits June 24, 2019 19:39
Refactor to merge some very similar code in different branches. This ended up slightly changing the order of arguments in Linux linker commands.
Gives us consistent results between macOS and Linux.
@beccadax
Copy link
Contributor Author

@swift-ci please test

@beccadax beccadax marked this pull request as ready for review June 25, 2019 02:43
@swift-ci

This comment has been minimized.

@swift-ci

This comment has been minimized.


SmallVector<std::string, 4> RuntimeLibPaths;
getRuntimeLibraryPaths(RuntimeLibPaths, context.Args,
context.OI.SDKPath, /*Shared=*/!wantsStaticStdlib);
Copy link
Contributor

Choose a reason for hiding this comment

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

Apple platforms don't support the static stdlib anymore, so you can probably just drop this.

Copy link
Contributor Author

@beccadax beccadax Jun 25, 2019

Choose a reason for hiding this comment

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

Doing this properly (i.e. with good diagnostics) will require a little more work than I want to do right now. I've filed https://bugs.swift.org/browse/SR-11015 to track the idea.

Turns out the order of the linker flags matters on Linux; see swiftlang#9958. I don’t know if the change I previously made would break something, but it’s not worth risking it.
@beccadax
Copy link
Contributor Author

@swift-ci please smoke test

@compnerd
Copy link
Member

@swift-ci please test Windows platform

1 similar comment
@shahmishal
Copy link
Member

@swift-ci please test Windows platform

@beccadax
Copy link
Contributor Author

@swift-ci please smoke test

@beccadax
Copy link
Contributor Author

@swift-ci please test Windows platform

@beccadax beccadax merged commit b818b44 into swiftlang:master Jun 25, 2019
nkcsgexi added a commit that referenced this pull request Jun 26, 2019
beccadax added a commit to beccadax/swift that referenced this pull request Jul 3, 2019
Cherry-picks swiftlang#25740:

> In swiftlang#23175, we started looking in the SDK for swiftmodules, but we want to look for the dylibs there too. Fixes <rdar://problem/52059706>.
beccadax added a commit that referenced this pull request Jul 8, 2019
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.

5 participants