chore: add Cross.toml for cross-compilation clang/llvm dependencies#83
chore: add Cross.toml for cross-compilation clang/llvm dependencies#83
Conversation
📝 WalkthroughWalkthroughAdded a new Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 1
🧹 Nitpick comments (1)
Cross.toml (1)
18-19:JEMALLOC_SYS_WITH_LG_PAGEpassthrough is currently ineffective in release CI.In
.github/workflows/release.yml(Lines 102-108),cross buildruns without exporting this variable first, so this entry has no effect for that workflow. Consider either setting it explicitly in the workflow or removing this passthrough to avoid dead config.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Cross.toml` around lines 18 - 19, The passthrough entry under [build.env] (passthrough = ["JEMALLOC_SYS_WITH_LG_PAGE"]) is ineffective because the release workflow runs cross build without exporting JEMALLOC_SYS_WITH_LG_PAGE first; either remove that dead passthrough entry from Cross.toml or update the release workflow to export the environment variable before invoking cross (i.e., ensure the CI job that runs the cross build sets/exports JEMALLOC_SYS_WITH_LG_PAGE in the environment), referencing the passthrough key, the variable name JEMALLOC_SYS_WITH_LG_PAGE, and the invocation of cross build.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Cross.toml`:
- Around line 8-10: Add an HTTPS timeout entry to the APT retry config so the
timeout/retry hardening applies to the rewritten HTTPS sources: after the
existing echo lines that create /etc/apt/apt.conf.d/80-retries (the lines
writing Acquire::http::Timeout and Acquire::ftp::Timeout), add a similar echo
for Acquire::https::Timeout with the same value (e.g., "60") so the file
includes Acquire::https::Timeout "60";.
---
Nitpick comments:
In `@Cross.toml`:
- Around line 18-19: The passthrough entry under [build.env] (passthrough =
["JEMALLOC_SYS_WITH_LG_PAGE"]) is ineffective because the release workflow runs
cross build without exporting JEMALLOC_SYS_WITH_LG_PAGE first; either remove
that dead passthrough entry from Cross.toml or update the release workflow to
export the environment variable before invoking cross (i.e., ensure the CI job
that runs the cross build sets/exports JEMALLOC_SYS_WITH_LG_PAGE in the
environment), referencing the passthrough key, the variable name
JEMALLOC_SYS_WITH_LG_PAGE, and the invocation of cross build.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| "echo 'Acquire::Retries \"3\";' > /etc/apt/apt.conf.d/80-retries", | ||
| "echo 'Acquire::http::Timeout \"60\";' >> /etc/apt/apt.conf.d/80-retries", | ||
| "echo 'Acquire::ftp::Timeout \"60\";' >> /etc/apt/apt.conf.d/80-retries", |
There was a problem hiding this comment.
Add HTTPS timeout since sources are switched to HTTPS.
Line 5 rewrites APT sources to https://, but Lines 9-10 only configure http/ftp timeouts. Add Acquire::https::Timeout so retry/timeout hardening applies to the protocol actually used.
🔧 Suggested patch
"echo 'Acquire::Retries \"3\";' > /etc/apt/apt.conf.d/80-retries",
"echo 'Acquire::http::Timeout \"60\";' >> /etc/apt/apt.conf.d/80-retries",
+ "echo 'Acquire::https::Timeout \"60\";' >> /etc/apt/apt.conf.d/80-retries",
"echo 'Acquire::ftp::Timeout \"60\";' >> /etc/apt/apt.conf.d/80-retries",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "echo 'Acquire::Retries \"3\";' > /etc/apt/apt.conf.d/80-retries", | |
| "echo 'Acquire::http::Timeout \"60\";' >> /etc/apt/apt.conf.d/80-retries", | |
| "echo 'Acquire::ftp::Timeout \"60\";' >> /etc/apt/apt.conf.d/80-retries", | |
| "echo 'Acquire::Retries \"3\";' > /etc/apt/apt.conf.d/80-retries", | |
| "echo 'Acquire::http::Timeout \"60\";' >> /etc/apt/apt.conf.d/80-retries", | |
| "echo 'Acquire::https::Timeout \"60\";' >> /etc/apt/apt.conf.d/80-retries", | |
| "echo 'Acquire::ftp::Timeout \"60\";' >> /etc/apt/apt.conf.d/80-retries", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Cross.toml` around lines 8 - 10, Add an HTTPS timeout entry to the APT retry
config so the timeout/retry hardening applies to the rewritten HTTPS sources:
after the existing echo lines that create /etc/apt/apt.conf.d/80-retries (the
lines writing Acquire::http::Timeout and Acquire::ftp::Timeout), add a similar
echo for Acquire::https::Timeout with the same value (e.g., "60") so the file
includes Acquire::https::Timeout "60";.
2 participants
Summary
Cross.tomlwith pre-build steps to installllvm-dev,libclang-dev, andclangin the cross Docker containerreth-mdbx-syscompilation which needsstdarg.hContext
Release CI (
release.yml) failed onv0.2.0tag push with:Test plan
v0.2.0and verify release workflow completesSummary by CodeRabbit