fix(cli): resolve raw binary asset for Windows self-update, not *-setup#13
Merged
Merged
Conversation
`digstore update` on Windows returned NotFound because it called
select_installer_asset, which looked for a bundled `*-setup.exe`/`.msi`
GUI installer. That installer moved to dig-installer and digstore's own
releases no longer publish one, so the lookup always found nothing.
Unify the Windows path with the existing macOS/Linux self-replace path:
resolve the raw `digstore-<ver>-windows-x64.exe` binary via
select_binary_asset, download it, then atomically self-replace via
install_binary. Windows needs an extra step here that Unix doesn't:
overwriting the currently-running process's own image file fails
directly ("Access is denied", os error 5), so install_binary now
renames the running binary aside first, moves the new one into place,
then best-effort deletes the old one.
Removes the now-dead select_installer_asset/select_windows_installer/
download_asset/launch_installer GUI-installer helpers and their tests.
Regression test spawns a copy of a system executable and replaces it
while the child process is executing it, reproducing the exact lock
digstore hits when updating itself.
Closes #303
Co-Authored-By: Claude <noreply@anthropic.com>
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
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.
What
digstore updateon Windows returnedNotFoundbecause the Windows self-update path calledselect_installer_asset, which looked for a bundled*-setup.exe/.msiGUI installer. That installer moved to dig-installer, so digstore's own releases no longer publish one — the lookup always found nothing.Fixes #303.
How
Unified the Windows update path with the existing macOS/Linux self-replace path —
digstorenow updates ITS OWN binary in place on every platform, no GUI installer step:perform_updateno longer branches on#[cfg(target_os = "windows")]vs not; one path resolves the rawdigstore-<ver>-windows-x64.exe(or per-OS/arch equivalent) asset viaselect_binary_asset, downloads it, sanity-checks the magic bytes, then callsinstall_binary.install_binarygained a Windows-specific step: a direct one-step overwrite of the currently-executing binary fails on Windows (Access is denied, os error 5), even though Windows lets you rename a running image aside. So on Windows: rename the current binary aside → rename the new one into place → best-effort delete the old one (harmless if it's still locked; it's never the activetargetpath again).fetch_bytesis no longercfg-gated off Windows (shared download path for every platform).select_installer_asset/select_windows_installer/download_asset/launch_installerGUI-installer helpers and their tests (gitnexus confirmedselect_installer_assethad exactly one caller,perform_update, in this same file — LOW risk, no other consumers in the repo).Verified
ping.exe, keeps it running as a child process, then callsinstall_binaryon that locked file — against the pre-fix code this failed withUpdateFailed("... Access is denied (os error 5) ..."), reproducing the exact class of bug reported (a plain rename over a currently-executing file fails on Windows). Confirmed RED, then implemented the fix, confirmed GREEN.selects_windows_x64_raw_binaryasserts the shared selector resolvesdigstore-<ver>-windows-x64.exe(not*-setup.exe/.msi) from the real release asset-name fixture.cargo fmt -p digstore-cli --check: clean.cargo clippy -p digstore-cli --all-targets --all-features -- -D warnings: clean.cargo clippy --workspace --all-targets --locked -- -D warnings <CI's two allowed lints>: clean (exact CI command).cargo build --workspace --locked: clean.cargo test -p digstore-cli --lib: 268 passed, 0 failed, 1 ignored (live-network smoke test, intentionally#[ignore]d).cargo test -p digstore-cli --tests: full installed-crate integration suite green (everytests/*.rsfile), 0 failed.cargo install --path crates/digstore-cli --force --locked→digstore --versionreports the new0.11.1; randigstore update --checkagainst the live GitHub API with the reinstalled binary — completes cleanly (already up to date (v0.11.1)), no regression in the surrounding command.detect_changes(gitnexus): risklow, 0 affected downstream execution flows — every changed symbol is contained incrates/digstore-cli/src/commands/update.rs.Full-workspace
cargo test --workspacelink phase could not complete locally due to the dev sandbox running low on disk (unrelated pre-existing environment constraint on this shared machine, not caused by this change); the scoped crate build/lint/test gates above are exhaustive for the change's actual blast radius, and CI's--workspacegate will run authoritatively on this PR.Blast radius
gitnexus
impact({target: "select_installer_asset", direction: "upstream"}): LOW risk, single caller (perform_update), same file — confirmed no other consumers exist anywhere in the repo (suggest_manual_asset/select_binary_asset/fetch_bytes/install_binary/resolve_self_pathsimilarly grepped clean of any cross-module callers). Change is fully contained tocrates/digstore-cli/src/commands/update.rs.Version bump
Patch:
0.11.0→0.11.1(workspace-shared version) — this is a backwards-compatible bug fix (fix:), no public API/behavior change beyond making the Windows update path actually work.Co-Authored-By: Claude noreply@anthropic.com