Skip to content

[dotnet] [build] Fix remote linkage in SourceLink#17495

Merged
nvborisenko merged 1 commit into
SeleniumHQ:trunkfrom
nvborisenko:dotnet-sl-fix-link
May 18, 2026
Merged

[dotnet] [build] Fix remote linkage in SourceLink#17495
nvborisenko merged 1 commit into
SeleniumHQ:trunkfrom
nvborisenko:dotnet-sl-fix-link

Conversation

@nvborisenko
Copy link
Copy Markdown
Member

The link should use full 40-length commit hash.

Continuation of #17467

💥 What does this PR do?

Use full commit hash.

🔧 Implementation Notes

Exposed new parameter in build-info script.

🤖 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

🔄 Types of changes

  • Bug fix (backwards compatible)

@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Fix SourceLink remote linkage with full commit hash

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Use full 40-character commit hash for SourceLink remote linkage
• Add STABLE_GIT_REVISION_FULL parameter to build-info scripts
• Update sourcelink.bzl to reference full revision instead of short hash
• Ensure consistent commit hash format across bash and PowerShell scripts
Diagram
flowchart LR
  A["build-info scripts<br/>bash & PowerShell"] -- "Generate STABLE_GIT_REVISION_FULL<br/>with full commit hash" --> B["sourcelink.bzl"]
  B -- "Use full hash in<br/>SourceLink JSON" --> C["Remote repository links"]
Loading

Grey Divider

File Changes

1. dotnet/private/sourcelink.bzl 🐞 Bug fix +3/-3

Use full commit hash in SourceLink JSON

• Changed input from ctx.version_file to ctx.info_file
• Updated grep pattern from STABLE_GIT_REVISION to STABLE_GIT_REVISION_FULL
• Now uses full 40-character commit hash instead of short hash for SourceLink JSON generation

dotnet/private/sourcelink.bzl


2. scripts/build-info.sh ✨ Enhancement +2/-0

Add full commit hash output to bash script

• Added full_revision variable capturing full commit hash via git rev-parse HEAD
• Added new output line echoing STABLE_GIT_REVISION_FULL with full hash and dirty flag
• Maintains existing STABLE_GIT_REVISION output for backward compatibility

scripts/build-info.sh


3. scripts/build-info.ps1 ✨ Enhancement +2/-0

Add full commit hash output to PowerShell script

• Added $full_revision variable capturing full commit hash via git rev-parse HEAD
• Added new output line echoing STABLE_GIT_REVISION_FULL with full hash and dirty flag
• Maintains existing STABLE_GIT_REVISION output for backward compatibility

scripts/build-info.ps1


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 (2)

Grey Divider


Remediation recommended

1. build-info.ps1 ignores git failures 📘 Rule violation ☼ Reliability
Description
The new git rev-parse HEAD call used to populate STABLE_GIT_REVISION_FULL does not check exit
status (e.g., $LASTEXITCODE) or validate output, so missing/failed git can silently emit empty
or incorrect build metadata. This can make build outputs (including SourceLink URLs) less
deterministic and harder to troubleshoot.
Code

scripts/build-info.ps1[2]

+$full_revision = (git rev-parse HEAD)
Evidence
PR Compliance ID 12 requires scripts/build steps to be hardened and not silently proceed on
failures. The cited additions that set full_revision via git rev-parse HEAD do not include any
exit-code or output validation, meaning the scripts can continue even when git is unavailable or
the command fails, and still emit an incorrect/empty revision value instead of failing
deterministically.

scripts/build-info.ps1[1-10]
scripts/build-info.sh[1-12]
Best Practice: Learned patterns

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

## Issue description
`scripts/build-info.ps1` and `scripts/build-info.sh` run `git rev-parse HEAD` to compute `STABLE_GIT_REVISION_FULL` but do not fail fast or validate the command succeeded, so they may continue and emit invalid/empty revision values.

## Issue Context
This PR introduces `STABLE_GIT_REVISION_FULL`; if `git` is missing or `git rev-parse` fails, downstream consumers (including SourceLink URL generation) can receive incorrect and/or non-deterministic values, violating PR Compliance ID 12’s requirement to avoid hidden failures in scripts.

## Fix Focus Areas
- scripts/build-info.ps1[1-10]
- scripts/build-info.sh[1-12]

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


2. sourcelink.bzl whitespace-fragile parsing 📘 Rule violation ☼ Reliability
Description
The SourceLink generator in dotnet/private/sourcelink.bzl parses the workspace status line with
whitespace-fragile logic (e.g., cut -d' ' -f2 / strict grep), which can mis-parse when
separators are multiple spaces/tabs or the expected key is missing/renamed, yielding an empty
commit. It then silently falls back to COMMIT=HEAD, producing mutable (moving-target) SourceLink
URLs and potentially incorrect released PDB source mapping in some environments.
Code

dotnet/private/sourcelink.bzl[16]

+COMMIT=$(grep "^STABLE_GIT_REVISION_FULL " "{status}" | cut -d' ' -f2 | tr -d '*')
Evidence
PR Compliance ID 12 requires hardening scripts to avoid fragile, non-deterministic behavior, and the
current pipeline violates this by relying on exact workspace-status formatting (grepping for an
exact STABLE_GIT_REVISION_FULL line and/or splitting on a single-space delimiter via `cut -d' '
-f2`). When that lookup or parse yields no commit (due to differing whitespace, missing/different
key, or differing status file contents), the implementation explicitly substitutes HEAD, so the
resulting SourceLink JSON can reference .../raw/HEAD/... rather than an immutable commit. Since
the workspace status command is wired to scripts/build-info.* (configured via .bazelrc) and now
emits STABLE_GIT_REVISION_FULL, any mismatch in availability/formatting triggers the fallback and
leads to incorrect SourceLink URLs.

dotnet/private/sourcelink.bzl[15-18]
dotnet/private/sourcelink.bzl[10-26]
.bazelrc[78-85]
scripts/build-info.sh[3-12]
scripts/build-info.ps1[1-10]
.bazelrc[123-125]
Best Practice: Learned patterns

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 SourceLink JSON generation in `dotnet/private/sourcelink.bzl` extracts the commit using whitespace- and format-fragile parsing (strict key match and/or `cut -d' ' -f2`), and when the expected `STABLE_GIT_REVISION_FULL` entry isn’t found or is mis-parsed it silently falls back to `COMMIT=HEAD`. This can publish SourceLink metadata pointing at a mutable `.../raw/HEAD/...` URL instead of an immutable commit, making released PDB source mapping incorrect.

## Issue Context
PR Compliance ID 12 calls for hardened scripts to reduce fragility and non-deterministic behavior. The workspace status command is configured via `.bazelrc` and `scripts/build-info.{sh,ps1}` emits `STABLE_GIT_REVISION_FULL ...`, but the current extraction requires an exact key/spacing and treats a miss as success by substituting `HEAD`, so minor differences in whitespace (multiple spaces/tabs), key naming (e.g., `GIT_REVISION_FULL`), or which status file contains the value can trigger the fallback.

## Fix Focus Areas
- dotnet/private/sourcelink.bzl[10-26]

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


Grey Divider

Qodo Logo

Copy link
Copy Markdown
Member

@AutomatedTester AutomatedTester left a comment

Choose a reason for hiding this comment

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

LGTM

@nvborisenko nvborisenko merged commit d0601c9 into SeleniumHQ:trunk May 18, 2026
25 checks passed
@nvborisenko nvborisenko deleted the dotnet-sl-fix-link branch May 18, 2026 12:17
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 Compliance violation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants