#2373: native package edition - #2500
Open
krystynaShatkovska wants to merge 6 commits into
Open
krystynaShatkovska wants to merge 6 commits into
krystynaShatkovska wants to merge 6 commits into
Conversation
Follow-up to devonfw#2306. Determine the edition together with the version for native package manager installs (Linux/macOS) using one consistent edition-resolution model across all operating systems. - GlobalToolCommandlet: replace the per-OS if-blocks in computeInstalledEditionAndVersion() with a single loop over getEditionNames(); add getEditionNames() and getInstalledVersionForEdition() hooks. - Docker: declare its two editions (docker/rancher), resolve each edition's version per OS, and cover both in the Windows registry map. Drop the separate rdctl-based Rancher detection in favour of the shared native-package path. - Tests: prove Linux editions resolve via getEditionNames() (not the tool-name placeholder); update Rancher + macOS tests to the new loop semantics. Fixes devonfw#2373
Complete OS parity for Docker edition resolution (part of devonfw#2373). The rancher edition now resolves on macOS from the Rancher Desktop.app bundle (via plutil), mirroring the docker edition, instead of returning null. Add a test proving the probe order (docker first) falls through to rancher when Docker Desktop is absent.
On Windows, GlobalToolCommandlet now validates that every declared edition has a matching entry in getWindowsRegistryAppNames() before probing the registry. A missing mapping is logged (warn) and the edition is skipped, instead of crashing or silently probing with a null app name. Add unit tests covering Docker+Rancher both installed (resolves to Docker, probed first) and Rancher-only (resolves to Rancher).
…-package-edition # Conflicts: # cli/src/test/java/com/devonfw/tools/ide/tool/GlobalToolCommandletTest.java
Collaborator
Coverage Report for CI Build 35580652092Warning No base build found for commit Coverage: 74.276%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats💛 - Coveralls |
Contributor
|
Thanks for the fix @krystynaShatkovska cd cli
mvn test -Dtest=GlobalToolCommandletTest,DockerTest
20 tests, BUILD SUCCESS.
Happy path (Docker Desktop installed) — pass
plutil -extract CFBundleShortVersionString raw /Applications/Docker.app/Contents/Info.plist
# 4.73.0
ide get-edition docker --installed
# docker
ide get-version docker --installed
# 4.73.0
Version matches Docker.app, not docker --version. Good.
When Docker.app is missing — I may have the wrong expectation here
I moved /Applications/Docker.app away (docker CLI still on PATH, Rancher not installed):
ide get-edition docker --installed
# exit=1
ide -d get-edition docker --installed
# plutil ... /Applications/Docker.app/Contents/Info.plist
# failed with exit code 1!
# Step 'ide' ended with failure.
What I expected:
Tool docker is not installed. (exit 1)
if only Rancher is installed → edition rancher
What I got:
exit 1 because plutil failed
no "not installed" message
rancher is never checked (docker is tried first, then it stops)
After restoring Docker.app it worked again: docker / 4.73.0.
I did not install Rancher, because this already stops before rancher is probed.
Is this the intended macOS behavior? Happy to retest if I missed something. |
plutil exits non-zero when a .app bundle is absent, which previously made 'get-edition'/'get-version' fail with 'failed with exit code 1!' instead of reporting the edition as not installed. The IdeLogLevel passed to runAndGetSingleOutput only controls output logging, not exit-code handling, so the default THROW_ERR still threw and the edition loop stopped before probing the next edition (e.g. rancher). Treat the failed plutil as 'not installed' (return null) so the edition loop can fall through to the next edition or report the tool as not installed. Extract the shared plutil logic into getMacAppVersion(). Update the two macOS 'app missing' unit tests to simulate the real failure (a non-zero exit, i.e. a thrown IllegalStateException) rather than a null return.
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.
This PR fixes #2373
Two commits building on #2306, unifying edition resolution for native-package (Linux/macOS)
installs into a single model that behaves consistently across all OSes.
Implemented changes:
GlobalToolCommandlet - one edition-resolution loop
computeInstalledEditionAndVersion() no longer has per-OS if (isLinux) / if (isWindows) branches. It now loops over the tool's editions once and reports the first that's actually installed:
[this.tool]).
This replaces the old behavior where Linux always reported the tool name as the edition (placeholder), even for multi-edition tools.
Docker - declare editions, resolve per OS
both are present).
isDockerInstalled/isRancherDesktopInstalled guards — Rancher is now detected through the same
native-package/bundle path as everything else.
Test instructions
Unit tests (fast, no Docker needed)
Run the two test classes:
cd CLI && mvn -o test -Dtest=GlobalToolCommandletTest,DockerTest
They cover the new behavior directly:
real edition from getEditionNames(), not the tool-name placeholder.
version from the rancher-desktop package (Docker's apt source not consulted).
version from Rancher Desktop.app (falling through when Docker Desktop is absent).
(no exception).
Manual test - Docker/Rancher edition + version
Run after install on each OS:
Expected: edition
dockerorrancher, version = the installed one, exit 0.If neither is installed:
Tool docker is not installed., exit 1.Windows
Windows
Install one or both (run the terminal as Administrator - winget install needs elevation):
winget install --id Docker.DockerDesktop
winget install --id SUSE.RancherDesktop
2. Verify the registry key exists (Git Bash mis-quotes the space; use PowerShell):
powershell -NoProfile -Command "(Get-ItemProperty
'HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall\Docker Desktop').DisplayVersion"
3. Run the two
idecommands.- Docker only →
docker/ that DisplayVersion- Rancher only →
rancher/ that DisplayVersion- Both →
docker(probed first)macOS
idecommands.docker/ CFBundleShortVersionString of Docker.apprancher/ version of Rancher Desktop.appnot installedLinux
idecommands.docker/ apt versionrancher/ package-manager versionnot installedChecklist for this PR
Make sure everything is checked before merging this PR. For further info please also see
our DoD.
mvn clean testlocally all tests pass and build is successful#«issue-id»: «brief summary»(e.g.#921: fixed setup.batand notfeature/921 fixed setup.bat). If no issue ID exists, title only.In Progressand assigned to you or there is no issue (might happen for very small PRs)with
internalpom.xmlfiles or otherwise if runtime dependencies changed, you have updated our LICENSE.asciidocChecklist for tool commandlets
Have you added a new
«tool»as commandlet? There are the following additional checks:«tool»«TOOL»_VERSIONand«TOOL»_EDITIONare honored by your commandlet