Skip to content

[test] Enable Serialization/runtime-import-from-sdk on non-Darwin platforms #81842

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

finagolfin
Copy link
Member

@finagolfin finagolfin commented May 29, 2025

Also, remove Swift runtime modules from Serialization/module_defining_interface,_client that are unused.

This seems to vary by platform today, so see what the CI finds...

@finagolfin finagolfin requested a review from xymus as a code owner May 29, 2025 18:41
@finagolfin finagolfin removed the request for review from xymus May 29, 2025 18:41
@finagolfin
Copy link
Member Author

@swift-ci smoke test

@finagolfin finagolfin marked this pull request as draft May 29, 2025 21:36
@finagolfin
Copy link
Member Author

@swift-ci smoke test

@finagolfin finagolfin force-pushed the resource-path branch 2 times, most recently from 64cd337 to 1e6641a Compare May 30, 2025 10:33
@finagolfin
Copy link
Member Author

@swift-ci smoke test

@finagolfin finagolfin changed the title DNM: check what the frontend is doing for some tests [test] Enable Serialization/runtime-import-from-sdk on non-Darwin platforms May 30, 2025
@finagolfin
Copy link
Member Author

@swift-ci smoke test

…tforms

Also, remove Swift runtime modules from Serialization/module_defining_interface,_client
that are unused.
@finagolfin
Copy link
Member Author

@swift-ci smoke test

@finagolfin finagolfin marked this pull request as ready for review June 1, 2025 11:21
@finagolfin
Copy link
Member Author

@beccadax, I've been looking into some issues with cross-compiling for Unix platforms through specifying an explicit -sdk and came across this test Serialization/runtime-import-from-sdk that you added in #23175 six years ago, which explicitly codifies some rules of where to look for Swift runtime modules but the test was never enabled for non-Darwin platforms.

By adding the correct platform-suffixed path, where the compiler looks for the runtime modules on non-Darwin platforms, and tweaking some formatting, I got this test to pass the CI on linux and Windows too. 😃 The two other tests added runtime modules to a mock SDK, but were never used because of the rule about the resource-dir taking precedence, so I removed those lines without a problem.

This small pull is ready for review, can finally enable it for non-Darwin.

@finagolfin
Copy link
Member Author

I'd also like to add another pull to change these two rules checked in this test, which currently boil down to the following:

  1. The Swift runtime modules found in a resource-dir, whether explicit or implicit, always take precedence over any runtime modules found in an explicitly specified -sdk.
  2. If the resource-dir doesn't have the runtime modules for the target platform and if an explicit -sdk is specified, fall back to using the runtime modules from the -sdk instead.

While these rules may have made sense six years ago when cross-compilation with an explicitly specified -sdk was first being spun up, I think they need to be tightened up today.

Here are the alternate rules I propose:

  1. Make a distinction between an explicitly specified -resource-dir and the implicit default resource-dir, which the compiler driver adds at bin/../lib/swift/ relative to the compiler. Change the precedence to first an explicit -resource-dir, then an explicit -sdk, and finally an implicit resource-dir. The difference to the current rule above is that -sdk is currently third, and I want to move it to second.
  2. Don't fall back between resource-dir and -sdk paths at all, which is a recipe for conflicts. Instead, just use the above order of
    a) explicit -resource-dir
    b) explicit -sdk
    c) implicit resource-dir, ie bin/../lib/swift/
    and only look in the first one specified with no fallback, erroring if the Swift runtime modules are not found in the first one specified.

The general principles of my rule changes are to always use the explicitly specified flag paths first, and only look in one path, no fallbacks. The latter is to avoid the risk of the runtime modules being pulled from one path while the runtime libraries are pulled from a completely different path, ie mixing and matching Swift SDK files from different locations and causing version conflicts.

I have implemented my updated rule 1. in #79621 and can expand it to clean up rule 2., if given the go-ahead.

Another consideration is the planned new cross-compilation model, which makes -sdk where to look for Swift runtime modules, -sysroot for the platform C/C++ sysroot, and deprecates the -resource-dir flag as a path to look in for Swift runtime modules and libraries. My new rules work well with this plan- simply remove -resource-dir from the top of the precedence list I gave- but the -sdk/-sysroot plan doesn't work with the current rule 1., which gives an implicit resource-dir precedence even over an explicit -sdk.

@beccadax, @etcwilde, @compnerd, and @kateinoigakukun , let me know what you think of these updated rules I propose, since you all have extensive experience with these cross-compilation flags. I'm fairly certain the updated rules will work well with non-Darwin platforms, but I have no experience with building for Darwin platforms, though my pull #79621 passing the macOS CI suggests my updated rule 1. will be fine.

As seen in that small pull, we can add temporary warnings and fallbacks initially before tightening up these rules as I propose over time.

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.

1 participant