Skip to content

fix: return an error when uninstalling a non-installed version#617

Open
suman2280 wants to merge 3 commits into
tofuutils:mainfrom
suman2280:fix/uninstall-error
Open

fix: return an error when uninstalling a non-installed version#617
suman2280 wants to merge 3 commits into
tofuutils:mainfrom
suman2280:fix/uninstall-error

Conversation

@suman2280

Copy link
Copy Markdown
Contributor

Summary

solves #616

tenv <tool> uninstall <version> used to silently report success when the requested version was not installed locally. This change makes it return a clear <Tool> <version> is not installed error and a non-zero exit code, matching the documented expected behaviour.

Fixes the bug described for tf, tofu, tg (and tm) where running

tenv <tool> uninstall 0.0.0

reported Uninstallation of <Tool> 0.0.0 successful (directory … removed) even though the version does not exist.

What this PR fixes

uninstallSpecificVersion in versionmanager/manager.go relied on os.RemoveAll, which is a no-op on a non-existent path and returns nil. Callers therefore reported success even when nothing was installed for the requested version.

Implementation

versionmanager/manager.go

  • New sentinel errVersionNotInstalled exported package-wide.
  • uninstallSpecificVersion stats the target directory first; missing path -> display the user-facing message and return wrapped errVersionNotInstalled; otherwise os.RemoveAll and only report success when it actually removes something.
  • Uninstall propagates the error in both branches: exact version and the constraint path (when SelectVersionsToUninstall yields an empty selection).
  • UninstallMultiple and the multi-version loop in Uninstall collect per-version errors via errors.Join.

versionmanager/tenvlib/lib.go

  • Tenv.Uninstall now delegates to manager.Uninstall(requestedVersion) instead of UninstallMultiple([]string{requestedVersion}), routing the single-version CLI path through the direct, non-interactive branch and ensuring the new error reaches callers.

Tests

versionmanager/uninstall_test.go (new):

  • Uninstall_not_installed_exact_version -- original bug.
  • UninstallMultiple_returns_error_for_not_installed.
  • UninstallMultiple_returns_joined_errors_for_multiple_not_installed -- verifies errors.Join wrapping.
  • Uninstall_installed_version_succeeds -- happy path regression.
  • Uninstall_same_version_twice_returns_error -- idempotency.

Commits

This PR is split into two commits to keep history readable:

  1. fix(uninstall): return an error when the target version is not installed -- manager logic + tests.
  2. fix(tenvlib): route single-version Uninstall through VersionManager.Uninstall -- public-API routing.

Test plan

  • go build ./...
  • go vet ./...
  • gofmt -l versionmanager/ clean
  • go test ./versionmanager/... ./versionmanager/tenvlib/... -- all new subtests pass; no regressions in existing suites.

Signed-off-by: Suman Sardar <sudarshansardar22@gmail.com>
…ninstall

Signed-off-by: Suman Sardar <sudarshansardar22@gmail.com>
- Add blank lines before return/break per nlreturn.
- Drop redundant nil check on sliced result of errors.As.

Signed-off-by: Suman Sardar <sudarshansardar22@gmail.com>
@suman2280
suman2280 force-pushed the fix/uninstall-error branch from 27e8193 to 635c3e3 Compare July 13, 2026 16:51
@suman2280

Copy link
Copy Markdown
Contributor Author

@kvendingoldo Can you please review this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant