#347: Change copy method behavior to be consistent (#359)

devonfw · May 28, 2024 · 1e650d7 · 1e650d7
1 parent e2ab777
commit 1e650d7
Show file tree

Hide file tree

Showing 6 changed files with 101 additions and 130 deletions.
diff --git a/cli/src/main/java/com/devonfw/tools/ide/commandlet/AbstractUpdateCommandlet.java b/cli/src/main/java/com/devonfw/tools/ide/commandlet/AbstractUpdateCommandlet.java
@@ -1,11 +1,5 @@
 package com.devonfw.tools.ide.commandlet;
 
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
-
 import com.devonfw.tools.ide.context.GitContext;
 import com.devonfw.tools.ide.context.IdeContext;
 import com.devonfw.tools.ide.property.StringProperty;
@@ -15,6 +9,12 @@
 import com.devonfw.tools.ide.tool.ToolCommandlet;
 import com.devonfw.tools.ide.variable.IdeVariables;
 
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
 /**
  * Abstract {@link Commandlet} base-class for both {@link UpdateCommandlet} and {@link CreateCommandlet}.
  */
@@ -78,8 +78,8 @@ private void setupConf(Path template, Path conf) {
           this.context.debug("Configuration {} already exists - skipping to copy from {}", confPath, child);
         } else {
           if (!basename.equals("settings.xml")) {
-            this.context.info("Copying template {} to {}.", child, confPath);
-            this.context.getFileAccess().copy(child, confPath);
+            this.context.info("Copying template {} to {}.", child, conf);
+            this.context.getFileAccess().copy(child, conf);
           }
         }
       }
@@ -104,8 +104,8 @@ private void updateSettings() {
           String message = "Missing your settings at " + settingsPath + " and no SETTINGS_URL is defined.\n"
               + "Further details can be found here: https://github.com/devonfw/IDEasy/blob/main/documentation/settings.asciidoc\n"
               + "Please contact the technical lead of your project to get the SETTINGS_URL for your project.\n"
-              + "In case you just want to test IDEasy you may simply hit return to install the default settings.\n"
-              + "Settings URL [" + IdeContext.DEFAULT_SETTINGS_REPO_URL + "]:";
+              + "In case you just want to test IDEasy you may simply hit return to install the default settings.\n" + "Settings URL ["
+              + IdeContext.DEFAULT_SETTINGS_REPO_URL + "]:";
           repository = this.context.askForInput(message, IdeContext.DEFAULT_SETTINGS_REPO_URL);
         } else if ("-".equals(repository)) {
           repository = IdeContext.DEFAULT_SETTINGS_REPO_URL;
@@ -127,8 +127,7 @@ private void updateSoftware() {
       Set<ToolCommandlet> toolCommandlets = new HashSet<>();
 
       // installed tools in IDE_HOME/software
-      List<Path> softwares = this.context.getFileAccess().listChildren(this.context.getSoftwarePath(),
-          Files::isDirectory);
+      List<Path> softwares = this.context.getFileAccess().listChildren(this.context.getSoftwarePath(), Files::isDirectory);
       for (Path software : softwares) {
         String toolName = software.getFileName().toString();
         ToolCommandlet toolCommandlet = this.context.getCommandletManager().getToolCommandletOrNull(toolName);

diff --git a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java
@@ -282,18 +282,20 @@ public void move(Path source, Path targetDir) {
   @Override
   public void copy(Path source, Path target, FileCopyMode mode) {
 
+    if (mode != FileCopyMode.COPY_TREE_CONTENT) {
+      // if we want to copy the file or folder "source" to the existing folder "target" in a shell this will copy 
+      // source into that folder so that we as a result have a copy in "target/source".
+      // With Java NIO the raw copy method will fail as we cannot copy "source" to the path of the "target" folder.
+      // For folders we want the same behavior as the linux "cp -r" command so that the "source" folder is copied
+      // and not only its content what also makes it consistent with the move method that also behaves this way.
+      // Therefore we need to add the filename (foldername) of "source" to the "target" path before.
+      // For the rare cases, where we want to copy the content of a folder (cp -r source/* target) we support
+      // it via the COPY_TREE_CONTENT mode.
+      target = target.resolve(source.getFileName());
+    }
     boolean fileOnly = mode.isFileOnly();
     if (fileOnly) {
       this.context.debug("Copying file {} to {}", source, target);
-      if (Files.isDirectory(target)) {
-        // if we want to copy "file.txt" to the existing folder "path/to/folder/" in a shell this will copy "file.txt"
-        // into that folder
-        // with Java NIO the raw copy method will fail as we cannot copy the file to the path of the target folder
-        // even worse if FileCopyMode is override the target folder ("path/to/folder/") would be deleted and the result
-        // of our "file.txt" would later appear in "path/to/folder". To prevent such bugs we append the filename to
-        // target
-        target = target.resolve(source.getFileName());
-      }
     } else {
       this.context.debug("Copying {} recursively to {}", source, target);
     }
@@ -501,12 +503,8 @@ public void extract(Path archiveFile, Path targetDir, Consumer<Path> postExtract
 
     if (Files.isDirectory(archiveFile)) {
       Path properInstallDir = archiveFile; // getProperInstallationSubDirOf(archiveFile, archiveFile);
-      if (extract) {
-        this.context.warning("Found directory for download at {} hence copying without extraction!", archiveFile);
-        copy(properInstallDir, targetDir, FileCopyMode.COPY_TREE_OVERRIDE_TREE);
-      } else {
-        move(properInstallDir, targetDir);
-      }
+      this.context.warning("Found directory for download at {} hence copying without extraction!", archiveFile);
+      copy(properInstallDir, targetDir, FileCopyMode.COPY_TREE_CONTENT);
       postExtractHook(postExtractHook, properInstallDir);
       return;
     } else if (!extract) {

diff --git a/cli/src/main/java/com/devonfw/tools/ide/io/FileCopyMode.java b/cli/src/main/java/com/devonfw/tools/ide/io/FileCopyMode.java
@@ -1,14 +1,12 @@
 package com.devonfw.tools.ide.io;
 
 /**
- * {@link Enum} with the available modes to
- * {@link FileAccess#copy(java.nio.file.Path, java.nio.file.Path, FileCopyMode) copy} files and folders.
+ * {@link Enum} with the available modes to {@link FileAccess#copy(java.nio.file.Path, java.nio.file.Path, FileCopyMode) copy} files and folders.
  */
 public enum FileCopyMode {
 
   /**
-   * Copy {@link #isFileOnly() only a single file} and
-   * {@link #isFailIfExists() fail if the target-file already exists}.
+   * Copy {@link #isFileOnly() only a single file} and {@link #isFailIfExists() fail if the target-file already exists}.
    */
   COPY_FILE_FAIL_IF_EXISTS,
 
@@ -22,23 +20,26 @@ public enum FileCopyMode {
   COPY_TREE_OVERRIDE_FILES,
 
   /**
-   * Copy {@link #isRecursive() recursively} and {@link FileAccess#delete(java.nio.file.Path) delete} the target-file if
-   * it exists before copying.
+   * Copy {@link #isRecursive() recursively} and {@link FileAccess#delete(java.nio.file.Path) delete} the target-file if it exists before copying.
    */
-  COPY_TREE_OVERRIDE_TREE;
+  COPY_TREE_OVERRIDE_TREE,
 
   /**
-   * @return {@code true} if only a single file shall be copied. Will fail if a directory is given to copy,
-   * {@code false} otherwise (to copy folders recursively).
+   * Copy {@link #isRecursive() recursively} and appends the file name to the target.
+   */
+  COPY_TREE_CONTENT;
+
+  /**
+   * @return {@code true} if only a single file shall be copied. Will fail if a directory is given to copy, {@code false} otherwise (to copy folders
+   * recursively).
    */
   public boolean isFileOnly() {
 
     return (this == COPY_FILE_FAIL_IF_EXISTS) || (this == COPY_FILE_OVERRIDE);
   }
 
   /**
-   * @return {@code true} if files and folders shall be copied recursively, {@code false} otherwise
-   * ({@link #isFileOnly() copy file copy}).
+   * @return {@code true} if files and folders shall be copied recursively, {@code false} otherwise ({@link #isFileOnly() copy file copy}).
    */
   public boolean isRecursive() {
 

diff --git a/cli/src/main/java/com/devonfw/tools/ide/tool/LocalToolCommandlet.java b/cli/src/main/java/com/devonfw/tools/ide/tool/LocalToolCommandlet.java
@@ -1,11 +1,5 @@
 package com.devonfw.tools.ide.tool;
 
-import java.io.IOException;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.StandardOpenOption;
-import java.util.Set;
-
 import com.devonfw.tools.ide.common.Tag;
 import com.devonfw.tools.ide.context.IdeContext;
 import com.devonfw.tools.ide.io.FileAccess;
@@ -15,6 +9,12 @@
 import com.devonfw.tools.ide.step.Step;
 import com.devonfw.tools.ide.version.VersionIdentifier;
 
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
+import java.util.Set;
+
 /**
  * {@link ToolCommandlet} that is installed locally into the IDE.
  */
@@ -25,8 +25,7 @@ public abstract class LocalToolCommandlet extends ToolCommandlet {
    *
    * @param context the {@link IdeContext}.
    * @param tool the {@link #getName() tool name}.
-   * @param tags the {@link #getTags() tags} classifying the tool. Should be created via {@link Set#of(Object) Set.of}
-   * method.
+   * @param tags the {@link #getTags() tags} classifying the tool. Should be created via {@link Set#of(Object) Set.of} method.
    */
   public LocalToolCommandlet(IdeContext context, String tool, Set<Tag> tags) {
 
@@ -42,14 +41,12 @@ public Path getToolPath() {
   }
 
   /**
-   * @return the {@link Path} where the executables of the tool can be found. Typically a "bin" folder inside
-   * {@link #getToolPath() tool path}.
+   * @return the {@link Path} where the executables of the tool can be found. Typically a "bin" folder inside {@link #getToolPath() tool path}.
    */
   public Path getToolBinPath() {
 
     Path toolPath = getToolPath();
-    Path binPath = this.context.getFileAccess().findFirst(toolPath, path -> path.getFileName().toString().equals("bin"),
-        false);
+    Path binPath = this.context.getFileAccess().findFirst(toolPath, path -> path.getFileName().toString().equals("bin"), false);
     if ((binPath != null) && Files.isDirectory(binPath)) {
       return binPath;
     }
@@ -70,8 +67,7 @@ protected boolean doInstall(boolean silent) {
       VersionIdentifier resolvedVersion = installation.resolvedVersion();
       if (resolvedVersion.equals(installedVersion) && !installation.newInstallation()) {
         IdeLogLevel level = silent ? IdeLogLevel.DEBUG : IdeLogLevel.INFO;
-        this.context.level(level).log("Version {} of tool {} is already installed", installedVersion,
-            getToolWithEdition());
+        this.context.level(level).log("Version {} of tool {} is already installed", installedVersion, getToolWithEdition());
         step.success();
         return false;
       }
@@ -88,8 +84,7 @@ protected boolean doInstall(boolean silent) {
       if (installedVersion == null) {
         step.success("Successfully installed {} in version {}", this.tool, resolvedVersion);
       } else {
-        step.success("Successfully installed {} in version {} replacing previous version {}", this.tool,
-            resolvedVersion, installedVersion);
+        step.success("Successfully installed {} in version {} replacing previous version {}", this.tool, resolvedVersion, installedVersion);
       }
       return true;
     } catch (RuntimeException e) {
@@ -102,12 +97,10 @@ protected boolean doInstall(boolean silent) {
   }
 
   /**
-   * Performs the installation of the {@link #getName() tool} managed by this
-   * {@link com.devonfw.tools.ide.commandlet.Commandlet} only in the central software repository without touching the
-   * IDE installation.
+   * Performs the installation of the {@link #getName() tool} managed by this {@link com.devonfw.tools.ide.commandlet.Commandlet} only in the central software
+   * repository without touching the IDE installation.
    *
-   * @param version the {@link VersionIdentifier} requested to be installed. May also be a
-   * {@link VersionIdentifier#isPattern() version pattern}.
+   * @param version the {@link VersionIdentifier} requested to be installed. May also be a {@link VersionIdentifier#isPattern() version pattern}.
    * @return the {@link ToolInstallation} in the central software repository matching the given {@code version}.
    */
   public ToolInstallation installInRepo(VersionIdentifier version) {
@@ -116,12 +109,10 @@ public ToolInstallation installInRepo(VersionIdentifier version) {
   }
 
   /**
-   * Performs the installation of the {@link #getName() tool} managed by this
-   * {@link com.devonfw.tools.ide.commandlet.Commandlet} only in the central software repository without touching the
-   * IDE installation.
+   * Performs the installation of the {@link #getName() tool} managed by this {@link com.devonfw.tools.ide.commandlet.Commandlet} only in the central software
+   * repository without touching the IDE installation.
    *
-   * @param version the {@link VersionIdentifier} requested to be installed. May also be a
-   * {@link VersionIdentifier#isPattern() version pattern}.
+   * @param version the {@link VersionIdentifier} requested to be installed. May also be a {@link VersionIdentifier#isPattern() version pattern}.
    * @param edition the specific edition to install.
    * @return the {@link ToolInstallation} in the central software repository matching the given {@code version}.
    */
@@ -131,30 +122,27 @@ public ToolInstallation installInRepo(VersionIdentifier version, String edition)
   }
 
   /**
-   * Performs the installation of the {@link #getName() tool} managed by this
-   * {@link com.devonfw.tools.ide.commandlet.Commandlet} only in the central software repository without touching the
-   * IDE installation.
+   * Performs the installation of the {@link #getName() tool} managed by this {@link com.devonfw.tools.ide.commandlet.Commandlet} only in the central software
+   * repository without touching the IDE installation.
    *
-   * @param version the {@link VersionIdentifier} requested to be installed. May also be a
-   * {@link VersionIdentifier#isPattern() version pattern}.
+   * @param version the {@link VersionIdentifier} requested to be installed. May also be a {@link VersionIdentifier#isPattern() version pattern}.
    * @param edition the specific edition to install.
    * @param toolRepository the {@link ToolRepository} to use.
    * @return the {@link ToolInstallation} in the central software repository matching the given {@code version}.
    */
   public ToolInstallation installInRepo(VersionIdentifier version, String edition, ToolRepository toolRepository) {
 
     VersionIdentifier resolvedVersion = toolRepository.resolveVersion(this.tool, edition, version);
-    Path toolPath = this.context.getSoftwareRepositoryPath().resolve(toolRepository.getId()).resolve(this.tool)
-        .resolve(edition).resolve(resolvedVersion.toString());
+    Path toolPath = this.context.getSoftwareRepositoryPath().resolve(toolRepository.getId()).resolve(this.tool).resolve(edition)
+        .resolve(resolvedVersion.toString());
     Path toolVersionFile = toolPath.resolve(IdeContext.FILE_SOFTWARE_VERSION);
     FileAccess fileAccess = this.context.getFileAccess();
     if (Files.isDirectory(toolPath)) {
       if (Files.exists(toolVersionFile)) {
         if (this.context.isForceMode()) {
           fileAccess.delete(toolPath);
         } else {
-          this.context.debug("Version {} of tool {} is already installed at {}", resolvedVersion,
-              getToolWithEdition(this.tool, edition), toolPath);
+          this.context.debug("Version {} of tool {} is already installed at {}", resolvedVersion, getToolWithEdition(this.tool, edition), toolPath);
           return createToolInstallation(toolPath, resolvedVersion, toolVersionFile);
         }
       } else {
@@ -166,8 +154,7 @@ public ToolInstallation installInRepo(VersionIdentifier version, String edition,
     fileAccess.mkdirs(toolPath.getParent());
     boolean extract = isExtract();
     if (!extract) {
-      this.context.trace("Extraction is disabled for '{}' hence just moving the downloaded file {}.", this.tool,
-          target);
+      this.context.trace("Extraction is disabled for '{}' hence just moving the downloaded file {}.", this.tool, target);
     }
     fileAccess.extract(target, toolPath, this::postExtract, extract);
     try {
@@ -181,17 +168,15 @@ public ToolInstallation installInRepo(VersionIdentifier version, String edition,
   }
 
   /**
-   * Post-extraction hook that can be overridden to add custom processing after unpacking and before moving to the final
-   * destination folder.
+   * Post-extraction hook that can be overridden to add custom processing after unpacking and before moving to the final destination folder.
    *
    * @param extractedDir the {@link Path} to the folder with the unpacked tool.
    */
   protected void postExtract(Path extractedDir) {
 
   }
 
-  private ToolInstallation createToolInstallation(Path rootDir, VersionIdentifier resolvedVersion, Path toolVersionFile,
-      boolean newInstallation) {
+  private ToolInstallation createToolInstallation(Path rootDir, VersionIdentifier resolvedVersion, Path toolVersionFile, boolean newInstallation) {
 
     Path linkDir = getMacOsHelper().findLinkDir(rootDir, this.tool);
     Path binDir = linkDir;
@@ -201,14 +186,12 @@ private ToolInstallation createToolInstallation(Path rootDir, VersionIdentifier
     }
     if (linkDir != rootDir) {
       assert (!linkDir.equals(rootDir));
-      this.context.getFileAccess().copy(toolVersionFile, linkDir.resolve(IdeContext.FILE_SOFTWARE_VERSION),
-          FileCopyMode.COPY_FILE_OVERRIDE);
+      this.context.getFileAccess().copy(toolVersionFile, linkDir, FileCopyMode.COPY_FILE_OVERRIDE);
     }
     return new ToolInstallation(rootDir, linkDir, binDir, resolvedVersion, newInstallation);
   }
 
-  private ToolInstallation createToolInstallation(Path rootDir, VersionIdentifier resolvedVersion,
-      Path toolVersionFile) {
+  private ToolInstallation createToolInstallation(Path rootDir, VersionIdentifier resolvedVersion, Path toolVersionFile) {
 
     return createToolInstallation(rootDir, resolvedVersion, toolVersionFile, false);
   }

diff --git a/cli/src/main/java/com/devonfw/tools/ide/tool/npm/Npm.java b/cli/src/main/java/com/devonfw/tools/ide/tool/npm/Npm.java
@@ -1,15 +1,15 @@
 package com.devonfw.tools.ide.tool.npm;
 
-import java.nio.file.Path;
-import java.util.Set;
-
 import com.devonfw.tools.ide.common.Tag;
 import com.devonfw.tools.ide.context.IdeContext;
 import com.devonfw.tools.ide.io.FileAccess;
 import com.devonfw.tools.ide.tool.LocalToolCommandlet;
 import com.devonfw.tools.ide.tool.ToolCommandlet;
 import com.devonfw.tools.ide.tool.node.Node;
 
+import java.nio.file.Path;
+import java.util.Set;
+
 /**
  * {@link ToolCommandlet} for <a href="https://www.npmjs.com/">npm</a>.
  */
@@ -47,10 +47,10 @@ protected void postExtract(Path extractedDir) {
       fileAccess.delete(nodeHomePath.resolve(npx));
       fileAccess.delete(nodeHomePath.resolve(npx + cmd));
 
-      fileAccess.copy(npmBinBath.resolve(npm), nodeHomePath.resolve(npm));
-      fileAccess.copy(npmBinBath.resolve(npm + cmd), nodeHomePath.resolve(npm + cmd));
-      fileAccess.copy(npmBinBath.resolve(npx), nodeHomePath.resolve(npx));
-      fileAccess.copy(npmBinBath.resolve(npx + cmd), nodeHomePath.resolve(npx + cmd));
+      fileAccess.copy(npmBinBath.resolve(npm), nodeHomePath);
+      fileAccess.copy(npmBinBath.resolve(npm + cmd), nodeHomePath);
+      fileAccess.copy(npmBinBath.resolve(npx), nodeHomePath);
+      fileAccess.copy(npmBinBath.resolve(npx + cmd), nodeHomePath);
     }
   }
 }