Skip to content

[dotnet] [build] Support deterministic build output#17497

Merged
nvborisenko merged 2 commits into
SeleniumHQ:trunkfrom
nvborisenko:dotnet-build-determenistic
May 18, 2026
Merged

[dotnet] [build] Support deterministic build output#17497
nvborisenko merged 2 commits into
SeleniumHQ:trunkfrom
nvborisenko:dotnet-build-determenistic

Conversation

@nvborisenko
Copy link
Copy Markdown
Member

Currently builds are not deterministic, regardless default /deterministic+ compiler flag. This is because bazel sandbox directory path. In normal dotnet build (SDK 8+ if I am not wrong), SDK determines CI environment automatically and normalizes input paths to be some neutral, environment agnostic.

This PR mimics the behavior.

💥 What does this PR do?

Use /pathmap compiler option.

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s):
    • What was generated:
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

Should be default behavior in rules_dotnet.

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Support deterministic build output via pathmap normalization

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add /pathmap compiler option to normalize Bazel sandbox paths
• Enable byte-for-byte reproducible deterministic builds
• Map execution root to stable prefix for environment-agnostic compilation
Diagram
flowchart LR
  A["Bazel Execution Root Path"] -- "Extract and normalize" --> B["Stable Prefix Mapping"]
  B -- "Apply /pathmap option" --> C["Deterministic Build Output"]
  D["Compiler Options"] -- "Add pathmap" --> C
Loading

Grey Divider

File Changes

1. dotnet/private/sourcelink.bzl ✨ Enhancement +7/-1

Add pathmap compiler option for deterministic builds

• Extract Bazel execution root path and create stable /pathmap mapping
• Map sandbox path to neutral /_/ prefix for reproducible builds
• Add pathmap compiler option to the compiler options list alongside sourcelink
• Include explanatory comment about deterministic build requirements

dotnet/private/sourcelink.bzl


Grey Divider

Qodo Logo

@selenium-ci selenium-ci added C-dotnet .NET Bindings B-build Includes scripting, bazel and CI integrations labels May 18, 2026
@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 18, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Wrong exec_root mapping ✓ Resolved 🐞 Bug ≡ Correctness
Description
exec_root is derived from ctx.bin_dir.path using split("/bin/"), which can leave it pointing
at the output bin directory (or include /bin/ at the end) instead of the Bazel execution root, so
/pathmap may not match and builds remain non-deterministic. This also breaks when the path ends
with /bin (no trailing slash), since the split won’t remove the bin segment at all.
Code

dotnet/private/sourcelink.bzl[R36-40]

+    # Map the Bazel execution root to a stable prefix so that builds
+    # are byte-for-byte reproducible regardless of sandbox path.
+    exec_root = ctx.bin_dir.path.split("/bin/")[0] + "/"
+    pathmap = "/pathmap:{}=/_/".format(exec_root)
+
Evidence
The new logic computes exec_root from ctx.bin_dir.path via split("/bin/") and immediately uses
it in /pathmap, but repo code that needs a real execution root uses bazel info execution_root
(not ctx.bin_dir). The repo also documents Bazel output paths as bazel-out/.../bin/..., where
ctx.bin_dir.path commonly ends in /bin, making split("/bin/") ineffective and leaving /bin/
in the computed prefix.

dotnet/private/sourcelink.bzl[29-81]
dotnet/private/docfx.bzl[35-49]
dotnet/private/nuget_pack.bzl[19-22]
rb/support/rbs_collection_update.rb[33-38]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The rule claims to map the Bazel execution root, but computes `exec_root` from `ctx.bin_dir.path` using `split("/bin/")`. This is fragile (won’t strip when the string ends with `/bin`) and does not reliably represent the execution root, so the `/pathmap` prefix may not match the paths that end up embedded in PDBs.

## Issue Context
Other code in this repo that truly needs the execution root obtains it at runtime via `bazel info execution_root` and handles Windows path formatting explicitly.

## Fix Focus Areas
- dotnet/private/sourcelink.bzl[29-81]
- dotnet/private/docfx.bzl[35-49]

## Suggested direction
- Stop using `ctx.bin_dir.path.split("/bin/")`.
- Either:
 - Derive the execroot/workspace root by stripping from the `/bazel-out/` segment (as other repo code does), using a robust operation like `rsplit`/`rpartition` (and handle both `.../bin` and `.../bin/...` cases), **or**
 - Move execroot detection to execution time (similar to `dotnet/private/docfx.bzl` using `bazel info execution_root`) by introducing a small wrapper that expands the correct `/pathmap` argument before invoking the compiler.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. /pathmap change lacks tests 📘 Rule violation ☼ Reliability
Description
The PR changes compilation behavior by adding a new /pathmap compiler option for deterministic
outputs but does not add/update tests to lock in the new contract. This increases the risk of
regressions (e.g., option removed/altered later) without detection.
Code

dotnet/private/sourcelink.bzl[79]

+        compiler_options = ctx.attr.compiler_options + [pathmap, "/sourcelink:" + sourcelink_json.path],
Evidence
PR Compliance ID 4 requires relevant tests when production behavior changes. The diff adds
/pathmap computation and injects it into compiler_options, but no test changes are present in
the PR to validate this new behavior.

