Improve mingw link flag handling by dzbarsky · Pull Request #3785 · bazelbuild/rules_rust

dzbarsky · 2025-12-18T19:22:41Z

Without this, we get errors like

          lld: error: unable to find library -lliblibcxx.static
          lld: error: unable to find library -lliblibcxxabi.static
          lld: error: unable to find library -lliblibunwind.static
          lld: error: unable to find library -llibmingw32
          lld: error: unable to find library -llibucrtbase

because get_lib_windows doesn't strip the leading lib.

non-msvc mode should use the same lib-naming conventions as non-windows.

UebelAndre

Thanks! I take it you have a mingw toolchain you're developing with? Can you check if there's a unit test for this that also needs to be updated? I feel like there is but don't remember where off hand.

UebelAndre · 2025-12-19T14:49:52Z

      ; exit 1 \
      ; }
 split_coverage_postprocessing_shell_commands: &split_coverage_postprocessing_shell_commands
+  - echo "9.0.0rc3" > .bazelversion


I'd prefer not to pin this here if possible.

I'll revert this bit before landing; this was to debug the coverage duplicate actions that broke in bazel 8.5 and is another cause of red CI currently. bazelbuild/bazel#28056

dzbarsky · 2025-12-19T15:04:33Z

Thanks! I take it you have a mingw toolchain you're developing with? Can you check if there's a unit test for this that also needs to be updated? I feel like there is but don't remember where off hand.

Yes we added hermetic mingw support to https://github.com/cerisier/toolchains_llvm_bootstrapped

I don't think there's a unit test for this, everything is green when run locally.

Tests can surely be added but given how many fixes I need to do here to get things functional, I don't know if I have cycles for adding all of them, they're a bit annoying to setup

JSGette · 2026-04-24T10:14:10Z

Cross-posting an adjacent MinGW-visible gap we ran into, in case it's on your radar while you're touching this area. Not a blocker for this PR -- just looking for a place to put the observation where a MinGW user will see it.

In rust/private/rustc.bzl (construct_arguments, ~L1162), cc_toolchain's link_env is merged into the rustc action env only inside the link-emitting branch:

if ("link" in emit and crate_info.type not in ["rlib", "lib"]) or add_flags_for_binary:
    ld, ld_is_direct_driver, link_args, link_env = get_linker_and_args(...)
    ...
    env.update(link_env)

Rlibs are excluded. On Windows-GNU this is wrong: rustc invokes dlltool.exe during rlib compile for crates with #[link(name=..., kind="dylib")] (e.g. getrandom), so rlib actions need mingw64/bin on PATH just as much as link-emitting actions do. On MSVC/Linux this branch happens to be fine because rustc doesn't shell out to toolchain binaries at rlib compile time there.

In our setup (also a non-MSVC Windows toolchain) the two workarounds that end up being necessary together are:

Patch rustc_compile_action leaks default_shell_env into published CrateInfo.rustc_env, clobbering cc_toolchain link_env in downstream rust_test(crate = ...) #3989 / PR Don't leak default_shell_env into published CrateInfo.rustc_env #3990 to stop rustc_compile_action from leaking default_shell_env into published CrateInfo.rustc_env -- otherwise the polluted PATH inherited via rust_test(crate = :lib) clobbers cc_toolchain.link_env for link actions.
Pin -Clinker=<abs> and -Cdlltool=<abs> via extra_rustc_flags / extra_exec_rustc_flags so rlib actions don't need PATH for those binaries.

Dropping either one fails. A cleaner long-term fix would be lifting env.update(link_env) out of the link-only branch (or wiring cc_common.get_environment_variables(ACTION_NAMES.cpp_compile, ...) for non-link actions), so rlib compile actions inherit cc_toolchain env the same way link-emitting ones do.

Happy to split this off as its own issue / PR if you think it's worth addressing separately -- just didn't want to file it into the void while windows-gnu is still commented out of SUPPORTED_T1_PLATFORM_TRIPLES and #3794 closed without discussion.

