Repository navigation
fix(doctor): stop removing other dtvem installs' shims from PATH - #298
Merged
Merged
Conversation
Every PATH entry that looked like a dtvem shims directory (…/dtvem/shims or …/.dtvem/shims) but wasn't the current root's was treated as stale, without checking whether it was. With a second install, or DTVEM_ROOT pointing away from the default install, the default install's real shims directory was reported as an error and removed by `dtvem doctor --fix` (also under --yes): from the User PATH on Windows, and from the shell config on Unix. `dtvem init` on Windows dropped it silently too. A non-current shims entry is now stale only when its directory no longer exists. A directory that still exists belongs to another dtvem root, which may still be in use: it is kept everywhere and only reported. - path.ClassifyShimsEntries sorts entries into kept, stale and other roots. PartitionStaleShimsEntries and FindStaleShimsEntries wrap it, so the doctor check, the Windows registry fix, the Unix shell-config fix and dtvem init all follow the same rule. - Entries are judged with environment references expanded (%VAR% on Windows, $VAR elsewhere). A registry spelling of the current shims directory, such as %USERPROFILE%\.dtvem\shims, used to compare unequal to the expanded current directory and was removed as stale; it is now recognized as current. - doctor reports other roots' shims as a warning with manual instructions and no fix; stale entries stay a fixable error. A directory listed in both the System and User PATH is reported once. - dtvem init on Windows keeps other roots' entries and warns about them.
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.
Description
dtvem doctor'sstale-shims-pathcheck treated everyPATHentry that looked like a dtvem shims directory, other than the current root's, as stale, without checking whether it was. With a second install, orDTVEM_ROOTpointing away from the default install, the default install's real shims directory was:dtvem doctor --fix(also under--yes): from the UserPATHon Windows, and from the shell config on Unix;dtvem initon Windows.A non-current shims entry is now stale only when its directory no longer exists. One that still exists belongs to another dtvem root, which may still be in use. It is kept everywhere and only reported.
Type of Change
Related Issues
Resolves #297
Changes Made
path.ClassifyShimsEntriessortsPATHentries into kept, stale (missing directory) and other roots (existing directory).PartitionStaleShimsEntriesandFindStaleShimsEntriesnow wrap it, so every place that decides what's stale follows the same rule:RemoveStaleShimsFromUserPath);removeDtvemMarkerBlocks);dtvem initon Windows (prependShimsDir).%VAR%on Windows (registry.ExpandString),$VARelsewhere (os.ExpandEnv). This also fixes a latent bug: a registry spelling of the current shims directory, such as%USERPROFILE%\.dtvem\shims, compared unequal to the expanded current directory, so the stale-shims fix removed it. It's now recognized as current.PATHis reported once.dtvem initon Windows keeps other roots' entries and warns:Left another dtvem install's shims on your User PATH: …. Unixinitalready only warned, and now warns only about missing directories.Testing
Test Environment
GOOS=linuxandGOOS=darwin)Test Steps
./rnr check: gofmt, golangci-lint (0 issues), all 27 packages pass. Lint also reports 0 issues withGOOS=linuxandGOOS=darwin.internal/path, all platforms:ClassifyShimsEntries: kept, stale and other roots, against real existing and missing directories.FindStaleShimsEntriesandFindOtherRootShimsEntries.# Added by dtvemblock and removes a missing one's. This test is Unix-only, since that parser splits"<dir>:$PATH"on:.internal/path, Windows:prependShimsDirkeeps and reports an existing other root, and doesn't duplicate a%USERPROFILE%spelling of the shims directory.removeStaleShimskeeps both of those and removes only the missing directory.internal/doctor: other root only (warning, not fixable, no "Found"), stale plus other root (fixable error, both listed, "left in place"), a repeated entry reported once. The existing tests are unchanged and pass.--no-fix):DTVEM_ROOTset to a temp root, with the real install's shims onPATH.Test Results
Before, the real install's shims directory was a fixable
✗ Stale dtvem shims entry in PATH. Now:Checklist
golangci-lint runand fixed all issues (includes gofmt, govet, and 40+ linters)go mod tidyto clean up dependencies (no dependency changes in this PR)go test ./...)Breaking Changes
None. One behaviour change to know about: a leftover shims directory that still exists on disk is no longer removed automatically, for example a pre-XDG
~/.dtvem/shimsthat was never deleted. Doctor reports it as another install and says how to remove itsPATHentry once the user confirms it's unused.Additional Notes
The Unix-only shell-config test is skipped on Windows and runs on CI's Linux and macOS jobs.