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

Frontend: correct `-sdk` search paths for !Darwin #25902

Merged
merged 1 commit into from Jul 3, 2019

Conversation

Projects
None yet
4 participants
@compnerd
Copy link
Collaborator

commented Jul 1, 2019

This adjusts the runtime library import paths to add the OS/architecture
subdirectory to the path. This allows the use of -sdk with the
compiler to target Android and Windows as well (and Linux). The SDK
image is identical to that generated by the current CMake setup without
the host tools. Ideally, we would change the layout for the modules on
other targets to be identical to the Darwin layout which converts to
OS//architecture. rather than
OS/architecture/..

Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.

Resolves SR-NNNN.

@compnerd

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 1, 2019

@compnerd

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 1, 2019

@swift-ci please test

@jrose-apple

This comment has been minimized.

Copy link
Member

commented Jul 1, 2019

Shouldn't this just be the architecture and not the platform? The latter should already be part of the SDK, no?

@compnerd

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 1, 2019

Yes, it really should be. However, unfortunately, the current CMake installation image doesn't construct that properly. I want to get that moved over, but couldn't figure out how to make that work. I really want to move the other targets over to the module as a directory approach so that we have the multiple architectures supported.

@brentdax

This comment has been minimized.

Copy link
Collaborator

commented Jul 1, 2019

I think we currently treat SDKs on non-Darwin platform as sysroots. Is this going to be compatible with that interpretation? Do we want to install our runtime libraries to platform- and architecture-specific directories on non-Darwin platforms?

@brentdax
Copy link
Collaborator

left a comment

This definitely needs tests before we can merge it.

@jrose-apple

This comment has been minimized.

Copy link
Member

commented Jul 1, 2019

@compnerd and I have talked a little about the sysroot aspect before, and one of the conclusions was that this is more useful for cross compilation than having to ship an SDK and a resource directory…especially when most of the normal resource directory can be shared.

@compnerd

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 1, 2019

@brentdax - a sysroot doesn't work on all targets. Particularly Windows, the sysroot wont work since there is no concept of sysroot. Android also does not have a sysroot but a set of paths that need to be configured. It seems better to just standardise on this. The SDK here comprises of all the swift pieces that are needed and lets you be agnostic to the root of the system.

@compnerd

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 1, 2019

@brentdax, @jrose-apple Im happy to add a test case for this but I don't see a good way to add a test case for this. Particularly for the case where we are doing cross-compilation.

@jrose-apple

This comment has been minimized.

Copy link
Member

commented Jul 1, 2019

Maybe you can set up a dummy resource dir and a dummy SDK and then try to get as far as -typecheck?

@compnerd

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 1, 2019

Hmm, wont that try to load the swift module? If so, it would fail, since we cannot have a valid swift module for a foreign target.

@jrose-apple

This comment has been minimized.

Copy link
Member

commented Jul 1, 2019

-parse-stdlib, maybe? We just need to import some module, and it can come from a swiftinterface now.

@compnerd

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 1, 2019

But if you use -parse-stdlib it wont load the standard library right? So, it doesn't test this.

@jrose-apple

This comment has been minimized.

Copy link
Member

commented Jul 1, 2019

Right, but you can import other things from the same path using normal import.

@compnerd compnerd force-pushed the compnerd:sdk-on-non-darwin branch from 77b0219 to 725cb48 Jul 2, 2019

@compnerd

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 2, 2019

@brentdax @jrose-apple okay, I managed to come up with a tricky test case

@compnerd

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 2, 2019

@swift-ci please test

@swift-ci

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2019

Build failed
Swift Test Linux Platform
Git Sha - 77b0219

@swift-ci

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2019

Build failed
Swift Test OS X Platform
Git Sha - 77b0219

@jrose-apple
Copy link
Member

left a comment

Thanks!

Show resolved Hide resolved test/Driver/android-sdk.swift Outdated
Show resolved Hide resolved test/Driver/android-sdk.swift Outdated

@compnerd compnerd force-pushed the compnerd:sdk-on-non-darwin branch from 725cb48 to d758a9a Jul 2, 2019

@compnerd

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 2, 2019

@swift-ci please test

@swift-ci

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2019

Build failed
Swift Test Linux Platform
Git Sha - d758a9a

@swift-ci

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

Build failed
Swift Test OS X Platform
Git Sha - d758a9a

@compnerd compnerd force-pushed the compnerd:sdk-on-non-darwin branch from d758a9a to c2d7918 Jul 3, 2019

@compnerd

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 3, 2019

@swift-ci please test and merge

Frontend: correct `-sdk` search paths for !Darwin
This adjusts the runtime library import paths to add the OS/architecture
subdirectory to the path.  This allows the use of `-sdk` with the
compiler to target Android and Windows as well (and Linux).  The SDK
image is identical to that generated by the current CMake setup without
the host tools.  Ideally, we would change the layout for the modules on
other targets to be identical to the Darwin layout which converts to
OS/<module>/architecture.<extension> rather than
OS/architecture/<module>.<extension>.

@compnerd compnerd force-pushed the compnerd:sdk-on-non-darwin branch from c2d7918 to f3655ed Jul 3, 2019

@compnerd

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 3, 2019

@swift-ci please test and merge

@swift-ci swift-ci merged commit 930699c into apple:master Jul 3, 2019

4 of 5 checks passed

Test and Merge Build started.
Details
Swift Test Linux Platform No test results found.
Details
Swift Test Linux Platform (smoke test)
Details
Swift Test OS X Platform No test results found.
Details
Swift Test OS X Platform (smoke test)
Details

@compnerd compnerd deleted the compnerd:sdk-on-non-darwin branch Jul 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.