…tch (#49862) ### What does this PR do? Removes the global `--action_env=PATH=...` / `--host_action_env=PATH=...` from the `bazel:test:windows-amd64` GitLab job, and replaces it with a scoped fix to `rules_rust` applied via `MODULE.bazel`. Concrete changes: - `.gitlab/build/bazel/test.yml`: drop the `--action_env=PATH` and `--host_action_env=PATH` overrides from the Windows test job. - `MODULE.bazel`: apply `bazel/patches/rules_rust-crateinfo-env-leak.patch` on top of the pinned `rules_rust` commit. - `bazel/patches/rules_rust-crateinfo-env-leak.patch`: stops `rustc_compile_action` (`rust/private/rustc.bzl`) from publishing `ctx.configuration.default_shell_env` into `CrateInfo.rustc_env`. Only `env_from_args` (the rustc-specific env) is persisted; Bazel re-injects `default_shell_env` at action execution anyway, so nothing is lost. - `.bazelrc`: leave commented-out `--@rules_rust//rust/settings:extra_rustc_env=PATH=...` / `extra_exec_rustc_env` hints near the other Windows-specific settings, as documentation for the fallback knob if a scoped PATH injection ever becomes necessary again. - `MODULE.bazel.lock`: regenerated to reflect the patched `rules_rust`. ### Motivation The previous `--action_env=PATH=C:/tools/msys64/mingw64/bin;...` was needed because `rustc` actions on Windows-GNU were ending up with a `PATH` that didn't include `mingw64/bin` — linking of `rust_test(crate = ...)` targets failed with `collect2.exe` unable to locate MinGW runtime DLLs. Root cause (debugged in detail — see bazelbuild/rules_rust#3989): at the tail of `rustc_compile_action`, `CrateInfo` was re-created with `rustc_env = env`, where `env = dict(default_shell_env) + env_from_args`. That baked host environment variables (including a Windows-native `PATH` without `mingw64/bin`) into the provider. Downstream `rust_test(crate = :lib)` inherited the polluted `rustc_env`, and in the test's own rustc action it overwrote the correct `PATH` that `cc_toolchain.link_env` had just supplied via `construct_arguments` — silently clobbering the C++ toolchain's env contract. `--action_env=PATH=...` worked around this by making `default_shell_env`'s `PATH` happen to contain `mingw64/bin`, but: - It invalidates the global action cache any time the value changes (or when adding/removing any env var pinned alongside it). - It leaks a host-specific path into every action's environment, not just Rust's. - It masks the underlying `rules_rust` bug for everyone in the repo. The in-tree patch fixes the root cause (scoped to Rust actions only), and the existing `-Clinker=<abs>` / `-Cdlltool=<abs>` flags in `rust.repository_set` (already on `main`) keep rlib compile actions working without needing `mingw64/bin` on `PATH`. Upstream tracking: - Issue: bazelbuild/rules_rust#3989 - PR with the same fix: bazelbuild/rules_rust#3990 Once the fix lands and we bump `rules_rust`, `bazel/patches/rules_rust-crateinfo-env-leak.patch` and the `patches = [...]` entry in `MODULE.bazel` can be removed. ### Describe how you validated your changes - `bazel test //comp/core/log/rust/...` and `//pkg/procmgr/rust/...` pass on Windows without `--action_env=PATH`. Without the patch these fail at link time with `collect2.exe` errors, reproducing the pre-patch symptom. - `bazel:test:windows-amd64` GitLab job on this PR (pipeline [109481274](https://gitlab.ddbuild.io/DataDog/datadog-agent/-/jobs/1627555041)). - Linux and macOS are unaffected (the leak exists but has nothing to clobber — most `cc_toolchain`s don't set `env_set` on link actions). ### Additional Notes - No agent binary behavior changes — build-system-only. - The commented-out `extra_rustc_env` lines in `.bazelrc` are intentional: they document the scoped-PATH injection path as a fallback knob in case anyone hits a similar issue again before we can drop the patch. - Related upstream: comment on bazelbuild/rules_rust#3785 noting an adjacent gap (rlib compile actions don't receive `cc_toolchain.link_env` at all — currently worked around by the `-Clinker`/`-Cdlltool` absolute paths). Co-authored-by: joseph.gette <joseph.gette@datadoghq.com>

dzbarsky · 2026-04-24T17:29:59Z

Cross-posting an adjacent MinGW-visible gap we ran into, in case it's on your radar while you're touching this area. Not a blocker for this PR -- just looking for a place to put the observation where a MinGW user will see it.

In rust/private/rustc.bzl (construct_arguments, ~L1162), cc_toolchain's link_env is merged into the rustc action env only inside the link-emitting branch:
if ("link" in emit and crate_info.type not in ["rlib", "lib"]) or add_flags_for_binary:
    ld, ld_is_direct_driver, link_args, link_env = get_linker_and_args(...)
    ...
    env.update(link_env)
Rlibs are excluded. On Windows-GNU this is wrong: rustc invokes dlltool.exe during rlib compile for crates with #[link(name=..., kind="dylib")] (e.g. getrandom), so rlib actions need mingw64/bin on PATH just as much as link-emitting actions do. On MSVC/Linux this branch happens to be fine because rustc doesn't shell out to toolchain binaries at rlib compile time there.

In our setup (also a non-MSVC Windows toolchain) the two workarounds that end up being necessary together are:
1. Patch [rustc_compile_action leaks default_shell_env into published CrateInfo.rustc_env, clobbering cc_toolchain link_env in downstream rust_test(crate = ...) #3989](https://github.com/bazelbuild/rules_rust/issues/3989) / PR [Don't leak default_shell_env into published CrateInfo.rustc_env #3990](https://github.com/bazelbuild/rules_rust/pull/3990) to stop `rustc_compile_action` from leaking `default_shell_env` into published `CrateInfo.rustc_env` -- otherwise the polluted `PATH` inherited via `rust_test(crate = :lib)` clobbers `cc_toolchain.link_env` for link actions.

2. Pin `-Clinker=<abs>` and `-Cdlltool=<abs>` via `extra_rustc_flags` / `extra_exec_rustc_flags` so rlib actions don't need `PATH` for those binaries.
Dropping either one fails. A cleaner long-term fix would be lifting env.update(link_env) out of the link-only branch (or wiring cc_common.get_environment_variables(ACTION_NAMES.cpp_compile, ...) for non-link actions), so rlib compile actions inherit cc_toolchain env the same way link-emitting ones do.

Happy to split this off as its own issue / PR if you think it's worth addressing separately -- just didn't want to file it into the void while windows-gnu is still commented out of SUPPORTED_T1_PLATFORM_TRIPLES and #3794 closed without discussion.

Thanks! There's actually quite a few mingw-related issues with rules_rust, https://github.com/hermeticbuild/rules_rs is the recommended approach for targeting mingw and is used in production to target that platform by OpenAI, IntelliJ, etc.

UebelAndre · 2026-04-24T17:52:14Z

I'd be happy to review changes to make mingw support better but until there's some common way for rules to identify msvc vs mingw (e.g. https://github.com/bazel-contrib/platforms_contrib/) then rules_rust doesn't have a way to distinguish between the two and therefore can't register toolchains by default.

If this change gets rebased I can do a review. Same for other mingw changes, as long as they're not adding constraints to rules_rust.

dzbarsky · 2026-04-25T19:02:04Z

Although rule_rust is not able to support it out of the box, rules_rs can and does. Also, rulesets should not be registering any toolchains by default. rules_rust doing that is exactly what leads to issues downstream and makes it extremely hard to configure toolchains in a non-default way. Anyway, rules_rust doesn't need to do anything differently if the maintainers are happy with the ergonomics, I'm just mentioning that folks who want to target platforms rules_rust is unable to support will have better luck with rules_rs, which does support them in an ergonomic way :)

UebelAndre · 2026-04-25T19:06:22Z

rules_rust doesn't need to do anything differently if the maintainers are happy with the ergonomics, I'm just mentioning that folks who want to target platforms rules_rust is unable to support will have better luck with rules_rs, which does support them in an ergonomic way :)

I would love for the rules to "just work" for folks. If there are improvements that can be made here we maintainers are always interested in making the repo better so PRs are welcome!

dzbarsky · 2026-04-25T19:09:45Z

rules_rust doesn't need to do anything differently if the maintainers are happy with the ergonomics, I'm just mentioning that folks who want to target platforms rules_rust is unable to support will have better luck with rules_rs, which does support them in an ergonomic way :)

