build(Bazel): Added all code examples to Bazel config. - #727
Open
marvin-hansen wants to merge 15 commits into
Open
build(Bazel): Added all code examples to Bazel config. #727marvin-hansen wants to merge 15 commits into
marvin-hansen wants to merge 15 commits into
Conversation
…onfigBuilder
Add CfdConfigBuilder::qtt_march, ::compressible_march and ::duct, so all seven
owned configs start at one entry. Each case entry takes the case name.
BREAKING CHANGE:
- QttMarchConfigBuilder::new / CompressibleMarchConfigBuilder::new and both
Default impls are crate-private; .name(..) is removed and the name is now
required. The "qtt_march" / "compressible_march" default names are gone.
- DuctConfig::new is crate-private; use CfdConfigBuilder::duct. DuctConfig now
carries a case name, so a duct Report is labelled with it instead of the fixed
literal "duct_march".
- DecNs is no longer exported and DecNs::config is crate-private; use
CfdConfigBuilder::dec_ns.
Migration: replace `XConfigBuilder::<R>::new().name(n)` with
`CfdConfigBuilder::x::<R>(n)`, and `DuctConfig::new(profile, inlet, gamma, pb,
cells, stop)` with the fluent `CfdConfigBuilder::duct::<R>(name)` chain.
ReferenceScales, PlumeImprint and PlumeNozzle now hold private fields behind
validated constructors and getters; DuctInlet and DuctStop are absorbed into
DuctConfigBuilder::inlet / ::stop and removed.
PlumeNozzle::new enforces the Cordell-Braun jet-gamma envelope [1.2, 1.4] that
was previously only stated in prose, so an extrapolated plume boundary is
refused at construction rather than reported as a validated one.
BREAKING CHANGE: these types can no longer be built as struct literals.
Migration: ReferenceScales { t_ref, n_ref, u_ref } becomes
.reference(t_ref, n_ref, u_ref); DuctInlet/DuctStop become .inlet(p0, t0) and
.stop(max_steps, residual_tol); PlumeImprint and PlumeNozzle take positional
constructors returning Result.
builder for BlendedMapConfig, lift the cylinder harness
- BlendedMapConfig::builder() replaces the seven-argument positional
constructor and validates the fan geometry before any metric field is
assembled. BlendedMapConfig::new is crate-private. BREAKING.
- Add Seed::UniformXPerturbed (symmetry-breaking wake seed) and
Observe::drag_split() (pressure/friction contributions as separate series,
off by default, summing to the combined drag by construction).
- Mesh::cut_registry() is public: the crate's surface-force diagnostics each
take a &CutCellRegistry and a Mesh-configured consumer had no way to get one.
- dec_cylinder_verification is configured through CfdConfigBuilder::march and
samples on run_with, dropping its hand-built lattice, registry, manifold and
DecNsSolver plus the duplicate instantaneous_drag. St = 0.1710 and
C_d = 1.345 are unchanged, and the full 214-step trace is identical.
- The five solver-level harnesses that keep direct construction now declare the
exemption and its reason in their module docs.
Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
…onfig builder from the public API Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
Applies the change's deltas: new cfd-config-entry capability (entry set, no-bypass rule, private-field rule, config-vs-input boundary, harness exemption class) plus 6 modified requirements across fluiddynamics-dsl, qtt-flow, duct-march, compressible-flow-host, dec-ns-validation, body-fitted-qtt-coordinate and surface-force-diagnostic. Corrects two standing drifts in the process: qtt-flow required a CfdFlow::qtt_march entry and duct-march a CfdFlow::duct_march entry, both replaced in code by the unified CfdFlow::march + MarchDispatch. Follow-up: nozzle-operating-map and blackout-flow-dsl still reference those retired verbs; they sit outside this change's capability set. Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
…er refactor The CfdConfigBuilder entry and .reference(..) collapsed 5 lines out of shared/world.rs, so four blueprint citations pointed past their quoted code — shared/world.rs:127-129 showed plume_imprint/build() instead of the publish_constant loop. Shifted by the exact delta. Also drops QttBody from the public API: every field was pub(crate), it had no public constructor and no accessor returned one, so a consumer could name the type and do nothing with it. Known remaining drift, pre-existing: couple-multiphysics.mdx cites compressible_march_run.rs:441-444 for a block now at 466. Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
The 52 [[example]] targets that live inside library crates — the CFD
verification harnesses and studies, the BRCD verification harnesses, the HAFT
category-theory examples, and three smaller ones — had no Bazel targets. They
are now rust_binary rules in each crate's existing BUILD.bazel, below the
rust_library under a `# Example binaries` comment.
The five BRCD harnesses that read bundled RCAEval cases resolved their data
through env!("CARGO_MANIFEST_DIR"), which under Bazel names the rustc sandbox
and is gone by the time the binary runs. They now go through a manifest_dir()
helper in verification/brcd/common.rs, the module all six already share, so
the resolver stays out of the published src/.
check_examples now walks both declaration sites, examples/*/Cargo.toml and
*/Cargo.toml, and covers all 166 targets across 22 packages.
Adds deep_causality_quantum to format.sh, which had never been formatted; the
first run cleaned up blank-line drift in its tests/BUILD.bazel.
Verified: bazel build //... green, bazel test //... 1153/1153, cargo clippy
--workspace --examples --all-targets clean, make format clean. Under bazel
run, verification_sockshop reproduces the python rankings exactly and
mms_taylor_green_verification passes both manufactured-solution gates.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
Cloudflare's build image has no Rust toolchain, so cargo doc cannot run where the CFD site is built. Every workaround (commit 28 MB of generated HTML, move the deploy into CI, host rustdoc on a second origin) fights either rustdoc's own automation or the working Cloudflare auto-deploy, to substitute for a page a normal crate release produces for free. Resolution: publish the crate and link to docs.rs. Reverted at 3/35 tasks; no delta reached openspec/specs/. Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #727 +/- ##
==========================================
- Coverage 97.02% 97.02% -0.01%
==========================================
Files 1282 1283 +1
Lines 75088 75449 +361
==========================================
+ Hits 72857 73207 +350
- Misses 2231 2242 +11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
28 issues found across 976 files
Not reviewed (too large): cylinder_wake.csv (~2,002 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="openspec/specs/qtt-flow/spec.md">
<violation number="1" location="openspec/specs/qtt-flow/spec.md:15">
P3: The requirement that 'its `Default` implementation SHALL be crate-private' is not implementable in Rust — a trait impl cannot be made crate-private, so if `Default` is ever implemented, `QttMarchConfigBuilder::default()` is publicly callable and the 'does not compile' scenario breaks. Rephrase as 'the builder SHALL NOT implement `Default`' (which is what the current code already does), keeping the requirement consistent with the scenario.</violation>
</file>
<file name="thirdparty/crates/parking_lot-0.12.5/.github/workflows/rust.yml">
<violation number="1" location="thirdparty/crates/parking_lot-0.12.5/.github/workflows/rust.yml:1">
P2: This workflow is placed inside the vendored third-party crate directory (thirdparty/crates/parking_lot-0.12.5/.github/workflows/), where GitHub Actions will never discover or run it — only the root-level .github/workflows/ is scanned. It is also unrelated to the PR's Bazel change and its benchmark job would fail anyway (`cd benchmark` has no target since the vendored copy excludes benchmark/). Recommend removing it or moving it to the repo root, and re-scoping the PR to its intended Bazel config; as shipped, it is inert CI that misleadingly implies the vendored crate is being tested.</violation>
</file>
<file name="thirdparty/crates/errno-0.3.14/BUILD.bazel">
<violation number="1" location="thirdparty/crates/errno-0.3.14/BUILD.bazel:39">
P3: This per-platform `"default"` feature is a no-op: errno's only features are `default = ["std"]` and `std`, and `"std"` is already unconditionally present in the base `crate_features` list, so no platform's effective feature set changes. The ~100 lines of select arms add no behavior and will only mislead readers into thinking feature flags are being tuned per target; consider reverting to the plain `"std",` list.</violation>
</file>
<file name="thirdparty/crates/parking_lot-0.12.5/release-plz.toml">
<violation number="1" location="thirdparty/crates/parking_lot-0.12.5/release-plz.toml:1">
P3: This release-plz.toml is out of scope for a Bazel config PR and is effectively dead config: the Bazel build never reads it, the repo already has its own ./release-plz.toml at the root, and release-plz discovers its config only at the workspace/package root, not inside a vendored third-party crate directory. Recommend removing this file from the PR since it has no effect on the build and this directory is a fixed upstream snapshot the repo does not release.</violation>
</file>
<file name="deep_causality_cfd/src/types/flow_config/seed.rs">
<violation number="1" location="deep_causality_cfd/src/types/flow_config/seed.rs:89">
P1: A `sigma` of `NaN` bypasses this guard and poisons the seed/projection instead of returning the documented invalid-width error. Reject non-finite widths alongside non-positive ones.</violation>
</file>
<file name="thirdparty/crates/parking_lot_core-0.9.12/src/thread_parker/redox.rs">
<violation number="1" location="thirdparty/crates/parking_lot_core-0.9.12/src/thread_parker/redox.rs:25">
P2: Redox builds never use this parker: the earlier `cfg(unix)` arm selects `unix.rs`, leaving this new implementation dead. Place the `target_os = "redox"` arm before `cfg(unix)` so Redox uses its futex parker.</violation>
</file>
<file name="thirdparty/crates/redox_syscall-0.5.18/src/scheme/seek.rs">
<violation number="1" location="thirdparty/crates/redox_syscall-0.5.18/src/scheme/seek.rs:33">
P2: Invalid seek origins report `EOVERFLOW` instead of `EINVAL`, so scheme implementations using this helper cannot distinguish a malformed `whence` from an offset overflow. Return `EINVAL` directly for the default match arm.</violation>
<violation number="2" location="thirdparty/crates/redox_syscall-0.5.18/src/scheme/seek.rs:36">
P1: Custom agent: **Flag Security Vulnerabilities**
The `calc_seek_offset_isize` helper documents that the result is "guaranteed to be positive," but if `buf_len` is negative and `new_offset` is non-negative, the function returns `Ok(cmp::min(new_offset, buf_len))` — which can be negative. For example, with `SEEK_SET`, `pos=1`, and `buf_len=-5`, the function returns `Ok(-5)`, violating its own contract. Since this is public API in syscall code, callers relying on the positive-result guarantee may end up with incorrect offsets that bypass bounds checks. Adding a `buf_len < 0` guard that returns `EINVAL`, or clamping `buf_len` to zero when negative, would fix the issue.</violation>
</file>
<file name="thirdparty/crates/redox_syscall-0.5.18/.gitlab-ci.yml">
<violation number="1" location="thirdparty/crates/redox_syscall-0.5.18/.gitlab-ci.yml:1">
P3: This `.gitlab-ci.yml` is dead configuration: it is the only GitLab CI file in the repo (the project has no root `.gitlab-ci.yml`), so nothing ever runs it. It also sits inside a `thirdparty/crates/redox_syscall-0.5.18/` vendored crate that is auto-generated by `crate_universe` (the `BUILD.bazel` carries a "DO NOT MODIFY" header), which is unrelated to the PR's Bazel-config purpose and may be discarded on regeneration. Suggest moving any CI config to the repo root or dropping this file.</violation>
</file>
<file name="thirdparty/crates/mio-1.2.2/src/net/uds/listener.rs">
<violation number="1" location="thirdparty/crates/mio-1.2.2/src/net/uds/listener.rs:44">
P2: On Solaris, an accept loop that drains the listener to `WouldBlock` will not re-arm its event-port registration, so later clients can arrive without another readable event. Wrap the backend accept in `IoSource::do_io`, as `TcpListener::accept` does.</violation>
</file>
<file name="thirdparty/crates/redox_syscall-0.5.18/src/scheme/generate.sh">
<violation number="1" location="thirdparty/crates/redox_syscall-0.5.18/src/scheme/generate.sh:3">
P2: Invoking this executable outside `src/scheme` silently generates empty files because paths are relative and `set -e` does not catch failed pipeline stages; resolve the script directory first and enable `pipefail`.</violation>
</file>
<file name="thirdparty/crates/redox_syscall-0.5.18/src/scheme/mod.rs">
<violation number="1" location="thirdparty/crates/redox_syscall-0.5.18/src/scheme/mod.rs:11">
P1: Safe scheme dispatch can invoke UB with a malformed `Packet`, because this dereferences unvalidated integer fields as a slice. Make dispatch unsafe with documented pointer-validity requirements, or validate/map packet memory before creating references.</violation>
</file>
<file name="examples/causal_discovery_examples/shared/paths.rs">
<violation number="1" location="examples/causal_discovery_examples/shared/paths.rs:11">
P3: PACKAGE_DIR is a hand-maintained magic string that must always match the crate's checked-in location under the workspace. If the package is ever relocated or the workspace layout changes, bazel runs silently resolve to a wrong/nonexistent data directory while cargo runs keep working, and manifest_dir() gives no signal that resolution failed. Consider deriving it or asserting the directory exists before returning.</violation>
</file>
<file name="thirdparty/crates/bytes-1.12.1/BUILD.bazel">
<violation number="1" location="thirdparty/crates/bytes-1.12.1/BUILD.bazel:38">
P2: This file is auto-generated (see the `@generated` / `DO NOT MODIFY` header; regenerated by `bazel run @@//thirdparty:crates_vendor`). `bytes` isn't listed in the `crates_vendor` packages in thirdparty/BUILD.bazel — it's a transitive dep whose `crate_features` are re-derived on the next regen, so this manual edit won't persist and won't reproduce. Also, bytes' default feature is `std` (default = ["std"]), already enabled, so the added `"default"` entry changes no behavior. If default features are actually needed, express them in the crates_vendor spec (e.g. add a `crate.spec` for bytes in thirdparty/BUILD.bazel) rather than editing the generated file.</violation>
</file>
<file name="deep_causality_cfd/src/types/flow_config/mesh.rs">
<violation number="1" location="deep_causality_cfd/src/types/flow_config/mesh.rs:188">
P2: The newly public `cut_registry` builds its registry from `CubicalReggeGeometry::uniform(self.spacing)`, ignoring any `Grading`, but `materialize` builds the run's registry from `self.base_geometry(&lattice)` (which applies the grading). Because `from_primitive` derives node coordinates from the geometry, a mesh with both a `Grading` and an immersed body yields two different registries, so the doc promise that a caller's diagnostics and the pipeline's observables describe the same body does not hold. Build the base from `self.base_geometry(&lattice)` here so the public registry matches what the run uses.</violation>
</file>
<file name="thirdparty/crates/parking_lot-0.12.5/bors.toml">
<violation number="1" location="thirdparty/crates/parking_lot-0.12.5/bors.toml:2">
P2: This bors.toml is placed inside a vendored third-party crate directory (thirdparty/crates/parking_lot-0.12.5/), so bors-ng will never read it (it only honors bors.toml at the repo root), and the whole directory is regenerated/overwritten on the next crate re-vendor. The referenced status checks 'build_tier_one' and 'build_other_platforms' also match no CI job in this repo (run_tests.yml uses 'build'), so they could never gate merges. Move any bors configuration to /bors.toml and reference real status names, or drop this file.</violation>
</file>
<file name="openspec/changes/reverted/add-cfd-api-docs/specs/documentation-code-parity/spec.md">
<violation number="1" location="openspec/changes/reverted/add-cfd-api-docs/specs/documentation-code-parity/spec.md:17">
P3: Consider adding a rule for classifying inline code before the check can be implemented consistently. As written, the requirement folds every inline-code span into 'a symbol that SHALL resolve or fail', yet the last scenario says filenames/CLI flags/field-name literals must not fail — without specifying how the verifier tells a symbol reference from a non-symbol identifier, the two normative statements conflict and an implementer must guess the classification heuristically (which risks either a noisy gate that gets disabled or one that lets real drift through).</violation>
</file>
<file name="README.md">
<violation number="1" location="README.md:323">
P3: The claim 'Every example is a Bazel binary' is not accurate: `example_ml_rca` in causal_discovery_examples is deliberately Cargo-only (listed in build/scripts/check_examples.sh's CARGO_ONLY and documented in its BUILD.bazel) and has no `bazel run` target. Consider qualifying the sentence, e.g. 'Every example except the deliberately Cargo-only `example_ml_rca` is a Bazel binary', so a reader doesn't assume a `bazel run` target exists for every example.</violation>
</file>
<file name="openspec/changes/reverted/add-cfd-api-docs/tasks.md">
<violation number="1" location="openspec/changes/reverted/add-cfd-api-docs/tasks.md:6">
P2: This tasks.md is a forward implementation plan added into the reverted change set for `add-cfd-api-docs`, but it contradicts the repo's current state: it marks tasks 1.1 and 1.2 as done `[x]` even though the sibling revert note (`openspec/changes/reverted/add-cfd-api-docs.md`, 'What was rolled back') says all phase-1 artifacts from those exact tasks were deleted. It also re-plans the cargo-doc-in-deploy / GitHub Actions deploy approach that the revert rejected because the Cloudflare build image has no Rust toolchain. As committed here, the checklist misleads a future reader into treating rolled-back work as complete and an already-falsified deployment premise as current. Either this file belongs in a re-opened proposal with the 1.1/1.2 boxes unchecked and the hosting decision resolved, or it should not be added to the reverted set; as a Bazel-scoped PR this looks like an accidental inclusion.</violation>
</file>
<file name="thirdparty/crates/scopeguard-1.2.0/examples/readme.rs">
<violation number="1" location="thirdparty/crates/scopeguard-1.2.0/examples/readme.rs:27">
P2: In this example, f() calls panic!() and is invoked first in main(), so the unwinding panic aborts the process and g() is never reached — the ScopeGuard file-write demo it exists to show never actually runs. Reorder so g() runs first (or drop the panic from the combined run) so the example exercises both paths.</violation>
</file>
<file name="thirdparty/crates/scopeguard-1.2.0/.github/workflows/ci.yaml">
<violation number="1" location="thirdparty/crates/scopeguard-1.2.0/.github/workflows/ci.yaml:1">
P2: This workflow is dead content: GitHub Actions only executes workflows from the repository-root .github/workflows/, and thirdparty/crates/ is an auto-generated crate_universe (crates_vendor) directory whose .cargo-checksum.json does not list this file, so it will be clobbered on the next regeneration. It is also unrelated to the PR's stated goal of adding examples to the Bazel config — please remove it rather than committing an upstream CI file into a generated vendored crate.</violation>
<violation number="2" location="thirdparty/crates/scopeguard-1.2.0/.github/workflows/ci.yaml:21">
P3: The workflow pins deprecated/outdated actions (actions/checkout@v2 and the archived actions-rs/toolchain@v1). Even setting aside that this file sits in a non-root directory and never runs, any future workflow should use maintained action versions (e.g. actions/checkout@v4 and dtolnay/rust-toolchain) for reliability and supply-chain safety.</violation>
</file>
<file name="thirdparty/crates/mio-1.2.2/examples/udp_server.rs">
<violation number="1" location="thirdparty/crates/mio-1.2.2/examples/udp_server.rs:61">
P2: The `?` on `send_to` inside the echo loop aborts the whole server if any single send returns `WouldBlock` (or another error). Since the socket is only registered for `READABLE` (not `WRITABLE`), that transient condition is possible and the example would die instead of breaking out and waiting for the next poll. Consider matching `send_to` errors like the `WouldBlock` arm in `recv_from`, e.g. break on `WouldBlock` and propagate only other errors, so a slow/bursty peer doesn't take the server down.</violation>
</file>
<file name="deep_causality_rand/BUILD.bazel">
<violation number="1" location="deep_causality_rand/BUILD.bazel:13">
P2: Enabling `os-random` unconditionally on the library forces `OsRandomRng` (and the `getrandom`/`libc` dependency) onto every Bazel consumer of `deep_causality_rand`, even ones that never opt in. In Cargo this feature is off by default (see Cargo.toml `os-random = ["getrandom"]`), and `rng()` returns the pure-Rust `ThreadRng` unless the feature is set — so this silently changes RNG behavior for consumers like `deep_causality_topology`, whose `os-random` feature forwards `deep_causality_rand/os-random` but whose BUILD has no `crate_features`, making the opt-out impossible under Bazel. Consider gating the feature at the consumer/example targets that actually need the Lund kernels (e.g. `deep_causality_physics`) rather than on the shared library, so standalone builds keep the documented default PRNG and no-libc build.</violation>
</file>
<file name="openspec/specs/fluiddynamics-dsl/spec.md">
<violation number="1" location="openspec/specs/fluiddynamics-dsl/spec.md:29">
P3: The blanket claim that every config is 'composed onto a caller-owned geometry' is now false for two of the seven families just added to the list. `qtt_march` and `duct` are geometry-free per the crate's own docs (QttMarchRun: 'no `.on(geometry)` stage'; duct: 'self-contained, no geometry stage'), so the requirement as written contradicts the implemented B1/borrow model. Suggest qualifying the sentence to say the march/uncertain families are composed onto a caller-owned geometry while `qtt_march` and `duct` are self-contained.</violation>
</file>
<file name="openspec/changes/reverted/add-cfd-api-docs.md">
<violation number="1" location="openspec/changes/reverted/add-cfd-api-docs.md:15">
P3: The documented premise does not match the crate manifest: neither deep_causality_cfd/Cargo.toml nor the workspace sets `publish = false`, so 'the crate is publish = false' is not accurate as stated, and the proposed remedy (lift `publish = false`, then docs.rs hosts it) is a no-op against the current config. The real reason docs.rs has no page is that the crate has no release on crates.io; suggest stating that instead of the publish flag.</violation>
<violation number="2" location="openspec/changes/reverted/add-cfd-api-docs.md:55">
P3: Internal inconsistency in the revert record: tasks.md marks 1.5 (nav entry) and 1.6 (Cargo.toml `documentation`) as not completed, and the deploy workflow has no task, yet the rolled-back list claims all of tasks 1.1–1.6 were reverted. A future reader can't tell what was actually implemented and rolled back from the count ('3 of 35') vs. the list (1.1–1.6). Align the list with the task checkboxes.</violation>
</file>
<file name="thirdparty/BUILD.bazel">
<violation number="1" location="thirdparty/BUILD.bazel:51">
P3: Encoding tokio as `features = ["full"]` vendors the entire tokio feature set and ~15+ transitive crates (mio, socket2, bytes, parking_lot, futures-core/util, ...) into the shared third-party registry just to support tokio_example, which only exercises `#[tokio::main]`, `tokio::spawn`, and `.await` — covered by `macros`, `rt`, and `rt-multi-thread`. Consider trimming the vendored feature set to what the example actually uses, which keeps the shared registry lean and reduces the surface vendored for every consumer of this repo.</violation>
</file>
Note: This PR contains a large number of files. cubic only reviews up to 200 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
Re-trigger cubic
| } => { | ||
| // The blob is stated in physical coordinates, so the vertex lattice indices are | ||
| // scaled by the per-axis spacing — the same convention `Observe::probe` uses. | ||
| if *sigma <= 0.0 { |
Contributor
There was a problem hiding this comment.
P1: A sigma of NaN bypasses this guard and poisons the seed/projection instead of returning the documented invalid-width error. Reject non-finite widths alongside non-positive ones.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At deep_causality_cfd/src/types/flow_config/seed.rs, line 89:
<comment>A `sigma` of `NaN` bypasses this guard and poisons the seed/projection instead of returning the documented invalid-width error. Reject non-finite widths alongside non-positive ones.</comment>
<file context>
@@ -54,6 +78,54 @@ impl Seed {
+ } => {
+ // The blob is stated in physical coordinates, so the vertex lattice indices are
+ // scaled by the per-axis spacing — the same convention `Observe::probe` uses.
+ if *sigma <= 0.0 {
+ return Err(PhysicsError::PhysicalInvariantBroken(format!(
+ "Seed::UniformXPerturbed requires a positive sigma, got {sigma}"
</file context>
Suggested change
| if *sigma <= 0.0 { | |
| if !sigma.is_finite() || *sigma <= 0.0 { |
| }; | ||
|
|
||
| unsafe fn str_from_raw_parts(ptr: *const u8, len: usize) -> Option<&'static str> { | ||
| let slice = slice::from_raw_parts(ptr, len); |
Contributor
There was a problem hiding this comment.
P1: Safe scheme dispatch can invoke UB with a malformed Packet, because this dereferences unvalidated integer fields as a slice. Make dispatch unsafe with documented pointer-validity requirements, or validate/map packet memory before creating references.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At thirdparty/crates/redox_syscall-0.5.18/src/scheme/mod.rs, line 11:
<comment>Safe scheme dispatch can invoke UB with a malformed `Packet`, because this dereferences unvalidated integer fields as a slice. Make dispatch unsafe with documented pointer-validity requirements, or validate/map packet memory before creating references.</comment>
<file context>
@@ -0,0 +1,73 @@
+};
+
+unsafe fn str_from_raw_parts(ptr: *const u8, len: usize) -> Option<&'static str> {
+ let slice = slice::from_raw_parts(ptr, len);
+ str::from_utf8(slice).ok()
+}
</file context>
| _ => None, | ||
| }; | ||
|
|
||
| match new_offset { |
Contributor
There was a problem hiding this comment.
P1: Custom agent: Flag Security Vulnerabilities
The calc_seek_offset_isize helper documents that the result is "guaranteed to be positive," but if buf_len is negative and new_offset is non-negative, the function returns Ok(cmp::min(new_offset, buf_len)) — which can be negative. For example, with SEEK_SET, pos=1, and buf_len=-5, the function returns Ok(-5), violating its own contract. Since this is public API in syscall code, callers relying on the positive-result guarantee may end up with incorrect offsets that bypass bounds checks. Adding a buf_len < 0 guard that returns EINVAL, or clamping buf_len to zero when negative, would fix the issue.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At thirdparty/crates/redox_syscall-0.5.18/src/scheme/seek.rs, line 36:
<comment>The `calc_seek_offset_isize` helper documents that the result is "guaranteed to be positive," but if `buf_len` is negative and `new_offset` is non-negative, the function returns `Ok(cmp::min(new_offset, buf_len))` — which can be negative. For example, with `SEEK_SET`, `pos=1`, and `buf_len=-5`, the function returns `Ok(-5)`, violating its own contract. Since this is public API in syscall code, callers relying on the positive-result guarantee may end up with incorrect offsets that bypass bounds checks. Adding a `buf_len < 0` guard that returns `EINVAL`, or clamping `buf_len` to zero when negative, would fix the issue.</comment>
<file context>
@@ -0,0 +1,41 @@
+ _ => None,
+ };
+
+ match new_offset {
+ Some(new_offset) if new_offset < 0 => Err(Error::new(EINVAL)),
+ Some(new_offset) => Ok(cmp::min(new_offset, buf_len)),
</file context>
| @@ -0,0 +1,68 @@ | |||
| name: Rust | |||
Contributor
There was a problem hiding this comment.
P2: This workflow is placed inside the vendored third-party crate directory (thirdparty/crates/parking_lot-0.12.5/.github/workflows/), where GitHub Actions will never discover or run it — only the root-level .github/workflows/ is scanned. It is also unrelated to the PR's Bazel change and its benchmark job would fail anyway (cd benchmark has no target since the vendored copy excludes benchmark/). Recommend removing it or moving it to the repo root, and re-scoping the PR to its intended Bazel config; as shipped, it is inert CI that misleadingly implies the vendored crate is being tested.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At thirdparty/crates/parking_lot-0.12.5/.github/workflows/rust.yml, line 1:
<comment>This workflow is placed inside the vendored third-party crate directory (thirdparty/crates/parking_lot-0.12.5/.github/workflows/), where GitHub Actions will never discover or run it — only the root-level .github/workflows/ is scanned. It is also unrelated to the PR's Bazel change and its benchmark job would fail anyway (`cd benchmark` has no target since the vendored copy excludes benchmark/). Recommend removing it or moving it to the repo root, and re-scoping the PR to its intended Bazel config; as shipped, it is inert CI that misleadingly implies the vendored crate is being tested.</comment>
<file context>
@@ -0,0 +1,68 @@
+name: Rust
+
+on:
</file context>
| const PARKED: i32 = 1; | ||
|
|
||
| // Helper type for putting a thread to sleep until some other thread wakes it up | ||
| pub struct ThreadParker { |
Contributor
There was a problem hiding this comment.
P2: Redox builds never use this parker: the earlier cfg(unix) arm selects unix.rs, leaving this new implementation dead. Place the target_os = "redox" arm before cfg(unix) so Redox uses its futex parker.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At thirdparty/crates/parking_lot_core-0.9.12/src/thread_parker/redox.rs, line 25:
<comment>Redox builds never use this parker: the earlier `cfg(unix)` arm selects `unix.rs`, leaving this new implementation dead. Place the `target_os = "redox"` arm before `cfg(unix)` so Redox uses its futex parker.</comment>
<file context>
@@ -0,0 +1,139 @@
+const PARKED: i32 = 1;
+
+// Helper type for putting a thread to sleep until some other thread wakes it up
+pub struct ThreadParker {
+ futex: AtomicI32,
+}
</file context>
| - nightly | ||
| - 1.20.0 # MSRV | ||
| steps: | ||
| - uses: actions/checkout@v2 |
Contributor
There was a problem hiding this comment.
P3: The workflow pins deprecated/outdated actions (actions/checkout@v2 and the archived actions-rs/toolchain@v1). Even setting aside that this file sits in a non-root directory and never runs, any future workflow should use maintained action versions (e.g. actions/checkout@v4 and dtolnay/rust-toolchain) for reliability and supply-chain safety.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At thirdparty/crates/scopeguard-1.2.0/.github/workflows/ci.yaml, line 21:
<comment>The workflow pins deprecated/outdated actions (actions/checkout@v2 and the archived actions-rs/toolchain@v1). Even setting aside that this file sits in a non-root directory and never runs, any future workflow should use maintained action versions (e.g. actions/checkout@v4 and dtolnay/rust-toolchain) for reliability and supply-chain safety.</comment>
<file context>
@@ -0,0 +1,49 @@
+ - nightly
+ - 1.20.0 # MSRV
+ steps:
+ - uses: actions/checkout@v2
+ - name: Install Rust
+ uses: actions-rs/toolchain@v1
</file context>
| those configs onto a caller-owned geometry and run them. Configuration objects SHALL hold no geometry | ||
| SHALL start each owned, validated configuration — the solver config (`dec_ns`) and every marching / | ||
| verification case container (`march`, `qtt_march`, `compressible_march`, `duct`, `verify`, | ||
| `uncertain_march`) — and the `CfdFlow` facade SHALL compose those configs onto a caller-owned |
Contributor
There was a problem hiding this comment.
P3: The blanket claim that every config is 'composed onto a caller-owned geometry' is now false for two of the seven families just added to the list. qtt_march and duct are geometry-free per the crate's own docs (QttMarchRun: 'no .on(geometry) stage'; duct: 'self-contained, no geometry stage'), so the requirement as written contradicts the implemented B1/borrow model. Suggest qualifying the sentence to say the march/uncertain families are composed onto a caller-owned geometry while qtt_march and duct are self-contained.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At openspec/specs/fluiddynamics-dsl/spec.md, line 29:
<comment>The blanket claim that every config is 'composed onto a caller-owned geometry' is now false for two of the seven families just added to the list. `qtt_march` and `duct` are geometry-free per the crate's own docs (QttMarchRun: 'no `.on(geometry)` stage'; duct: 'self-contained, no geometry stage'), so the requirement as written contradicts the implemented B1/borrow model. Suggest qualifying the sentence to say the march/uncertain families are composed onto a caller-owned geometry while `qtt_march` and `duct` are self-contained.</comment>
<file context>
@@ -24,16 +24,23 @@ downcasting to `f64`.
-those configs onto a caller-owned geometry and run them. Configuration objects SHALL hold no geometry
+SHALL start each owned, validated configuration — the solver config (`dec_ns`) and every marching /
+verification case container (`march`, `qtt_march`, `compressible_march`, `duct`, `verify`,
+`uncertain_march`) — and the `CfdFlow` facade SHALL compose those configs onto a caller-owned
+geometry and run them. The entry set SHALL be complete: no configuration family SHALL be reachable
+through a public constructor outside `CfdConfigBuilder`. Configuration objects SHALL hold no geometry
</file context>
|
|
||
| ## What was rolled back | ||
|
|
||
| Implementation (phase 1, tasks 1.1–1.6, all reverted): |
Contributor
There was a problem hiding this comment.
P3: Internal inconsistency in the revert record: tasks.md marks 1.5 (nav entry) and 1.6 (Cargo.toml documentation) as not completed, and the deploy workflow has no task, yet the rolled-back list claims all of tasks 1.1–1.6 were reverted. A future reader can't tell what was actually implemented and rolled back from the count ('3 of 35') vs. the list (1.1–1.6). Align the list with the task checkboxes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At openspec/changes/reverted/add-cfd-api-docs.md, line 55:
<comment>Internal inconsistency in the revert record: tasks.md marks 1.5 (nav entry) and 1.6 (Cargo.toml `documentation`) as not completed, and the deploy workflow has no task, yet the rolled-back list claims all of tasks 1.1–1.6 were reverted. A future reader can't tell what was actually implemented and rolled back from the count ('3 of 35') vs. the list (1.1–1.6). Align the list with the task checkboxes.</comment>
<file context>
@@ -0,0 +1,76 @@
+
+## What was rolled back
+
+Implementation (phase 1, tasks 1.1–1.6, all reverted):
+
+- `build/scripts/docs.sh` and the `make docs` target — deleted.
</file context>
|
|
||
| ## What the change set assumed | ||
|
|
||
| That `deep_causality_cfd`'s ~297 public names have no published API reference — true, and the reason |
Contributor
There was a problem hiding this comment.
P3: The documented premise does not match the crate manifest: neither deep_causality_cfd/Cargo.toml nor the workspace sets publish = false, so 'the crate is publish = false' is not accurate as stated, and the proposed remedy (lift publish = false, then docs.rs hosts it) is a no-op against the current config. The real reason docs.rs has no page is that the crate has no release on crates.io; suggest stating that instead of the publish flag.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At openspec/changes/reverted/add-cfd-api-docs.md, line 15:
<comment>The documented premise does not match the crate manifest: neither deep_causality_cfd/Cargo.toml nor the workspace sets `publish = false`, so 'the crate is publish = false' is not accurate as stated, and the proposed remedy (lift `publish = false`, then docs.rs hosts it) is a no-op against the current config. The real reason docs.rs has no page is that the crate has no release on crates.io; suggest stating that instead of the publish flag.</comment>
<file context>
@@ -0,0 +1,76 @@
+
+## What the change set assumed
+
+That `deep_causality_cfd`'s ~297 public names have no published API reference — true, and the reason
+is that the crate is `publish = false`, so docs.rs builds no page for it. The plan was a hybrid: ship
+generated rustdoc at `/api/` on the CFD site for complete signatures, plus a curated MDX guide in the
</file context>
Suggested change
| That `deep_causality_cfd`'s ~297 public names have no published API reference — true, and the reason | |
| That `deep_causality_cfd`'s ~297 public names have no published API reference — true, and the reason is that the crate has no release on crates.io, so docs.rs builds no page for it. |
| ), | ||
| "tokio": crate.spec( | ||
| default_features = True, | ||
| features = ["full"], |
Contributor
There was a problem hiding this comment.
P3: Encoding tokio as features = ["full"] vendors the entire tokio feature set and ~15+ transitive crates (mio, socket2, bytes, parking_lot, futures-core/util, ...) into the shared third-party registry just to support tokio_example, which only exercises #[tokio::main], tokio::spawn, and .await — covered by macros, rt, and rt-multi-thread. Consider trimming the vendored feature set to what the example actually uses, which keeps the shared registry lean and reduces the surface vendored for every consumer of this repo.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At thirdparty/BUILD.bazel, line 51:
<comment>Encoding tokio as `features = ["full"]` vendors the entire tokio feature set and ~15+ transitive crates (mio, socket2, bytes, parking_lot, futures-core/util, ...) into the shared third-party registry just to support tokio_example, which only exercises `#[tokio::main]`, `tokio::spawn`, and `.await` — covered by `macros`, `rt`, and `rt-multi-thread`. Consider trimming the vendored feature set to what the example actually uses, which keeps the shared registry lean and reduces the surface vendored for every consumer of this repo.</comment>
<file context>
@@ -39,6 +46,12 @@ crates_vendor(
),
+ "tokio": crate.spec(
+ default_features = True,
+ features = ["full"],
+ package = "tokio",
+ version = "1",
</file context>
Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
Make `bazel build/test --config=remote //...` work from an arm64 macOS host
against BuildBuddy's x86_64 Linux workers, and run the same setup on CI.
The workspace already reached the BuildBuddy RBE but every action failed:
Bazel selected the aarch64-apple-darwin rustc for exec-configuration actions
and shipped it to a Linux worker, which failed as
`rustc: Syntax error: ")" unexpected`. Fixing that exposed a missing C/C++
toolchain, which exposed a doctest link failure. All three are addressed here.
Hermetic LLVM toolchain
-----------------------
Adopt hermeticbuild/hermetic-llvm (`bazel_dep(name = "llvm")`) as the C/C++
toolchain for Linux exec platforms. rules_rust links every binary and test
through `@bazel_tools//tools/cpp:toolchain_type`, and with
`--incompatible_enable_cc_toolchain_resolution` that toolchain must come from
toolchain resolution; without one, every remote target failed analysis with
"No matching toolchains found". Only Linux exec platforms are registered, so
local macOS builds keep using the auto-detected Apple toolchain and are
unaffected.
Add the Linux target triples to the Rust toolchain so the same host can
cross-compile to the workers.
Platforms
---------
Linux platforms inherit `@llvm//platforms:linux_*` rather than restating their
constraints, picking up the two values hermetic-llvm keys its cc_toolchain on:
`constraints/libc:gnu.2.28` and `constraints/cxxstdlib:libcxx`. Inheriting keeps
them in step with the toolchain instead of drifting from it.
Drop the `@bazel_tools//tools/cpp:gcc` marker and the intermediate `:linux`
platform that carried it. That value is a marker on the cc_compiler constraint
setting, matched by BuildBuddy's `ubuntu_cc_toolchain` through
`exec_compatible_with`; with a clang toolchain it is dead and would mismatch.
Remote platforms pin `docker://ubuntu:22.04`, replacing a custom toolchain-
bearing image. Hermetic LLVM ships the compiler, headers, CRT objects and
unwinder from the CAS, so the worker needs no build tools — but it still
executes dynamically linked outputs, so its glibc must be at least the 2.28 the
toolchain targets. BuildBuddy's default executor image is Ubuntu 16.04 /
glibc 2.23, on which every produced binary dies at startup with
"libc.so.6: version `GLIBC_2.28' not found".
Remote configuration
--------------------
Pin `--host_platform`, `--platforms` and `--extra_execution_platforms` to the
remote platform. `--host_platform` is the one that mattered: left at the
auto-detected darwin host, exec-configuration actions (rules_rust's
process_wrapper and friends) were built for macOS and executed on Linux.
Move the remote endpoints out of unconditional `common` and under
`remote-shared`, so a plain `bazel build` no longer executes remotely with the
local platform.
Fix three latent rc bugs:
* `--extra_execution_platforms` is a non-repeatable list flag in Bazel 9 (its
help text, unlike `--extra_toolchains`, has no "may be used multiple
times"). Three separate lines collapsed to the last, silently selecting a
platform lacking the constraints the cc toolchain resolves on.
* `--remote_download_minimal` and `--remote_download_toplevel` both expand to
`--remote_download_outputs`, so setting both left the winner to rc ordering.
Keep toplevel only.
* `common:remote-shared--experimental_remote_cache_chunking` was missing a
space, defining a phantom config instead of enabling the flag.
Drop `--cpu=k8` (legacy under platform-based resolution, and it discarded the
analysis cache on every switch) and the `remote.buildbuddy.io` overrides that
contradicted the org endpoint.
Pin Lean actions to local execution. The toolchain in `@lake_deps` is downloaded
for the fetching host and its `toolchain()` declares no exec constraints, so
`//lean/...` would ship macOS binaries to a Linux worker — the same failure mode
as the rustc one, just not yet triggered. `no-remote-exec` keeps those actions
local while still allowing remote caching.
Patch rules_rust so doc tests link
----------------------------------
Every `rust_doc_test` failed remotely with `undefined symbol: _Unwind_Resume`
while all other tests passed.
`add_native_link_flags` emits the cc toolchain's `static_runtime_lib` search
path as `-Lnative=` unconditionally but gates the matching `-lstatic=` behind
`include_link_flags`, which rustdoc sets to False (rules_rust#2467).
rules_rust#4080 added a compensating `-Clink-arg=-l` loop for doc tests, but it
iterates only `dep_info.transitive_noncrates`, which never contains the
toolchain runtime libs: Bazel injects those into C++ link actions, and rustdoc
drives the doctest link itself.
Not a hermetic-LLVM bug — any cc toolchain enabling `static_link_cpp_runtimes`
hits it. A system GNU toolchain hides it because rustc's `-lgcc_s` resolves to a
real libgcc_s there, whereas hermetic-LLVM's is an intentionally empty stub
archive. Still present in rules_rust 0.73.0 (#3741 landed in 0.68.0, #4080 in
0.71.0) and unreported upstream.
The fix needs two edits. Emitting the `-l` alone fails with "unable to find
library": the runtime libs are built in a different configuration than the crate
outputs, so their root is absent from `rustdoc_test.bzl`'s `--strip_substring`
list and the `-Lnative=` path stays an execroot path that does not exist under
runfiles. So `rustdoc.bzl` emits the `-l` and returns the libs in its action
struct, and `rustdoc_test.bzl` adds their root to the strip list. They already
reach runfiles via `ctx.runfiles(transitive_files = action.inputs)`.
Applied through `single_version_override`, one patch file per source file:
Bazel's built-in patcher mis-attributes hunks across file boundaries in a
multi-file unified diff (it applied rustdoc.bzl's hunks to rustdoc_test.bzl)
even though GNU patch accepts the same diff, and `single_version_override` has
no `patch_tool` option. Drop the patch once an upstream fix lands; it fails
loudly at fetch time if it stops applying.
CI
--
Add buildbuddy.yaml running build and test on pull requests to main, and a
`--config=ci` profile. It shares the execution setup with `--config=remote` and
adds the ROLE/USER build metadata, which moved out of `remote-shared` so
developer machines no longer report themselves as CI runs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
…and doesnt even build on remote Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
Contributor
There was a problem hiding this comment.
4 issues found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="buildbuddy.yaml">
<violation number="1" location="buildbuddy.yaml:4">
P1: The trigger key `triggers.pr` may not match BuildBuddy CI's schema, which uses `pull_request` for PR event triggers. If `pr` is unrecognized, this BuildBuddyCI action will never fire on pull requests, silently disabling CI for the new Bazel pipeline. Please confirm against the BuildBuddy CI config docs (https://buildbuddy.io/docs/ci-config) and use the documented `pull_request` key if `pr` is not accepted. Note: this is validated against external BuildBuddy behavior, so confirm before merging.</violation>
<violation number="2" location="buildbuddy.yaml:11">
P3: The two CI steps use different Bazel configs: the build step runs with `--config=ci` while the test step runs with `--config=remote`. Both configs expand to the same `--config=remote-shared` execution setup, but `ci` additionally sets `--build_metadata=ROLE=CI` and `--build_metadata=USER=gh`, which is exactly the mechanism the .bazelrc comment says exists "so the BuildBuddy dashboard can separate CI runs from developer machines." Because the test step uses `remote` instead of `ci`, all CI test executions will be classified as non-CI/local on the BuildBuddy dashboard, defeating the tagging intent introduced in this same PR. Recommend using `--config=ci` for both steps for consistent CI attribution.</violation>
</file>
<file name="thirdparty/patches/rules_rust_doctest_runtime_libs_rustdoc.patch">
<violation number="1" location="thirdparty/patches/rules_rust_doctest_runtime_libs_rustdoc.patch:43">
P3: The field/list named `static_runtime_libs` actually holds *dynamic* runtime libraries whenever the crate is a `dylib`/`cdylib`, because the `if crate_info.type in ["dylib", "cdylib"]` branch calls `cc_toolchain.dynamic_runtime_lib(...)` and its results are appended into this same list. The name is misleading for a large subset of crates and could mislead future maintenance (e.g. someone relying on the name to skip stripping). Consider naming it `cc_runtime_libs` (or `toolchain_runtime_libs`) so the name matches what's actually stored across both crate-type branches.</violation>
<violation number="2" location="thirdparty/patches/rules_rust_doctest_runtime_libs_rustdoc.patch:45">
P3: This added block (the `-l` prefix + `-Clink-arg=%s` emission) is byte-for-byte identical to the pre-existing upstream block that immediately follows it in the same file and is now reused as the body of the new `for lib in runtime_libs.to_list():` loop. After the patch there are two copies of the same 6-line flag-emission logic (one for the `transitive_noncrates` static libs, one for the cc_toolchain runtime libs), which will drift independently. Since the two loops only differ in which lib list they iterate, consider extracting a small local helper (e.g. `def _add_lib_link_flag(rustdoc_flags, arg, for_windows)`) and calling it from both sites to keep the behavior on Windows/PIC consistent and maintainable against upstream upgrades.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| actions: | ||
| - name: "BazelCI" | ||
| triggers: | ||
| pr: |
Contributor
There was a problem hiding this comment.
P1: The trigger key triggers.pr may not match BuildBuddy CI's schema, which uses pull_request for PR event triggers. If pr is unrecognized, this BuildBuddyCI action will never fire on pull requests, silently disabling CI for the new Bazel pipeline. Please confirm against the BuildBuddy CI config docs (https://buildbuddy.io/docs/ci-config) and use the documented pull_request key if pr is not accepted. Note: this is validated against external BuildBuddy behavior, so confirm before merging.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At buildbuddy.yaml, line 4:
<comment>The trigger key `triggers.pr` may not match BuildBuddy CI's schema, which uses `pull_request` for PR event triggers. If `pr` is unrecognized, this BuildBuddyCI action will never fire on pull requests, silently disabling CI for the new Bazel pipeline. Please confirm against the BuildBuddy CI config docs (https://buildbuddy.io/docs/ci-config) and use the documented `pull_request` key if `pr` is not accepted. Note: this is validated against external BuildBuddy behavior, so confirm before merging.</comment>
<file context>
@@ -0,0 +1,11 @@
+actions:
+ - name: "BazelCI"
+ triggers:
+ pr:
+ branches:
+ - "main"
</file context>
| # Build all targets | ||
| - run: "bazel build --config=ci //..." | ||
| # Run all tests | ||
| - run: "bazel test --config=remote //..." |
Contributor
There was a problem hiding this comment.
P3: The two CI steps use different Bazel configs: the build step runs with --config=ci while the test step runs with --config=remote. Both configs expand to the same --config=remote-shared execution setup, but ci additionally sets --build_metadata=ROLE=CI and --build_metadata=USER=gh, which is exactly the mechanism the .bazelrc comment says exists "so the BuildBuddy dashboard can separate CI runs from developer machines." Because the test step uses remote instead of ci, all CI test executions will be classified as non-CI/local on the BuildBuddy dashboard, defeating the tagging intent introduced in this same PR. Recommend using --config=ci for both steps for consistent CI attribution.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At buildbuddy.yaml, line 11:
<comment>The two CI steps use different Bazel configs: the build step runs with `--config=ci` while the test step runs with `--config=remote`. Both configs expand to the same `--config=remote-shared` execution setup, but `ci` additionally sets `--build_metadata=ROLE=CI` and `--build_metadata=USER=gh`, which is exactly the mechanism the .bazelrc comment says exists "so the BuildBuddy dashboard can separate CI runs from developer machines." Because the test step uses `remote` instead of `ci`, all CI test executions will be classified as non-CI/local on the BuildBuddy dashboard, defeating the tagging intent introduced in this same PR. Recommend using `--config=ci` for both steps for consistent CI attribution.</comment>
<file context>
@@ -0,0 +1,11 @@
+ # Build all targets
+ - run: "bazel build --config=ci //..."
+ # Run all tests
+ - run: "bazel test --config=remote //..."
</file context>
| + else: | ||
| + runtime_libs = cc_toolchain.static_runtime_lib(feature_configuration = feature_configuration) | ||
| + for lib in runtime_libs.to_list(): | ||
| + static_runtime_libs.append(lib) |
Contributor
There was a problem hiding this comment.
P3: The field/list named static_runtime_libs actually holds dynamic runtime libraries whenever the crate is a dylib/cdylib, because the if crate_info.type in ["dylib", "cdylib"] branch calls cc_toolchain.dynamic_runtime_lib(...) and its results are appended into this same list. The name is misleading for a large subset of crates and could mislead future maintenance (e.g. someone relying on the name to skip stripping). Consider naming it cc_runtime_libs (or toolchain_runtime_libs) so the name matches what's actually stored across both crate-type branches.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At thirdparty/patches/rules_rust_doctest_runtime_libs_rustdoc.patch, line 43:
<comment>The field/list named `static_runtime_libs` actually holds *dynamic* runtime libraries whenever the crate is a `dylib`/`cdylib`, because the `if crate_info.type in ["dylib", "cdylib"]` branch calls `cc_toolchain.dynamic_runtime_lib(...)` and its results are appended into this same list. The name is misleading for a large subset of crates and could mislead future maintenance (e.g. someone relying on the name to skip stripping). Consider naming it `cc_runtime_libs` (or `toolchain_runtime_libs`) so the name matches what's actually stored across both crate-type branches.</comment>
<file context>
@@ -0,0 +1,55 @@
++ else:
++ runtime_libs = cc_toolchain.static_runtime_lib(feature_configuration = feature_configuration)
++ for lib in runtime_libs.to_list():
++ static_runtime_libs.append(lib)
++ arg = get_lib_name(lib)
+ if not for_windows:
</file context>
| + for lib in runtime_libs.to_list(): | ||
| + static_runtime_libs.append(lib) | ||
| + arg = get_lib_name(lib) | ||
| if not for_windows: |
Contributor
There was a problem hiding this comment.
P3: This added block (the -l prefix + -Clink-arg=%s emission) is byte-for-byte identical to the pre-existing upstream block that immediately follows it in the same file and is now reused as the body of the new for lib in runtime_libs.to_list(): loop. After the patch there are two copies of the same 6-line flag-emission logic (one for the transitive_noncrates static libs, one for the cc_toolchain runtime libs), which will drift independently. Since the two loops only differ in which lib list they iterate, consider extracting a small local helper (e.g. def _add_lib_link_flag(rustdoc_flags, arg, for_windows)) and calling it from both sites to keep the behavior on Windows/PIC consistent and maintainable against upstream upgrades.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At thirdparty/patches/rules_rust_doctest_runtime_libs_rustdoc.patch, line 45:
<comment>This added block (the `-l` prefix + `-Clink-arg=%s` emission) is byte-for-byte identical to the pre-existing upstream block that immediately follows it in the same file and is now reused as the body of the new `for lib in runtime_libs.to_list():` loop. After the patch there are two copies of the same 6-line flag-emission logic (one for the `transitive_noncrates` static libs, one for the cc_toolchain runtime libs), which will drift independently. Since the two loops only differ in which lib list they iterate, consider extracting a small local helper (e.g. `def _add_lib_link_flag(rustdoc_flags, arg, for_windows)`) and calling it from both sites to keep the behavior on Windows/PIC consistent and maintainable against upstream upgrades.</comment>
<file context>
@@ -0,0 +1,55 @@
++ for lib in runtime_libs.to_list():
++ static_runtime_libs.append(lib)
++ arg = get_lib_name(lib)
+ if not for_windows:
+ arg = "-l" + arg
+ if type(rustdoc_flags) == "Args":
</file context>
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
1 participant
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.
Describe your changes
build(Bazel): Added all code examples to Bazel config.
Issue ticket number and link
Code checklist before requesting a review
For details on make, please see BUILD.md
Note: The CI runs all of the above and fixing things before they hit CI speeds
up the review and merge process. Thank you.
Summary by cubic
Adds Bazel targets for all code examples, unifies CFD config entry points via
CfdConfigBuilder, and enables remote build/test with BuildBuddy using a hermetic LLVM toolchain. Also removes thewebsitefrom Bazel to avoid ~3k extra targets and remote build failures.New Features
rust_binarytargets for all examples (inexamples/*and library crates); stable I/O paths viamanifest_dir();build/scripts/check_examples.shandmake check_examplesensure every Cargo[[example]]has a Bazel target.CfdConfigBuilder::{dec_ns,march,qtt_march,compressible_march,duct,verify}; newDuctConfigBuilder;BlendedMapConfig::builder();Observe::drag_split();Mesh::cut_registry()is public.--config=remotewith hermeticllvmC/C++ toolchain for Linux exec platforms, Linux platforms inheriting@llvm//platforms:*, and patchedrules_rustdoctest linking viaMODULE.bazeloverrides; CI wired viabuildbuddy.yamland--config=ci.README.md; coverage note and example-target rule inAGENTS.md; minor build scripts/format updates; removed unusedcylinder_wake.csv.websitefrom the build graph to shrink targets and fix remote builds.Migration
QttMarchConfigBuilder::<R>::new().name(n)→CfdConfigBuilder::qtt_march::<R>(n).CompressibleMarchConfigBuilder::<R>::new().name(n)→CfdConfigBuilder::compressible_march::<R>(n).DecNs::config()→CfdConfigBuilder::dec_ns().DuctConfig::new(...)→CfdConfigBuilder::duct::<R>(name)...build().ReferenceScales,PlumeImprint,PlumeNozzle→ use builders/constructors.manifest_dir()and can be run viabazel run //examples/<package>:<name>.Written for commit 6bfd485. Summary will update on new commits.