fix: patch pcre2-sys for reproducibility - #11090
Merged
basvandijk merged 3 commits intoAug 10, 2026
Merged
Conversation
Commit 9e29205 switched pcre2-sys to compiling the bundled PCRE2 C sources (PCRE2_SYS_STATIC=1). The crate's build.rs enumerates those sources with an unsorted std::fs::read_dir, so the member order of the produced libpcre2.a is the build machine's filesystem iteration order — stable on any one machine, but different across machines (ext4 iterates in name-hash order with a per-filesystem seed). lld records an STT_FILE symbol per archive member it loads, in load order, so the order leaked into //rs/ic_os/open_rootfs (the only consumer shipped in an IC-OS image, via partition_tools; all pcre2 code is GC'd, only the STT_FILE markers remain). Result: the GuestOS update image built by repro-check on a dev machine hashed differently from the CI artifact, with open_rootfs as the only differing file. CI's Build Determinism job did not catch this because (a) it was skipped on that commit (Bazel Test All's test step failed after the artifacts were already uploaded, and build-determinism has no `if:` so a failed need skips it) and (b) both compared builds run on the uniformly-provisioned dind fleet, which agrees with itself on readdir order; only a build outside the fleet exposes the difference. Fix: patch build.rs (bazel/pcre2-sys.patch) to collect and sort the sources before handing them to cc::Build, making the archive byte-identical regardless of the build machine. Verified: ar t of the rebuilt libpcre2.a is sorted, and bazel build //... --nobuild plus buildifier pass. Upstream PR: BurntSushi/rust-pcre2#58 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
nmattia
approved these changes
Aug 10, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a reproducibility issue in IC-OS GuestOS builds by making the pcre2-sys vendored C source enumeration deterministic when PCRE2_SYS_STATIC=1 is set (avoiding filesystem-dependent read_dir ordering leaking into produced archives and downstream binaries).
Changes:
- Add a Bazel-managed patch for
pcre2-sys 0.2.8to sort C source files before passing them tocc::Build. - Wire the patch into the Rust crate universe configuration (
crate.annotation) and the pinned crate lock metadata. - Repin
Cargo.Bazel.json.lockto include the patch information forpcre2-sys.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| Cargo.Bazel.json.lock | Repins crate metadata and records the pcre2-sys patch application details. |
| bazel/rust.MODULE.bazel | Adds a crate.annotation for pcre2-sys to apply the determinism patch under Bazel. |
| bazel/pcre2-sys.patch | Implements deterministic ordering by collecting and sorting vendored PCRE2 C sources in build.rs. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…inistic-source-order-2026-08-10
basvandijk
enabled auto-merge
August 10, 2026 14:43
basvandijk
deleted the
ai/pcre2-sys-deterministic-source-order-2026-08-10
branch
August 10, 2026 15:30
pietrodimarco-dfinity
added a commit
that referenced
this pull request
Aug 10, 2026
The cherry-pick of #11090 ("fix: patch pcre2-sys for reproducibility") carried over master's `Cargo.Bazel.json.lock`, whose `checksum` is a digest over the crate_universe config and splicing manifest — i.e. over `bazel/rust.MODULE.bazel` plus every workspace `Cargo.toml`. Those differ between master and this RC branch (master has since bumped `comparable` to the dfinity fork, added `axum-otel-metrics`, `duration-string`, `exitcode`, bumped `canbench-rs` and `ciborium`, …), so the digest is stale here and every Bazel invocation fails at analysis time: Error: Digests do not match: Current Digest("987c09bb853448361351d6b167b9e528c23cea8fdaa95ce28b65cc3a9114fbce") != Expected Digest("4991a9aa71efd82294bf1844c7f4e677fc0fdad5c5d280d67766a7edd824e45b") The current `lockfile` is out of date for 'crate_index'. Regenerated with `./bin/bazel-pin.sh`. The resolved crate set was already correct for this branch, so the only change is the recorded checksum, which now matches the digest CI computes. `bazel query @crate_index//:all` passes again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pietrodimarco-dfinity
added a commit
that referenced
this pull request
Aug 10, 2026
The cherry-pick of #11090 ("fix: patch pcre2-sys for reproducibility") carried over master's `Cargo.Bazel.json.lock`, whose `checksum` is a digest over the crate_universe config and splicing manifest — i.e. over `bazel/rust.MODULE.bazel` plus every workspace `Cargo.toml`. Those differ between master and this RC branch, which is 28 commits behind (master has since moved `comparable` to the dfinity fork, added `axum-otel-metrics`, `duration-string` and `exitcode`, bumped `canbench-rs` and `ciborium`, …), so the digest is stale here and every Bazel invocation fails at analysis time: Error: Digests do not match: Current Digest("987c09bb853448361351d6b167b9e528c23cea8fdaa95ce28b65cc3a9114fbce") != Expected Digest("4991a9aa71efd82294bf1844c7f4e677fc0fdad5c5d280d67766a7edd824e45b") The current `lockfile` is out of date for 'crate_index'. Regenerated with `./bin/bazel-pin.sh`. The resolved crate set was already correct for this branch, so the only change is the recorded checksum, which now matches the digest CI computes. `bazel query @crate_index//:all` and `bazel build --nobuild //publish/canisters:bundle` pass again. Same fix as the one applied to rc--2026-08-06_04-21-revert_dmt-fix-repro. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pull Bot
pushed a commit
to mikeyhodl/ic
that referenced
this pull request
Aug 17, 2026
…nity#11091) ## Problem `build-determinism` (which compares per-artifact digests between the remote-cache-enabled **Bazel Test All** build and the no-cache **Build IC** build) declared `needs: [build-ic, bazel-test-all]` with no `if:` — so any test failure in Bazel Test All skipped it. By the time tests run in that job, its build step has succeeded, its execution logs are uploaded (the `.github/actions/bazel` action uploads them `if: success() || failure()`), and on release builds the artifacts have **already been published to the CDN** by the "Upload artifacts" step. Net effect: an ordinary flaky/failing test disables the determinism gate for artifacts that still ship. That's exactly how `9e2920593e` and `f18ab1c843` reached the CDN with the check silently skipped (both master pushes had test failures; verified via the jobs API — `Build Determinism: skipped` on both runs). The pcre2-sys non-reproducibility introduced by `9e2920593e` (fixed in dfinity#11090) went unnoticed until `repro-check` failed on a dev machine. ## Fix ```yaml if: ${{ !cancelled() && needs.build-ic.result == 'success' }} ``` - Bazel Test All test failures no longer skip the comparison — the execlogs it needs exist even when that job fails. - `build-ic` must still have succeeded: it produces the no-cache side of the comparison, and it is skipped on `merge_group` runs — where the determinism job stays skipped exactly as before. - Cancelled runs stay skipped. Found while root-causing the GuestOS reproducibility regression (see dfinity#11090 for the full analysis). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
repro-check --guestosfails for every commit since 9e29205 (the zig cc → hermetic-llvm switch, #10991): the locally built GuestOSupdate-img.tar.zsthashes differently from the CI/CDN artifact (e.g. localfc824312…vs CDN2f31b0e5…at 9e29205), while being perfectly stable across rebuilds on the same machine.Root cause
9e29205 set
PCRE2_SYS_STATIC=1onpcre2-sys(to fix the host-library-Lleak class under the hermetic toolchain), which makes the crate's build script compile the bundled PCRE2 C sources. Thatbuild.rsenumerates the sources with an unsortedstd::fs::read_dir, so the member order of the producedlibpcre2.ais the build machine's filesystem iteration order — stable per machine, but different across machines (ext4 iterates in name-hash order with a per-filesystem seed chosen at mkfs time).The order leaks into
open_rootfs(the only pcre2 consumer shipped in an IC-OS image, viapartition_tools): lld records oneSTT_FILEsymbol per loaded archive member, in load order. All pcre2 code is GC'd (--gc-sections— nothing inopen_rootfscalls it), so the binaries differ only in the order of theirpcre2_*.cSTT_FILEsymtab entries and the corresponding.strtabbytes. Diffing the local vs CDN images file-by-file:/opt/ic/bin/open_rootfsis the only differing file in the entire rootfs (boot partition differences are the cascade: the initramfs embedsopen_rootfs,boot_argscarries the root's verity hash, andlaunch-measurements.jsonfollows).Why the "Build Determinism" job didn't catch it: (a) on 9e29205 (and its parent) it was skipped —
needs: [build-ic, bazel-test-all]with noif:, and Bazel Test All's test step failed after the artifacts had already been uploaded; (b) even when it runs (verified green on later master commits with the defect present), it compares two builds within the uniformly-provisioned dind fleet, which agrees with itself on readdir order — only a build outside the fleet exposes the difference.Fix
Patch the crate's
build.rsto collect and sort the sources before handing them tocc::Build(bazel/pcre2-sys.patch, wired viacrate.annotationlike the existingrustix/askamadeterminism patches, lockfile repinned via./bin/bazel-pin.sh). Same file set, deterministic order → the archive is byte-identical regardless of the build machine.Upstream PR: BurntSushi/rust-pcre2#58
Verification
patch -p1applies cleanly against pristine pcre2-sys 0.2.8;bazel build //... --nobuildandbazel run //:buildifierpass.bazel build --config=local //rs/ic_os/open_rootfs:open_rootfsin the dev container: the rebuiltlibpcre2.amember order is sorted (ar t | sort -c), i.e. no longer filesystem-dependent (before the patch it matched the machine's raw readdir order 1:1).repro-check --guestosagainst a CI build that includes this commit.🤖 Generated with Claude Code