I would love for the rules to "just work" for folks. If there are improvements that can be made here we maintainers are always interested in making the repo better so PRs are welcome!

As you're very aware, I've sent plenty of PRs that have been ignored for months, I think saying PRs are welcome is very aspirational but doesn't reflect the lived experience of contributors :) In this case, you said you are not interested in adding constraints to rules_rust, so you are unable to improve the ergonomics. That's your choice, I've made different choices in rules_rs. That's one of the great things about open source, people have choices!

UebelAndre · 2026-04-25T20:06:50Z

As you're very aware, I've sent plenty of PRs that have been ignored for months, I think saying PRs are welcome is very aspirational but doesn't reflect the lived experience of contributors :) In this case, you said you are not interested in adding constraints to rules_rust, so you are unable to improve the ergonomics. That's your choice, I've made different choices in rules_rs. That's one of the great things about open source, people have choices!

We have since made improvements to maintainer organization and will continue to improve and be more structured. While we are not accepting constraints here (as we feel they belong in https://github.com/bazelbuild/platforms or https://github.com/bazel-contrib/platforms_contrib), there are still plenty of things I'm sure we could improve. I apologize for past experiences and would invite anyone interested to continue engaging as we maintainers are interested in engaging with the community :)

Improve mingw link flag handling

ba92203

dzbarsky mentioned this pull request Dec 18, 2025

Action conflicts: 'foo.repo_mapping' is generated by these conflicting actions bazelbuild/bazel#28056

Closed

dzbarsky added 2 commits December 18, 2025 16:58

coverage

068d341

loadz

beaf021

UebelAndre requested changes Dec 19, 2025

View reviewed changes

JSGette mentioned this pull request Apr 24, 2026

Replace global --action_env=PATH on Windows with scoped rules_rust patch DataDog/datadog-agent#49862

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve mingw link flag handling#3785

Improve mingw link flag handling#3785
dzbarsky wants to merge 3 commits intobazelbuild:mainfrom
hermeticbuild:zbarsky/linkz

dzbarsky commented Dec 18, 2025

Uh oh!

UebelAndre left a comment

Uh oh!

UebelAndre Dec 19, 2025

Uh oh!

dzbarsky Dec 19, 2025

Uh oh!

dzbarsky commented Dec 19, 2025

Uh oh!

JSGette commented Apr 24, 2026

Uh oh!

dzbarsky commented Apr 24, 2026

Uh oh!

UebelAndre commented Apr 24, 2026

Uh oh!

dzbarsky commented Apr 25, 2026

Uh oh!

UebelAndre commented Apr 25, 2026

Uh oh!

dzbarsky commented Apr 25, 2026

Uh oh!

UebelAndre commented Apr 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

dzbarsky commented Dec 18, 2025

Uh oh!

UebelAndre left a comment

Choose a reason for hiding this comment

Uh oh!

UebelAndre Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

dzbarsky Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

dzbarsky commented Dec 19, 2025

Uh oh!

JSGette commented Apr 24, 2026

Uh oh!

dzbarsky commented Apr 24, 2026

Uh oh!

UebelAndre commented Apr 24, 2026

Uh oh!

dzbarsky commented Apr 25, 2026

Uh oh!

UebelAndre commented Apr 25, 2026

Uh oh!

dzbarsky commented Apr 25, 2026

Uh oh!

UebelAndre commented Apr 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants