Skip to content

[build] pin the rules_rs zlib bootstrap to an immutable snapshot - #17985

Merged
titusfortner merged 1 commit into
SeleniumHQ:trunkfrom
titusfortner:rules-rs-zlib-snapshot
Sep 5, 2026
Merged

titusfortner merged 1 commit into
SeleniumHQ:trunkfrom
titusfortner:rules-rs-zlib-snapshot

Conversation

@titusfortner

@titusfortner titusfortner commented Sep 5, 2026

Copy link
Copy Markdown
Member

🔗 Related Issues

Upstream: hermeticbuild/rules_rs#238

💥 What does this PR do?

  • Fixes failing tests on scheduled Rust runs caused by a link reference bug in the version of rules_rs we're using

🔧 Implementation Notes

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): Claude Code
    • What was generated: root-cause investigation and the override wiring
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

🔄 Types of changes

  • Bug fix (backwards compatible)

🤖 Generated with Claude Code

@selenium-ci selenium-ci added the B-build Includes scripting, bazel and CI integrations label Sep 5, 2026
@qodo-code-review

qodo-code-review Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Pin rules_rs zlib bootstrap to immutable Ubuntu snapshots

🐞 Bug fix ⚙️ Configuration changes 🕐 Less than 10 minutes

Grey Divider

AI Description

• Applies the upstream rules_rs fix without adopting a currently blocked dependency upgrade.
• Pins ARM64 and x86_64 zlib bootstrap artifacts to immutable Ubuntu snapshots.
• Restores reliable scheduled Rust builds while retaining existing artifact checksums.
Diagram

graph TD
  A["Bazel module"] -->|applies| B["Snapshot patch"] -->|modifies| C["rules_rs bootstrap"] -->|downloads zlib| D["Ubuntu snapshot"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Upgrade rules_rs to 0.0.108 or newer
  • ➕ Removes the local patch and maintenance burden
  • ➕ Consumes the zlib fix through an upstream release
  • ➖ Currently exposes another known upstream bug
  • ➖ Broadens the change beyond the failing zlib bootstrap
2. Mirror zlib artifacts internally
  • ➕ Provides direct control over artifact availability
  • ➕ Avoids reliance on Ubuntu hosting
  • ➖ Adds artifact hosting and maintenance overhead
  • ➖ Duplicates immutable packages already available from Ubuntu snapshots

Recommendation: Use the local upstream patch as a temporary, low-risk fix because it preserves rules_rs 0.0.96 and existing checksums while eliminating mutable URLs. Upgrade to rules_rs 0.0.108 or newer and remove the override once the separate upstream blocker is resolved.

Files changed (2) +27 / -0

Bug fix (1) +18 / -0
rules_rs_zlib_snapshot.patchRedirect zlib bootstrap downloads to immutable snapshots +18/-0

Redirect zlib bootstrap downloads to immutable snapshots

• Repoints ARM64 and x86_64 zlib package URLs from mutable Ubuntu archive locations to a timestamped snapshot. Package versions and SHA-256 checksums remain unchanged.

third_party/bazel/rules_rs_zlib_snapshot.patch

Other (1) +9 / -0
MODULE.bazelApply the temporary rules_rs zlib snapshot patch +9/-0

Apply the temporary rules_rs zlib snapshot patch

• Adds a single-version override that patches rules_rs 0.0.96 during Bazel module resolution. Documents that the override should be removed when upgrading to rules_rs 0.0.108 or newer.

MODULE.bazel

@qodo-code-review

qodo-code-review Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)

Grey Divider


Action required

1. Patch added under third_party 📘 Rule violation ⚙ Maintainability
Description
The PR adds rules_rs_zlib_snapshot.patch beneath third_party/, which the checklist explicitly
treats as read-only. Relocate the patch to a source-controlled directory outside third_party/ and
update the override label.
Code

third_party/bazel/rules_rs_zlib_snapshot.patch[1]

+diff --git a/rs/private/rustc_repository.bzl b/rs/private/rustc_repository.bzl
Evidence
PR Compliance ID 3 prohibits source changes under third_party/. The cited branch file is newly
added entirely within third_party/bazel/, directly meeting the rule's failure criterion.

AGENTS.md: Do Not Modify Third-Party or Generated Output Directories
third_party/bazel/rules_rs_zlib_snapshot.patch[1-20]

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 new `rules_rs_zlib_snapshot.patch` is stored under `third_party/`, despite the compliance requirement prohibiting modifications in that directory.

## Issue Context
Preserve the immutable Ubuntu snapshot override, but move the repository-maintained patch to an appropriate source directory outside `third_party/` and update its Bazel label.

## Fix Focus Areas
- third_party/bazel/rules_rs_zlib_snapshot.patch[1-20]
- MODULE.bazel[46-52]

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


Grey Divider

Context sources
✅ Web pages:
  +7 more
Review mode: ⚖️ Balanced: This localized build override changes dependency patching and immutable download URLs, so correctness and supply-chain behavior warrant a complete review.

Grey Divider

Tip of the day
💡 Did you know, you can keep summaries lean with Finding overflow, which tucks the rest behind 'View more'

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread third_party/bazel/rules_rs_zlib_snapshot.patch
@titusfortner
titusfortner marked this pull request as draft September 5, 2026 21:40
@titusfortner
titusfortner force-pushed the rules-rs-zlib-snapshot branch from fd85e2a to a946ac1 Compare September 5, 2026 21:58
@titusfortner
titusfortner marked this pull request as ready for review September 5, 2026 21:59
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit a946ac1

@titusfortner
titusfortner merged commit b0b4a7a into SeleniumHQ:trunk Sep 5, 2026
56 checks passed
This was referenced Sep 9, 2026
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants