From 3ddcdd7586eb92b5ff77c9d907f1326c71e1b213 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Hohwiller?= Date: Tue, 24 Sep 2024 16:36:59 +0200 Subject: [PATCH] #650: various improvements (git, processContext, step success) (#645) --- CHANGELOG.adoc | 1 + .../tools/ide/context/AbstractIdeContext.java | 30 ++-- .../devonfw/tools/ide/context/GitContext.java | 12 ++ .../tools/ide/context/GitContextImpl.java | 153 +++++++----------- .../tools/ide/process/ProcessResultImpl.java | 13 +- .../com/devonfw/tools/ide/step/StepImpl.java | 2 +- .../ide/commandlet/UpdateCommandletTest.java | 2 +- .../tools/ide/context/GitContextMock.java | 12 ++ .../ide/process/ProcessContextImplTest.java | 4 +- .../tool/androidstudio/AndroidStudioTest.java | 2 +- .../tools/ide/tool/intellij/IntellijTest.java | 2 +- 11 files changed, 115 insertions(+), 118 deletions(-) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 60b905066..bd742fe61 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -22,6 +22,7 @@ Release with new features and bugfixes: * https://github.com/devonfw/IDEasy/issues/612[#612]: Automatically generated issue URL is still pointing to ide instead of IDEasy * https://github.com/devonfw/IDEasy/issues/52[#52]: Adjusting Intellij settings in ide-settings * https://github.com/devonfw/IDEasy/issues/588[#588]: ide create installs wrong Java version +* https://github.com/devonfw/IDEasy/issues/650[#650]: Improve default success message of step The full list of changes for this release can be found in https://github.com/devonfw/IDEasy/milestone/13?closed=1[milestone 2024.09.002]. diff --git a/cli/src/main/java/com/devonfw/tools/ide/context/AbstractIdeContext.java b/cli/src/main/java/com/devonfw/tools/ide/context/AbstractIdeContext.java index e8ccdc4d7..a7bbd5cab 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/context/AbstractIdeContext.java +++ b/cli/src/main/java/com/devonfw/tools/ide/context/AbstractIdeContext.java @@ -122,6 +122,8 @@ public abstract class AbstractIdeContext implements IdeContext { private StepImpl currentStep; + private Boolean online; + /** * The constructor. * @@ -544,20 +546,22 @@ public boolean isOfflineMode() { @Override public boolean isOnline() { - boolean online = false; - try { - int timeout = 1000; - //open a connection to github.com and try to retrieve data - //getContent fails if there is no connection - URLConnection connection = new URL("https://www.github.com").openConnection(); - connection.setConnectTimeout(timeout); - connection.getContent(); - online = true; - - } catch (Exception ignored) { - + if (this.online == null) { + // we currently assume we have only a CLI process that runs shortly + // therefore we run this check only once to save resources when this method is called many times + try { + int timeout = 1000; + //open a connection to github.com and try to retrieve data + //getContent fails if there is no connection + URLConnection connection = new URL("https://www.github.com").openConnection(); + connection.setConnectTimeout(timeout); + connection.getContent(); + this.online = Boolean.TRUE; + } catch (Exception ignored) { + this.online = Boolean.FALSE; + } } - return online; + return this.online.booleanValue(); } @Override diff --git a/cli/src/main/java/com/devonfw/tools/ide/context/GitContext.java b/cli/src/main/java/com/devonfw/tools/ide/context/GitContext.java index 2674fb0fe..99754889c 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/context/GitContext.java +++ b/cli/src/main/java/com/devonfw/tools/ide/context/GitContext.java @@ -203,4 +203,16 @@ default void reset(Path targetRepository, String branch) { */ String retrieveGitUrl(Path repository); + /** + * @param repository the {@link Path} to the folder where the git repository is located. + * @return the name of the current branch. + */ + String determineCurrentBranch(Path repository); + + /** + * @param repository the {@link Path} to the folder where the git repository is located. + * @return the name of the default origin. + */ + String determineRemote(Path repository); + } diff --git a/cli/src/main/java/com/devonfw/tools/ide/context/GitContextImpl.java b/cli/src/main/java/com/devonfw/tools/ide/context/GitContextImpl.java index 326b1637b..263b78e38 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/context/GitContextImpl.java +++ b/cli/src/main/java/com/devonfw/tools/ide/context/GitContextImpl.java @@ -3,12 +3,8 @@ import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; -import java.util.AbstractMap; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Objects; -import java.util.Optional; import com.devonfw.tools.ide.cli.CliOfflineException; import com.devonfw.tools.ide.process.ProcessContext; @@ -34,7 +30,7 @@ public class GitContextImpl implements GitContext { public GitContextImpl(IdeContext context) { this.context = context; - this.processContext = this.context.newProcess().executable("git").withEnvVar("GIT_TERMINAL_PROMPT", "0"); + this.processContext = this.context.newProcess().executable("git").withEnvVar("GIT_TERMINAL_PROMPT", "0").errorHandling(ProcessErrorHandling.WARNING); } @Override @@ -56,20 +52,14 @@ public boolean fetchIfNeeded(Path targetRepository, String remote, String branch } @Override - public boolean isRepositoryUpdateAvailable(Path targetRepository) { + public boolean isRepositoryUpdateAvailable(Path repository) { - this.processContext.directory(targetRepository); - ProcessResult result; - - // get local commit code - result = this.processContext.addArg("rev-parse").addArg("HEAD").run(PROCESS_MODE_FOR_FETCH); + ProcessResult result = this.processContext.directory(repository).addArg("rev-parse").addArg("HEAD").run(PROCESS_MODE_FOR_FETCH); if (!result.isSuccessful()) { this.context.warning("Failed to get the local commit hash."); return false; } - - String local_commit_code = result.getOut().stream().findFirst().orElse("").trim(); - + String localCommitHash = result.getOut().stream().findFirst().orElse("").trim(); // get remote commit code result = this.processContext.addArg("rev-parse").addArg("@{u}").run(PROCESS_MODE_FOR_FETCH); if (!result.isSuccessful()) { @@ -77,53 +67,45 @@ public boolean isRepositoryUpdateAvailable(Path targetRepository) { return false; } String remote_commit_code = result.getOut().stream().findFirst().orElse("").trim(); - return !local_commit_code.equals(remote_commit_code); + return !localCommitHash.equals(remote_commit_code); } @Override - public void pullOrCloneAndResetIfNeeded(String repoUrl, Path targetRepository, String branch, String remoteName) { + public void pullOrCloneAndResetIfNeeded(String repoUrl, Path repository, String branch, String remoteName) { - pullOrCloneIfNeeded(repoUrl, branch, targetRepository); + pullOrCloneIfNeeded(repoUrl, branch, repository); - reset(targetRepository, "master", remoteName); + reset(repository, "master", remoteName); - cleanup(targetRepository); + cleanup(repository); } @Override - public void pullOrClone(String gitRepoUrl, Path targetRepository) { + public void pullOrClone(String gitRepoUrl, Path repository) { - pullOrClone(gitRepoUrl, targetRepository, null); + pullOrClone(gitRepoUrl, repository, null); } @Override - public void pullOrClone(String gitRepoUrl, Path targetRepository, String branch) { + public void pullOrClone(String gitRepoUrl, Path repository, String branch) { - Objects.requireNonNull(targetRepository); + Objects.requireNonNull(repository); Objects.requireNonNull(gitRepoUrl); - if (!gitRepoUrl.startsWith("http")) { throw new IllegalArgumentException("Invalid git URL '" + gitRepoUrl + "'!"); } - - if (Files.isDirectory(targetRepository.resolve(GIT_FOLDER))) { + if (Files.isDirectory(repository.resolve(GIT_FOLDER))) { // checks for remotes - this.processContext.directory(targetRepository); - ProcessResult result = this.processContext.addArg("remote").run(ProcessMode.DEFAULT_CAPTURE); - List remotes = result.getOut(); - if (remotes.isEmpty()) { - String message = targetRepository + " is a local git repository with no remote - if you did this for testing, you may continue...\n" + String remote = determineRemote(repository); + if (remote == null) { + String message = repository + " is a local git repository with no remote - if you did this for testing, you may continue...\n" + "Do you want to ignore the problem and continue anyhow?"; this.context.askToContinue(message); } else { - this.processContext.errorHandling(ProcessErrorHandling.WARNING); - - if (!this.context.isOffline()) { - pull(targetRepository); - } + pull(repository); } } else { - clone(new GitUrl(gitRepoUrl, branch), targetRepository); + clone(new GitUrl(gitRepoUrl, branch), repository); } } @@ -185,89 +167,66 @@ public void clone(GitUrl gitRepoUrl, Path targetRepository) { } @Override - public void pull(Path targetRepository) { - - this.processContext.directory(targetRepository); - ProcessResult result; - // pull from remote - result = this.processContext.addArg("--no-pager").addArg("pull").addArg("--quiet").run(PROCESS_MODE); + public void pull(Path repository) { + if (this.context.isOffline()) { + this.context.info("Skipping git pull on {} because offline", repository); + return; + } + ProcessResult result = this.processContext.directory(repository).addArg("--no-pager").addArg("pull").addArg("--quiet").run(PROCESS_MODE); if (!result.isSuccessful()) { - Map remoteAndBranchName = retrieveRemoteAndBranchName(); - this.context.warning("Git pull for {}/{} failed for repository {}.", remoteAndBranchName.get("remote"), remoteAndBranchName.get("branch"), - targetRepository); - handleErrors(targetRepository, result); + String branchName = determineCurrentBranch(repository); + this.context.warning("Git pull on branch {} failed for repository {}.", branchName, repository); + handleErrors(repository, result); } } @Override public void fetch(Path targetRepository, String remote, String branch) { - if ((remote == null) || (branch == null)) { - Optional remoteAndBranchOpt = getLocalRemoteAndBranch(targetRepository); - if (remoteAndBranchOpt.isEmpty()) { - context.warning("Could not determine the remote or branch for the git repository at {}", targetRepository); - return; // false; - } - String[] remoteAndBranch = remoteAndBranchOpt.get(); - if (remote == null) { - remote = remoteAndBranch[0]; - } - if (branch == null) { - branch = remoteAndBranch[1]; - } + if (branch == null) { + branch = determineCurrentBranch(targetRepository); } - - this.processContext.directory(targetRepository); - ProcessResult result; - - result = this.processContext.addArg("fetch").addArg(remote).addArg(branch).run(PROCESS_MODE_FOR_FETCH); + if (remote == null) { + remote = determineRemote(targetRepository); + } + ProcessResult result = this.processContext.directory(targetRepository).addArg("fetch").addArg(remote).addArg(branch).run(PROCESS_MODE_FOR_FETCH); if (!result.isSuccessful()) { this.context.warning("Git fetch for '{}/{} failed.'.", remote, branch); } } - private Map retrieveRemoteAndBranchName() { - - Map remoteAndBranchName = new HashMap<>(); - ProcessResult remoteResult = this.processContext.addArg("branch").addArg("-vv").run(PROCESS_MODE); - List remotes = remoteResult.getOut(); - if (!remotes.isEmpty()) { - for (String remote : remotes) { - if (remote.startsWith("*")) { - String checkedOutBranch = remote.substring(remote.indexOf("[") + 1, remote.indexOf("]")); - remoteAndBranchName.put("remote", checkedOutBranch.substring(0, checkedOutBranch.indexOf("/"))); - // check if current repo is behind remote and omit message - if (checkedOutBranch.contains(":")) { - remoteAndBranchName.put("branch", checkedOutBranch.substring(checkedOutBranch.indexOf("/") + 1, checkedOutBranch.indexOf(":"))); - } else { - remoteAndBranchName.put("branch", checkedOutBranch.substring(checkedOutBranch.indexOf("/") + 1)); - } - - } + @Override + public String determineCurrentBranch(Path repository) { + + ProcessResult remoteResult = this.processContext.directory(repository).addArg("branch").addArg("--show-current").run(ProcessMode.DEFAULT_CAPTURE); + if (remoteResult.isSuccessful()) { + List remotes = remoteResult.getOut(); + if (!remotes.isEmpty()) { + assert (remotes.size() == 1); + return remotes.get(0); } } else { - return Map.ofEntries(new AbstractMap.SimpleEntry<>("remote", "unknown"), new AbstractMap.SimpleEntry<>("branch", "unknown")); + this.context.warning("Failed to determine current branch of git repository {}", repository); } - - return remoteAndBranchName; + return null; } - private Optional getLocalRemoteAndBranch(Path repositoryPath) { - - this.processContext.directory(repositoryPath); - ProcessResult result = this.processContext.addArg("rev-parse").addArg("--abbrev-ref").addArg("--symbolic-full-name").addArg("@{u}") - .run(PROCESS_MODE_FOR_FETCH); - if (result.isSuccessful()) { - String upstream = result.getOut().stream().findFirst().orElse(""); - if (upstream.contains("/")) { - return Optional.of(upstream.split("/", 2)); // Split into remote and branch + @Override + public String determineRemote(Path repository) { + + ProcessResult remoteResult = this.processContext.directory(repository).addArg("remote").run(ProcessMode.DEFAULT_CAPTURE); + if (remoteResult.isSuccessful()) { + List remotes = remoteResult.getOut(); + if (!remotes.isEmpty()) { + assert (remotes.size() == 1); + return remotes.get(0); } } else { - this.context.warning("Failed to determine the remote tracking branch."); + this.context.warning("Failed to determine current origin of git repository {}", repository); } - return Optional.empty(); + return null; } @Override diff --git a/cli/src/main/java/com/devonfw/tools/ide/process/ProcessResultImpl.java b/cli/src/main/java/com/devonfw/tools/ide/process/ProcessResultImpl.java index 88bea84fb..968d82354 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/process/ProcessResultImpl.java +++ b/cli/src/main/java/com/devonfw/tools/ide/process/ProcessResultImpl.java @@ -1,5 +1,6 @@ package com.devonfw.tools.ide.process; +import java.util.Collections; import java.util.List; /** @@ -24,8 +25,16 @@ public ProcessResultImpl(int exitCode, List out, List err) { super(); this.exitCode = exitCode; - this.out = out; - this.err = err; + if (out == null) { + this.out = Collections.emptyList(); + } else { + this.out = out; + } + if (err == null) { + this.err = Collections.emptyList(); + } else { + this.err = err; + } } @Override diff --git a/cli/src/main/java/com/devonfw/tools/ide/step/StepImpl.java b/cli/src/main/java/com/devonfw/tools/ide/step/StepImpl.java index cb8db2b32..56c0dc3d8 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/step/StepImpl.java +++ b/cli/src/main/java/com/devonfw/tools/ide/step/StepImpl.java @@ -156,7 +156,7 @@ private void end(Boolean newSuccess, Throwable error, boolean suppress, String m if (message != null) { this.context.success(message, args); } else if (!this.silent) { - this.context.success(this.name); + this.context.success("Successfully ended step '{}'.", this.name); } this.context.debug("Step '{}' ended successfully.", this.name); } else { diff --git a/cli/src/test/java/com/devonfw/tools/ide/commandlet/UpdateCommandletTest.java b/cli/src/test/java/com/devonfw/tools/ide/commandlet/UpdateCommandletTest.java index 6c3746a30..ff651acbc 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/commandlet/UpdateCommandletTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/commandlet/UpdateCommandletTest.java @@ -80,6 +80,6 @@ public void testRunUpdateSoftwareDoesNotFailOnFailedSoftwareInstallations() { assertThat(context).logAtError().hasMessage("Installation of java failed!"); assertThat(context).logAtError().hasMessage("Installation of mvn failed!"); assertThat(context).logAtSuccess().hasMessage("Successfully updated settings repository."); - assertThat(context).logAtSuccess().hasMessage("Install or update software"); + assertThat(context).logAtSuccess().hasMessageContaining("Install or update software"); } } diff --git a/cli/src/test/java/com/devonfw/tools/ide/context/GitContextMock.java b/cli/src/test/java/com/devonfw/tools/ide/context/GitContextMock.java index df6176b5d..cb5cdcca8 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/context/GitContextMock.java +++ b/cli/src/test/java/com/devonfw/tools/ide/context/GitContextMock.java @@ -77,4 +77,16 @@ public boolean isRepositoryUpdateAvailable(Path targetRepository) { return false; } + + @Override + public String determineCurrentBranch(Path repository) { + + return "main"; + } + + @Override + public String determineRemote(Path repository) { + + return "origin"; + } } diff --git a/cli/src/test/java/com/devonfw/tools/ide/process/ProcessContextImplTest.java b/cli/src/test/java/com/devonfw/tools/ide/process/ProcessContextImplTest.java index 5664f531e..6c0b8806e 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/process/ProcessContextImplTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/process/ProcessContextImplTest.java @@ -168,8 +168,8 @@ public void enablingBackgroundProcessShouldNotBeAwaitedAndShouldNotPassStreams(P verify(this.processMock, never()).waitFor(); - assertThat(result.getOut()).isNull(); - assertThat(result.getErr()).isNull(); + assertThat(result.getOut()).isEmpty(); + assertThat(result.getErr()).isEmpty(); } diff --git a/cli/src/test/java/com/devonfw/tools/ide/tool/androidstudio/AndroidStudioTest.java b/cli/src/test/java/com/devonfw/tools/ide/tool/androidstudio/AndroidStudioTest.java index 64ef14d09..d50fe9ec9 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/tool/androidstudio/AndroidStudioTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/tool/androidstudio/AndroidStudioTest.java @@ -81,7 +81,7 @@ private void checkInstallation(IdeTestContext context) { // commandlet - android-studio assertThat(context.getSoftwarePath().resolve("android-studio/.ide.software.version")).exists().hasContent("2024.1.1.1"); assertThat(context).logAtSuccess().hasMessage("Successfully installed android-studio in version 2024.1.1.1"); - assertThat(context).logAtSuccess().hasMessage("Install plugin: mockedPlugin"); + assertThat(context).logAtSuccess().hasMessageContaining("Install plugin: mockedPlugin"); assertThat(context.getPluginsPath().resolve("android-studio").resolve("mockedPlugin").resolve("MockedClass.class")).exists(); } diff --git a/cli/src/test/java/com/devonfw/tools/ide/tool/intellij/IntellijTest.java b/cli/src/test/java/com/devonfw/tools/ide/tool/intellij/IntellijTest.java index 60f0efec2..d21a865bb 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/tool/intellij/IntellijTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/tool/intellij/IntellijTest.java @@ -143,7 +143,7 @@ private void checkInstallation(IdeTestContext context) { assertThat(context.getSoftwarePath().resolve("intellij/.ide.software.version")).exists().hasContent("2023.3.3"); assertThat(context).logAtSuccess().hasEntries("Successfully installed java in version 17.0.10_7", "Successfully installed intellij in version 2023.3.3", - "Install plugin: mockedPlugin"); + "Successfully ended step 'Install plugin: mockedPlugin'."); assertThat(context.getPluginsPath().resolve("intellij").resolve("mockedPlugin").resolve("MockedClass.class")).exists(); }