Skip to content

Replace host_windows condition with @platforms//os:windows#3909

Open
davido wants to merge 1 commit intobazel-contrib:5.xfrom
davido:fix/bazel8-host-conditions
Open

Replace host_windows condition with @platforms//os:windows#3909
davido wants to merge 1 commit intobazel-contrib:5.xfrom
davido:fix/bazel8-host-conditions

Conversation

@davido
Copy link
Copy Markdown

@davido davido commented Apr 5, 2026

Bazel 8 emits warnings for deprecated bazel_tools condition targets such as @bazel_tools//src/conditions:host_windows_x64_constraint.

rules_nodejs still uses @bazel_tools//src/conditions:host_windows in a few widely used rules (e.g. js_library, npm_link, pkg_npm), causing consumers to see large numbers of warnings from generated npm targets.

Replace these host Windows checks with @platforms//os:windows in the affected user-facing rules. This is a minimal change to eliminate the Bazel 8 warning flood without attempting a full migration of all bazel_tools condition usages.

Architecture-specific and toolchain-selection cases are intentionally left unchanged, as they require a more careful migration.

Fixes Bazel 8 warnings reported in #2440

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

No tests/docs needed: this change only replaces deprecated condition labels without changing behavior.

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

With Bazel 8, builds using rules_nodejs produce a large number of warnings due to usage of deprecated condition targets such as:

@bazel_tools//src/conditions:host_windows_x64_constraint

This happens because several widely used rules (js_library, npm_link, pkg_npm, etc.) still rely on legacy @bazel_tools//src/conditions:host_windows.

As a result, consumers see a flood of warnings during analysis, especially from generated npm targets.

Issue: #2440

What is the new behavior?

The affected host Windows checks are replaced with @platforms//os:windows in user-facing rules.

This removes the Bazel 8 deprecation warnings while preserving existing behavior.

The change is intentionally minimal and does not migrate all remaining bazel_tools//src/conditions usages (e.g. architecture-specific or toolchain-related cases).

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Although the 5.x line is no longer actively developed, this small fix significantly improves the user experience for consumers migrating to Bazel 8 by eliminating a large volume of warnings.

Having this included in a final 5.8.x patch release would make migration smoother until users can fully move to newer alternatives (e.g. rules_js).

Bazel 8 emits warnings for deprecated bazel_tools condition targets such
as @bazel_tools//src/conditions:host_windows_x64_constraint.

rules_nodejs still uses @bazel_tools//src/conditions:host_windows in a
few widely used rules (e.g. js_library, npm_link, pkg_npm), causing
consumers to see large numbers of warnings from generated npm targets.

Replace these host Windows checks with @platforms//os:windows in the
affected user-facing rules. This is a minimal change to eliminate the
Bazel 8 warning flood without attempting a full migration of all
bazel_tools condition usages.

Architecture-specific and toolchain-selection cases are intentionally
left unchanged, as they require a more careful migration.

Fixes Bazel 8 warnings reported in bazel-contrib#2440
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a66fc4faa3

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

srcs = srcs,
is_windows = select({
"@bazel_tools//src/conditions:host_windows": True,
"@platforms//os:windows": True,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep Windows copy mode tied to host, not target platform

This select now uses @platforms//os:windows, which follows the target platform, but _copy_to_bin_impl uses is_windows to choose copy_cmd vs copy_bash for the action runner; copy_cmd invokes cmd.exe (third_party/github.com/bazelbuild/bazel-skylib/rules/private/copy_file_private.bzl, copy_cmd). On a Linux/macOS host building Windows targets (for example with --platforms=@platforms//os:windows), this flips to cmd.exe even though execution is not on Windows, causing copy actions to fail. The previous host_windows condition avoided that by matching host OS.

Useful? React with 👍 / 👎.

@davido
Copy link
Copy Markdown
Author

davido commented Apr 5, 2026

The CI failure appears to come from the Bazel 5.0.0 test lane:

  @platforms//os:windows is not a valid select() condition

This change is specifically targeting Bazel 8, where legacy @bazel_tools//src/conditions:host_* targets are deprecated and produce a large number of analysis warnings.

Using @platforms//os:windows is the recommended replacement in newer Bazel versions and resolves the warning flood for users on Bazel 8.

Given that, this PR intentionally focuses on improving behavior with current Bazel versions rather than maintaining compatibility with discontinued Bazel 5 setups.

Please let me know if Bazel 5 compatibility is still a requirement for this repository; otherwise this change should be correct for newer Bazel releases.

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