bazel: bump bazel-orfs for single-writer 1_synth.sdc (fixes RBE)#10823
bazel: bump bazel-orfs for single-writer 1_synth.sdc (fixes RBE)#10823oharboe wants to merge 3 commits into
Conversation
1_synth.sdc had two writers in ORFS: yosys' synth.tcl copied the user's SDC_FILE into place, then synth_odb.tcl overwrote it in place with the OpenSTA-canonicalized version. cp preserves mode bits, so with read-only inputs (RBE) the copy is read-only and the write fails; locally it only accidentally worked. Supersedes The-OpenROAD-Project#10818's chmod +w workaround. - Bump BAZEL_ORFS_COMMIT to bazel-orfs#754, which fixes this and also stops touch-stubbing missing primary synth artifacts, so a silently-missing output fails the action instead of flowing downstream as an empty file (which the gcd idempotency tests would happily compare as "identical"). - Mirror the ORFS-side fix (ORFS#4333) as bazel-orfs-patches/0036, adapted to the pinned ORFS_COMMIT, plus the flow/BUILD.bazel exports_files(scripts/synth.tcl) newer bazel-orfs needs. Both drop at the next ORFS bump. - Mirror bazel-orfs's sed single_version_override (gnulib memchr.c vs glibc 2.41+); non-root overrides don't apply, same story as the existing scip tinycthread patch. No-op on older glibc (CI). bazelisk test //test/orfs/gcd/...: 9/9 pass, including all six gcd_idempotent_test_* stages. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the pinned bazel-orfs commit, adds a patch to ensure a single writer for 1_synth.sdc during synthesis, and introduces a workaround override for sed to fix compilation issues on glibc 2.41+ with clang. The review feedback correctly identifies that the sed -i command used in the sed override is not portable to macOS (BSD sed) and suggests a portable alternative using a backup extension.
|
@hzeller I'm running Ubuntu 26 so Claude relitigated the memchr.c incompatibility fix |
BCR's yosys-slang 0.0.0 source.json pins the GitHub tarball of povik/sv-elab@6760afa2; after the repo's rename GitHub regenerated that tarball (new bytes, top-level dir now sv-elab-<sha>), so any fresh fetch fails the integrity check (Check-Bazel-Lock). archive_override the same commit with the corrected integrity and strip_prefix; extracted content verified byte-identical to the upstream git tree. Drop once BCR publishes a fixed entry. Use sed -i.bak in the sed override's patch_cmds so BSD sed on macOS accepts it (review feedback). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
|
/gemini review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
bazel-orfs#754 squash-merged as 90ad965f (identical tree to the PR-head commit pinned before); pin the SHA that stays reachable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
|
/gemini review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
@codex review |
| archive_override( | ||
| module_name = "yosys-slang", |
There was a problem hiding this comment.
How is this related to the sdc? I think you have another PR for this change
There was a problem hiding this comment.
FYI, I also filed povik/sv-elab#362 to have stable hashes.
I will create a new updated BCR module.
There was a problem hiding this comment.
Created new BCR release for sv-elab: bazelbuild/bazel-central-registry#9571
There was a problem hiding this comment.
@hzeller I take it you will create a PR when it lands?
Claude added that slang noice to this PR because it failed locally, not because it was related to bumping bazel-orfs.
Your problem goes away if you create a PR with bazelisk run @bazel-orfs//:bump, though you may want to hold off on that until this strom of bazel bcr upgrades is done....
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
never mind, will be picked up by next bazel-orfs bump. |
3 participants
Root-cause replacement for #10818.
1_synth.sdchad two writers in ORFS: yosys'synth.tclcopied the user'sSDC_FILEinto place (a leftover from before SDC canonicalization existed), thensynth_odb.tcloverwrote it in place with the OpenSTA-canonicalized version.cppreserves mode bits, so with read-only inputs (RBE) the copy is read-only and the canonicalization write fails — locally it only accidentally worked.BAZEL_ORFS_COMMITto the head of bazel-orfs#754 (created ahead of its merge so this PR can bake; will be updated to the merged SHA if it differs). Besides carrying the ORFS fix, it stops bazel-orfs from touch-stubbing missing primary synth artifacts — a silently-missing output now fails the bazel action with "declared output was not created" instead of flowing downstream as an empty file, which the gcd idempotency tests would have compared as "identical".bazel/bazel-orfs-patches/0036, adapted to the pinnedORFS_COMMIT(nosynth_syn.tclthere) plus theflow/BUILD.bazelexports_files(scripts/synth.tcl)newer bazel-orfs needs. Both evaporate at the next ORFS bump.sedsingle_version_override(gnulibmemchr.cvs glibc 2.41+): non-root overrides don't apply when OpenROAD is the root module — same story as the existing scip tinycthread patch. No-op on the older-glibc CI runners.Testing
bazelisk test //test/orfs/gcd/...— 9/9 pass, including all sixgcd_idempotent_test_*stages.Supersedes #10818: no
chmod +wneeded since nothing overwrites a file copied from a read-only input anymore, and this class of bug now fails hard and early on all machines, not just RBE.🤖 Generated with Claude Code