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

Driver: introduce -sysroot option for non-Darwin targets #72352

Merged
merged 1 commit into from
Jul 12, 2024

Conversation

compnerd
Copy link
Member

This introduces a secondary flag -sysroot for the non-Darwin targets, primarily Unicies. The intention here is to support a split -sdk, -sysroot model where the -sdk parameter provides the Swift "SDK" which augments the native platform's C sysroot which is indicated as -sysroot. For the case of Android, this would allow us to provide a path to the NDK sysroot and the Swift SDK allowing us to cross-compile Android binaries from Windows.

@hyp
Copy link
Contributor

hyp commented Mar 15, 2024

Could we also pass --sysroot flag to the Clang importer using the value passed to this new flag? Otherwise, I still have to pass -Xcc -sysroot <path-to-ndk> when building for android.

@compnerd
Copy link
Member Author

Could we also pass --sysroot flag to the Clang importer using the value passed to this new flag? Otherwise, I still have to pass -Xcc -sysroot <path-to-ndk> when building for android.

Yes, that is the intention - to setup the clang importer to use the specified sysroot. It is odd that the value for the SDK is still being used as when -sysroot is specified, it should take precedence over the -sdk value.

@finagolfin
Copy link
Contributor

For the case of Android, this would allow us to provide a path to the NDK sysroot and the Swift SDK allowing us to cross-compile Android binaries from Windows.

Note that there already is a long-established way to do this, ie to pass -sdk NDK C sysroot and -resource-dir Swift SDK to the compiler which should already work today on Windows. You simply believe that passing the -resource-dir is not a good way to do it.

I am fine in principle with switching that flag combo to your new combo of -sdk Swift SDK -sysroot NDK C sysroot, although that will require fixing swiftlang/swift-driver#1562 first, but that raises the question of what you plan to do about the old -sdk/-resource-dir combo, phase it out completely? Do you plan on switching the toolchain build over to your new combo too?

@compnerd
Copy link
Member Author

I am fine in principle with switching that flag combo to your new combo of -sdk Swift SDK -sysroot NDK C sysroot, although that will require fixing swiftlang/swift-driver#1562 first, but that raises the question of what you plan to do about the old -sdk/-resource-dir combo, phase it out completely? Do you plan on switching the toolchain build over to your new combo too?

I don't mind working through the issues that are required for this. I've tried to structure changes such that -sysroot would be given preference if specified. If unspecified, try to behave like the old way. This is unfortunately a requirement for driver level changes IMO (we need to have some sort of migration story). The Windows cross-compiling support is likely to attempt to use this mechanism rather than the old mechanism, I don't have plans to go modifying build-script/build-script-impl.

@finagolfin
Copy link
Contributor

I don't mind working through the issues that are required for this.

I will look into that swift-driver issue in the coming week, though it will probably be good if more people look into it too.

I've tried to structure changes such that -sysroot would be given preference if specified.

Yep, I see you override where the -sdk path is currently passed in to the clangImporter as the sysroot.

This is unfortunately a requirement for driver level changes IMO (we need to have some sort of migration story). The Windows cross-compiling support is likely to attempt to use this mechanism rather than the old mechanism, I don't have plans to go modifying build-script/build-script-impl.

Understood, you and @etcwilde seemed to think -resource-dir is being misused, so I wondered how far you wanted to go to change it. That's fine: once this is in, we can slowly transition the toolchain build and SDK bundles to use this new flag, if wanted.

@compnerd
Copy link
Member Author

@swift-ci please test

@finagolfin
Copy link
Contributor

@compnerd, have you worked on changing the meaning of the -resource-dir flag also, as described in your new cross-compilation doc? If you have these pulls ready, people who cross-compile extensively for "Unicies" like me can apply them locally and test them out.

A head's up to @MaxDesiatov too, you'll want to change the SwiftPM SDK bundle config to use these new flags, as shown in the linked doc, once they're updated in the compiler and swift-driver.

@compnerd
Copy link
Member Author

No, I'm trying to get the initial work in place so that this is usable and working so that we have a working state that we can work from rather than accidentally break something and then try to fix it.

@finagolfin
Copy link
Contributor

Oh, that's my point: this new flag in this pull is not going to break anything, so it should be fine, but your plans to change what -resource-dir means could break something. So I'm asking if you have any separate local patches in use yet to change the meaning of -resource-dir and that you slap them up as draft pulls at least if so. It sounds like you haven't implemented the -resource-dir changes yet though.

The use of the -resource-dir flag through this codebase is a mess, I don't envy you having to track those down. I just found another bug with its use, for which I'm trying to formulate a clean repro now before filing it.

This introduces a secondary flag `-sysroot` for the non-Darwin targets,
primarily Unicies. The intention here is to support a split `-sdk`,
`-sysroot` model where the `-sdk` parameter provides the Swift "SDK"
which augments the native platform's C sysroot which is indicated as
`-sysroot`. For the case of Android, this would allow us to provide a
path to the NDK sysroot and the Swift SDK allowing us to cross-compile
Android binaries from Windows.
@compnerd
Copy link
Member Author

@swift-ci please test

@compnerd
Copy link
Member Author

@swift-ci please test macOS platform

@compnerd compnerd merged commit 7cc7e2b into swiftlang:main Jul 12, 2024
5 checks passed
@compnerd compnerd deleted the sysroot branch July 12, 2024 15:09
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