Add Static Linux Swift SDK support for cross-compilation#1813
Add Static Linux Swift SDK support for cross-compilation#1813adincebic wants to merge 4 commits into
Conversation
8d7f3be to
7ae62ec
Compare
| # `swiftc` defaults to it, `swift-symbolgraph-extract` (used for XCTest | ||
| # test discovery) matches the module layout's triple exactly and fails | ||
| # to load modules built for `*-windows-msvc` if it is omitted. | ||
| return "%s-unknown-windows-msvc" % arch |
There was a problem hiding this comment.
What are the other windows target triples supported by swiftc ?
| it cannot target by itself. The `swift` extension can download these and | ||
| define matching Swift and C/C++ toolchains, so that plain `swift_library` |
There was a problem hiding this comment.
Should you make this opt-out to allow c/c++ toolchains to be registered by a different module ?
|
|
||
| filegroup( | ||
| name = "sdk_files", | ||
| srcs = glob(["{bundle_dir}/**"]), |
There was a problem hiding this comment.
Should you use source directories ?
| ) | ||
|
|
||
| constraint_value( | ||
| name = "static_linux", |
There was a problem hiding this comment.
If static_linux really implies musl, I would model that using libc semantic. That way everything will align when bazel-contrib/platforms_contrib#5 lands.
There was a problem hiding this comment.
+1. and if you want you can reuse @llvm's libc/musl constraint for now, that is what rules_rs does as well. Once platforms_contrib ships, we will make ours a forwarding alias
There was a problem hiding this comment.
it does imply musl, but maybe the problem is you "can't" just use any old musl cc toolchain, since apple's clang fork that ships with swift is tied to who knows what implementation details. so that probably still means we need some custom constraint like this? (and maybe should also add the musl constraint for random select() statements)
|
|
||
| * a `swift_toolchain` that compiles against the SDK's sysroot and Swift | ||
| resource directory, and links against its static Swift runtime; and | ||
| * a rules_cc `cc_toolchain` that drives the matching clang for the target |
There was a problem hiding this comment.
is it possible to reuse existing cc toolchain like hermetic-llvm? I feel like otherwise there will be a lot of duplicated clang binaries in the output base and more maintenance burden overall. We already support wasm, happy to look at what's missing to make android work better
There was a problem hiding this comment.
So the problem I think is that unless those are used for exec tools, the LLVM version used by Apple Clang is that of their own and mismatch means either failures or no LTO
| _HOST_LINKER_INPUTS = "swift_sdk_linker_inputs" | ||
|
|
||
| _CC_TOOLCHAIN_TEMPLATE = """ | ||
| cc_tool( |
There was a problem hiding this comment.
i think a lot of this could be cleaner if it was an actual .bzl file that had a macro to stamp out these targets and this .bzl emitted a load to it. would make LSPs less confused
| triple = "wasm32-unknown-wasip1", | ||
| ) | ||
|
|
||
| repository_ctx.file("BUILD.bazel", build_content) |
There was a problem hiding this comment.
please make sure you mark all repo rules and module extensions reproducible = True as appropriate!
| doc = "The Android API level to target.", | ||
| mandatory = True, | ||
| ), | ||
| "host_swiftc": attr.label( |
There was a problem hiding this comment.
do you actually need to pass this or can you just construct the label yourself? (repo rule can fabricate labels). That's how rules_rs deals with host_cargo
| ) | ||
| return content | ||
|
|
||
| def static_linux_sdk_toolchains_for_platform(platform, sdk_repository, archs): |
There was a problem hiding this comment.
does this mean you are duplicating all the SDK bytes just to change the target to musl? Or is it reusing the tools but only emitting new toolchain targets? (Finding it a bit hard to trace what is materialized where, the way this is written)
| ]), | ||
| ) | ||
|
|
||
| def _python_executable_works(repository_ctx, python_bin): |
There was a problem hiding this comment.
shouldn't we be using the python toolchain? this seems pretty hacky (though I see its partially preexisting)
| if os == "linux": | ||
| return "%s-unknown-linux-gnu" % arch | ||
| elif os == "windows": | ||
| # The MSVC cc toolchain reports a `target_gnu_system_name` of "local", |
There was a problem hiding this comment.
what is the "MSVC cc toolchain", do you mean the default one created by rules_cc? What about hermetic ones like @llvm? this feels like it's baking in a lot of assumptions about other toolchains?
|
I'll review this one once logan's stuff is in! |
7ae62ec to
eaad696
Compare
keith
left a comment
There was a problem hiding this comment.
still reviewing but flushing some comments
| toolchain_static_linux_x86_64_test = make_action_command_line_test_rule( | ||
| config_settings = { | ||
| "//command_line_option:extra_toolchains": [ | ||
| "//test/fixtures/toolchains:cc_toolchain_static_linux_x86_64", | ||
| "//test/fixtures/toolchains:toolchain_static_linux_x86_64", | ||
| ], | ||
| }, | ||
| ) |
There was a problem hiding this comment.
can we use the real toolchain we're registering for this instead of creating some fake ones? this doesn't prove much since we're just testing the behavior of the copy pasted fixture AFAIUI?
| def _static_linux_resource_path(target_settings, triple): | ||
| resource_path = target_settings.get("swiftStaticResourcesPath") | ||
| field = "swiftStaticResourcesPath" | ||
| if not resource_path: | ||
| resource_path = target_settings.get("swiftResourcesPath") | ||
| field = "swiftResourcesPath" | ||
| return _relative_metadata_path(resource_path, field, triple) |
There was a problem hiding this comment.
when doesn't it include swiftStaticResourcesPath?
There was a problem hiding this comment.
Well it actually includes it every time at least starting from Swift 6.3.2. However since both swiftStaticResourcesPath and swiftResourcesPath are present in swift-sdk.json I first check for the swiftStaticResourcesPath and fall back to the swiftResourcesPath
{
"schemaVersion": "4.0",
"targetTriples": {
"x86_64-swift-linux-musl": {
"toolsetPaths": [
"toolset.json"
],
"sdkRootPath": "musl-1.2.5.sdk/x86_64",
"swiftResourcesPath": "musl-1.2.5.sdk/x86_64/usr/lib/swift_static",
"swiftStaticResourcesPath": "musl-1.2.5.sdk/x86_64/usr/lib/swift_static"
},
"aarch64-swift-linux-musl": {
"toolsetPaths": [
"toolset.json"
],
"sdkRootPath": "musl-1.2.5.sdk/aarch64",
"swiftResourcesPath": "musl-1.2.5.sdk/aarch64/usr/lib/swift_static",
"swiftStaticResourcesPath": "musl-1.2.5.sdk/aarch64/usr/lib/swift_static"
}
}
}
There was a problem hiding this comment.
Ok I downloaded some of the older ones and all of them have the same shape, so I am dropping the swiftResourcesPath since it has the same value as swiftStaticResourcesPath anyway.
| linker_arg_expected = False | ||
| for arg in args: | ||
| if linker_arg_expected: | ||
| if arg.startswith("-undefined=") and arg != "-undefined=": | ||
| linkopts.append("-Wl,-u," + arg.removeprefix("-undefined=")) | ||
| else: | ||
| linkopts.extend(["-Xlinker", arg]) | ||
| linker_arg_expected = False | ||
| elif arg == "-Xlinker": | ||
| linker_arg_expected = True | ||
| else: | ||
| linkopts.append(arg) | ||
|
|
||
| if linker_arg_expected: | ||
| linkopts.append("-Xlinker") |
There was a problem hiding this comment.
why are we processing these? why not take them as they are in the file?
There was a problem hiding this comment.
Because we can't just take the contents as is:
-static
-lswiftCore
-lswift_RegexParser
-Xlinker
-undefined=pthread_self
-Xlinker
-undefined=pthread_once
-Xlinker
-undefined=pthread_key_create
-ldispatch
-lBlocksRuntime
-lpthread
-ldl
-lc++
-lm
purely because of -Xlinker flags.
There was a problem hiding this comment.
Perfect. Removed.
| linux_static_dir_relative, | ||
| repo_root): | ||
| swiftrt_relative = "{}/{}/swiftrt.o".format(linux_static_dir_relative, arch) | ||
| args_relative = linux_static_dir_relative + "/static-executable-args.lnk" |
There was a problem hiding this comment.
what is the difference between this one and the static-stdlib-args.lnk? looks like the args aren't 1:1, so maybe we need some combo? fwiw for android and wasm we didn't fetch from this file we hardcoded the args instead. this is probably a bit nicer for when new ones are introduced, assuming that all these args make sense in bazel.
There was a problem hiding this comment.
static-stdlib-args.lnk is for the case where you want to statically link Swift standard library swiftc -static-stdlib which we don't need here, at least not yet.
There was a problem hiding this comment.
Also those flags that come from static-executable-args.lnk seem to make sense with Bazel unless I am missing something. I wouldn't like us to hardcode them if we don't have to.
| ) | ||
|
|
||
| constraint_value( | ||
| name = "static_linux", |
There was a problem hiding this comment.
it does imply musl, but maybe the problem is you "can't" just use any old musl cc toolchain, since apple's clang fork that ships with swift is tied to who knows what implementation details. so that probably still means we need some custom constraint like this? (and maybe should also add the musl constraint for random select() statements)
| platform( | ||
| name = "static-linux-x86_64", | ||
| constraint_values = [ | ||
| "@platforms//cpu:x86_64", | ||
| "@platforms//os:linux", | ||
| "//swift/toolchains:static_linux", | ||
| ], | ||
| ) |
There was a problem hiding this comment.
if this is something we end up requiring, we should move this somewhere that users can reference it too
There was a problem hiding this comment.
Probably at the root?
da7fa11 to
1984608
Compare
1984608 to
e166267
Compare
Adding a new swift.static_linux_sdk(...) module extension tag, which downloads the Static Linux SDK artifact bundle from swift.org, generates matching Swift and C/C++ toolchains for x86_64-swift-linux-musl and aarch64-swift-linux-musl, and
wires those toolchains into the existing standalone-toolchain flow.
I am also adding a dedicated //swift/toolchains:static_linux constraint to distinguish Static Linux targets from generic Linux targets. Further more, I tried documenting Static Linux SDK setup and toolchain registration
One important detail is toolchain registration order: Static Linux SDK toolchains must be registered before generic Linux Swift toolchains, since both are compatible with @platforms//os:linux and Bazel prefers earlier registered toolchains. Open to discussing if there is a better way to do this.