#2466: remove broken software links on uninstall

[HadiDawoud]

This PR fixes #2466

Implemented changes:

• UninstallCommandlet no longer checks isInstalled() and simply calls toolCommandlet.uninstall(), so the bug is fixed in the uninstall routine itself
• uninstall() detects broken software symlinks with NOFOLLOW_LINKS and removes them; the "never installed" warning stays in uninstall()
• For package-manager based tools (e.g. pip) the tool path points to the parent tool, so PackageManagerBasedLocalToolCommandlet.uninstall() removes leftover software/«tool» folders (e.g. only .ide.software.version) or warns, without invoking the package manager
• Added unit tests for the broken-link case, the leftover folder case and the not-installed pip case

---

Testing instructions

1. Run mvn -Dtest=UninstallCommandletTest test in the cli folder and verify all 12 tests pass
2. Optional manual reproduce from the issue: ide install gradle, delete the version under $IDE_ROOT/_ide/software/default/gradle, then ide uninstall gradle - the broken link under $IDE_HOME/software/gradle should be removed
3. Optional: create $IDE_HOME/software/pip/.ide.software.version without a pip installation, then ide uninstall pip - the folder should be removed

---

Checklist for this PR

Make sure everything is checked before merging this PR. For further info see
our DoD.

• When running mvn clean test locally all tests pass and build is successful
• PR title is of the form #«issue-id»: «brief summary» (e.g. #921: fixed setup.bat and not feature/921 fixed setup.bat). If no issue ID exists, title only.
• PR top-level comment summaries what has been done and contains link to addressed issue(s)
• PR and issue(s) have suitable labels
• Issue is set to In Progress and assigned to you or there is no issue (might happen for very small PRs)
• You followed all coding conventions
• You have added the issue implemented by your PR in CHANGELOG.adoc unless issue is labelled
  with internal
• You have not changed any dependency in pom.xml files or otherwise if runtime dependencies changed, you have updated our LICENSE.asciidoc
• You have formulated clear instructions on how to test your contribution under "Testing instructions"

[CLAassistant]

All committers have signed the CLA.

[coveralls]

Coverage Report for CI Build 38900708598

Coverage increased (+0.03%) to 74.789%

Details

• Coverage increased (+0.03%) from the base build.
• Patch coverage: No coverable lines changed in this PR.
• 6 coverage regressions across 3 files.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

6 previously-covered lines in 3 files lost coverage.

File	Lines Losing Coverage	Coverage
com/devonfw/tools/ide/tool/PackageManagerBasedLocalToolCommandlet.java	4	92.38%
com/devonfw/tools/ide/commandlet/UninstallCommandlet.java	1	88.0%
com/devonfw/tools/ide/version/VersionSegment.java	1	89.76%

---

Coverage Stats

Relevant Lines:	19649
Covered Lines:	15351
Line Coverage:	78.13%
Relevant Branches:	8731
Covered Branches:	5874
Branch Coverage:	67.28%
Branches in Coverage %:	Yes
Coverage Strength:	3.36 hits per line

---

💛 - Coveralls

[shodiBoy1]

Thanks @HadiDawoud, nice fix.

Tested this on Mac and Windows.

Unit tests: UninstallCommandletTest - 11/11 passed.

Manual: installed gradle, deleted $IDE_ROOT/_ide/software/default/gradle so software/gradle became a broken link, then ran uninstall from this branch. Got Successfully uninstalled gradle and the link was gone. Same on Windows (Explorer showed “location is not available” before uninstall, folder gone after).

Looks good to me, the original issue is fixed.

[hohwille]

@HadiDawoud thanks for your PR and fixing the problem. 👍
You found the right spots and tricks (NOFOLLOW_LINKS) to fix it.
I would suggest fixing the bug as its root avoiding redundant knowledge and implementation details about uninstalling tools outside the uninstall routine.
Then we can merge.

[hohwille]

IMHO the bug is that we are using isInstalled() to see if the tool is correctly installed.
Actually the uninstall() method of [Local]ToolCommandlet should be able to uninstall a tool including such leftovers.
Therefore I would suggest to fix the bug in this method rather than keeping the bug as is and adding a fix outside as a workaround.
So my suggestion is to remove the if condition here and change this to just:

      toolCommandlet.uninstall();

Then in the uninstall() method we should handle the warning logging that if I call uninstall on a tool that was never installed (not even broken leftover) to raise the warning.

[hohwille]

So we already have that warning.
Excellent catch that you added the NOFOLLOW_LINKS option here! 👍

[HadiDawoud]

@HadiDawoud thanks for your PR and fixing the problem. 👍 You found the right spots and tricks (NOFOLLOW_LINKS) to fix it. I would suggest fixing the bug as its root avoiding redundant knowledge and implementation details about uninstalling tools outside the uninstall routine. Then we can merge.

Thanks for the review! I removed the isInstalled() check, so UninstallCommandlet now just calls uninstall(). The warning and the NOFOLLOW_LINKS fix are inside uninstall(). For pip-like tools the tool path points to the parent tool, so I added a small override that only cleans up the leftover software/pip folder (or warns) without touching the package manager.