From f0ab0950a2b4cb875e60683f6e9d261db57b4db9 Mon Sep 17 00:00:00 2001 From: Matthias Thieroff <93515444+mthieroff@users.noreply.github.com> Date: Wed, 24 Nov 2021 14:23:20 +0100 Subject: [PATCH] Fix move in modify command for windows (#1872) On Windows the path after moving files was not recognized correctly by git, hg and svn. The move resulted in a deletion without adding new files. Therefore we now change the path to unix style before adding files whenever we detect a non unix style file separator. --- .../fix_move_command_for_windows.yaml | 2 ++ .../repository/spi/ModifyWorkerHelper.java | 19 +++++++--- .../repository/spi/HgModifyCommandTest.java | 36 +++++++++---------- 3 files changed, 35 insertions(+), 22 deletions(-) create mode 100644 gradle/changelog/fix_move_command_for_windows.yaml diff --git a/gradle/changelog/fix_move_command_for_windows.yaml b/gradle/changelog/fix_move_command_for_windows.yaml new file mode 100644 index 0000000000..41cff90054 --- /dev/null +++ b/gradle/changelog/fix_move_command_for_windows.yaml @@ -0,0 +1,2 @@ +- type: fixed + description: Move command for Windows ([#1872](https://github.com/scm-manager/scm-manager/pull/1872)) diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/ModifyWorkerHelper.java b/scm-core/src/main/java/sonia/scm/repository/spi/ModifyWorkerHelper.java index f0f0cd046d..6ad96a5eb3 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/ModifyWorkerHelper.java +++ b/scm-core/src/main/java/sonia/scm/repository/spi/ModifyWorkerHelper.java @@ -79,12 +79,23 @@ public interface ModifyWorkerHelper extends ModifyCommand.Worker { Files.createDirectories(targetFile.getParent()); try { Path pathAfterMove = Files.move(getTargetFile(source), targetFile); - doScmMove(source, getWorkDir().toPath().relativize(pathAfterMove).normalize().toString()); - } catch(FileAlreadyExistsException e) { + doScmMove(source, toScmPath(pathAfterMove)); + } catch (FileAlreadyExistsException e) { throw AlreadyExistsException.alreadyExists(ContextEntry.ContextBuilder.entity("File", target).in(getRepository())); } } + /** + * @since 2.28.0 + */ + default String toScmPath(Path pathAfterMove) { + String path = getWorkDir().toPath().relativize(pathAfterMove).normalize().toString(); + if (File.separator.equals("/")) { + return path; + } + return path.replace(File.separator, "/"); + } + /** * @since 2.28.0 */ @@ -100,7 +111,7 @@ public interface ModifyWorkerHelper extends ModifyCommand.Worker { Path targetPath = getTargetFile(path); if (Files.isDirectory(targetPath)) { try (Stream list = Files.list(targetPath)) { - list.forEach(subPath -> addRecursive(path + File.separator + subPath.getFileName())); + list.forEach(subPath -> addRecursive(path + "/" + subPath.getFileName())); } catch (IOException e) { throw new InternalRepositoryException(getRepository(), "Could not add files to scm", e); } @@ -171,7 +182,7 @@ public interface ModifyWorkerHelper extends ModifyCommand.Worker { // mind whether 'path' starts with a '/', the nio api does. // So using the file api the two paths '/file' and 'file' // lead to the same result, whereas the nio api would - // lead to an absolute path starting at the ssytem root in the + // lead to an absolute path starting at the system root in the // first example starting with a '/'. Path targetFile = new File(workDir, path).toPath().normalize(); doThrow() diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgModifyCommandTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgModifyCommandTest.java index f4cff11d33..8ecc0bb18f 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgModifyCommandTest.java +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgModifyCommandTest.java @@ -259,8 +259,8 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { request.setAuthor(new Person("Trillian Astra", "trillian@hitchhiker.com")); hgModifyCommand.execute(request); - assertThat(cmdContext.open().tip().getDeletedFiles().contains("a.txt")).isTrue(); - assertThat(cmdContext.open().tip().getAddedFiles().contains("g.txt")).isTrue(); + assertThat(cmdContext.open().tip().getDeletedFiles()).contains(new File("a.txt").toString()); + assertThat(cmdContext.open().tip().getAddedFiles()).contains(new File("g.txt").toString()); } @Test(expected = AlreadyExistsException.class) @@ -281,10 +281,10 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { request.setAuthor(new Person("Trillian Astra", "trillian@hitchhiker.com")); hgModifyCommand.execute(request); - assertThat(cmdContext.open().tip().getDeletedFiles().contains("c/d.txt")).isTrue(); - assertThat(cmdContext.open().tip().getDeletedFiles().contains("c/e.txt")).isTrue(); - assertThat(cmdContext.open().tip().getAddedFiles().contains("notc/d.txt")).isTrue(); - assertThat(cmdContext.open().tip().getAddedFiles().contains("notc/e.txt")).isTrue(); + assertThat(cmdContext.open().tip().getDeletedFiles()).contains(new File("c/d.txt").toString()); + assertThat(cmdContext.open().tip().getDeletedFiles()).contains(new File("c/e.txt").toString()); + assertThat(cmdContext.open().tip().getAddedFiles()).contains(new File("notc/d.txt").toString()); + assertThat(cmdContext.open().tip().getAddedFiles()).contains(new File("notc/e.txt").toString()); } @Test @@ -295,10 +295,10 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { request.setAuthor(new Person("Trillian Astra", "trillian@hitchhiker.com")); hgModifyCommand.execute(request); - assertThat(cmdContext.open().tip().getDeletedFiles().contains("a.txt")).isTrue(); - assertThat(cmdContext.open().tip().getAddedFiles().contains("c/z.txt")).isTrue(); - assertThat(cmdContext.open().tip().getDeletedFiles().contains("c/d.txt")).isFalse(); - assertThat(cmdContext.open().tip().getDeletedFiles().contains("c/e.txt")).isFalse(); + assertThat(cmdContext.open().tip().getDeletedFiles()).contains(new File("a.txt").toString()); + assertThat(cmdContext.open().tip().getAddedFiles()).contains(new File("c/z.txt").toString()); + assertThat(cmdContext.open().tip().getDeletedFiles()).doesNotContain(new File("c/d.txt").toString()); + assertThat(cmdContext.open().tip().getDeletedFiles()).doesNotContain(new File("c/e.txt").toString()); } @Test @@ -309,8 +309,8 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { request.setAuthor(new Person("Trillian Astra", "trillian@hitchhiker.com")); hgModifyCommand.execute(request); - assertThat(cmdContext.open().tip().getDeletedFiles().contains("g/h/j.txt")).isTrue(); - assertThat(cmdContext.open().tip().getAddedFiles().contains("y/h/j.txt")).isTrue(); + assertThat(cmdContext.open().tip().getDeletedFiles()).contains(new File("g/h/j.txt").toString()); + assertThat(cmdContext.open().tip().getAddedFiles()).contains(new File("y/h/j.txt").toString()); } @Test @@ -321,8 +321,8 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { request.setAuthor(new Person("Trillian Astra", "trillian@hitchhiker.com")); hgModifyCommand.execute(request); - assertThat(cmdContext.open().tip().getDeletedFiles().contains("a.txt")).isTrue(); - assertThat(cmdContext.open().tip().getAddedFiles().contains("y/z.txt")).isTrue(); + assertThat(cmdContext.open().tip().getDeletedFiles()).contains(new File("a.txt").toString()); + assertThat(cmdContext.open().tip().getAddedFiles()).contains(new File("y/z.txt").toString()); } @Test @@ -333,9 +333,9 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { request.setAuthor(new Person("Trillian Astra", "trillian@hitchhiker.com")); hgModifyCommand.execute(request); - assertThat(cmdContext.open().tip().getDeletedFiles().contains("c/d.txt")).isTrue(); - assertThat(cmdContext.open().tip().getDeletedFiles().contains("c/e.txt")).isTrue(); - assertThat(cmdContext.open().tip().getAddedFiles().contains("j/k/c/d.txt")).isTrue(); - assertThat(cmdContext.open().tip().getAddedFiles().contains("j/k/c/e.txt")).isTrue(); + assertThat(cmdContext.open().tip().getDeletedFiles()).contains(new File("c/d.txt").toString()); + assertThat(cmdContext.open().tip().getDeletedFiles()).contains(new File("c/e.txt").toString()); + assertThat(cmdContext.open().tip().getAddedFiles()).contains(new File("j/k/c/d.txt").toString()); + assertThat(cmdContext.open().tip().getAddedFiles()).contains(new File("j/k/c/e.txt").toString()); } }