build: add optional Rust and CXX bridge support - #7580
Conversation
Eventually, `rust/` will be hosting primarily subtrees, whose contents we cannot directly influence, so linting it makes little sense.
Co-authored-by: pasta <pasta@dashboost.org>
Needed for cross-compilation to work correctly
This can be removed alongside the whole minimal crate when we have actual crates to slot in.
We're using `depends` to provide us with the Rust compiler and `cxxbridge`, we can drop it from the CI container and save some space.
The conflict occurs due a difference in glibc version between the Guix environment and the targets compiled, this is only a problem for Linux targets so the issue doesn't extend to Windows
…ging Merging per-crate staticlibs with 'ar -x' into a flat directory silently drops object files once member basenames collide, which happens as soon as two crates share dependencies (measured: 373 of 374 members collide between two crates built from one workspace). Replace the merge with an umbrella staticlib crate, rust/dashrust, that depends on each component as an rlib: cargo deduplicates shared dependencies, applies LTO across the whole graph, and emits a single archive. Component crates keep their own cxx bridges and codegen; per-crate Makefile includes now contribute only cxxbridge artifacts. Optional components become cargo features on the umbrella crate, plumbed from configure via RUST_CRATE_FEATURES. Per-crate dist-hook recipes are replaced by an aggregate dist-hook so additional crates do not collide on automake's one-recipe-per-Makefile rule.
The dashpay/platform v4.1.0 workspace declares rust-version 1.92; rustc 1.85.1 refuses nine of its crates by name. Hashes regenerated with contrib/devtools/update-rust-hashes.py. cxx/cxxbridge stay at 1.0.192, which compiles unchanged under 1.92.
The offline cargo configuration was generated into the source tree, which pollutes srcdir and races when multiple out-of-tree builds for different hosts share one checkout. Cargo discovers .cargo/config.toml by walking up from its invocation directory, so a config under the build tree's root works for both in-tree and out-of-tree builds.
cargo vendor handles git dependencies, but consuming the vendor directory offline requires per-git-source replacement stanzas in .cargo/config.toml, which the build system previously did not generate (only crates.io was redirected). Derive the stanzas deterministically from Cargo.lock with contrib/devtools/cargo-vendor-git-sources.sh — output verified byte-identical to what cargo vendor itself prints — and append them when generating the offline config. Git dependencies are unavoidable for upcoming components: crates.io copies of the dashpay/platform crates are stale or name-squatted.
|
⛔ Blockers found — Opus deferred (commit eeb3b91) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e286842c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe change adds optional Rust support to the build system. It provisions Rust and standard-library packages, vendors Cargo dependencies, configures targets and linkers, and adds Linux Rust CI jobs. It introduces the Estimated code review effort: 4 (Complex) | ~60 minutes Mergeability Score: 🔵 Low · up to The optional Rust build currently lacks the big-endian PowerPC standard-library target mapping, so Rust-enabled builds for that platform would fail. This is a bounded cross-platform compatibility gap; the PR is mergeable with explicit owner awareness or follow-up. Sequence Diagram(s)sequenceDiagram
participant Configure
participant Depends
participant Cargo
participant Cxxbridge
participant CppTargets
Configure->>Depends: Configure Rust tools and targets
Depends->>Cargo: Vendor dependencies and install Rust targets
Cargo->>Cxxbridge: Build chirp and generate bridge artifacts
Cxxbridge->>CppTargets: Provide bridge sources and headers
Cargo->>CppTargets: Provide libdashrust.a
CppTargets->>CppTargets: Link Rust and CXX bridge libraries
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@depends/funcs.mk`:
- Line 88: Remove the tar -P option from every archive-extraction command at
depends/funcs.mk lines 88, 293-296, and 337-340, and depends/Makefile lines 216,
224, and 237. Preserve the existing checksum validation and all other extraction
options.
In `@depends/Makefile`:
- Around line 221-228: The $(host_prefix)/$(CRATE_REGISTRY) target must depend
on the workspace Cargo.lock so lockfile changes invalidate the vendored
registry. Update this target’s prerequisites using the existing workspace path,
and ensure the $(CRATE_ARCHIVE) path is regenerated or validated against that
same lockfile before extraction.
In `@rust/Makefile.chirp.include`:
- Around line 39-43: Guard the cp branches in both lib.cpp
(rust/Makefile.chirp.include lines 39-43) and lib.h (rust/Makefile.chirp.include
lines 47-51) recipes with a source/destination inequality check, so copying is
skipped when $(LIBRUST_CHIRP_GEN_SRCDIR) resolves to the build output path;
retain the existing CXXBRIDGE generation fallback.
In `@rust/Makefile.libs.include`:
- Around line 55-56: Update the $(LIBRUSTDEPS) rule prerequisites to include
$(CARGO_CONFIGURED), the root Cargo.toml, and Cargo.lock, ensuring configuration
is generated before Cargo runs and changes to workspace metadata rebuild
libdashrust.a.
In `@src/init.cpp`:
- Around line 1483-1485: Add a targeted C++ unit test for the ENABLE_RUST bridge
behavior, invoking chirp::chirp() and asserting its expected result. Place the
test with the existing C++ unit-test suite and ensure it is included in the
relevant test target; leave the startup logging in the init path unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d68cf288-94bf-4484-9351-5b69296956da
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockdepends/patches/native_cxxbridge/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (36)
.cargo/config.toml.offline.github/workflows/build.yml.gitignoreCargo.tomlMakefile.amci/dash/matrix.shci/test/00_setup_env_native_rust.shconfigure.accontrib/devtools/cargo-vendor-git-sources.shcontrib/devtools/update-native-cxxbridge.pycontrib/devtools/update-rust-hashes.pydepends/Makefiledepends/config.site.independs/funcs.mkdepends/packages/native_cxxbridge.mkdepends/packages/native_rust.mkdepends/packages/packages.mkdepends/packages/rust_stdlib.mkdepends/packages/rustcxx.mkdepends/patches/native_cxxbridge/cargo-config.tomldepends/patches/native_rust/fix-elf-interpreter.shrust-toolchain.tomlrust/Makefile.amrust/Makefile.chirp.includerust/Makefile.libs.includerust/chirp/Cargo.tomlrust/chirp/build.rsrust/chirp/src/lib.rsrust/dashrust/Cargo.tomlrust/dashrust/src/lib.rssrc/Makefile.amsrc/Makefile.bench.includesrc/Makefile.qt.includesrc/Makefile.qttest.includesrc/Makefile.test.includesrc/init.cpp
Potential PR merge conflictsThis is advisory only. It does not block CI, but it marks PRs that will likely need a rebase depending on merge order. If this PR merges firstThese open PRs will likely need a rebase:
If these PRs merge firstThis PR will likely need a rebase:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0afa9e49d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
|
@coderabbitai review Please review the current head ( 🤖 Posted autonomously by Codex on behalf of pasta. |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
depends/packages/rust_stdlib.mk (1)
40-58: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftDocument the GNU-linker/musl-target contract. The mapping is intentional to avoid Guix glibc version conflicts, but the build links musl Rust libraries through GNU host compilers. Document the static-CRT and C++ linking requirements, and add link coverage for each supported Linux target.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@depends/packages/rust_stdlib.mk` around lines 40 - 58, Update the rust_stdlib target mappings to document that GNU host compilers link against musl Rust libraries to avoid Guix glibc conflicts, including the required static CRT and C++ linking behavior. Add link-coverage checks for every supported Linux target defined by the target and hash variables, including ARMv7, AArch64, and i686 variants.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@depends/packages/rust_stdlib.mk`:
- Around line 40-58: Update the rust_stdlib target mappings to document that GNU
host compilers link against musl Rust libraries to avoid Guix glibc conflicts,
including the required static CRT and C++ linking behavior. Add link-coverage
checks for every supported Linux target defined by the target and hash
variables, including ARMv7, AArch64, and i686 variants.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 926c7495-9815-4ea2-80d8-5c7872b7c39e
📒 Files selected for processing (7)
.gitignoreconfigure.accontrib/devtools/update-rust-hashes.pydepends/Makefiledepends/packages/rust_stdlib.mkrust/Makefile.amrust/Makefile.common.include
🚧 Files skipped from review as they are similar to previous changes (4)
- .gitignore
- rust/Makefile.am
- rust/Makefile.common.include
- contrib/devtools/update-rust-hashes.py
|
I evaluated the remaining CodeRabbit nitpick against the scope of this foundational PR and am not expanding it into an all-target Linux link matrix:
I’m treating broader cross-target link coverage as follow-up portability work, not a blocker for the opt-in DashPay Rust/CXX foundation. 🤖 Posted autonomously by Codex on behalf of pasta. |
|
@coderabbitai review Please review the current head ( 🤖 Posted autonomously by Codex on behalf of pasta. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c94e2c6e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
git status --short
git show --no-ext-diff --format=fuller --stat 9c94e2c6e63c
git diff --no-ext-diff 9c94e2c6e63c^ 9c94e2c6e63c -- \
configure.ac \
depends/funcs.mk \
depends/packages/native_rust.mk \
depends/patches/native_rust/fix-elf-interpreter.sh \
depends/Makefile \
depends/packages/native_cxxbridge.mk
printf '\n--- Candidate files ---\n'
fd -a -t f '^(configure\.ac|funcs\.mk|native_rust\.mk|native_cxxbridge\.mk|fix-elf-interpreter\.sh)$' . \
| sort
printf '\n--- Relevant definitions and invocations ---\n'
rg -n -C 5 -P \
'CARGO.*VERSION|cargo.*version|RUSTUP_TOOLCHAIN|fix-elf-interpreter|int_.*(hash|stage)|_stage_cmds|_hash' \
configure.ac dependsLength of output: 73172 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Package path definitions ---'
sed -n '45,115p' depends/funcs.mk
sed -n '1,80p' depends/packages/native_cxxbridge.mk
sed -n '1,75p' depends/packages/native_rust.mk
printf '%s\n' '--- All interpreter-fix references and files ---'
rg -n -C 3 -F 'fix-elf-interpreter.sh' .
find depends/patches -type f -name 'fix-elf-interpreter.sh' -print
printf '%s\n' '--- Cargo detection and Rust version checks ---'
sed -n '115,165p' configure.ac
sed -n '1745,1810p' configure.ac
sed -n '1860,1930p' configure.ac
rg -n -C 3 -P '\$CARGO\s+--version|CARGO_REQUIRED_VERSION|AC_PATH_PROG\(\[CARGO\]' configure.ac
printf '%s\n' '--- Make database expansion for relevant package variables ---'
make -s -C depends -pn RUST=1 2>/dev/null \
| grep -E '^(native_(cxxbridge|rust)_(patch_dir|patches|recipe_hash|all_file_checksums)|PATCHES_PATH) ?[:=]' \
| sort || trueLength of output: 20640 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Depends patch staging rules ---'
rg -n -C 8 -P 'patch_dir|_patches|\.patches-|cp .*patch|for .*patch' depends/funcs.mk depends/Makefile
printf '%s\n' '--- Expanded native_cxxbridge commands and variables ---'
make -C depends -pn RUST=1 \
| grep -E '^(native_cxxbridge_(patch_dir|patches|preprocess_cmds|stage_cmds|recipe_hash|all_file_checksums)|base_build_dir|PATCHES_PATH) ?[:=]' \
| sed -n '1,120p'
printf '%s\n' '--- Cargo-check introduction and relevant history ---'
git log --no-ext-diff --format='%H %s' --all -- configure.ac | head -40
git log --no-ext-diff -S'CARGO_REQUIRED_VERSION' --format=fuller -- configure.ac
git log --no-ext-diff -S'rust_uses_depends' --format=fuller -- configure.ac
printf '%s\n' '--- Current toolchain selection references ---'
rg -n -C 6 -P '\b(CARGO|RUSTC|CXXBRIDGE)\s*=|depends_prefix|rust_uses_depends|RUSTUP_TOOLCHAIN' \
configure.ac depends/config.site.in rust dependsLength of output: 5275 🏁 Script executed: #!/bin/bash
set -u
printf '%s\n' '--- Cargo version-check introduction ---'
git log --no-ext-diff -S'CARGO_REQUIRED_VERSION' --format='%H%n%s%n%b%n' -- configure.ac || true
printf '%s\n' '--- Introducing diff, if present ---'
cargo_commit=$(git log --no-ext-diff -S'CARGO_REQUIRED_VERSION' --format='%H' -n 1 -- configure.ac || true)
if [ -n "${cargo_commit}" ]; then
git show --no-ext-diff --format=fuller --find-renames "$cargo_commit" -- configure.ac
fi
printf '%s\n' '--- All Cargo assignments and validation use ---'
rg -n -C 5 -P 'AC_PATH_PROG\(\[CARGO\]|\bCARGO=|CARGO_REQUIRED_VERSION|\$CARGO\s+--version|rust_uses_depends' \
configure.ac depends/config.site.in rust .cargo 2>/dev/null || true
printf '%s\n' '--- All helper declarations, copy operations, and executions ---'
rg -n -C 4 -P 'fix-elf-interpreter\.sh|foreach patch.*_patches|cp \$\(patch\)' \
depends || true
printf '%s\n' '--- Script static syntax check ---'
bash -n depends/patches/native_rust/fix-elf-interpreter.shLength of output: 11145
I found no issues in the requested areas.
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 684495e16d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
|
@coderabbitai review Please review the current head ( 🤖 Posted autonomously by Codex on behalf of pasta. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
head_commit="a0cee6845e2f"
base_commit="$(git rev-parse "${head_commit}~3")"
printf '%s\n' '--- Latest three commits ---'
git log --no-ext-diff --format='%H%n%s%n%b' "${base_commit}..${head_commit}"
printf '%s\n' '--- Changed files ---'
git diff --no-ext-diff --name-status "${base_commit}..${head_commit}"
printf '%s\n' '--- Diff for updater workflows and their data sources ---'
git diff --no-ext-diff --find-renames "${base_commit}..${head_commit}" -- \
contrib/devtools/update-rust-hashes.py \
contrib/devtools/update-native-cxxbridge.py \
depends/packages/native_rust.mk \
depends/packages/rust_stdlib.mk \
depends/packages/native_cxxbridge.mk \
depends/packages/rustcxx.mk \
depends/packages/packages.mk \
depends/Makefile \
rust-toolchain.toml \
Cargo.lock
printf '%s\n' '--- Updater script structure ---'
ast-grep outline contrib/devtools/update-rust-hashes.py --items all
ast-grep outline contrib/devtools/update-native-cxxbridge.py --items all
printf '%s\n' '--- Current recipe declarations and updater references ---'
rg -n -C 4 -P \
'RUST(_|_STDLIB_|CXXBRIDGE)|native_(rust|cxxbridge)|rustcxx|CROSS_TARGETS|NATIVE_TARGETS|update_(rust|native-cxxbridge)' \
depends contrib/devtools rust-toolchain.toml Cargo.toml Cargo.lock
printf '%s\n' '--- Python syntax check ---'
python3 -m py_compile \
contrib/devtools/update-rust-hashes.py \
contrib/devtools/update-native-cxxbridge.pyLength of output: 27631 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- cxxbridge updater ---'
nl -ba contrib/devtools/update-native-cxxbridge.py | sed -n '1,190p'
printf '%s\n' '--- Rust hash updater ---'
nl -ba contrib/devtools/update-rust-hashes.py | sed -n '1,190p'
printf '%s\n' '--- cxxbridge depends recipe and patch lockfile ---'
nl -ba depends/packages/native_cxxbridge.mk | sed -n '1,120p'
nl -ba depends/patches/native_cxxbridge/Cargo.lock | sed -n '1,220p'
printf '%s\n' '--- workspace manifests and cxx-related lock entries ---'
nl -ba Cargo.toml | sed -n '1,160p'
for path in rust/chirp/Cargo.toml rust/dashrust/Cargo.toml; do
printf '%s\n' "--- ${path} ---"
nl -ba "$path" | sed -n '1,140p'
done
rg -n -C 5 -P '^name = "(cxx|cxx-build|cxxbridge-cmd|cxxbridge-flags|cxxbridge-macro)"$|^version = "1\.0\.198"$' \
Cargo.lock depends/patches/native_cxxbridge/Cargo.lock
printf '%s\n' '--- Cargo lockfile copy and workspace-update history ---'
git log --no-ext-diff --format='%H %s' -S'cargo update' -- contrib/devtools/update-native-cxxbridge.py
git blame -L 112,150 -- contrib/devtools/update-native-cxxbridge.py
printf '%s\n' '--- Depends download stamp patterns ---'
find depends/sources/download-stamps -maxdepth 1 -type f -name '*native_cxxbridge*' -print -exec sh -c 'echo "--- $1"; cat "$1"' _ {} \; 2>/dev/null || true
rg -n -C 3 -P 'stamp_fetched|download-stamps|sha256.*file_name' depends/funcs.mk depends/MakefileLength of output: 1796 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- cxxbridge updater ---'
cat -n contrib/devtools/update-native-cxxbridge.py
printf '%s\n' '--- Rust hash updater ---'
cat -n contrib/devtools/update-rust-hashes.py
printf '%s\n' '--- cxxbridge depends recipe and patch lockfile ---'
cat -n depends/packages/native_cxxbridge.mk
sed -n '1,220p' depends/patches/native_cxxbridge/Cargo.lock
printf '%s\n' '--- workspace manifests and cxx-related lock entries ---'
cat -n Cargo.toml
for path in rust/chirp/Cargo.toml rust/dashrust/Cargo.toml; do
printf '%s\n' "--- ${path} ---"
cat -n "$path"
done
rg -n -C 5 -P '^name = "(cxx|cxx-build|cxxbridge-cmd|cxxbridge-flags|cxxbridge-macro)"$|^version = "1\.0\.198"$' \
Cargo.lock depends/patches/native_cxxbridge/Cargo.lock
printf '%s\n' '--- Cargo lockfile copy and workspace-update history ---'
git log --no-ext-diff --format='%H %s' -S'cargo update' -- contrib/devtools/update-native-cxxbridge.py
git blame -L 112,150 -- contrib/devtools/update-native-cxxbridge.py
printf '%s\n' '--- Depends download stamp patterns ---'
find depends/sources/download-stamps -maxdepth 1 -type f -name '*native_cxxbridge*' -print -exec sh -c 'echo "--- $1"; cat "$1"' _ {} \; 2>/dev/null || true
rg -n -C 3 -P 'stamp_fetched|download-stamps|sha256.*file_name' depends/funcs.mk depends/MakefileLength of output: 33777 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Rust recipe declarations ---'
sed -n '1,180p' depends/packages/native_rust.mk
sed -n '1,260p' depends/packages/rust_stdlib.mk
printf '%s\n' '--- Version pins used by each updater ---'
rg -n -C 2 -P \
'^\$\(package\)_version:=|^channel = "|^RUSTC_REQUIRED_VERSION="|^CXXBRIDGE_REQUIRED_VERSION="' \
depends/packages/native_rust.mk \
depends/packages/rust_stdlib.mk \
depends/packages/native_cxxbridge.mk \
rust-toolchain.toml \
configure.ac
printf '%s\n' '--- Independent target and declaration consistency check ---'
python3 - <<'PY'
import re
from pathlib import Path
rust_updater = Path("contrib/devtools/update-rust-hashes.py").read_text()
native_recipe = Path("depends/packages/native_rust.mk").read_text()
stdlib_recipe = Path("depends/packages/rust_stdlib.mk").read_text()
cross_block = re.search(r"CROSS_TARGETS = \[(.*?)\n\]", rust_updater, re.S).group(1)
cross_targets = re.findall(r'"([^"]+)"', cross_block)
native_block = re.search(r"NATIVE_TARGETS = \[(.*?)\n\]", rust_updater, re.S).group(1)
native_ids = dict(re.findall(r'\("([^"]+)", "([^"]+)"\)', native_block))
declared_native = set(re.findall(r'^\$\(package\)_sha256_hash_([a-z0-9_]+):=', native_recipe, re.M))
declared_stdlib = set(re.findall(r'^\$\(package\)_sha256_hash_([a-z0-9_-]+):=', stdlib_recipe, re.M))
print("native updater IDs:", sorted(native_ids.values()))
print("native recipe IDs:", sorted(declared_native))
print("native missing from updater:", sorted(declared_native - set(native_ids.values())))
print("native extra in updater:", sorted(set(native_ids.values()) - declared_native))
print("stdlib updater targets:", sorted(cross_targets))
print("stdlib recipe targets:", sorted(declared_stdlib))
print("stdlib missing from updater:", sorted(declared_stdlib - set(cross_targets)))
print("stdlib extra in updater:", sorted(set(cross_targets) - declared_stdlib))
PY
printf '%s\n' '--- Updater documentation, test, and CI references ---'
rg -n -C 3 -P \
'update-rust-hashes\.py|update-native-cxxbridge\.py|native_cxxbridge.*Cargo\.lock|rust-toolchain\.toml' \
.github ci test contrib depends \
-g '!depends/sources/**' 2>/dev/null || true
printf '%s\n' '--- Changed commit patch checks ---'
git show --no-ext-diff --check a0cee6845e2f
git diff --no-ext-diff --check a0cee6845e2f~3..a0cee6845e2fLength of output: 23489
I found no issues in the Rust and cxxbridge updater workflows.
|
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Codex/Sol only (Phase 2 disabled)
The current head fixes all five previously reported Rust/CXX build-path defects, and the bridge behavior test requested by CodeRabbit is present and wired into the Rust-enabled unit-test target. One non-blocking history-quality finding remains: the 55-commit series retains abandoned implementation paths and numerous corrective commits that should be consolidated before the history becomes permanent.
Source: Codex general reviewer backend gpt-5.6-sol; Codex dash-core-commit-history reviewer backend gpt-5.6-sol; final verifier backend gpt-5.6-sol. Orchestration-only: openclaw-agent/cliproxy/gpt-5.6-sol (not reviewer evidence).
Validated zero-blocker Codex/Sol precheck evidence was promoted to final because Phase 2 (Sonnet/Opus) is temporarily disabled. This is Codex/Sol-only final validation, not Codex + Sonnet/Opus coverage.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— dash-core-commit-history (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet/Opus: not run (Phase 2 disabled — temporary Codex/Sol-only final)
- Secondary pass: disabled (
temporary_phase2_sonnet_disable)
🟡 1 suggestion(s)
1 additional finding(s) omitted (not in diff).
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `<commit:6e28684>`:
- [SUGGESTION] <commit:6e28684>:1: Rewrite temporary detours and fixups into durable commits
The 55-commit series still records temporary paths that have no effect in the final tree. For example, `e189a09` adds CI-container Rust support that `8e74d07` removes, `fc23f36` adds broad Rust lint exclusions that `6e28684` reverses, and the Guix integration from `9c88a95` and `a1a681b` is later removed because Guix support is outside this PR. The series also contains roughly thirty follow-up corrections after `6e28684`, including several narrowly scoped repairs to build logic introduced earlier in the same unmerged stack. This makes future bisection traverse known-incomplete states and preserves review-process detours as project history. Drop the abandoned paths and fold corrective commits into the commits that introduced the affected logic, while retaining independently meaningful feature, hardening, test, and generated-update steps.
|
CI status on current head
This PR does not alter Qt/DBus code, so no unrelated suppression is being added here. The independent final review also reports no blockers for 🤖 Posted autonomously by Codex on behalf of pasta. |
The Rust stdlib/target provisioning covered Android, FreeBSD, ARMv7, i686, PowerPC and more, inherited from the Zcash-derived tooling and later ABI-correctness fixes, even though only x86-64 Linux is CI-covered and Guix Rust builds are disabled. Presence in the build system implies a support commitment we cannot honor, and untested mappings can bitrot or emit broken binaries. Trim CROSS_TARGETS, rust_stdlib.mk, native_rust.mk and RS_SET_TRIPLE to the hosts we actually validate: the narrowed Guix release set (x86_64/aarch64/riscv64 Linux, x86_64 Windows, both macOS) plus native development hosts. Any other host now fails --enable-rust explicitly instead of fetching a standard library we never test. Android triples are rejected explicitly since they would otherwise match the generic Linux arms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
RUST_MACOS_DEPLOYMENT_TARGET fell back to the host OS version (sw_vers) whenever OSX_MIN_VERSION was not set as a shell variable. The depends config.site passes the deployment target to C/C++ inside CXXFLAGS (-mmacos-version-min=14.0) without exporting OSX_MIN_VERSION, so C++ objects were built for macOS 14.0 while libdashrust.a claimed the host version (e.g. minos 26.5), producing binaries whose Rust code assumes a newer macOS than the binary advertises. Derive the Rust deployment target from the C++ compiler's effective target instead: honor OSX_MIN_VERSION and MACOSX_DEPLOYMENT_TARGET when set, otherwise probe __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ through $CXX $CXXFLAGS so version-min flags embedded in CXXFLAGS are respected. Verified: configure now reports 14.0 under the depends config.site and the built archive carries minos 14.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
dist-chirp copied the generated bridge sources before the version stamp, so in the extracted tarball the stamp was newer than the sources and the strictly-newer staleness check always failed, forcing cxxbridge regeneration and defeating the pre-generated dist sources. Copy the stamp first so the shipped sources compare newer than it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Pushed three commits addressing review feedback on target scope and a macOS correctness issue:
The PR description's target-support wording has been updated to match. Re Guix: it has not been exercised for the Rust lane on this branch — Guix/release Rust remains intentionally disabled in this foundation PR, and a flag-on Guix determinism run is tracked as a release gate in #7512. 🤖 Posted autonomously by Claude on behalf of pasta. |
|
Guix Automation has failed due to the HEAD commit not being signed by an authorized core-team member. Please rebase and sign or push a new empty signed commit to allow Guix build to happen. |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
The optional Rust/CXX integration has one blocking source-distribution defect: ustar timestamp precision defeats the strict freshness checks, so extracted tarballs regenerate the bridge instead of using the distributed artifacts. The bridge behavior test is present and correctly wired, while durable documentation for the new opt-in workflow is still needed.
Source: Codex general reviewer backend gpt-5.6-sol; final verifier backend gpt-5.6-sol. Orchestration-only: openclaw-agent/cliproxy/gpt-5.6-sol (not reviewer evidence).
Validated blockers were found in the Codex precheck. Opus is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 1 blocking | 🟡 1 suggestion(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `rust/Makefile.chirp.include`:
- [BLOCKING] rust/Makefile.chirp.include:52-64: Allow equal timestamps for distributed bridge artifacts
Copying the stamp before the generated files does not preserve their ordering in the source archive. Automake emits a ustar archive, whose mtimes have one-second precision; these three copies normally occur in the same second and therefore have identical timestamps after extraction. In an out-of-tree build, both strict `artifact -nt $(LIBRUST_CHIRP_GEN_SRCSTAMP)` checks fail and invoke `cxxbridge`, so the shipped bridge files are never accepted even though they have the matching version stamp. Accept an artifact when it is not older than the versioned stamp, while retaining the strict comparison against `lib.rs`.
In `configure.ac`:
- [SUGGESTION] configure.ac:153-163: Document the optional Rust build workflow
The repository has no durable documentation explaining how to use the new build path. Users currently must infer `RUST=1` for depends, the Rust/Cargo/cxxbridge version requirements, supported hosts, the default offline `RUST_VENDORED_SOURCES` requirement, and the purpose of `--enable-online-rust` from build scripts and configure diagnostics. Add a short build document covering both depends-backed and system-toolchain workflows and link it from the relevant build documentation.
| test $(LIBRUST_CHIRP_GEN_SRCDIR)/src/lib.cpp -nt $(LIBRUST_CHIRP_GEN_SRCSTAMP) && \ | ||
| test "$(LIBRUST_CHIRP_GEN_SRCDIR)" != "$(LIBRUST_CHIRP_GEN_DIR)"; then \ | ||
| cp $(LIBRUST_CHIRP_GEN_SRCDIR)/src/lib.cpp $@; \ | ||
| else \ | ||
| $(CXXBRIDGE) $< -o $@; \ | ||
| fi | ||
|
|
||
| $(LIBRUST_CHIRP_H): $(LIBRUST_CHIRP_SRCS) $(LIBRUST_CHIRP_GEN_STAMP) | ||
| $(AM_V_at)$(MKDIR_P) $(@D) | ||
| $(AM_V_GEN)if test -f $(LIBRUST_CHIRP_GEN_SRCDIR)/include/rust/chirp/lib.h && \ | ||
| test -f $(LIBRUST_CHIRP_GEN_SRCSTAMP) && \ | ||
| test $(LIBRUST_CHIRP_GEN_SRCDIR)/include/rust/chirp/lib.h -nt $< && \ | ||
| test $(LIBRUST_CHIRP_GEN_SRCDIR)/include/rust/chirp/lib.h -nt $(LIBRUST_CHIRP_GEN_SRCSTAMP) && \ |
There was a problem hiding this comment.
🔴 Blocking: Allow equal timestamps for distributed bridge artifacts
Copying the stamp before the generated files does not preserve their ordering in the source archive. Automake emits a ustar archive, whose mtimes have one-second precision; these three copies normally occur in the same second and therefore have identical timestamps after extraction. In an out-of-tree build, both strict artifact -nt $(LIBRUST_CHIRP_GEN_SRCSTAMP) checks fail and invoke cxxbridge, so the shipped bridge files are never accepted even though they have the matching version stamp. Accept an artifact when it is not older than the versioned stamp, while retaining the strict comparison against lib.rs.
source: ['codex']
| AC_ARG_ENABLE([rust], | ||
| [AS_HELP_STRING([--enable-rust], | ||
| [build the Rust/C++ bridge smoke component (default is no)])], | ||
| [enable_rust=$enableval], | ||
| [enable_rust=no]) | ||
|
|
||
| AC_ARG_ENABLE([online-rust], | ||
| [AS_HELP_STRING([--enable-online-rust], | ||
| [allow Cargo to fetch Rust dependencies from the internet (default is no, using vendored dependencies offline)])], | ||
| [enable_online_rust=$enableval], | ||
| [enable_online_rust=no]) |
There was a problem hiding this comment.
🟡 Suggestion: Document the optional Rust build workflow
The repository has no durable documentation explaining how to use the new build path. Users currently must infer RUST=1 for depends, the Rust/Cargo/cxxbridge version requirements, supported hosts, the default offline RUST_VENDORED_SOURCES requirement, and the purpose of --enable-online-rust from build scripts and configure diagnostics. Add a short build document covering both depends-backed and system-toolchain workflows and link it from the relevant build documentation.
source: ['codex']
4 participants
Issue being fixed or feature implemented
Dash Core needs a reproducible, cross-platform Rust and C++ bridge foundation before the Dash Platform GUI components can be split into reviewable follow-up pull requests. The earlier draft #7109 established this direction but is now stale and conflicting.
This PR carries that work forward on current
develop, preserves the original commit authorship, and keeps Rust completely opt-in so normal developer and release builds are unaffected.What was done?
--enable-rustbuild path backed by Rust 1.92.0 and cxxbridge 1.0.198.--enable-ruston any other host fails explicitly rather than fetching a Rust standard library we never build or test; additional targets can be provisioned later together with CI lanes that actually exercise them.linux64_rustdepends and source CI lane.This replaces #7109 and is intended as the base for the Rust-backed DashPay GUI work currently demonstrated in PastaPastaPasta#67.
How Has This Been Tested?
Tested locally on macOS arm64:
RUST=1and completed a Rust-enabled Dash Core build.dashdon regtest and confirmed the Rust chirp bridge executed at runtime.cargo check --workspace --lockedand verified formatting withcargo fmt --all -- --check.RUST=1, verified no Rust packages or vendoring ran, then completed a clean Dash Core build with--disable-rust.test/lint/all-lint.pysuccessfully.Breaking Changes
None. Rust support is disabled by default and existing build invocations retain their current behavior.
Checklist:
This pull request was created by Codex.