Skip to content

Look for runtime library modules in the SDK, too #23175

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 2 commits into from
Mar 13, 2019
Merged

Look for runtime library modules in the SDK, too #23175

merged 2 commits into from
Mar 13, 2019

Conversation

beccadax
Copy link
Contributor

@beccadax beccadax commented Mar 8, 2019

Once module stability arrives, we'll want to load modules (and particularly parsable interfaces) from the SDK instead of the toolchain. This PR starts to stage that change in.

Resolves rdar://problem/46132288.

@beccadax
Copy link
Contributor Author

beccadax commented Mar 8, 2019 •
edited
Loading

The WIP commit doesn't have tests yet, but as far as I know it will work.

@beccadax beccadax force-pushed the theres-a-path-for-everyone branch from e68c834 to 019f8ba Compare March 9, 2019 01:37
@beccadax beccadax marked this pull request as ready for review March 9, 2019 01:41
@beccadax
Copy link
Contributor Author

beccadax commented Mar 9, 2019

@swift-ci please test

@swift-ci

This comment has been minimized.

Sign in to view
@swift-ci

This comment has been minimized.

Sign in to view
@beccadax
Copy link
Contributor Author

beccadax commented Mar 9, 2019

This will require a matching change in lldb:

18:24:38 /home/buildnode/jenkins/workspace/swift-PR-Linux/branch-master/lldb/source/Symbol/SwiftASTContext.cpp:4762:40: error: no member named 'RuntimeLibraryImportPath' in 'swift::SearchPathOptions'; did you mean 'RuntimeLibraryPath'?
18:24:38       m_ast_context_ap->SearchPathOpts.RuntimeLibraryImportPath.c_str());
18:24:38                                        ^~~~~~~~~~~~~~~~~~~~~~~~
18:24:38                                        RuntimeLibraryPath

beccadax added a commit to beccadax/swift-lldb that referenced this pull request Mar 11, 2019
@beccadax
Support multiple runtime library import paths
49c716a 
Matches changes in swiftlang/swift#23175.
@beccadax beccadax mentioned this pull request Mar 11, 2019
Merged
@beccadax
Copy link
Contributor Author

Please test with following pull request:
apple/swift-lldb#1365

@swift-ci test

@beccadax beccadax requested a review from jrose-apple March 11, 2019 20:02
jrose-apple
jrose-apple reviewed Mar 11, 2019
View reviewed changes
Copy link
Contributor

@jrose-apple jrose-apple left a comment

Choose a reason for hiding this comment

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

Looks pretty good, only minor comments.

@beccadax
[NFC] Support many runtime library import paths
46ddb2a 
Replaces SearchPathOptions::RuntimeLibraryImportPath with an equivalent std::vector of paths. Also reimplements SearchPathOptions::SkipRuntimeLibraryImportPaths to cause the list of runtime library import paths to be empty, rather than exiting early from SerializedModuleLoader::findModule().
@beccadax beccadax force-pushed the theres-a-path-for-everyone branch from 019f8ba to 09f87d3 Compare March 12, 2019 02:07
@beccadax beccadax requested a review from jrose-apple March 12, 2019 02:07
@beccadax
Copy link
Contributor Author

Please test with following pull request:
apple/swift-lldb#1365

@swift-ci test

@swiftlang swiftlang deleted a comment from swift-ci Mar 12, 2019
@swiftlang swiftlang deleted a comment from swift-ci Mar 12, 2019
jrose-apple
jrose-apple reviewed Mar 12, 2019
View reviewed changes
@beccadax beccadax force-pushed the theres-a-path-for-everyone branch from 09f87d3 to ba09af0 Compare March 12, 2019 18:13
@beccadax

This comment has been minimized.

Sign in to view
@beccadax

This comment has been minimized.

Sign in to view
@beccadax
Copy link
Contributor Author

Please test with following pull request:
apple/swift-lldb#1365

@swift-ci please smoke test

@beccadax beccadax force-pushed the theres-a-path-for-everyone branch from ba09af0 to 9b56c6c Compare March 13, 2019 00:15
@beccadax
Copy link
Contributor Author

The new test caused a different error message on Linux than it did on macOS. Tweaked the test to use an empty SDK instead of an SDK with an unloadable standard library; that ought to give the same message on both platforms.

@beccadax
Copy link
Contributor Author

Please test with following pull request:
apple/swift-lldb#1365

@swift-ci please smoke test

@jrose-apple
Copy link
Contributor

Tweaked the test to use an empty SDK instead of an SDK with an unloadable standard library; that ought to give the same message on both platforms.

Er, but that's no longer testing that we prefer the SDK over the toolchain.

@beccadax
Copy link
Contributor Author

Why doesn't Github have 🤦‍♂️ as a reaction?

@beccadax
Load runtime library modules from SDK
c177674 
With this change, swiftc will still look for standard library and overlay modules in ../lib/swift (relative to the compiler), but if it doesn’t find them there, it will now look in usr/lib/swift in the SDK.
@beccadax beccadax force-pushed the theres-a-path-for-everyone branch from 9b56c6c to c177674 Compare March 13, 2019 02:47
@beccadax
Copy link
Contributor Author

@jrose-apple I don't think there's a way to distinguish between "standard library is invalid" and "standard library is missing" on non-Darwin platforms, so I'll go back to the old test design and make it conditional on objc_interop. I filed SR-10097 to improve the errors.

I need to get this tested and merged so it can be cherry-picked, but if you're not happy with where this is at, I can do some follow-up work in another PR.

@beccadax
Copy link
Contributor Author

Please test with following pull request:
apple/swift-lldb#1365

@swift-ci please smoke test

@beccadax beccadax merged commit 21b96d4 into swiftlang:master Mar 13, 2019
beccadax added a commit to beccadax/swift that referenced this pull request Mar 13, 2019
@beccadax
Merge pull request swiftlang#23175 from brentdax/theres-a-path-for-ev…
465162f 
…eryone

Look for runtime library modules in the SDK, too
@beccadax beccadax mentioned this pull request Mar 13, 2019
Merged
@jrose-apple
Copy link
Contributor

I don't think there's a way to distinguish between "standard library is invalid" and "standard library is missing" on non-Darwin platforms, so I'll go back to the old test design and make it conditional on objc_interop.

The folder doesn't do that?

beccadax added a commit to beccadax/swift that referenced this pull request Jun 25, 2019
@beccadax
Look up runtime libraries in SDK
6a1da3b 
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 beccadax mentioned this pull request Jun 25, 2019
Merged
beccadax added a commit that referenced this pull request Jun 25, 2019
@beccadax
Look up runtime libraries in SDK (#25740)
b818b44 
In #23175, we started looking in the SDK for swiftmodules, but we want to look for the dylibs there too. Fixes <rdar://problem/52059706>.
@beccadax beccadax mentioned this pull request Jun 25, 2019
Merged
beccadax added a commit to beccadax/swift that referenced this pull request Jul 3, 2019
@beccadax
Look up runtime libraries in SDK (swiftlang#25740)
d06cfb2 
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>.
This was referenced Jun 1, 2025
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrose-apple jrose-apple jrose-apple approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@beccadax @swift-ci @jrose-apple