Repository navigation
Conversation
…ate-git-as-global-tool # Conflicts: # CHANGELOG.adoc
Coverage Report for CI Build 40359028650Coverage increased (+0.2%) to 75.245%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions252 previously-covered lines in 7 files lost coverage.
Coverage Stats💛 - Coveralls |
- small improvement in log messages
- added support for git on mac-os
- improvements
…it-as-global-tool # Conflicts: # CHANGELOG.adoc # cli/src/main/java/com/devonfw/tools/ide/commandlet/CommandletManagerImpl.java # cli/src/main/java/com/devonfw/tools/ide/tool/GlobalToolCommandlet.java
- added tests for git
quando632
left a comment
There was a problem hiding this comment.
Nice continuation of #2360. The isPackageManagerBasedInstallation() and getInstallerArguments() hooks keep Git's OS-specific behaviour in Git instead of the shared base class, and the DisplayName check in WindowsHelperImpl fixes a real mismatch, since a registry search for "Git" also hits keys like GitHub CLI. The new Windows registry tests and the no-Git fallback tests in GitOperationTest cover the tricky branches well.
| } else if ((urlsPath != null) && Files.isDirectory(urlsPath)) { | ||
| LOG.debug("Git is not available. Using existing URL metadata without updating it."); | ||
| } else { | ||
| gitContext.findGitRequired(); |
There was a problem hiding this comment.
On a machine without Git and without _ide/urls (a fresh Windows setup, the use case of #2273), ide install git resolves its version via getUrls() and lands here. findGitRequired() then aborts with the new hint "Try running ide install git" from GitContextImpl.java:351, so the user is sent in a circle. Please either keep the download hint without the ide install git advice when the urls repository is missing, or provide a way to get the Git metadata without Git.
|
|
||
| if (this.context.getSystemInfo().isLinux()) { | ||
| // on Linux global tools are typically installed via the package manager of the OS | ||
| if (isPackageManagerBasedInstallation()) { |
There was a problem hiding this comment.
This branch conflicts with main: #2500 rewrote computeInstalledEditionAndVersion() so that every non-Windows OS already goes through getInstalledVersionForEdition() and so through getNativePackageVersion(), which already covers macOS for Git. When rebasing, this hunk can be dropped in favour of main's version, and isPackageManagerBasedInstallation() is then only needed in doInstall and resolveVersionForInstall.
| return DEFAULT_WINDOWS_GIT_PATH; | ||
| } | ||
|
|
||
| private boolean isInstallingGit(AbstractCommandlet cmd) { |
There was a problem hiding this comment.
isInstallingGit puts tool-specific knowledge into the generic context and reads InstallCommandlet.tool directly. A generic guard such as getGitContext().findGit() != null around the settings-update check would cover ide install git the same way, without coupling AbstractIdeContext to Git, and would also avoid the same failure for other commands run without Git.
| return null; | ||
| } | ||
|
|
||
| protected List<String> getInstallerArguments() { |
There was a problem hiding this comment.
getInstallerArguments() is a new protected extension point but has no Javadoc, unlike its neighbours such as getNativePackages().
| protected List<String> getInstallerArguments() { | |
| /** | |
| * @return the additional arguments passed to the downloaded installer executable (e.g. silent mode flags). The default is an empty {@link List}. | |
| */ | |
| protected List<String> getInstallerArguments() { |
This PR fixes #2273
This PR continues the work of #2360.
Implemented changes:
aptandzypper).brew).DisplayName.Testing instructions:
Please add conscise, understandable instructions on how a reviewer can test/verify the functionality of your contribution here:
build-local-dev.shide install gitand / oride uninstall gitChecklist 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