Stop Windows MSVC D9025 duplicate flag warnings

[tameware]

Summary

• Remove /O2//Od from Windows DDS_CPPOPTS so Bazel compilation_mode owns optimisation (avoids cl D9025 overriding /Od with /O2).
• Patch rules_cc so MSVC default_cpp_std is /std:c++20 (not /std:c++17), and drop /std from Windows DDS_CPPOPTS so googletest and every Windows cc_* target get C++20 without a second /std (D9025) or a host --cxxopt=/std:... that would leak into wasm transitions.
• Point Windows CI at --config=opt and add //python:ci_windows_cppopts_test to lock the invariants in.

Test plan

• bazelisk test //python:ci_windows_cppopts_test //python:ci_bazelisk_test
• Confirm Windows CI is green (no D9025 spam; --config=opt build/test)
• Optional: local Windows bazelisk build --config=opt //examples:analyse_play_pbn and check compile lines lack D9025

[Copilot]

Pull request overview

This PR reduces Windows MSVC D9025 “overriding option” noise by ensuring Bazel’s compilation_mode is the single source of truth for /O2 vs /Od, and by preventing rules_cc from injecting a conflicting default C++ standard flag on Windows hosts. It also adds a CI guard test and updates Windows CI to explicitly use --config=opt.

Changes:

• Remove /O2 and /Od from Windows DDS_CPPOPTS so Bazel owns optimization flags via compilation_mode.
• Disable default_cpp_std on Windows hosts and set --host_cxxopt=/std:c++20 while keeping target /std:c++20 in DDS_CPPOPTS.
• Update Windows CI to build/test with --config=opt and add a Python test to lock these invariants.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
specs/build-system.md	Documents the Windows flag invariants (optimization via compilation_mode, std handling via -default_cpp_std + /std:c++20).
python/tests/ci_windows_cppopts_test.py	Adds a CI guard test to prevent reintroducing conflicting MSVC flags and to assert Windows CI uses --config=opt.
python/BUILD.bazel	Registers the new CI guard py_test and its data dependencies.
CPPVARIABLES.bzl	Removes Windows /O2 and /Od from DDS_CPPOPTS and clarifies the intended ownership.
BUILD.bazel	Adds a restricted filegroup (and exports) to make config inputs available to the CI guard test.
.github/workflows/ci_windows.yml	Updates Windows CI build/test commands to pass --config=opt.
.bazelrc	Adds build:windows --features=-default_cpp_std and build:windows --host_cxxopt=/std:c++20.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[zzcgumn]

Looks sensible to me.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (1)

MODULE.bazel:16

• PR description says to disable rules_cc default_cpp_std on Windows and keep /std:c++20 in DDS_CPPOPTS, but this change set does the opposite: it patches default_cpp_std to /std:c++20 and removes /std from DDS_CPPOPTS (and the new CI guard enforces that default_cpp_std stays enabled). Please update the PR description (or the implementation, if the description is the true intent) so reviewers/users aren’t misled about where the C++ standard is set on Windows.

# MSVC default_cpp_std is hardcoded to /std:c++17 in rules_cc. Raise it to
# /std:c++20 so googletest and every Windows cc_* compile match the project
# baseline without restating /std in DDS_CPPOPTS (D9025) or build:windows
# --cxxopt (which would leak into wasm transitions on Windows hosts).
single_version_override(
    module_name = "rules_cc",
    version = "0.2.18",
    patches = ["//:patches/rules_cc_msvc_default_cpp_std_cxx20.patch"],
    patch_strip = 1,
)

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (1)

python/tests/ci_windows_cppopts_test.py:133

• The guard hard-codes the rules_cc version (0.2.18) in the regex and failure message. This makes routine rules_cc upgrades fail the test even when the patch remains correctly applied, and it contradicts the build-system spec’s goal of keeping exact dependency versions confined to MODULE.bazel/MODULE.bazel.lock.

        match = re.search(
            r"single_version_override\(\s*"
            r'module_name\s*=\s*"rules_cc"\s*,\s*'
            r'version\s*=\s*"0\.2\.18"\s*,\s*'
            r"patches\s*=\s*\[\s*"

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (3)

python/tests/ci_windows_cppopts_test.py:140

• _rules_cc_override_version() hard-codes the argument order and exact structure of single_version_override(...). This makes the guard brittle to harmless refactors (reordering args, adding new fields, formatting changes). Consider matching by presence of the required fields instead of a fixed layout.

def _rules_cc_override_version(module_bazel: str) -> str:
    match = re.search(
        r"single_version_override\(\s*"
        r'module_name\s*=\s*"rules_cc"\s*,\s*'
        r'version\s*=\s*"([^"]+)"\s*,\s*'
        r"patches\s*=\s*\[\s*"
        r'"//:patches/rules_cc_msvc_default_cpp_std_cxx20\.patch"\s*,?\s*'
        r"\]\s*,\s*"
        r"patch_strip\s*=\s*1\s*,?\s*"
        r"\)",
        module_bazel,
        flags=re.DOTALL,
    )
    if match is None:
        raise AssertionError(
            "expected single_version_override(module_name=\"rules_cc\", "
            "version=..., patches=[//:patches/rules_cc_msvc_default_cpp_std_cxx20.patch], "
            "patch_strip=1)"
        )
    return match.group(1)

python/tests/ci_windows_cppopts_test.py:283

• The exports_files() guard regex requires an exact file ordering and formatting, so it may fail on harmless reformatting (e.g., reordering entries) even if visibility remains correctly restricted. Matching required entries via lookaheads would keep the invariant while reducing churn.

        match = re.search(
            r"exports_files\(\s*"
            r"\[\s*"
            r'"\.bazelrc",\s*'
            r'"CPPVARIABLES\.bzl",\s*'
            r'"MODULE\.bazel",\s*'
            r'"patches/rules_cc_msvc_default_cpp_std_cxx20\.patch",\s*'
            r"\]\s*,\s*"
            r"visibility\s*=\s*\[\s*\"//python:__pkg__\"\s*\]\s*,?\s*"
            r"\)",
            text,
            flags=re.DOTALL,
        )

python/tests/ci_windows_cppopts_test.py:118

• _rules_cc_bazel_dep_version() is very sensitive to MODULE.bazel formatting: it only matches when name comes before version and when there are no additional arguments. That can cause false failures if MODULE.bazel is reformatted or extended while remaining semantically correct.

This issue also appears in the following locations of the same file:

• line 121
• line 271

def _rules_cc_bazel_dep_version(module_bazel: str) -> str:
    match = re.search(
        r'bazel_dep\(\s*name\s*=\s*"rules_cc"\s*,\s*version\s*=\s*"([^"]+)"\s*\)',
        module_bazel,
    )
    if match is None:
        raise AssertionError('expected bazel_dep(name = "rules_cc", version = "...")')
    return match.group(1)

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (2)

python/tests/ci_windows_cppopts_test.py:69

• The optimization-flag guard only rejects "/Od" and "/O2", but other MSVC optimization level flags (e.g. "/O1", "/Ox") would also override Bazel’s compilation_mode defaults and can reintroduce D9025. Expanding the regex makes the guard match the stated invariant (no /O* level flags in DDS_CPPOPTS).

            self.assertNotRegex(
                block,
                r'"/O[d2]"',
                f"{label} must not set /Od or /O2; use compilation_mode instead",
            )

python/tests/ci_windows_cppopts_test.py:51

• _bazelisk_invocation_has_config_opt can be satisfied by a commented-out line (e.g. "# bazelisk build --config=opt ..."), because the regex searches anywhere in the file. Anchoring to non-comment line starts makes the CI guard harder to accidentally bypass while still supporting indented YAML and "run:" prefixes.

This issue also appears on line 65 of the same file.

def _bazelisk_invocation_has_config_opt(text: str, subcommand: str) -> bool:
    """True if any bazelisk <subcommand> invocation includes --config=opt.

    Flag order after the subcommand is not significant, and a YAML `run:`
    prefix on the same line is allowed.
    """
    pattern = rf"bazelisk\s+{re.escape(subcommand)}\b[^\n]*--config=opt\b"
    return re.search(pattern, text) is not None

[tameware]

Addressed the latest Copilot suppressed comments in adaf3ff:

• Expanded the Windows DDS_CPPOPTS optimisation guard from /O[d2] to any MSVC /O* level flag (/O1, /Ox, /Og, …) via _MSVC_OPT_LEVEL_COPT.
• _bazelisk_invocation_has_config_opt now skips full-line # comments (including indented YAML comments), so a commented-out --config=opt line cannot satisfy the CI guard.

bazelisk test //python:ci_windows_cppopts_test is green.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (1)

python/tests/ci_windows_cppopts_test.py:66

• _bazelisk_invocation_has_config_opt() can be fooled by inline # comments (valid in both YAML and PowerShell): a line like bazelisk build //... # --config=opt would satisfy the regex even though the flag is not passed. Since this test is meant to lock CI invariants, it should ignore trailing comments before matching.

def _bazelisk_invocation_has_config_opt(text: str, subcommand: str) -> bool:
    """True if any bazelisk <subcommand> invocation includes --config=opt.

    Flag order after the subcommand is not significant, and a YAML `run:`
    prefix on the same line is allowed. Full-line comments (leading `#`) are
    ignored so a commented-out invocation cannot satisfy the CI guard.
    """
    pattern = re.compile(
        rf"^[ \t]*(?:run:[ \t]+)?bazelisk\s+{re.escape(subcommand)}\b[^\n]*--config=opt\b",
        re.MULTILINE,
    )
    for line in text.splitlines():
        if line.lstrip().startswith("#"):
            continue
        if pattern.search(line):
            return True
    return False

[tameware]

Addressed the latest Copilot suppressed comment in $(git rev-parse --short HEAD):

• _bazelisk_invocation_has_config_opt now strips trailing # ... comments before matching, so bazelisk build //... # --config=opt no longer satisfies the guard (full-line comments were already ignored).

bazelisk test //python:ci_windows_cppopts_test is green.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.