Repository navigation
fix(install): regenerate lockfile with --force on global install - #33970
Conversation
…tall When `deno install --global --force` (or --reload) was used, the auto-generated lockfile at `<bin>/.<name>/deno.lock` still pinned the old version, so resolution returned the previously locked version instead of the latest. Remove the stale lockfile before re-running resolution so --force / --reload actually picks up newer versions. Closes denoland#33934
Deno Individual Contributor License AgreementAll contributors have signed the CLA. Thank you! This is an automated message from CLA Assistant |
fibibot
left a comment
There was a problem hiding this comment.
Fix is well-scoped — deletion is limited to dir.join("deno.lock") inside the per-install config dir (~/.deno/bin/.<name>/), and the if lockfile_path.exists() guard handles the fresh-install case cleanly.
Two test nits:
install_reload_regenerates_lockfilesetsreload: trueandforce: true. Sinceforce || flags.reloadshort-circuits on the first true, the test doesn't actually exercise the reload-only path — passingforce: falsehere would isolate it.- The same test calls
rustls::crypto::aws_lc_rs::default_provider().install_default()whileinstall_force_regenerates_lockfiledoesn't. Both fetch fromhttp://localhost:4545(no TLS), so the rustls init looks cargo-culted — unless there's a code path I'm missing that lands on a TLS request.
Holding approval until CI is green (12 shards still in_progress at the head SHA).
fibibot
left a comment
There was a problem hiding this comment.
fdcb6e9d narrows the condition from force || flags.reload → force only. The linked issue #33934 explicitly lists both flags in the expected behaviour ("When --force (or --reload) is specified... the lockfile should be regenerated"). Was the reload removal intentional, or a misread of my prior nit about test isolation? The nit was "make the reload test actually exercise reload-only", not "drop reload from scope."
If --force-only is what you want, two test follow-ups:
install_reload_regenerates_lockfilestill hasforce: true, so it's now a misleadingly-named force test. Either rename it or convert it to assert thatforce: false, reload: truedoes NOT remove the lockfile — that pins the new (narrowed) behaviour and prevents accidental reflow back toforce || reload.- The
rustls::default_provider().install_default()call in that test is now even more orphaned (no TLS path at all).
Holding approval until the scope question is settled and CI lands green.
fibibot
left a comment
There was a problem hiding this comment.
537d8416 renames the test to install_reload_preserves_lockfile, flips force: false, asserts post_reload_content == stale_lockfile, and drops the orphaned rustls init. That pins the narrowed --force-only scope and prevents accidental drift back to force || reload — both prior nits resolved and the scope question is answered by the test itself.
Heads-up on the linked issue: #33934's expected-behaviour section still names both flags, so if @bartlomieju wants --reload to also clear the lockfile that's a maintainer call — code is internally consistent either way.
Holding approval until CI lands green on 537d8416 (most shards still in_progress).
|
|
…oland#33970) ## Summary Fixes denoland#33934. When `deno install --global --force` (or `--reload`) was used, the auto-generated lockfile at `<bin>/.<name>/deno.lock` still pinned the old version, so version resolution returned the previously locked version instead of the latest. This change removes the stale lockfile before re-running resolution in `setup_config_dir` when `--force` or `--reload` is specified. The user is explicitly asking for a fresh install in those cases, so honoring a stale auto-generated lockfile is a footgun. ## Test plan - [x] Added `install_force_regenerates_lockfile` unit test - [x] Added `install_reload_regenerates_lockfile` unit test - [x] `cargo build --bin deno` passes - [x] `./tools/format.js` + `./tools/lint.js` clean --------- Co-authored-by: Bartek Iwańczuk <[email protected]>
Summary
Fixes #33934.
When
deno install --global --force(or--reload) was used, the auto-generated lockfile at<bin>/.<name>/deno.lockstill pinned the old version, so version resolution returned the previously locked version instead of the latest.This change removes the stale lockfile before re-running resolution in
setup_config_dirwhen--forceor--reloadis specified. The user is explicitly asking for a fresh install in those cases, so honoring a stale auto-generated lockfile is a footgun.Test plan
install_force_regenerates_lockfileunit testinstall_reload_regenerates_lockfileunit testcargo build --bin denopasses./tools/format.js+./tools/lint.jsclean