#650: various improvements (git, processContext, step success) (#645)

Author: hohwille

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].
 

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

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);
+
 }

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,74 +52,60 @@ 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()) {
       this.context.warning("Failed to get the remote commit hash.");
       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<String> 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<String, String> 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<String[]> 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<String, String> retrieveRemoteAndBranchName() {
-
-    Map<String, String> remoteAndBranchName = new HashMap<>();
-    ProcessResult remoteResult = this.processContext.addArg("branch").addArg("-vv").run(PROCESS_MODE);
-    List<String> 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<String> 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<String[]> 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<String> 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

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<String> out, List<String> 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

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 {

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");
   }
 }

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";
+  }
 }

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();
 
   }
 

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();
   }