AGENTS.md
dotnet/private/sourcelink.bzl[36-40]
dotnet/private/sourcelink.bzl[79-79]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A behavior change was introduced by adding the `/pathmap` compiler option, but there are no accompanying tests to ensure the option remains present and effective.

## Issue Context
The deterministic build behavior depends on passing `/pathmap` consistently in the compile action. Without a test, future refactors to `AssemblyAction` wiring or `compiler_options` composition could silently drop or change this flag.

## Fix Focus Areas
- dotnet/private/sourcelink.bzl[36-40]
- dotnet/private/sourcelink.bzl[79-79]
- dotnet/private/BUILD.bazel[1-200]
- dotnet/private/sourcelink_test.bzl[1-200]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Windows separator mismatch ✓ Resolved 🐞 Bug ☼ Reliability
Description
The new /pathmap uses POSIX separators and is passed even when is_windows selects the .bat
wrapper, but other Windows rules in this repo explicitly convert / to \ before invoking tools.
If the compiler wrapper or compiler sees \-style paths, the /pathmap prefix won’t match, leaving
Windows builds non-deterministic.
Code

dotnet/private/sourcelink.bzl[R38-43]

+    exec_root = ctx.bin_dir.path.split("/bin/")[0] + "/"
+    pathmap = "/pathmap:{}=/_/".format(exec_root)
+
    return AssemblyAction(
        ctx.actions,
        ctx.executable._compiler_wrapper_bat if is_windows else ctx.executable._compiler_wrapper_sh,
Evidence
_compile_action selects a Windows .bat wrapper when is_windows is true, but the added
/pathmap is always constructed with forward slashes. Elsewhere, Windows-specific actions
explicitly replace / with \ before invoking cmd.exe or generating .bat scripts, showing
separator normalization is required for correctness on Windows.

dotnet/private/sourcelink.bzl[29-81]
dotnet/private/copy_files.bzl[13-31]
dotnet/private/docfx.bzl[18-24]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
On Windows, the rule still constructs `exec_root`/`pathmap` using forward slashes and the POSIX-only `split("/bin/")`, but the rest of the repo treats Windows tool invocations as requiring backslash paths. This can prevent `/pathmap` from applying on Windows.

## Issue Context
Existing Windows actions convert Bazel-provided paths (which often contain `/`) into `\` before invoking `cmd.exe` or writing `.bat` files.

## Fix Focus Areas
- dotnet/private/sourcelink.bzl[31-43]
- dotnet/private/sourcelink.bzl[78-81]
- dotnet/private/copy_files.bzl[13-31]
- dotnet/private/docfx.bzl[18-24]

## Suggested direction
- When `is_windows` is true, build a Windows-normalized `pathmap` (e.g., convert the computed prefix to `\` consistently) or ensure the wrapper uses consistent separator normalization for both the paths being mapped and the `/pathmap` prefix.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread dotnet/private/sourcelink.bzl Outdated
@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 18, 2026

Persistent review updated to latest commit 2a92102

@nvborisenko nvborisenko merged commit ddd6153 into SeleniumHQ:trunk May 18, 2026
20 checks passed
@nvborisenko nvborisenko deleted the dotnet-build-determenistic branch May 18, 2026 13:08
shs96c added a commit to shs96c/selenium that referenced this pull request May 19, 2026
* origin/trunk: (97 commits)
  [py] update python dependencies (SeleniumHQ#17490)
  [build] fix renovate reported issues with configuration
  [build] remove base-ref from renovate workflows it does not work for the use case I had for them
  [build] add renovate dependency workflow (SeleniumHQ#17504)
  [build] simplify commit-changes workflow (SeleniumHQ#17503)
  [build] clarify dependency pin and update tasks (SeleniumHQ#17463)
  [build] do not rerun or attempt to upload logs unless workflow failure is from the Bazel step
  [build] fix renovate ignore rules_python to v2 until upstream fixed
  [build] renovate ignore rules_python until upstream fixed
  [build] bump rules_closure version (SeleniumHQ#17500)
  [build] bump rules_jvm_external (SeleniumHQ#17501)
  [js] remove npm dependency by using bazel for everything (SeleniumHQ#17499)
  [build] bump ruby versions to latest patch releases (SeleniumHQ#17496)
  [dotnet] [build] Support deterministic build output (SeleniumHQ#17497)
  [build] remove renovate update requests pending work done in SeleniumHQ#17427 (SeleniumHQ#17498)
  [dotnet] [build] Fix remote linkage in SourceLink (SeleniumHQ#17495)
  [rust] update reqwest to 0.13 (SeleniumHQ#17488)
  [build] bump low-risk Bazel module dependencies (SeleniumHQ#17494)
  [dotnet] run format against slnx instead of looping csproj (SeleniumHQ#17483)
  [build] ignore renovate.json references in renovate recommendations
  ...

# Conflicts:
#	MODULE.bazel
#	rust/BUILD.bazel
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations C-dotnet .NET Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants