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

Create CrossCompilationModel.md #73581

Merged
merged 1 commit into from
Jun 22, 2024
Merged

Create CrossCompilationModel.md #73581

merged 1 commit into from
Jun 22, 2024

Conversation

compnerd
Copy link
Member

Add some documentation to describe the proposed model for cross-compilation, associated flags, and the reasoning for the structure. This should allow us to have a reference for the design allowing us to evolve the model.

@compnerd
Copy link
Member Author

CC: @etcwilde

Copy link
Contributor

@etcwilde etcwilde left a comment

Choose a reason for hiding this comment

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

Many editorial requests.

Contentwise, I think we're missing some details on the behavior of the flags, specifically, where the standard library and other libraries live. It's not really an overlay, it's the standard library. Neither are Foundation nor Dispatch, which are also parts of the SDK. If I had the energy and time, I could totally build a Linux distro with the Swift standard library installed natively into /usr/lib, which would then make it part of the sysroot at /.

At least the way I understand it, the C/C++ headers should go under -sysroot. Any and all Swift bits should go under -sdk (except the bits that are specific to a given compiler, which are compiler-resources. I'm aware that I'm ignoring the binary swiftmodules right now.). If -sysroot is left unspecified, it seems like it should default to whatever -sdk was set to so that existing builds continue to work as they used to.

docs/CrossCompilationModel.md Outdated Show resolved Hide resolved
docs/CrossCompilationModel.md Outdated Show resolved Hide resolved
docs/CrossCompilationModel.md Outdated Show resolved Hide resolved
docs/CrossCompilationModel.md Outdated Show resolved Hide resolved
These pieces compose in a particular manner to build a system image to build
code against.

The compiler resources are content for the compiler that the compiler provides.
Copy link
Contributor

Choose a reason for hiding this comment

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

something something "matches that specific compiler" something something "not really meant to be shared across different compilers" something something.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, but also there is the compatibility required for the standard library. The Swift standard library depends on some of the compiler resources. The reason for calling out the packging here is that at least on Windows, we package the SwiftShims into the SDK to maintain the compatibility with the standard library. But I agree with you that we should make this bit explicit.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah... SwiftShims is unfortunate since it combines headers that are definitely compiler-ABI details with headers that are for the stdlib. So for that one, you can flip a coin to choose whether it's compiler-specific or stdlib-specific and you'd be equally right and wrong at the same time.

docs/CrossCompilationModel.md Outdated Show resolved Hide resolved
docs/CrossCompilationModel.md Outdated Show resolved Hide resolved
docs/CrossCompilationModel.md Outdated Show resolved Hide resolved
These pieces compose in a particular manner to build a system image to build
code against.

The compiler resources are content for the compiler that the compiler provides.
Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, but also there is the compatibility required for the standard library. The Swift standard library depends on some of the compiler resources. The reason for calling out the packging here is that at least on Windows, we package the SwiftShims into the SDK to maintain the compatibility with the standard library. But I agree with you that we should make this bit explicit.

docs/CrossCompilationModel.md Outdated Show resolved Hide resolved
docs/CrossCompilationModel.md Outdated Show resolved Hide resolved
docs/CrossCompilationModel.md Outdated Show resolved Hide resolved
@finagolfin
Copy link
Contributor

Great to finally see this documented. 🥳

@compnerd
Copy link
Member Author

Contentwise, I think we're missing some details on the behavior of the flags, specifically, where the standard library and other libraries live. It's not really an overlay, it's the standard library. Neither are Foundation nor Dispatch, which are also parts of the SDK. If I had the energy and time, I could totally build a Linux distro with the Swift standard library installed natively into /usr/lib, which would then make it part of the sysroot at /.

The standard library is part of the "SDK Overlay", which seems odd, but makes sense after realization of what is going on. The "SDK Overlay" overlays the Darwin SDK - aka the sysroot. I was trying to avoid creating a new term as an existing term of art exists. The "SDK Overlay" is generally disjoint and what you end up with is a union of two filesystems (a la unionfs). This provides all the binary swiftmodules and auxiliary content required for Swift compilation for a platform. It would contain the standard library, dispatch, foundation, and potentially XCTest. The Swift shims also are provided through this union.

@compnerd compnerd requested a review from etcwilde May 13, 2024 19:16
Copy link
Contributor

@etcwilde etcwilde left a comment

Choose a reason for hiding this comment

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

yeah, looking better

These pieces compose in a particular manner to build a system image to build
code against.

The compiler resources are content for the compiler that the compiler provides.
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah... SwiftShims is unfortunate since it combines headers that are definitely compiler-ABI details with headers that are for the stdlib. So for that one, you can flip a coin to choose whether it's compiler-specific or stdlib-specific and you'd be equally right and wrong at the same time.

docs/CrossCompilationModel.md Outdated Show resolved Hide resolved

Currently, we do not have a good way to isolate the SDK overlay from the
remainder of the required content. On Darwin platforms, this content is shipped
as part of the SDK. As a result the singular `-sdk` flag allows control over the
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that the new swift-driver does not look in -sdk for the SDK overlay, only for the C/C++ headers, swiftlang/swift-driver#1562.

@finagolfin
Copy link
Contributor

I'm fine with shifting the cross-compilation model over to this new set of flags, but it's misleading to say only "compiler resources are controlled via the driver flag -resource-dir," as it has been long-standing practice that the SDK overlay location is also set by this flag.

I get that you guys don't like the way that flag was named and feel it was misused, but given the way it has long been used, including by
the compiler today, maybe that current use should be mentioned, at least to clear up any confusion?

@compnerd
Copy link
Member Author

I'm fine with shifting the cross-compilation model over to this new set of flags, but it's misleading to say only "compiler resources are controlled via the driver flag -resource-dir," as it has been long-standing practice that the SDK overlay location is also set by this flag.

I don't think that the change really changes the existing behaviour per se. It does however try to homogenise the behaviour between the various platforms. The behaviour on Windows and Darwin is already pretty similar, at least from the user's experience.

The equivalent of the -sysroot value is computed either by the environment, scanning the system, or by the user specifying it on Windows. It is packaged on Darwin into the single SDK parameter.

The default value for -sdk on Windows is specified by the environment variable SDKROOT. On Darwin, SDKROOT is honoured but defaults to the SDKs near the compiler.

This would make all the platforms behave similarly making the experience a bit better.

@finagolfin
Copy link
Contributor

I'm not talking about those other flags: I'm talking about -resource-dir, which the new swift-driver passes to the frontend on all platforms, whether -resource-dir was passed in to the driver or not (this is a change from the legacy C++ Driver, which would only pass along this flag if it was given a -resource-dir itself). The frontend then only uses the SDK overlay at that location, no matter what is in the -sdk.

My point is that -resource-dir is seemingly currently used on all platforms by the frontend to find the SDK overlay and runtime libraries. If you want -resource-dir to only specify the few "compiler resources" you listed instead, that is a change in behavior.

I am fine with changing what -resource-dir means, but we should document such an important change and warn users.

@compnerd
Copy link
Member Author

I think that for having a road to migrate we should be incrementally changing the behaviour. What this means is that we should preserve the existing behaviour of -resource-dir selecting the SDK, but if -sdk is specified, it would take precedence.

@finagolfin
Copy link
Contributor

As I said, I'm all for making these changes, but I think we should mention that -resource-dir has long been used to select the SDK overlay in this document, and that we are changing the precedence as you just detailed.

@compnerd
Copy link
Member Author

Ah, I see what you are asking for - sure, making a remark about the historical behaviour is reasonable.

@compnerd
Copy link
Member Author

CC: @xedin

Copy link
Contributor

@etcwilde etcwilde left a comment

Choose a reason for hiding this comment

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

more comments

docs/CrossCompilationModel.md Outdated Show resolved Hide resolved
docs/CrossCompilationModel.md Outdated Show resolved Hide resolved
docs/CrossCompilationModel.md Outdated Show resolved Hide resolved
docs/CrossCompilationModel.md Outdated Show resolved Hide resolved
@compnerd compnerd force-pushed the compnerd/cross-compile-model branch 2 times, most recently from ab66d12 to 68554ec Compare June 20, 2024 21:55
@compnerd
Copy link
Member Author

@swift-ci please smoke test

Copy link
Contributor

@etcwilde etcwilde left a comment

Choose a reason for hiding this comment

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

Only the one comment on the definition of the Swift SDK. Other than that, I think this looks good.

docs/CrossCompilationModel.md Outdated Show resolved Hide resolved
Add some documentation to describe the proposed model for cross-compilation, associated flags, and the reasoning for the structure. This should allow us to have a reference for the design allowing us to evolve the model.

Co-authored-by: Evan Wilde <ewilde@apple.com>
Co-authored-by: Alexander Smarus <bender@readdle.com>
Co-authored-by: Danielle <dani@builds.terrible.systems>
@compnerd compnerd force-pushed the compnerd/cross-compile-model branch from 68554ec to 3e6aa17 Compare June 21, 2024 21:11
@compnerd
Copy link
Member Author

@swift-ci please smoke test

Copy link
Contributor

@etcwilde etcwilde left a comment

Choose a reason for hiding this comment

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

LGTM

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.

7 participants