RFC: Add --@bazel_build_rules_swift//swift:static_stdlib #706

liuliu · 2021-10-28T01:34:31Z

This PR added the ability to compile Swift binary with stdlib in Linux.

It added -static-stdlib to swiftc invocation and added
static-stdlib-args.lnk to linker flag.

However, there is an issue with Bazel cache:

bazel build examples:ddpg --@bazel_build_rules_swift//swift:static_stdlib

It compiles.

Then:

bazel build examples:ddpg

See error:

/usr/bin/ld: cannot find -lDispatchStubs
/usr/bin/ld: cannot find -lCoreFoundation

It seems reused the previous autolink extract results, because:

bazel clean --expunge
bazel build examples:ddpg

Worked again.

liuliu · 2021-10-28T02:54:41Z

The issue mentioned in this PR seems related to persistent worker mode. Disabling it (changing to "wrap") will not trigger this.

liuliu · 2021-10-28T03:25:02Z

tools/worker/work_processor.cc

@@ -77,6 +78,8 @@ void WorkProcessor::ProcessWorkRequest(
    } else if (prev_arg == "-output-file-map") {
      output_file_map_path = arg;
      arg.clear();
+    } else if (arg == "-static-stdlib") {
+      is_static_stdlib = true;


I am not happy with this. This appears to be a bug in swiftc which should invalidate previous cache when this parameter changed. It is perfectly capable of doing incremental build for static stdlib build otherwise.

If I have a way to invalidate the current persistent worker and restart when swift.static_stdlib enabled / disabled, that would be ideal.

Huh interesting, can you reproduce this outside of bazel? Wondering if we should report something. Are you testing with 5.5 as well?

Yeah, I am on 5.5.1. Would need to dig this.

keith · 2021-10-28T04:04:36Z

swift/internal/swift_toolchain.bzl

@@ -181,6 +199,7 @@ def _swift_toolchain_impl(ctx):
        ctx.attr.os,
        ctx.label,
        toolchain_root,
+        SWIFT_FEATURE_STATIC_STDLIB in ctx.features,


Hrm. I wouldn't expect this code to be re-evaluated if this changed, especially if you didn't change the value of --features and instead set it on features on the binary. I wonder if we will instead have to add this in a similar place to how you're adding the compilation flag, and removing the default path in that case (emulating your if / else)

ah this is what caused the issue you mentioned in the description as well

Yeah. Not sure what's the best to do here, I didn't consider the situation of set features on the binary use case. I have a few options:

Generate static linkopts and dynamic linkopts in swift_toolchain.bzl, populate that into the SwiftToolchainInfo struct. In linking.bzl, select which one to use based on the feature_configuration.

Introduce a new action LINKING, register a new action_config that runs with linking.bzl like what we do in compiling.bzl.

Either way, it seems that if we are going to move the selection to linking.bzl, populating user_linker_flag in swift_toolchain.bzl is quite meaningless because most of these depend on whether we want to -static-stdlib or not.

@allevato may need your input on what's the use-case of the common user_linker_flag populated with swift_toolchain.bzl, and whether that should / can move to linking.bzl

TBH, this may need some refactoring to be suitable for open-source use cases if you want to support swift_binary targets using both linking modes in the same build graph. An older version of this function used to have an is_static argument and it was called by swift_binary, but it was removed in a refactoring since we were only supporting dynamic linking at the time.

Do you care about being able to support static and dynamic linking for different binaries in the same build, or would you be ok with just supporting one globally for an entire build? If you just want one global setting, then it's easier: you could define a Starlark build flag to control it (instead of a feature) and add that flag as a dependency of the toolchain. Then you would just pass something like --@bazel_build_rules_swift//swift:static_stdlib to your build command.

If you want to be able to support static linking for some binaries and not others, then it'll be a bit more work. By itself, introducing a new LINKING action won't help you much because the action is still carried out by the C++ toolchain. What you need is a pseudo-action that can give you the command line flags without actually registering the action with ctx.actions, and then you could request that action to get the flags from the toolchain and then pass the extra flags to the C++ linking API, but the rules don't have a framework in place for those kinds of fake actions yet.

Yes, I only care about generate static binaries on Linux (as that will be for all my binaries). There are some practical limitations such as XCTest doesn't have static lib counter-part and can error out compiling swift_test, but just have static binaries on Linux is my goal.

From that perspective, I think that I need to read a bit more about what is Starlark build flag and how to do something like --@bazel_build_rules_swift//swift:static_stdlib.

@allevato upon further looking, wouldn't the new things we added simply similar to how "autolink" happens? We just need to disambiguite whether static_stdlib feature enabled or not, and pass proper link flags here? https://github.com/bazelbuild/rules_swift/blob/master/swift/internal/linking.bzl#L154

I think we need to be able to support both, because there are cases where statically linking doesn't work, specifically if you're using SwiftSyntax and its internal syntax parser library. In that case if you ran tests on linux that needed to exercise that binary as well as other binaries, it would have to be dynamically linked, while the others could be statically linked.

liuliu · 2021-11-06T17:06:59Z

@allevato did the update with --@bazel_build_rules_swift//swift:static_stdlib I am still open to modify this PR to support the later behaviour (supporting features in swift_binary) if you have better pointers for where I should start.

This PR added the ability to compile Swift binary with stdlib in Linux. It added `-static-stdlib` to `swiftc` invocation and added `static-stdlib-args.lnk` to linker flag. However, there is an issue with Bazel cache: ``` bazel build examples:ddpg --features=swift.static_stdlib ``` It compiles. Then: ``` bazel build examples:ddpg ``` See error: ``` /usr/bin/ld: cannot find -lDispatchStubs /usr/bin/ld: cannot find -lCoreFoundation ``` It seems reused the previous autolink extract results. Disable incremental build when -static-stdlib is presented.

liuliu · 2022-06-03T04:01:36Z

Looked a bit more today for this. If I want to support

swift_binary(
    features = ["swift.static_stdlib"],
)

It seems that I need some way to propagate this up to swift_library to make sure these are compiled properly with -static-stdlib flag. What's the suggested way to do that? Thanks!

google-cla bot added the cla: yes label Oct 28, 2021

liuliu mentioned this pull request Oct 28, 2021

Support fully-static linking #8

Open

liuliu commented Oct 28, 2021

View reviewed changes

keith reviewed Oct 28, 2021

View reviewed changes

liuliu changed the title ~~RFC: Add --features=swift.static_stdlib~~ RFC: Add --@bazel_build_rules_swift//swift:static_stdlib Nov 6, 2021

liuliu force-pushed the liu/support-static-stdlib branch from 917000a to 15698bf Compare June 3, 2022 02:27

liuliu requested review from BalestraPatrick, brentleyjones, segiddins and thii as code owners June 3, 2022 02:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC: Add --@bazel_build_rules_swift//swift:static_stdlib #706

RFC: Add --@bazel_build_rules_swift//swift:static_stdlib #706

liuliu commented Oct 28, 2021 •

edited

liuliu commented Oct 28, 2021 •

edited

liuliu Oct 28, 2021

keith Oct 28, 2021

liuliu Oct 28, 2021

keith Oct 28, 2021

keith Oct 28, 2021

liuliu Oct 28, 2021 •

edited

allevato Nov 5, 2021

liuliu Nov 5, 2021

liuliu Nov 6, 2021

keith Nov 18, 2021

liuliu commented Nov 6, 2021

liuliu commented Jun 3, 2022

RFC: Add --@bazel_build_rules_swift//swift:static_stdlib #706

Are you sure you want to change the base?

RFC: Add --@bazel_build_rules_swift//swift:static_stdlib #706

Conversation

liuliu commented Oct 28, 2021 • edited

liuliu commented Oct 28, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liuliu Oct 28, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liuliu commented Nov 6, 2021

liuliu commented Jun 3, 2022

liuliu commented Oct 28, 2021 •

edited

liuliu commented Oct 28, 2021 •

edited

liuliu Oct 28, 2021 •

edited