fix(ci): statically link MinGW runtime for Windows builds#769
Conversation
Adds -static-libgcc, -static-libstdc++, and static winpthread rustflags for the x86_64-pc-windows-gnu target so the Windows binary no longer depends on MinGW runtime DLLs absent on user machines. Also sets OPENSSL_STATIC=1 on the build step to statically link OpenSSL. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughConfigures Windows GNU builds for static linking (MinGW runtimes, OpenSSL) and enables bundled SQLite; updates CI to verify the produced Windows binary is self-contained and removes the prior Windows DLL fetch step. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
- Add `bundled` feature to rusqlite so SQLite is compiled from source and statically linked, eliminating the sqlite3.dll dependency - Remove the Windows libsql CI step that downloaded sqlite3.dll and created an import library with a broken (null) DLL name entry - Add post-build verification step that inspects the PE import table and fails if null DLL imports or MinGW runtime DLLs are found Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes Windows release artifacts failing on clean machines due to unintended dynamic DLL dependencies and a broken SQLite import library that produced (null).DLL in the PE import table.
Changes:
- Enable
rusqlite’sbundledfeature so SQLite is compiled and statically linked (removing the externalsqlite3.dlldependency). - Update Windows GNU target rustflags to statically link MinGW runtime components.
- Simplify the release workflow by removing the SQLite DLL/import-lib step, enabling static OpenSSL linking, and adding a PE import-table sanity check for Windows artifacts.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| Cargo.toml | Enables rusqlite bundled SQLite to avoid shipping sqlite3.dll. |
| Cargo.lock | Lockfile updates consistent with enabling bundled SQLite (e.g., cc added). |
| .github/workflows/release.yml | Removes Windows SQLite download/import-lib step; adds static OpenSSL env and a Windows import-table self-containment check. |
| .cargo/config.toml | Adds Windows GNU target rustflags to statically link MinGW runtimes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
104-122: Solid verification step with explicit failure — well done.The sanity check correctly:
- Inspects the PE import table using
objdump- Fails explicitly with descriptive error messages when null imports or MinGW runtime DLLs are found
- Provides clear success output
Consider expanding DLL checks (optional):
For additional assurance, you could also verify that
sqlite3.dlland OpenSSL DLLs (libssl-*.dll,libcrypto-*.dll) are not in the import table, confirming the bundled SQLite and static OpenSSL are working as expected.,
💡 Optional: expand DLL verification
# Fail if MinGW runtime DLLs are dynamically linked - for dll in libgcc_s_seh-1.dll libwinpthread-1.dll libstdc++-6.dll; do + for dll in libgcc_s_seh-1.dll libwinpthread-1.dll libstdc++-6.dll sqlite3.dll libssl libcrypto; do if echo "$IMPORTS" | grep -qi "$dll"; then echo "::error::Binary dynamically links $dll — must be statically linked" exit 1 fi done - echo "✅ Binary is self-contained — no MinGW runtime or null DLL dependencies" + echo "✅ Binary is self-contained — no MinGW runtime, SQLite, OpenSSL, or null DLL dependencies"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 104 - 122, Update the "Verify Windows binary is self-contained" step to also reject imports of sqlite3 and OpenSSL DLLs by extending the IMPORTS checks: after computing IMPORTS, add checks that fail if any case-insensitive match for "sqlite3.dll" or wildcard OpenSSL names like "libssl-" or "libcrypto-" appears (use grep -qi with patterns such as "sqlite3.dll" and "libssl-\\|libcrypto-"), referencing the existing IMPORTS variable and reusing the same error/exit pattern used for the MinGW DLL checks so the step fails with a descriptive message if these DLLs are dynamically linked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 104-122: Update the "Verify Windows binary is self-contained" step
to also reject imports of sqlite3 and OpenSSL DLLs by extending the IMPORTS
checks: after computing IMPORTS, add checks that fail if any case-insensitive
match for "sqlite3.dll" or wildcard OpenSSL names like "libssl-" or "libcrypto-"
appears (use grep -qi with patterns such as "sqlite3.dll" and
"libssl-\\|libcrypto-"), referencing the existing IMPORTS variable and reusing
the same error/exit pattern used for the MinGW DLL checks so the step fails with
a descriptive message if these DLLs are dynamically linked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d4c0eab6-74a2-4628-a48f-97dc939b62e3
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
.cargo/config.toml.github/workflows/release.ymlCargo.toml
Insert the wallet record before the address row in `register_test_address` so the `addresses.seed_hash` FK (enforced when SQLite is compiled with `SQLITE_DEFAULT_FOREIGN_KEYS=1` via the `bundled` feature) is satisfied. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…770) * ci: add binary dependency verification for Linux, macOS, and Flatpak Add post-build sanity checks to catch missing or unexpected shared library dependencies before release artifacts are published: - Linux: ldd check with allowlist, fail on "not found" - macOS (ARM64 + x86): otool -L check, fail on non-system libs - Flatpak: ldd check inside build-dir, fail on "not found" Complements the Windows PE verification from #769. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(ci): prevent grep exit code 1 from failing dependency checks When all binary dependencies are valid system libraries, `grep -Ev` filters out all lines and returns exit code 1 (no matches). Under `bash -e` (GitHub Actions default), this kills the script before it can report success. Add `|| true` to all grep commands inside command substitutions across Linux and macOS verification steps. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(ci): run Flatpak ldd check inside sandbox, not on host The ldd check was running against the host runner's libraries, which don't match the Flatpak runtime. Use `flatpak build build-dir` to execute ldd inside the sandbox where libraries resolve against the Flatpak runtime (/usr) and app (/app) directories. Addresses review comment from copilot-pull-request-reviewer on PR #770. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Fixes #768 — Windows users get
"The code execution cannot proceed because (null).DLL was not found"when running dash-evo-tool.Root cause: Two issues in the Windows cross-compilation setup:
libgcc_s_seh-1.dll,libwinpthread-1.dll,libstdc++-6.dll) that don't exist on normal Windows machinessqlite3.dllimport library was generated bydlltoolwithout the-Dflag, producing a null DLL name in the PE import table — the direct cause of the(null).DLLerrorFix:
rustflagsto.cargo/config.tomlforx86_64-pc-windows-gnuthat statically links GCC runtime, C++ stdlib, and pthreadsbundledfeature torusqliteso SQLite is compiled from C source and statically linked — eliminatessqlite3.dlldependency entirelyWindows libsqlCI step that downloaded and created the broken import libraryOPENSSL_STATIC=1in CI build step to statically link OpenSSLTest plan
🤖 Co-authored by Claudius the Magnificent AI Agent
Summary by CodeRabbit
Chores
Tests