Improve cargo xtask dev-setup: cmake detection and xtask self-build lock#136
Open
Improve cargo xtask dev-setup: cmake detection and xtask self-build lock#136
Conversation
cmake became a hard prereq for `cargo build --workspace` after quarto-highlight (commit 1fa33da) enabled the `wasm` feature on tree-sitter, pulling in wasmtime-c-api-impl whose build.rs shells out to cmake. CI runners ship with cmake; local dev boxes often do not. check_cmake() in dev_setup.rs follows the check_pandoc() pattern. On missing cmake, warns with platform-specific install commands (scoop/winget on Windows, brew on macOS, apt/dnf on Linux). No version floor — any modern cmake works for wasmtime-c-api. Warning is stronger than pandoc's because missing cmake means the workspace build fails entirely, not just four comparison tests. verify.rs step 3 now skips the xtask crate on Windows. Cargo could not re-link target/debug/xtask.exe while the currently-running xtask held a file lock on it ("Accès refusé (os error 5)"). Linux/macOS tolerate overwriting a running binary, so the fix is cfg(windows)-gated. Step 5 (nextest) still compiles xtask as a test binary on all platforms, preserving warnings coverage. CONTRIBUTING.md lists cmake as a prereq with per-platform install commands, mirroring the existing macOS Homebrew LLVM section.
Review feedback on the prior commit: verify.rs: the Windows `--exclude xtask` workaround left a gap — `cargo xtask verify --skip-rust-tests` would now never compile or check xtask at all, letting a broken xtask crate slip through. Added `cargo check -p xtask` after step 3 on Windows so xtask still gets the `-D warnings` pass. `cargo check` does not relink, so it sidesteps the file lock that motivated the exclusion in the first place. dev_setup.rs: `check_cmake()` now uses `let-else` to mirror the neighbouring `check_pandoc()` helper, and the warning prefix is `Warning:` instead of `WARNING:` for consistency. Extracted `warn_cmake_missing()` so the early-return paths share one message. CONTRIBUTING.md: `brew install cmake` is a separate formula from `brew install llvm`, not bundled with it.
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.
Two cross-platform dev-ergonomics fixes surfaced while verifying PR #109 on Windows. Both are orthogonal to the WASM work.
Background
quarto-highlight(already on main) enables thewasmfeature ontree-sitterfor native builds. That pulls inwasmtime-c-api-implas a transitive dependency, whosebuild.rsshells out to cmake. Result: cmake is now a hard prerequisite forcargo build --workspaceon every platform. GitHub Actions runners have it pre-installed. Local dev boxes frequently do not.Fix 1 —
check_cmake()indev_setup.rsFollows the existing
check_pandoc()pattern. Detects cmake viacmake --version. If missing, prints a warning with platform-specific install commands (scoop/winget on Windows, brew on macOS, apt/dnf on Linux). No version floor — any modern cmake works for wasmtime-c-api. Warning only;cargo dev-setupstill exits 0.Fix 2 —
cfg(windows)-gated--exclude xtaskinverify.rsstep 3On Windows, Cargo could not re-link
target/debug/xtask.exewhile the currently-running xtask held a file lock on it (Accès refusé (os error 5)). Linux/macOS tolerate overwriting a running binary, so the fix is Windows-only.To avoid creating a gap where
cargo xtask verify --skip-rust-testson Windows would never compile or check xtask at all, a follow-upcargo check -p xtaskruns on Windows.cargo checkdoes not relink, so no file lock, and xtask still gets the-D warningspass.Fix 3 —
CONTRIBUTING.mdcmake sectionAdded after the existing macOS Homebrew LLVM section, with per-platform install commands.
Test plan
cargo xtask dev-setupreportscmake version 4.3.2 — detectedon Windowscargo xtask verify --skip-rust-tests --skip-hub-build --skip-hub-tests --skip-treesitter-tests --skip-trace-viewer-build --skip-trace-viewer-testscompletes step 3 on Windows without theos error 5file lockcargo nextest run -p xtask— 9/9 tests passRUSTFLAGS="-D warnings" cargo check -p xtaskpassescargo nextest run --workspaceon Linux/macOS CITracked in beads bd-tjbr.