From c9702656bbc11f1284e69def71c903b1c940b941 Mon Sep 17 00:00:00 2001 From: "CORP\\mmrzik" Date: Mon, 13 Nov 2023 14:19:04 +0100 Subject: [PATCH 01/17] #139: relative symlink feature --- .../com/devonfw/tools/ide/io/FileAccess.java | 26 ++ .../devonfw/tools/ide/io/FileAccessImpl.java | 71 +++++- .../tools/ide/io/FileAccessImplTest.java | 234 ++++++++++++++++++ .../ide/version/VersionIdentifierTest.java | 2 +- 4 files changed, 324 insertions(+), 9 deletions(-) create mode 100644 cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java diff --git a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java index 82fe25090..6864f7269 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java +++ b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java @@ -51,6 +51,32 @@ public interface FileAccess { */ void move(Path source, Path targetDir); + /** + * Symbolic links can point to relative or absolute paths. Here the link is converted to be relative. If the target of + * the link is again a link, then that lead is followed, until the target is not a link. + * + * @param link the {@link Path} of the symbolic link. + */ + void makeSymlinkRelative(Path link); + + /** + * Symbolic links can point to relative or absolute paths. Here the link is converted to be relative. + * + * @param link the {@link Path} of the symbolic link. + * @param followTarget - {@code true} if the target of the link is again a link, then that lead is followed, until the + * target is not a link. - {@code false} if the target of the link is again a link, then that lead is no + * followed. + */ + void makeSymlinkRelative(Path link, boolean followTarget); + + /** + * Creates a symbolic relative link. + * + * @param source the source {@link Path} to link to. + * @param targetLink the {@link Path} where the symbolic link shall be created pointing to {@code source}. + */ + void relativeSymlink(Path targetLink, Path source); + /** * @param source the source {@link Path} to link to. * @param targetLink the {@link Path} where the symbolic link shall be created pointing to {@code source}. 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 index 3a916fcd1..87e527fb8 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java +++ b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java @@ -15,8 +15,10 @@ import java.net.http.HttpResponse; import java.nio.file.FileSystemException; import java.nio.file.Files; +import java.nio.file.LinkOption; import java.nio.file.Path; import java.nio.file.Paths; +import java.nio.file.attribute.BasicFileAttributes; import java.security.DigestInputStream; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; @@ -282,22 +284,74 @@ private void copyRecursive(Path source, Path target, FileCopyMode mode) throws I } } + @Override + public void makeSymlinkRelative(Path link) { + + makeSymlinkRelative(link, false); + } + + @Override + public void makeSymlinkRelative(Path link, boolean followTarget) { + + if (!Files.isSymbolicLink(link)) { + throw new IllegalStateException( + "Can't call makeSymlinkRelative on " + link + " since it is not a symbolic link."); + } + Path linkTarget = null; + try { + linkTarget = followTarget ? link.toRealPath() : Files.readSymbolicLink(link); + } catch (IOException e) { + throw new RuntimeException("For link " + link + " the call to " + + (followTarget ? "toRealPath" : "readSymbolicLink") + " in method makeSymlinkRelative failed.", e); + } + this.context.getFileAccess().delete(link); // delete old absolute link + this.context.getFileAccess().relativeSymlink(link, linkTarget); // and replace it by the new relative link + } + + @Override + public void relativeSymlink(Path link, Path source) { + + Path relativeSource = link.getParent().relativize(source); + // to make relative links like this work: dir/link -> dir + relativeSource = (relativeSource.toString().isEmpty()) ? Paths.get(".") : relativeSource; + symlink(relativeSource, link); + } + @Override public void symlink(Path source, Path targetLink) { this.context.trace("Creating symbolic link {} pointing to {}", targetLink, source); try { - if (Files.exists(targetLink) && Files.isSymbolicLink(targetLink)) { - this.context.debug("Deleting symbolic link to be re-created at {}", targetLink); - Files.delete(targetLink); + if (Files.exists(targetLink)) { + if (Files.isSymbolicLink(targetLink)) { + this.context.debug("Deleting symbolic link to be re-created at {}", targetLink); + Files.delete(targetLink); + } else { + BasicFileAttributes attr = Files.readAttributes(targetLink, BasicFileAttributes.class, + LinkOption.NOFOLLOW_LINKS); + if (attr.isOther() && attr.isDirectory()) { + this.context.debug("Deleting symbolic link (junction) to be re-created at {}", targetLink); + Files.delete(targetLink); + } + } } Files.createSymbolicLink(targetLink, source); } catch (FileSystemException e) { if (this.context.getSystemInfo().isWindows()) { - info( - "Due to lack of permissions, Microsofts mklink with junction had to be used to create a Symlink. See https://github.com/devonfw/IDEasy/blob/main/documentation/symlinks.asciidoc for further details. Error was: " - + e.getMessage()); - + String infoMsg = "Due to lack of permissions, Microsofts mklink with junction had to be used to create " + + "a Symlink. See https://github.com/devonfw/IDEasy/blob/main/documentation/symlinks.asciidoc for " + + "further details. Error was: " + e.getMessage(); + info(infoMsg); + if (!source.isAbsolute()) { + throw new IllegalStateException( + infoMsg + "\\n These junctions can only point to absolute paths. Please make sure that the targetLink (" + + targetLink + ") is absolute."); + } + if (!Files.isDirectory(source)) { // if source is a junction. This returns true as well. + throw new IllegalStateException(infoMsg + + "\\n These junctions can only point to directories or other junctions. Please make sure that the source (" + + source + ") is one of these."); + } context.newProcess().executable("cmd") .addArgs("/c", "mklink", "/d", "/j", targetLink.toString(), source.toString()).run(); } else { @@ -391,8 +445,9 @@ public void delete(Path path) { try { if (Files.isSymbolicLink(path)) { Files.delete(path); + } else { + deleteRecursive(path); } - deleteRecursive(path); } catch (IOException e) { throw new IllegalStateException("Failed to delete " + path, e); } diff --git a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java new file mode 100644 index 000000000..d8096c3c3 --- /dev/null +++ b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java @@ -0,0 +1,234 @@ +package com.devonfw.tools.ide.io; + +import static org.junit.jupiter.api.Assertions.*; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.function.Function; + +import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import com.devonfw.tools.ide.context.IdeContext; +import com.devonfw.tools.ide.context.IdeTestContextMock; + +public class FileAccessImplTest extends Assertions { + + void arrangetestRelativeSymlinks(Path tempDir, FileAccess fileAccess) { + + } + + @Test + void testSymlink(@TempDir Path tempDir) { + + IdeContext context = IdeTestContextMock.get(); + FileAccess fileAccess = new FileAccessImpl(context); + // create a new directory + Path dir = tempDir.resolve("dir"); + fileAccess.mkdirs(dir); + + // create a new file using nio + Path file = tempDir.resolve("file"); + try { + Files.write(file, "Hello World!".getBytes()); + } catch (IOException e) { + throw new RuntimeException(e); + } + + // try to create a symlink to the file using Files.createSymbolicLink + Path link = tempDir.resolve("link"); + Path linkToLink = tempDir.resolve("linkToLink"); + + boolean junctionsUsed = false; + try { + Files.createSymbolicLink(link, file); + } catch (IOException e) { // if permission is not hold, then junctions are used instead of symlinks (on Windows) + if (context.getSystemInfo().isWindows()) { + junctionsUsed = true; + // this should work + fileAccess.symlink(dir, link); + fileAccess.symlink(dir, link); // should work again + fileAccess.symlink(link, linkToLink); + + IllegalStateException e1 = assertThrows(IllegalStateException.class, () -> { + fileAccess.symlink(file, link); + }); + assertThat(e1).hasMessageContaining("These junctions can only point to directories or other junctions"); + + IllegalStateException e2 = assertThrows(IllegalStateException.class, () -> { + fileAccess.symlink(Paths.get("dir"), link); + }); + assertThat(e2).hasMessageContaining("These junctions can only point to absolute paths"); + } else { + throw new RuntimeException( + "Creating symbolic link with Files.createSymbolicLink failed and junctions can not be used since the OS is not windows: " + + e.getMessage()); + } + } + + // test for normal symlinks (not junctions) + if (!junctionsUsed) { + try { + fileAccess.symlink(file, link); // should work again + fileAccess.symlink(link, linkToLink); + } catch (Exception e) { + fail("Creating symbolic links failed: " + e.getMessage()); + } + try { + assertEquals(linkToLink.toRealPath(), file); + assertEquals(Files.readSymbolicLink(linkToLink), link); + } catch (IOException e) { + fail("Reading symbolic links failed: " + e.getMessage()); + } + } + } + + @Test + void testMakeSymlinkRelative(@TempDir Path tempDir) { + + // arrange + IdeContext context = IdeTestContextMock.get(); + FileAccess fileAccess = new FileAccessImpl(context); + Path parent = tempDir.resolve("parent"); + Path d1 = parent.resolve("d1"); + Path d11 = d1.resolve("d11"); + Path d111 = d11.resolve("d111"); + Path d1111 = d111.resolve("d1111"); + Path d2 = parent.resolve("d2"); + Path d22 = d2.resolve("d22"); + Path d222 = d22.resolve("d222"); + Path[] dirPaths = new Path[] { parent, d1, d11, d111, d1111, d2, d22, d222 }; + for (Path dirPath : dirPaths) { + fileAccess.mkdirs(dirPath); + } + Path link_d11_d1 = d11.resolve("link_d11_d1"); + fileAccess.symlink(d1, link_d11_d1); + + Path link_d11_d11 = d11.resolve("link_d11_d11"); + fileAccess.symlink(d11, link_d11_d11); + + Path link_d11_d111 = d11.resolve("link_d11_d111"); + fileAccess.symlink(d111, link_d11_d111); + + Path link_d11_d1111 = d11.resolve("link_d11_d1111"); + fileAccess.symlink(d1111, link_d11_d1111); + + Path link_d11_d2 = d11.resolve("link_d11_d2"); + fileAccess.symlink(d2, link_d11_d2); + + Path link_d11_d22 = d11.resolve("link_d11_d22"); + fileAccess.symlink(d22, link_d11_d22); + + Path link_d11_d222 = d11.resolve("link_d11_d222"); + fileAccess.symlink(d222, link_d11_d222); + + Path link_d22_link_d11_d1 = d22.resolve("link_d22_link_d11_d1"); + fileAccess.symlink(link_d11_d1, link_d22_link_d11_d1); + + Path link_d2_link_d11_d1 = d2.resolve("link_d2_link_d11_d1"); + fileAccess.symlink(link_d11_d1, link_d2_link_d11_d1); + + Path link_parent_link_d2_link_d11_d1 = parent.resolve("link_parent_link_d2_link_d11_d1"); + fileAccess.symlink(link_d2_link_d11_d1, link_parent_link_d2_link_d11_d1); + + Path[] links = new Path[] { link_d11_d1, link_d11_d11, link_d11_d111, link_d11_d1111, link_d11_d2, link_d11_d22, + link_d11_d222, link_d22_link_d11_d1, link_d2_link_d11_d1, link_parent_link_d2_link_d11_d1 }; + + // act: check if moving breaks absolute symlinks + Path parent2 = tempDir.resolve("parent2"); + fileAccess.move(parent, parent2); + + // assert: check if moving breaks absolute symlinks + Function transformPath = path -> { + String newPath = path.toString().replace("_parent_", "_par_").replace("parent", "parent2").replace("_par_", + "_parent_"); + return Paths.get(newPath); + }; + for (Path link : links) { + try { + Path linkInParent2 = transformPath.apply(link); + assertThat(linkInParent2).existsNoFollowLinks(); + Path realPath = linkInParent2.toRealPath(); + if (Files.exists(realPath)) { + fail("The link target " + realPath + " (from toRealPath) should not exist"); + } + Path readPath = Files.readSymbolicLink(linkInParent2); + if (!Files.exists(readPath)) { + fail("The link target " + readPath + " (from readSymbolicLink) should not exist"); + } + } catch (IOException e) { + assertThat(e).isInstanceOf(IOException.class); + } + } + + // assert: Can't call makeSymlinkRelative since it is not a symbolic link + IllegalStateException e1 = assertThrows(IllegalStateException.class, () -> { + fileAccess.makeSymlinkRelative(d1); + }); + assertThat(e1).hasMessageContaining("is not a symbolic link"); + + boolean junctionsUsed = false; + try { + Files.createSymbolicLink(tempDir.resolve("my_test_link"), parent2); + } catch (IOException e) { + if (!context.getSystemInfo().isWindows()) { + fail("Creating symbolic link with Files.createSymbolicLink failed and junctions can not be used" + + " since the OS is not windows: " + e.getMessage()); + } + junctionsUsed = true; + IllegalStateException e2 = assertThrows(IllegalStateException.class, () -> { + fileAccess.makeSymlinkRelative(link_d2_link_d11_d1); + }); + assertThat(e2).hasMessageContaining("is not a symbolic link"); + } + + // act: make symlinks relative and move + fileAccess.move(parent2, parent); // revert previous move + if (!junctionsUsed) { + for (Path link : links) { + if (link.equals(link_d2_link_d11_d1)) { + fileAccess.makeSymlinkRelative(link, false); + } else { + fileAccess.makeSymlinkRelative(link, true); + } + } + + // redo move, and check later if symlinks still work + fileAccess.move(parent, parent2); + + // assert + for (Path link : links) { + Path linkInParent2 = transformPath.apply(link); + if (link.equals(link_d2_link_d11_d1)) { + try { // checking if the transformation of absolute to relative path with flag followTarget=false works + Path correct = transformPath.apply(link_d11_d1); + assertEquals(correct, linkInParent2.getParent().resolve(Files.readSymbolicLink(linkInParent2)) + .toRealPath(LinkOption.NOFOLLOW_LINKS)); + } catch (IOException e) { + throw new RuntimeException("Couldn't get path of link where followTarget was set to false: ", e); + } + } + assertThat(linkInParent2).existsNoFollowLinks(); + try { + Path realPath = linkInParent2.toRealPath(); + assertThat(realPath).existsNoFollowLinks(); + assertThat(realPath).exists(); + } catch (IOException e) { + throw new RuntimeException("Could not call toRealPath on moved relative link: " + linkInParent2, e); + } + try { + Path readPath = Files.readSymbolicLink(linkInParent2); + assertThat(linkInParent2.getParent().resolve(readPath)).existsNoFollowLinks(); + assertThat(linkInParent2.getParent().resolve(readPath)).exists(); + } catch (IOException e) { + throw new RuntimeException("Could not call Files.readSymbolicLink on moved relative link: " + linkInParent2, + e); + } + } + } + } +} diff --git a/cli/src/test/java/com/devonfw/tools/ide/version/VersionIdentifierTest.java b/cli/src/test/java/com/devonfw/tools/ide/version/VersionIdentifierTest.java index 9256e6e7d..20fa0ed8d 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/version/VersionIdentifierTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/version/VersionIdentifierTest.java @@ -93,7 +93,7 @@ public void testIllegal() { for (String version : illegalVersions) { try { VersionIdentifier.of(version); - fail("Illegal verion '" + version + "' did not cause an exception!"); + fail("Illegal version '" + version + "' did not cause an exception!"); } catch (Exception e) { assertThat(e).isInstanceOf(IllegalArgumentException.class); assertThat(e).hasMessageContaining(version); From a360f9ff263237b725455520200d005c6652d9c6 Mon Sep 17 00:00:00 2001 From: "CORP\\mmrzik" Date: Mon, 13 Nov 2023 14:36:41 +0100 Subject: [PATCH 02/17] #139: clean up --- .../java/com/devonfw/tools/ide/io/FileAccess.java | 4 ++-- .../com/devonfw/tools/ide/io/FileAccessImplTest.java | 11 ++--------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java index 6864f7269..3ce7a57f2 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java +++ b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java @@ -53,7 +53,7 @@ public interface FileAccess { /** * Symbolic links can point to relative or absolute paths. Here the link is converted to be relative. If the target of - * the link is again a link, then that lead is followed, until the target is not a link. + * the link is again a link, then that lead is not followed. * * @param link the {@link Path} of the symbolic link. */ @@ -64,7 +64,7 @@ public interface FileAccess { * * @param link the {@link Path} of the symbolic link. * @param followTarget - {@code true} if the target of the link is again a link, then that lead is followed, until the - * target is not a link. - {@code false} if the target of the link is again a link, then that lead is no + * target is not a link. - {@code false} if the target of the link is again a link, then that lead is not * followed. */ void makeSymlinkRelative(Path link, boolean followTarget); diff --git a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java index d8096c3c3..59f9883d6 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java @@ -18,28 +18,21 @@ public class FileAccessImplTest extends Assertions { - void arrangetestRelativeSymlinks(Path tempDir, FileAccess fileAccess) { - - } - @Test void testSymlink(@TempDir Path tempDir) { IdeContext context = IdeTestContextMock.get(); FileAccess fileAccess = new FileAccessImpl(context); - // create a new directory Path dir = tempDir.resolve("dir"); fileAccess.mkdirs(dir); - // create a new file using nio Path file = tempDir.resolve("file"); try { Files.write(file, "Hello World!".getBytes()); } catch (IOException e) { - throw new RuntimeException(e); + throw new RuntimeException("When preparing testSymlink writing of " + file + " failed. ", e); } - // try to create a symlink to the file using Files.createSymbolicLink Path link = tempDir.resolve("link"); Path linkToLink = tempDir.resolve("linkToLink"); @@ -197,7 +190,7 @@ void testMakeSymlinkRelative(@TempDir Path tempDir) { } } - // redo move, and check later if symlinks still work + // redo move, and check in assert if symlinks still work fileAccess.move(parent, parent2); // assert From e295e9df026e19734ad2d9683b6d68e62e0c6838 Mon Sep 17 00:00:00 2001 From: "CORP\\mmrzik" Date: Thu, 16 Nov 2023 13:23:42 +0100 Subject: [PATCH 03/17] #139: added check for windows when rewriting junction --- cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 87e527fb8..948d3957f 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java +++ b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java @@ -329,7 +329,7 @@ public void symlink(Path source, Path targetLink) { } else { BasicFileAttributes attr = Files.readAttributes(targetLink, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS); - if (attr.isOther() && attr.isDirectory()) { + if (attr.isOther() && attr.isDirectory() && this.context.getSystemInfo().isWindows()) { this.context.debug("Deleting symbolic link (junction) to be re-created at {}", targetLink); Files.delete(targetLink); } From 66f09cff9f5fcb9d7fe4ea0ee3cc0d6c6d092361 Mon Sep 17 00:00:00 2001 From: "CORP\\mmrzik" Date: Tue, 28 Nov 2023 19:28:35 +0100 Subject: [PATCH 04/17] #139: rewrote test, one still missing --- .../com/devonfw/tools/ide/io/FileAccess.java | 13 +- .../devonfw/tools/ide/io/FileAccessImpl.java | 47 +- .../tools/ide/io/FileAccessImplTest.java | 412 ++++++++++-------- 3 files changed, 262 insertions(+), 210 deletions(-) diff --git a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java index 3ce7a57f2..bcbc4a34d 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java +++ b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java @@ -70,18 +70,15 @@ public interface FileAccess { void makeSymlinkRelative(Path link, boolean followTarget); /** - * Creates a symbolic relative link. - * * @param source the source {@link Path} to link to. * @param targetLink the {@link Path} where the symbolic link shall be created pointing to {@code source}. */ - void relativeSymlink(Path targetLink, Path source); + void symlink(Path source, Path targetLink, boolean relative); - /** - * @param source the source {@link Path} to link to. - * @param targetLink the {@link Path} where the symbolic link shall be created pointing to {@code source}. - */ - void symlink(Path source, Path targetLink); + default void symlink(Path source, Path targetLink) { + + symlink(source, targetLink, true); + } /** * @param source the source {@link Path file or folder} to copy. 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 index 948d3957f..d84bd7b52 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java +++ b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java @@ -1,6 +1,7 @@ package com.devonfw.tools.ide.io; import static com.devonfw.tools.ide.logging.Log.info; +import static com.devonfw.tools.ide.logging.Log.warn; import java.io.BufferedOutputStream; import java.io.FileInputStream; @@ -297,30 +298,36 @@ public void makeSymlinkRelative(Path link, boolean followTarget) { throw new IllegalStateException( "Can't call makeSymlinkRelative on " + link + " since it is not a symbolic link."); } - Path linkTarget = null; + Path linkTarget; try { linkTarget = followTarget ? link.toRealPath() : Files.readSymbolicLink(link); } catch (IOException e) { throw new RuntimeException("For link " + link + " the call to " + (followTarget ? "toRealPath" : "readSymbolicLink") + " in method makeSymlinkRelative failed.", e); } - this.context.getFileAccess().delete(link); // delete old absolute link - this.context.getFileAccess().relativeSymlink(link, linkTarget); // and replace it by the new relative link + delete(link); // delete old absolute link + symlink(link, linkTarget); // and replace it by the new relative link } @Override - public void relativeSymlink(Path link, Path source) { + public void symlink(Path source, Path targetLink, boolean relative) { - Path relativeSource = link.getParent().relativize(source); - // to make relative links like this work: dir/link -> dir - relativeSource = (relativeSource.toString().isEmpty()) ? Paths.get(".") : relativeSource; - symlink(relativeSource, link); - } - - @Override - public void symlink(Path source, Path targetLink) { + if (!source.isAbsolute()) { + throw new IllegalStateException("When creating a symbolic link the source (" + source + + ") must be an absolute path. If you want to create a relative link, " + + "then pass the source as an absolute path and set the relative flag to true."); + } + Path relativeSource = null; + if (relative) { + relativeSource = targetLink.getParent().relativize(source); + // to make relative links like this work: dir/link -> dir + relativeSource = (relativeSource.toString().isEmpty()) ? Paths.get(".") : relativeSource; + this.context.trace("Creating a relative symbolic link {} pointing to {}, with absolute path {}", targetLink, + relativeSource, source); + } else { + this.context.trace("Creating symbolic link {} pointing to {}", targetLink, source); + } - this.context.trace("Creating symbolic link {} pointing to {}", targetLink, source); try { if (Files.exists(targetLink)) { if (Files.isSymbolicLink(targetLink)) { @@ -335,17 +342,16 @@ public void symlink(Path source, Path targetLink) { } } } - Files.createSymbolicLink(targetLink, source); + Files.createSymbolicLink(targetLink, relative ? relativeSource : source); } catch (FileSystemException e) { if (this.context.getSystemInfo().isWindows()) { - String infoMsg = "Due to lack of permissions, Microsofts mklink with junction had to be used to create " + String infoMsg = "Due to lack of permissions, Microsoft's mklink with junction had to be used to create " + "a Symlink. See https://github.com/devonfw/IDEasy/blob/main/documentation/symlinks.asciidoc for " + "further details. Error was: " + e.getMessage(); info(infoMsg); - if (!source.isAbsolute()) { - throw new IllegalStateException( - infoMsg + "\\n These junctions can only point to absolute paths. Please make sure that the targetLink (" - + targetLink + ") is absolute."); + if (relative) { + warn("You are on Windows and you do not have permissions to create symbolic links. Junctions are used as an " + + "alternative, however, these can not point to relative paths. So the flag \"relative = true\" is ignored."); } if (!Files.isDirectory(source)) { // if source is a junction. This returns true as well. throw new IllegalStateException(infoMsg @@ -358,7 +364,8 @@ public void symlink(Path source, Path targetLink) { throw new RuntimeException(e); } } catch (IOException e) { - throw new IllegalStateException("Failed to create a symbolic link " + targetLink + " pointing to " + source, e); + throw new IllegalStateException("Failed to create a symbolic link " + targetLink + " pointing to " + source + + " with the flag \"relative\" set to " + relative, e); } } diff --git a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java index 59f9883d6..c559f47f5 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java @@ -1,227 +1,275 @@ package com.devonfw.tools.ide.io; -import static org.junit.jupiter.api.Assertions.*; +import static com.devonfw.tools.ide.logging.Log.info; +import static org.junit.jupiter.api.Assertions.assertThrows; import java.io.IOException; import java.nio.file.Files; -import java.nio.file.LinkOption; import java.nio.file.Path; -import java.nio.file.Paths; -import java.util.function.Function; -import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import com.devonfw.tools.ide.context.AbstractIdeContextTest; import com.devonfw.tools.ide.context.IdeContext; import com.devonfw.tools.ide.context.IdeTestContextMock; -public class FileAccessImplTest extends Assertions { +public class FileAccessImplTest extends AbstractIdeContextTest { + + private boolean windowsJunctionsAreUsed(IdeContext context, Path dir) { + + Path source = dir.resolve("checkIfWindowsJunctionsAreUsed"); + Path link = dir.resolve("checkIfWindowsJunctionsAreUsedLink"); + context.getFileAccess().mkdirs(source); + try { + Files.createSymbolicLink(link, source); + } catch (IOException e) { + return context.getSystemInfo().isWindows(); + } + return false; + } @Test - void testSymlink(@TempDir Path tempDir) { + public void testSymlinkNotRelative(@TempDir Path tempDir) { + + // relative links are checked in testRelativeLinksWorkAfterMoving + // arrange IdeContext context = IdeTestContextMock.get(); FileAccess fileAccess = new FileAccessImpl(context); - Path dir = tempDir.resolve("dir"); - fileAccess.mkdirs(dir); + Path dir = tempDir.resolve("parent"); + createDirs(fileAccess, dir); - Path file = tempDir.resolve("file"); - try { - Files.write(file, "Hello World!".getBytes()); - } catch (IOException e) { - throw new RuntimeException("When preparing testSymlink writing of " + file + " failed. ", e); + // act + createSymlinks(fileAccess, dir, false); + + // assert + assertSymlinksExist(dir); + assertSymlinksWork(dir, false, false); + } + + @Test + public void testSymlinkAbsoluteAsFallback(@TempDir Path tempDir) { + + // arrange + IdeContext context = IdeTestContextMock.get(); + if (!windowsJunctionsAreUsed(context, tempDir)) { + info("Can not check the Test: testSymlinkAbsoluteAsFallback since windows junctions are not used."); + return; } + FileAccess fileAccess = new FileAccessImpl(context); + Path dir = tempDir.resolve("parent"); + createDirs(fileAccess, dir); - Path link = tempDir.resolve("link"); - Path linkToLink = tempDir.resolve("linkToLink"); + // act + createSymlinks(fileAccess, dir, true); - boolean junctionsUsed = false; - try { - Files.createSymbolicLink(link, file); - } catch (IOException e) { // if permission is not hold, then junctions are used instead of symlinks (on Windows) - if (context.getSystemInfo().isWindows()) { - junctionsUsed = true; - // this should work - fileAccess.symlink(dir, link); - fileAccess.symlink(dir, link); // should work again - fileAccess.symlink(link, linkToLink); - - IllegalStateException e1 = assertThrows(IllegalStateException.class, () -> { - fileAccess.symlink(file, link); - }); - assertThat(e1).hasMessageContaining("These junctions can only point to directories or other junctions"); - - IllegalStateException e2 = assertThrows(IllegalStateException.class, () -> { - fileAccess.symlink(Paths.get("dir"), link); - }); - assertThat(e2).hasMessageContaining("These junctions can only point to absolute paths"); - } else { - throw new RuntimeException( - "Creating symbolic link with Files.createSymbolicLink failed and junctions can not be used since the OS is not windows: " - + e.getMessage()); - } + // assert + assertSymlinksExist(dir); + assertSymlinksWork(dir, false, false); + } + + @Test + public void testAbsoluteLinksBreakAfterMoving(@TempDir Path tempDir) throws IOException { + + // arrange + IdeContext context = IdeTestContextMock.get(); + if (windowsJunctionsAreUsed(context, tempDir)) { + info("Can not check the Test: testAbsoluteLinksBreakAfterMoving since windows junctions are used."); + return; } + FileAccess fileAccess = new FileAccessImpl(context); + Path dir = tempDir.resolve("parent"); + createDirs(fileAccess, dir); + createSymlinks(fileAccess, dir, false); - // test for normal symlinks (not junctions) - if (!junctionsUsed) { - try { - fileAccess.symlink(file, link); // should work again - fileAccess.symlink(link, linkToLink); - } catch (Exception e) { - fail("Creating symbolic links failed: " + e.getMessage()); - } - try { - assertEquals(linkToLink.toRealPath(), file); - assertEquals(Files.readSymbolicLink(linkToLink), link); - } catch (IOException e) { - fail("Reading symbolic links failed: " + e.getMessage()); - } + // act + Path sibling = dir.resolveSibling("parent2"); + fileAccess.move(dir, sibling); + + // assert + assertSymlinksExist(sibling); + assertSymlinksAreBroken(sibling); + } + + @Test + public void testRelativeLinksWorkAfterMoving(@TempDir Path tempDir) { + + // arrange + IdeContext context = IdeTestContextMock.get(); + if (windowsJunctionsAreUsed(context, tempDir)) { + info("Can not check the Test: testRelativeLinksWorkAfterMoving since windows junctions are used."); + return; } + FileAccess fileAccess = new FileAccessImpl(context); + Path dir = tempDir.resolve("parent"); + createDirs(fileAccess, dir); + createSymlinks(fileAccess, dir, true); + + // act + Path sibling = dir.resolveSibling("parent2"); + fileAccess.move(dir, sibling); + + // assert + assertSymlinksExist(sibling); + assertSymlinksWork(sibling, true, false); + } + + public void testMakeSymlinksRelative(@TempDir Path tempDir) { + + // test follow target true and false + + // test on junctions, dir and file, } @Test - void testMakeSymlinkRelative(@TempDir Path tempDir) { + public void testWindowsJunctionsCanNotPointToFiles(@TempDir Path tempDir) throws IOException { // arrange IdeContext context = IdeTestContextMock.get(); + if (!windowsJunctionsAreUsed(context, tempDir)) { + info("Can not check the Test: testWindowsJunctionsCanNotPointToFiles since windows junctions are not used."); + return; + } + Path file = tempDir.resolve("file"); + Files.createFile(file); FileAccess fileAccess = new FileAccessImpl(context); - Path parent = tempDir.resolve("parent"); - Path d1 = parent.resolve("d1"); - Path d11 = d1.resolve("d11"); - Path d111 = d11.resolve("d111"); - Path d1111 = d111.resolve("d1111"); - Path d2 = parent.resolve("d2"); - Path d22 = d2.resolve("d22"); - Path d222 = d22.resolve("d222"); - Path[] dirPaths = new Path[] { parent, d1, d11, d111, d1111, d2, d22, d222 }; - for (Path dirPath : dirPaths) { - fileAccess.mkdirs(dirPath); + + // act & assert + IllegalStateException e1 = assertThrows(IllegalStateException.class, () -> { + fileAccess.symlink(file, tempDir.resolve("linkToFile")); + }); + assertThat(e1).hasMessageContaining("These junctions can only point to directories or other junctions"); + } + + private void createDirs(FileAccess fileAccess, Path dir) { + + fileAccess.mkdirs(dir.resolve("d1/d11/d111/d1111")); + fileAccess.mkdirs(dir.resolve("d2/d22/d222")); + } + + private void createSymlinks(FileAccess fa, Path dir, boolean relative) { + + fa.symlink(dir.resolve("d1/d11"), dir.resolve("d1/d11/link1"), relative); + // test if symbolic links or junctions can be overwritten with symlink() + fa.symlink(dir.resolve("d1"), dir.resolve("d1/d11/link1"), relative); + + fa.symlink(dir.resolve("d1/d11"), dir.resolve("d1/d11/link2"), relative); + fa.symlink(dir.resolve("d1/d11/d111"), dir.resolve("d1/d11/link3"), relative); + fa.symlink(dir.resolve("d1/d11/d111/d1111"), dir.resolve("d1/d11/link4"), relative); + fa.symlink(dir.resolve("d2"), dir.resolve("d1/d11/link5"), relative); + fa.symlink(dir.resolve("d2/d22"), dir.resolve("d1/d11/link6"), relative); + fa.symlink(dir.resolve("d2/d22/d222"), dir.resolve("d1/d11/link7"), relative); + + fa.symlink(dir.resolve("d1/d11/link1"), dir.resolve("d2/d22/link8"), relative); + fa.symlink(dir.resolve("d1/d11/link1"), dir.resolve("d2/link9"), relative); + fa.symlink(dir.resolve("d2/link9"), dir.resolve("link10"), relative); + } + + private void assertSymlinksExist(Path dir) { + + assertThat(dir.resolve("d1/d11/link1")).existsNoFollowLinks(); + assertThat(dir.resolve("d1/d11/link2")).existsNoFollowLinks(); + assertThat(dir.resolve("d1/d11/link3")).existsNoFollowLinks(); + assertThat(dir.resolve("d1/d11/link4")).existsNoFollowLinks(); + assertThat(dir.resolve("d1/d11/link5")).existsNoFollowLinks(); + assertThat(dir.resolve("d1/d11/link6")).existsNoFollowLinks(); + assertThat(dir.resolve("d1/d11/link7")).existsNoFollowLinks(); + assertThat(dir.resolve("d2/d22/link8")).existsNoFollowLinks(); + assertThat(dir.resolve("d2/link9")).existsNoFollowLinks(); + assertThat(dir.resolve("link10")).existsNoFollowLinks(); + } + + private void assertSymlinksAreBroken(Path dir) throws IOException { + + assertSymlinkIsBroken(dir.resolve("d1/d11/link1")); + assertSymlinkIsBroken(dir.resolve("d1/d11/link2")); + assertSymlinkIsBroken(dir.resolve("d1/d11/link3")); + assertSymlinkIsBroken(dir.resolve("d1/d11/link4")); + assertSymlinkIsBroken(dir.resolve("d1/d11/link5")); + assertSymlinkIsBroken(dir.resolve("d1/d11/link6")); + assertSymlinkIsBroken(dir.resolve("d1/d11/link7")); + assertSymlinkIsBroken(dir.resolve("d2/d22/link8")); + assertSymlinkIsBroken(dir.resolve("d2/link9")); + assertSymlinkIsBroken(dir.resolve("link10")); + } + + private void assertSymlinkIsBroken(Path link) throws IOException { + + Path realPath = link.toRealPath(); + if (Files.exists(realPath)) { + fail("The link target " + realPath + " (from toRealPath) should not exist"); } - Path link_d11_d1 = d11.resolve("link_d11_d1"); - fileAccess.symlink(d1, link_d11_d1); - - Path link_d11_d11 = d11.resolve("link_d11_d11"); - fileAccess.symlink(d11, link_d11_d11); - - Path link_d11_d111 = d11.resolve("link_d11_d111"); - fileAccess.symlink(d111, link_d11_d111); - - Path link_d11_d1111 = d11.resolve("link_d11_d1111"); - fileAccess.symlink(d1111, link_d11_d1111); - - Path link_d11_d2 = d11.resolve("link_d11_d2"); - fileAccess.symlink(d2, link_d11_d2); - - Path link_d11_d22 = d11.resolve("link_d11_d22"); - fileAccess.symlink(d22, link_d11_d22); - - Path link_d11_d222 = d11.resolve("link_d11_d222"); - fileAccess.symlink(d222, link_d11_d222); - - Path link_d22_link_d11_d1 = d22.resolve("link_d22_link_d11_d1"); - fileAccess.symlink(link_d11_d1, link_d22_link_d11_d1); - - Path link_d2_link_d11_d1 = d2.resolve("link_d2_link_d11_d1"); - fileAccess.symlink(link_d11_d1, link_d2_link_d11_d1); - - Path link_parent_link_d2_link_d11_d1 = parent.resolve("link_parent_link_d2_link_d11_d1"); - fileAccess.symlink(link_d2_link_d11_d1, link_parent_link_d2_link_d11_d1); - - Path[] links = new Path[] { link_d11_d1, link_d11_d11, link_d11_d111, link_d11_d1111, link_d11_d2, link_d11_d22, - link_d11_d222, link_d22_link_d11_d1, link_d2_link_d11_d1, link_parent_link_d2_link_d11_d1 }; - - // act: check if moving breaks absolute symlinks - Path parent2 = tempDir.resolve("parent2"); - fileAccess.move(parent, parent2); - - // assert: check if moving breaks absolute symlinks - Function transformPath = path -> { - String newPath = path.toString().replace("_parent_", "_par_").replace("parent", "parent2").replace("_par_", - "_parent_"); - return Paths.get(newPath); - }; - for (Path link : links) { - try { - Path linkInParent2 = transformPath.apply(link); - assertThat(linkInParent2).existsNoFollowLinks(); - Path realPath = linkInParent2.toRealPath(); - if (Files.exists(realPath)) { - fail("The link target " + realPath + " (from toRealPath) should not exist"); - } - Path readPath = Files.readSymbolicLink(linkInParent2); - if (!Files.exists(readPath)) { - fail("The link target " + readPath + " (from readSymbolicLink) should not exist"); - } - } catch (IOException e) { - assertThat(e).isInstanceOf(IOException.class); + Path readPath = Files.readSymbolicLink(link); + if (!Files.exists(readPath)) { + fail("The link target " + readPath + " (from readSymbolicLink) should not exist"); + } + } + + /*** + * + * @param readLinks - {@code true} only pass this when junctions are not used. + */ + private void assertSymlinksWork(Path dir, boolean readLinks, boolean followTarget) { + + assertSymlinkToRealPath(dir.resolve("d1/d11/link1"), dir.resolve("d1")); + assertSymlinkToRealPath(dir.resolve("d1/d11/link2"), dir.resolve("d1/d11")); + assertSymlinkToRealPath(dir.resolve("d1/d11/link3"), dir.resolve("d1/d11/d111")); + assertSymlinkToRealPath(dir.resolve("d1/d11/link4"), dir.resolve("d1/d11/d111/d1111")); + assertSymlinkToRealPath(dir.resolve("d1/d11/link5"), dir.resolve("d2")); + assertSymlinkToRealPath(dir.resolve("d1/d11/link6"), dir.resolve("d2/d22")); + assertSymlinkToRealPath(dir.resolve("d1/d11/link7"), dir.resolve("d2/d22/d222")); + assertSymlinkToRealPath(dir.resolve("d2/d22/link8"), dir.resolve("d1")); + assertSymlinkToRealPath(dir.resolve("d2/link9"), dir.resolve("d1")); + assertSymlinkToRealPath(dir.resolve("link10"), dir.resolve("d1")); + + if (readLinks) { + assertSymlinkRead(dir.resolve("d1/d11/link1"), dir.resolve("d1")); + assertSymlinkRead(dir.resolve("d1/d11/link2"), dir.resolve("d1/d11")); + assertSymlinkRead(dir.resolve("d1/d11/link3"), dir.resolve("d1/d11/d111")); + assertSymlinkRead(dir.resolve("d1/d11/link4"), dir.resolve("d1/d11/d111/d1111")); + assertSymlinkRead(dir.resolve("d1/d11/link5"), dir.resolve("d2")); + assertSymlinkRead(dir.resolve("d1/d11/link6"), dir.resolve("d2/d22")); + assertSymlinkRead(dir.resolve("d1/d11/link7"), dir.resolve("d2/d22/d222")); + if (followTarget) { + assertSymlinkRead(dir.resolve("d2/d22/link8"), dir.resolve("d1")); + assertSymlinkRead(dir.resolve("d2/link9"), dir.resolve("d1")); + assertSymlinkRead(dir.resolve("link10"), dir.resolve("d1")); + } else { + assertSymlinkRead(dir.resolve("d2/d22/link8"), dir.resolve("d1/d11/link1")); + assertSymlinkRead(dir.resolve("d2/link9"), dir.resolve("d1/d11/link1")); + assertSymlinkRead(dir.resolve("link10"), dir.resolve("d2/link9")); } } + } - // assert: Can't call makeSymlinkRelative since it is not a symbolic link - IllegalStateException e1 = assertThrows(IllegalStateException.class, () -> { - fileAccess.makeSymlinkRelative(d1); - }); - assertThat(e1).hasMessageContaining("is not a symbolic link"); + private void assertSymlinkToRealPath(Path link, Path trueTarget) { - boolean junctionsUsed = false; + Path realPath = null; try { - Files.createSymbolicLink(tempDir.resolve("my_test_link"), parent2); + realPath = link.toRealPath(); } catch (IOException e) { - if (!context.getSystemInfo().isWindows()) { - fail("Creating symbolic link with Files.createSymbolicLink failed and junctions can not be used" - + " since the OS is not windows: " + e.getMessage()); - } - junctionsUsed = true; - IllegalStateException e2 = assertThrows(IllegalStateException.class, () -> { - fileAccess.makeSymlinkRelative(link_d2_link_d11_d1); - }); - assertThat(e2).hasMessageContaining("is not a symbolic link"); + fail("Could not call toRealPath on link: " + link, e); // TODO this may also be moved to method declarations + // "throws IOException" } + assertThat(realPath).exists(); + assertThat(realPath).existsNoFollowLinks(); + assertThat(realPath).isEqualTo(trueTarget); - // act: make symlinks relative and move - fileAccess.move(parent2, parent); // revert previous move - if (!junctionsUsed) { - for (Path link : links) { - if (link.equals(link_d2_link_d11_d1)) { - fileAccess.makeSymlinkRelative(link, false); - } else { - fileAccess.makeSymlinkRelative(link, true); - } - } + } - // redo move, and check in assert if symlinks still work - fileAccess.move(parent, parent2); - - // assert - for (Path link : links) { - Path linkInParent2 = transformPath.apply(link); - if (link.equals(link_d2_link_d11_d1)) { - try { // checking if the transformation of absolute to relative path with flag followTarget=false works - Path correct = transformPath.apply(link_d11_d1); - assertEquals(correct, linkInParent2.getParent().resolve(Files.readSymbolicLink(linkInParent2)) - .toRealPath(LinkOption.NOFOLLOW_LINKS)); - } catch (IOException e) { - throw new RuntimeException("Couldn't get path of link where followTarget was set to false: ", e); - } - } - assertThat(linkInParent2).existsNoFollowLinks(); - try { - Path realPath = linkInParent2.toRealPath(); - assertThat(realPath).existsNoFollowLinks(); - assertThat(realPath).exists(); - } catch (IOException e) { - throw new RuntimeException("Could not call toRealPath on moved relative link: " + linkInParent2, e); - } - try { - Path readPath = Files.readSymbolicLink(linkInParent2); - assertThat(linkInParent2.getParent().resolve(readPath)).existsNoFollowLinks(); - assertThat(linkInParent2.getParent().resolve(readPath)).exists(); - } catch (IOException e) { - throw new RuntimeException("Could not call Files.readSymbolicLink on moved relative link: " + linkInParent2, - e); - } - } + private void assertSymlinkRead(Path link, Path trueTarget) { + + // only call this on relative links + + Path readPath = null; + try { + readPath = Files.readSymbolicLink(link); + } catch (IOException e) { + fail("Could not call Files.readSymbolicLink on link: " + link, e); } + assertThat(link.resolveSibling(readPath)).existsNoFollowLinks(); + assertThat(link.resolveSibling(readPath)).exists(); + assertThat(readPath.getFileName()).isEqualTo((trueTarget.getFileName())); } } From 75eb7092e64c6db9c678825b4a22999daebff7e1 Mon Sep 17 00:00:00 2001 From: "CORP\\mmrzik" Date: Wed, 29 Nov 2023 12:57:18 +0100 Subject: [PATCH 05/17] #139: improved test, removed makeSymlinkRelative --- .../com/devonfw/tools/ide/io/FileAccess.java | 18 ---- .../devonfw/tools/ide/io/FileAccessImpl.java | 42 ++------ .../tools/ide/io/FileAccessImplTest.java | 102 +++++++++--------- 3 files changed, 59 insertions(+), 103 deletions(-) diff --git a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java index bcbc4a34d..bc2809aa1 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java +++ b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java @@ -51,24 +51,6 @@ public interface FileAccess { */ void move(Path source, Path targetDir); - /** - * Symbolic links can point to relative or absolute paths. Here the link is converted to be relative. If the target of - * the link is again a link, then that lead is not followed. - * - * @param link the {@link Path} of the symbolic link. - */ - void makeSymlinkRelative(Path link); - - /** - * Symbolic links can point to relative or absolute paths. Here the link is converted to be relative. - * - * @param link the {@link Path} of the symbolic link. - * @param followTarget - {@code true} if the target of the link is again a link, then that lead is followed, until the - * target is not a link. - {@code false} if the target of the link is again a link, then that lead is not - * followed. - */ - void makeSymlinkRelative(Path link, boolean followTarget); - /** * @param source the source {@link Path} to link to. * @param targetLink the {@link Path} where the symbolic link shall be created pointing to {@code source}. 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 index d84bd7b52..04fdf577e 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java +++ b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java @@ -17,6 +17,7 @@ import java.nio.file.FileSystemException; import java.nio.file.Files; import java.nio.file.LinkOption; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.attribute.BasicFileAttributes; @@ -285,30 +286,6 @@ private void copyRecursive(Path source, Path target, FileCopyMode mode) throws I } } - @Override - public void makeSymlinkRelative(Path link) { - - makeSymlinkRelative(link, false); - } - - @Override - public void makeSymlinkRelative(Path link, boolean followTarget) { - - if (!Files.isSymbolicLink(link)) { - throw new IllegalStateException( - "Can't call makeSymlinkRelative on " + link + " since it is not a symbolic link."); - } - Path linkTarget; - try { - linkTarget = followTarget ? link.toRealPath() : Files.readSymbolicLink(link); - } catch (IOException e) { - throw new RuntimeException("For link " + link + " the call to " - + (followTarget ? "toRealPath" : "readSymbolicLink") + " in method makeSymlinkRelative failed.", e); - } - delete(link); // delete old absolute link - symlink(link, linkTarget); // and replace it by the new relative link - } - @Override public void symlink(Path source, Path targetLink, boolean relative) { @@ -333,15 +310,18 @@ public void symlink(Path source, Path targetLink, boolean relative) { if (Files.isSymbolicLink(targetLink)) { this.context.debug("Deleting symbolic link to be re-created at {}", targetLink); Files.delete(targetLink); - } else { - BasicFileAttributes attr = Files.readAttributes(targetLink, BasicFileAttributes.class, - LinkOption.NOFOLLOW_LINKS); - if (attr.isOther() && attr.isDirectory() && this.context.getSystemInfo().isWindows()) { - this.context.debug("Deleting symbolic link (junction) to be re-created at {}", targetLink); - Files.delete(targetLink); - } } } + try { // since broken junctions are not detected by Files.exists(brokenJunction) + BasicFileAttributes attr = Files.readAttributes(targetLink, BasicFileAttributes.class, + LinkOption.NOFOLLOW_LINKS); + if (attr.isOther() && attr.isDirectory() && this.context.getSystemInfo().isWindows()) { + this.context.debug("Deleting symbolic link (junction) to be re-created at {}", targetLink); + Files.delete(targetLink); + } + } catch (NoSuchFileException e) { + // ignore, since there is no previous file at the location, which is fine + } Files.createSymbolicLink(targetLink, relative ? relativeSource : source); } catch (FileSystemException e) { if (this.context.getSystemInfo().isWindows()) { diff --git a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java index c559f47f5..ebc52c376 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java @@ -5,6 +5,7 @@ import java.io.IOException; import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import org.junit.jupiter.api.Test; @@ -39,13 +40,15 @@ public void testSymlinkNotRelative(@TempDir Path tempDir) { FileAccess fileAccess = new FileAccessImpl(context); Path dir = tempDir.resolve("parent"); createDirs(fileAccess, dir); + boolean readLinks = !windowsJunctionsAreUsed(context, tempDir); + boolean relative = false; // act - createSymlinks(fileAccess, dir, false); + createSymlinks(fileAccess, dir, relative); // assert assertSymlinksExist(dir); - assertSymlinksWork(dir, false, false); + assertSymlinksWork(dir, readLinks); } @Test @@ -54,19 +57,22 @@ public void testSymlinkAbsoluteAsFallback(@TempDir Path tempDir) { // arrange IdeContext context = IdeTestContextMock.get(); if (!windowsJunctionsAreUsed(context, tempDir)) { - info("Can not check the Test: testSymlinkAbsoluteAsFallback since windows junctions are not used."); + info("Can not check the Test: testSymlinkAbsoluteAsFallback since windows junctions are not used and fallback " + + "from relative to absolute paths as link target is not used."); return; } FileAccess fileAccess = new FileAccessImpl(context); Path dir = tempDir.resolve("parent"); createDirs(fileAccess, dir); + boolean readLinks = false; // bc windows junctions are used, which can't be read with Files.readSymbolicLink(link); + boolean relative = true; // set to true, such that the fallback to absolute paths is used since junctions are used // act - createSymlinks(fileAccess, dir, true); + createSymlinks(fileAccess, dir, relative); // assert assertSymlinksExist(dir); - assertSymlinksWork(dir, false, false); + assertSymlinksWork(dir, readLinks); } @Test @@ -74,14 +80,11 @@ public void testAbsoluteLinksBreakAfterMoving(@TempDir Path tempDir) throws IOEx // arrange IdeContext context = IdeTestContextMock.get(); - if (windowsJunctionsAreUsed(context, tempDir)) { - info("Can not check the Test: testAbsoluteLinksBreakAfterMoving since windows junctions are used."); - return; - } FileAccess fileAccess = new FileAccessImpl(context); Path dir = tempDir.resolve("parent"); createDirs(fileAccess, dir); createSymlinks(fileAccess, dir, false); + boolean readLinks = !windowsJunctionsAreUsed(context, tempDir); // act Path sibling = dir.resolveSibling("parent2"); @@ -89,7 +92,7 @@ public void testAbsoluteLinksBreakAfterMoving(@TempDir Path tempDir) throws IOEx // assert assertSymlinksExist(sibling); - assertSymlinksAreBroken(sibling); + assertSymlinksAreBroken(sibling, readLinks); } @Test @@ -105,6 +108,7 @@ public void testRelativeLinksWorkAfterMoving(@TempDir Path tempDir) { Path dir = tempDir.resolve("parent"); createDirs(fileAccess, dir); createSymlinks(fileAccess, dir, true); + boolean readLinks = true; // junctions are not used, so links can be read with Files.readSymbolicLink(link); // act Path sibling = dir.resolveSibling("parent2"); @@ -112,14 +116,7 @@ public void testRelativeLinksWorkAfterMoving(@TempDir Path tempDir) { // assert assertSymlinksExist(sibling); - assertSymlinksWork(sibling, true, false); - } - - public void testMakeSymlinksRelative(@TempDir Path tempDir) { - - // test follow target true and false - - // test on junctions, dir and file, + assertSymlinksWork(sibling, readLinks); } @Test @@ -180,37 +177,42 @@ private void assertSymlinksExist(Path dir) { assertThat(dir.resolve("link10")).existsNoFollowLinks(); } - private void assertSymlinksAreBroken(Path dir) throws IOException { - - assertSymlinkIsBroken(dir.resolve("d1/d11/link1")); - assertSymlinkIsBroken(dir.resolve("d1/d11/link2")); - assertSymlinkIsBroken(dir.resolve("d1/d11/link3")); - assertSymlinkIsBroken(dir.resolve("d1/d11/link4")); - assertSymlinkIsBroken(dir.resolve("d1/d11/link5")); - assertSymlinkIsBroken(dir.resolve("d1/d11/link6")); - assertSymlinkIsBroken(dir.resolve("d1/d11/link7")); - assertSymlinkIsBroken(dir.resolve("d2/d22/link8")); - assertSymlinkIsBroken(dir.resolve("d2/link9")); - assertSymlinkIsBroken(dir.resolve("link10")); + private void assertSymlinksAreBroken(Path dir, boolean readLinks) throws IOException { + + assertSymlinkIsBroken(dir.resolve("d1/d11/link1"), readLinks); + assertSymlinkIsBroken(dir.resolve("d1/d11/link2"), readLinks); + assertSymlinkIsBroken(dir.resolve("d1/d11/link3"), readLinks); + assertSymlinkIsBroken(dir.resolve("d1/d11/link4"), readLinks); + assertSymlinkIsBroken(dir.resolve("d1/d11/link5"), readLinks); + assertSymlinkIsBroken(dir.resolve("d1/d11/link6"), readLinks); + assertSymlinkIsBroken(dir.resolve("d1/d11/link7"), readLinks); + assertSymlinkIsBroken(dir.resolve("d2/d22/link8"), readLinks); + assertSymlinkIsBroken(dir.resolve("d2/link9"), readLinks); + assertSymlinkIsBroken(dir.resolve("link10"), readLinks); } - private void assertSymlinkIsBroken(Path link) throws IOException { + private void assertSymlinkIsBroken(Path link, boolean readLinks) throws IOException { - Path realPath = link.toRealPath(); - if (Files.exists(realPath)) { - fail("The link target " + realPath + " (from toRealPath) should not exist"); + try { + Path realPath = link.toRealPath(); + if (Files.exists(realPath)) { + fail("The link target " + realPath + " (from toRealPath) should not exist"); + } + } catch (IOException e) { // toRealPath() throws exception for junctions + assertThat(e).isInstanceOf(NoSuchFileException.class); } - Path readPath = Files.readSymbolicLink(link); - if (!Files.exists(readPath)) { - fail("The link target " + readPath + " (from readSymbolicLink) should not exist"); + if (readLinks) { + Path readPath = Files.readSymbolicLink(link); + if (!Files.exists(readPath)) { + fail("The link target " + readPath + " (from readSymbolicLink) should not exist"); + } } } - /*** - * - * @param readLinks - {@code true} only pass this when junctions are not used. + /* + * only pass readLinks = true when junctions are not used. */ - private void assertSymlinksWork(Path dir, boolean readLinks, boolean followTarget) { + private void assertSymlinksWork(Path dir, boolean readLinks) { assertSymlinkToRealPath(dir.resolve("d1/d11/link1"), dir.resolve("d1")); assertSymlinkToRealPath(dir.resolve("d1/d11/link2"), dir.resolve("d1/d11")); @@ -231,15 +233,9 @@ private void assertSymlinksWork(Path dir, boolean readLinks, boolean followTarge assertSymlinkRead(dir.resolve("d1/d11/link5"), dir.resolve("d2")); assertSymlinkRead(dir.resolve("d1/d11/link6"), dir.resolve("d2/d22")); assertSymlinkRead(dir.resolve("d1/d11/link7"), dir.resolve("d2/d22/d222")); - if (followTarget) { - assertSymlinkRead(dir.resolve("d2/d22/link8"), dir.resolve("d1")); - assertSymlinkRead(dir.resolve("d2/link9"), dir.resolve("d1")); - assertSymlinkRead(dir.resolve("link10"), dir.resolve("d1")); - } else { - assertSymlinkRead(dir.resolve("d2/d22/link8"), dir.resolve("d1/d11/link1")); - assertSymlinkRead(dir.resolve("d2/link9"), dir.resolve("d1/d11/link1")); - assertSymlinkRead(dir.resolve("link10"), dir.resolve("d2/link9")); - } + assertSymlinkRead(dir.resolve("d2/d22/link8"), dir.resolve("d1/d11/link1")); + assertSymlinkRead(dir.resolve("d2/link9"), dir.resolve("d1/d11/link1")); + assertSymlinkRead(dir.resolve("link10"), dir.resolve("d2/link9")); } } @@ -249,18 +245,16 @@ private void assertSymlinkToRealPath(Path link, Path trueTarget) { try { realPath = link.toRealPath(); } catch (IOException e) { - fail("Could not call toRealPath on link: " + link, e); // TODO this may also be moved to method declarations - // "throws IOException" + fail("Could not call toRealPath on link: " + link, e); } assertThat(realPath).exists(); assertThat(realPath).existsNoFollowLinks(); assertThat(realPath).isEqualTo(trueTarget); - } private void assertSymlinkRead(Path link, Path trueTarget) { - // only call this on relative links + // only call this method if junctions are not used Path readPath = null; try { From cf287fe8329e1e7437683b5d2b82417c5e0f20e4 Mon Sep 17 00:00:00 2001 From: "CORP\\mmrzik" Date: Wed, 29 Nov 2023 13:18:37 +0100 Subject: [PATCH 06/17] #139: small bugfix in test --- .../com/devonfw/tools/ide/io/FileAccessImplTest.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java index ebc52c376..3c3730dcc 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java @@ -1,6 +1,5 @@ package com.devonfw.tools.ide.io; -import static com.devonfw.tools.ide.logging.Log.info; import static org.junit.jupiter.api.Assertions.assertThrows; import java.io.IOException; @@ -57,8 +56,9 @@ public void testSymlinkAbsoluteAsFallback(@TempDir Path tempDir) { // arrange IdeContext context = IdeTestContextMock.get(); if (!windowsJunctionsAreUsed(context, tempDir)) { - info("Can not check the Test: testSymlinkAbsoluteAsFallback since windows junctions are not used and fallback " - + "from relative to absolute paths as link target is not used."); + context.info( + "Can not check the Test: testSymlinkAbsoluteAsFallback since windows junctions are not used and fallback " + + "from relative to absolute paths as link target is not used."); return; } FileAccess fileAccess = new FileAccessImpl(context); @@ -101,7 +101,7 @@ public void testRelativeLinksWorkAfterMoving(@TempDir Path tempDir) { // arrange IdeContext context = IdeTestContextMock.get(); if (windowsJunctionsAreUsed(context, tempDir)) { - info("Can not check the Test: testRelativeLinksWorkAfterMoving since windows junctions are used."); + context.info("Can not check the Test: testRelativeLinksWorkAfterMoving since windows junctions are used."); return; } FileAccess fileAccess = new FileAccessImpl(context); @@ -125,7 +125,8 @@ public void testWindowsJunctionsCanNotPointToFiles(@TempDir Path tempDir) throws // arrange IdeContext context = IdeTestContextMock.get(); if (!windowsJunctionsAreUsed(context, tempDir)) { - info("Can not check the Test: testWindowsJunctionsCanNotPointToFiles since windows junctions are not used."); + context + .info("Can not check the Test: testWindowsJunctionsCanNotPointToFiles since windows junctions are not used."); return; } Path file = tempDir.resolve("file"); From 24f33076f3bafd970181fd57b11b5b1c8e8db4bf Mon Sep 17 00:00:00 2001 From: MattesMrzik Date: Wed, 29 Nov 2023 14:05:26 +0100 Subject: [PATCH 07/17] #139: fixed linux bug --- .../devonfw/tools/ide/io/FileAccessImplTest.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java index 3c3730dcc..c309d8914 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java @@ -4,6 +4,7 @@ import java.io.IOException; import java.nio.file.Files; +import java.nio.file.LinkOption; import java.nio.file.NoSuchFileException; import java.nio.file.Path; @@ -125,8 +126,8 @@ public void testWindowsJunctionsCanNotPointToFiles(@TempDir Path tempDir) throws // arrange IdeContext context = IdeTestContextMock.get(); if (!windowsJunctionsAreUsed(context, tempDir)) { - context - .info("Can not check the Test: testWindowsJunctionsCanNotPointToFiles since windows junctions are not used."); + context.info( + "Can not check the Test: testWindowsJunctionsCanNotPointToFiles since windows junctions are not used."); return; } Path file = tempDir.resolve("file"); @@ -204,7 +205,7 @@ private void assertSymlinkIsBroken(Path link, boolean readLinks) throws IOExcept } if (readLinks) { Path readPath = Files.readSymbolicLink(link); - if (!Files.exists(readPath)) { + if (Files.exists(readPath)) { fail("The link target " + readPath + " (from readSymbolicLink) should not exist"); } } @@ -265,6 +266,10 @@ private void assertSymlinkRead(Path link, Path trueTarget) { } assertThat(link.resolveSibling(readPath)).existsNoFollowLinks(); assertThat(link.resolveSibling(readPath)).exists(); - assertThat(readPath.getFileName()).isEqualTo((trueTarget.getFileName())); + try { + assertThat(link.resolveSibling(readPath).toRealPath(LinkOption.NOFOLLOW_LINKS)).isEqualTo(trueTarget); + } catch (IOException e) { + fail("Couldn't link.resolveSibling(readPath).toRealPath() in assertSymlinkRead:", e); + } } } From 496e43411b27973060333da2b8719d0cecde573f Mon Sep 17 00:00:00 2001 From: "CORP\\mmrzik" Date: Mon, 4 Dec 2023 12:03:26 +0100 Subject: [PATCH 08/17] #139: added comments to test --- .../tools/ide/io/FileAccessImplTest.java | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java index c309d8914..6b2831447 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java @@ -15,6 +15,9 @@ import com.devonfw.tools.ide.context.IdeContext; import com.devonfw.tools.ide.context.IdeTestContextMock; +/** + * Test of {@link FileAccessImpl}. + */ public class FileAccessImplTest extends AbstractIdeContextTest { private boolean windowsJunctionsAreUsed(IdeContext context, Path dir) { @@ -30,6 +33,9 @@ private boolean windowsJunctionsAreUsed(IdeContext context, Path dir) { return false; } + /** + * Test of {@link FileAccessImpl#symlink(Path, Path, boolean)} with "relative = false". + */ @Test public void testSymlinkNotRelative(@TempDir Path tempDir) { @@ -51,6 +57,10 @@ public void testSymlinkNotRelative(@TempDir Path tempDir) { assertSymlinksWork(dir, readLinks); } + /** + * Test of {@link FileAccessImpl#symlink(Path, Path, boolean)} with "relative = true". But Windows junctions are used + * and therefore the fallback from relative to absolute paths is tested. + */ @Test public void testSymlinkAbsoluteAsFallback(@TempDir Path tempDir) { @@ -76,6 +86,10 @@ public void testSymlinkAbsoluteAsFallback(@TempDir Path tempDir) { assertSymlinksWork(dir, readLinks); } + /** + * Test of {@link FileAccessImpl#symlink(Path, Path, boolean)} with "relative = false". Furthermore, it is tested that + * the links are broken after moving them. + */ @Test public void testAbsoluteLinksBreakAfterMoving(@TempDir Path tempDir) throws IOException { @@ -84,7 +98,8 @@ public void testAbsoluteLinksBreakAfterMoving(@TempDir Path tempDir) throws IOEx FileAccess fileAccess = new FileAccessImpl(context); Path dir = tempDir.resolve("parent"); createDirs(fileAccess, dir); - createSymlinks(fileAccess, dir, false); + boolean relative = false; + createSymlinks(fileAccess, dir, relative); boolean readLinks = !windowsJunctionsAreUsed(context, tempDir); // act @@ -96,6 +111,10 @@ public void testAbsoluteLinksBreakAfterMoving(@TempDir Path tempDir) throws IOEx assertSymlinksAreBroken(sibling, readLinks); } + /** + * Test of {@link FileAccessImpl#symlink(Path, Path, boolean)} with "relative = true". Furthermore, it is tested that + * the links still work after moving them. + */ @Test public void testRelativeLinksWorkAfterMoving(@TempDir Path tempDir) { @@ -108,7 +127,8 @@ public void testRelativeLinksWorkAfterMoving(@TempDir Path tempDir) { FileAccess fileAccess = new FileAccessImpl(context); Path dir = tempDir.resolve("parent"); createDirs(fileAccess, dir); - createSymlinks(fileAccess, dir, true); + boolean relative = true; + createSymlinks(fileAccess, dir, relative); boolean readLinks = true; // junctions are not used, so links can be read with Files.readSymbolicLink(link); // act @@ -120,6 +140,10 @@ public void testRelativeLinksWorkAfterMoving(@TempDir Path tempDir) { assertSymlinksWork(sibling, readLinks); } + /** + * Test of {@link FileAccessImpl#symlink(Path, Path, boolean)} when Windows junctions are used and the source is a + * file. + */ @Test public void testWindowsJunctionsCanNotPointToFiles(@TempDir Path tempDir) throws IOException { From 8d84beeac54e9af654a4bf76a05367485216baef Mon Sep 17 00:00:00 2001 From: "CORP\\mmrzik" Date: Mon, 11 Dec 2023 14:04:50 +0100 Subject: [PATCH 09/17] #139: impl. first change req from PR --- .../com/devonfw/tools/ide/io/FileAccess.java | 1 + .../devonfw/tools/ide/io/FileAccessImpl.java | 110 ++++++++++++------ 2 files changed, 73 insertions(+), 38 deletions(-) diff --git a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java index ebcf0ef7b..703d9f5fc 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java +++ b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java @@ -61,6 +61,7 @@ public interface FileAccess { /** * @param source the source {@link Path} to link to. * @param targetLink the {@link Path} where the symbolic link shall be created pointing to {@code source}. + * @param relative - {@code true} if the symbolic link shall be relative, {@code false} if it shall be absolute. */ void symlink(Path source, Path targetLink, boolean relative); 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 index d4fc740e4..d7c898752 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java +++ b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java @@ -293,67 +293,101 @@ private void copyRecursive(Path source, Path target, FileCopyMode mode) throws I } } - @Override - public void symlink(Path source, Path targetLink, boolean relative) { + /** + * Deletes the given {@link Path} if it is a symbolic link or a Windows junction. And throws an + * {@link IllegalStateException} if there is a file at the given {@link Path} that is neither a symbolic link nor a + * Windows junction. + * + * @param path the {@link Path} to delete. + * @throws IOException if the actual {@link Files#delete deletion} fails. + */ + private void deleteLinkIfExists(Path path) throws IOException { - if (!source.isAbsolute()) { - throw new IllegalStateException("When creating a symbolic link the source (" + source - + ") must be an absolute path. If you want to create a relative link, " - + "then pass the source as an absolute path and set the relative flag to true."); - } - Path relativeSource = null; - if (relative) { - relativeSource = targetLink.getParent().relativize(source); - // to make relative links like this work: dir/link -> dir - relativeSource = (relativeSource.toString().isEmpty()) ? Paths.get(".") : relativeSource; - this.context.trace("Creating a relative symbolic link {} pointing to {}, with absolute path {}", targetLink, - relativeSource, source); - } else { - this.context.trace("Creating symbolic link {} pointing to {}", targetLink, source); + if (Files.exists(path) && Files.isSymbolicLink(path)) { + this.context.debug("Deleting symbolic link to be re-created at {}", path); + Files.delete(path); + return; } - - try { - if (Files.exists(targetLink)) { - if (Files.isSymbolicLink(targetLink)) { - this.context.debug("Deleting symbolic link to be re-created at {}", targetLink); - Files.delete(targetLink); - } - } + if (this.context.getSystemInfo().isWindows()) { try { // since broken junctions are not detected by Files.exists(brokenJunction) - BasicFileAttributes attr = Files.readAttributes(targetLink, BasicFileAttributes.class, - LinkOption.NOFOLLOW_LINKS); - if (attr.isOther() && attr.isDirectory() && this.context.getSystemInfo().isWindows()) { - this.context.debug("Deleting symbolic link (junction) to be re-created at {}", targetLink); - Files.delete(targetLink); + BasicFileAttributes attr = Files.readAttributes(path, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS); + if (attr.isOther() && attr.isDirectory()) { + this.context.debug("Deleting symbolic link (junction) to be re-created at {}", path); + Files.delete(path); + return; } } catch (NoSuchFileException e) { - // ignore, since there is no previous file at the location, which is fine + // ignore, since there is no previous file at the location, so nothing to delete + return; } - Files.createSymbolicLink(targetLink, relative ? relativeSource : source); + } + throw new IllegalStateException( + "Failed to delete " + path + " as it is not a symbolic link or a Windows junction."); + } + + /** + * Adapts the given {@link Path} to be relative or absolute depending on the given {@code relative} flag. + * + * @param source the {@link Path} to adapt. + * @param targetLink the {@link Path} used to calculate the relative path to the {@code source} if {@code relative} is + * set to {@code true}. + * @param relative the {@code relative} flag. + * @return the adapted {@link Path}. + */ + private Path adaptPath(Path source, Path targetLink, boolean relative) { + + if (relative && source.isAbsolute()) { + return source.toAbsolutePath(); + } + if (!relative && !source.isAbsolute()) { + source = targetLink.getParent().relativize(source); + // to make relative links like this work: dir/link -> dir + return (source.toString().isEmpty()) ? Paths.get(".") : source; + } + return source; + } + + @Override + public void symlink(Path source, Path targetLink, boolean relative) { + + Path adaptedSource = adaptPath(source, targetLink, relative); + this.context.trace("Creating {} symbolic link {} pointing to {}", adaptedSource.isAbsolute() ? "" : "relative", + targetLink, adaptedSource); + + try { + deleteLinkIfExists(targetLink); + } catch (IOException e) { + throw new IllegalStateException("Failed to delete previous symlink or Windows junction at " + targetLink, e); + } + + try { + Files.createSymbolicLink(targetLink, adaptedSource); } catch (FileSystemException e) { if (this.context.getSystemInfo().isWindows()) { String infoMsg = "Due to lack of permissions, Microsoft's mklink with junction had to be used to create " + "a Symlink. See https://github.com/devonfw/IDEasy/blob/main/documentation/symlinks.asciidoc for " + "further details. Error was: " + e.getMessage(); this.context.info(infoMsg); - if (relative) { + if (!adaptedSource.isAbsolute()) { this.context.warning( "You are on Windows and you do not have permissions to create symbolic links. Junctions are used as an " - + "alternative, however, these can not point to relative paths. So the flag \"relative = true\" is ignored."); + + "alternative, however, these can not point to relative paths. So the source (" + adaptedSource + + ") is interpreted as " + "absolute path (" + adaptedSource.toAbsolutePath() + ")."); } - if (!Files.isDirectory(source)) { // if source is a junction. This returns true as well. + if (!Files.isDirectory(adaptedSource.toAbsolutePath())) { // if source is a junction. This returns true as well. throw new IllegalStateException(infoMsg + "\\n These junctions can only point to directories or other junctions. Please make sure that the source (" - + source + ") is one of these."); + + adaptedSource.toAbsolutePath() + ") is one of these."); } this.context.newProcess().executable("cmd") - .addArgs("/c", "mklink", "/d", "/j", targetLink.toString(), source.toString()).run(); + .addArgs("/c", "mklink", "/d", "/j", targetLink.toString(), adaptedSource.toAbsolutePath().toString()) + .run(); } else { throw new RuntimeException(e); } } catch (IOException e) { - throw new IllegalStateException("Failed to create a symbolic link " + targetLink + " pointing to " + source - + " with the flag \"relative\" set to " + relative, e); + throw new IllegalStateException("Failed to create a " + (adaptedSource.isAbsolute() ? "" : "relative") + + "symbolic link " + targetLink + " pointing to " + source, e); } } From 5d70c602eb66be6ed8dbd48ca71173fb0242e6e2 Mon Sep 17 00:00:00 2001 From: "CORP\\mmrzik" Date: Mon, 11 Dec 2023 16:29:17 +0100 Subject: [PATCH 10/17] '139: added comments --- .../java/com/devonfw/tools/ide/io/FileAccess.java | 13 +++++++++++++ .../com/devonfw/tools/ide/io/FileAccessImpl.java | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java index 703d9f5fc..182c9fd91 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java +++ b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java @@ -59,12 +59,25 @@ public interface FileAccess { void move(Path source, Path targetDir); /** + * Creates a symbolic link to the given {@link Path}. If the given {@code targetLink} already exists and is a symbolic + * link or a Windows junction, it will be replaced. In case of missing privileges, Windows Junctions may be * used, + * which must point to absolute paths. The created link will therefore be absolute. + * * @param source the source {@link Path} to link to. * @param targetLink the {@link Path} where the symbolic link shall be created pointing to {@code source}. * @param relative - {@code true} if the symbolic link shall be relative, {@code false} if it shall be absolute. */ void symlink(Path source, Path targetLink, boolean relative); + /** + * Creates a relative symbolic link to the given {@link Path}. If the given {@code targetLink} already exists and is a + * symbolic link or a Windows junction, it will be replaced. In case of missing privileges, Windows Junctions may be + * used, which must point to absolute paths. Hence, the created link will be absolute instead of relative. + * + * + * @param source the source {@link Path file or folder} to link to. + * @param targetLink the {@link Path} where the symbolic link shall be created pointing to {@code source}. + */ default void symlink(Path source, Path targetLink) { symlink(source, targetLink, true); 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 index d7c898752..d26b156f9 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java +++ b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java @@ -299,7 +299,7 @@ private void copyRecursive(Path source, Path target, FileCopyMode mode) throws I * Windows junction. * * @param path the {@link Path} to delete. - * @throws IOException if the actual {@link Files#delete deletion} fails. + * @throws IOException if the actual {@link Files#delete(Path) deletion} fails. */ private void deleteLinkIfExists(Path path) throws IOException { From bbeb7e4e3cd5d02840b8820b40f4c106bae1e82b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Hohwiller?= Date: Mon, 11 Dec 2023 16:51:07 +0100 Subject: [PATCH 11/17] Update FileAccess.java: improved JavaDoc --- .../main/java/com/devonfw/tools/ide/io/FileAccess.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java index 182c9fd91..8d6387bd6 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java +++ b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java @@ -60,8 +60,8 @@ public interface FileAccess { /** * Creates a symbolic link to the given {@link Path}. If the given {@code targetLink} already exists and is a symbolic - * link or a Windows junction, it will be replaced. In case of missing privileges, Windows Junctions may be * used, - * which must point to absolute paths. The created link will therefore be absolute. + * link or a Windows junction, it will be replaced. In case of missing privileges, Windows Junctions may be used as fallback, + * which must point to absolute paths. The created link will therefore be absolute in such case. * * @param source the source {@link Path} to link to. * @param targetLink the {@link Path} where the symbolic link shall be created pointing to {@code source}. @@ -72,8 +72,8 @@ public interface FileAccess { /** * Creates a relative symbolic link to the given {@link Path}. If the given {@code targetLink} already exists and is a * symbolic link or a Windows junction, it will be replaced. In case of missing privileges, Windows Junctions may be - * used, which must point to absolute paths. Hence, the created link will be absolute instead of relative. - * + * used as fallback, which must point to absolute paths. Hence, the created link will be absolute instead of relative + * in such case. * * @param source the source {@link Path file or folder} to link to. * @param targetLink the {@link Path} where the symbolic link shall be created pointing to {@code source}. From 18ec98f379970c7f4ae0c64fabf5ba8fb44ccb07 Mon Sep 17 00:00:00 2001 From: MattesMrzik Date: Mon, 11 Dec 2023 20:30:58 +0100 Subject: [PATCH 12/17] #139: fixed bug --- .../devonfw/tools/ide/io/FileAccessImpl.java | 49 +++++++++++-------- .../tools/ide/io/FileAccessImplTest.java | 5 +- 2 files changed, 30 insertions(+), 24 deletions(-) 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 index d26b156f9..785b71450 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java +++ b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java @@ -297,53 +297,59 @@ private void copyRecursive(Path source, Path target, FileCopyMode mode) throws I * Deletes the given {@link Path} if it is a symbolic link or a Windows junction. And throws an * {@link IllegalStateException} if there is a file at the given {@link Path} that is neither a symbolic link nor a * Windows junction. - * + * * @param path the {@link Path} to delete. * @throws IOException if the actual {@link Files#delete(Path) deletion} fails. */ private void deleteLinkIfExists(Path path) throws IOException { - if (Files.exists(path) && Files.isSymbolicLink(path)) { - this.context.debug("Deleting symbolic link to be re-created at {}", path); - Files.delete(path); - return; - } + boolean exists = false; + boolean isJunction = false; if (this.context.getSystemInfo().isWindows()) { try { // since broken junctions are not detected by Files.exists(brokenJunction) BasicFileAttributes attr = Files.readAttributes(path, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS); - if (attr.isOther() && attr.isDirectory()) { - this.context.debug("Deleting symbolic link (junction) to be re-created at {}", path); - Files.delete(path); - return; - } + exists = true; + isJunction = attr.isOther() && attr.isDirectory(); } catch (NoSuchFileException e) { // ignore, since there is no previous file at the location, so nothing to delete - return; } } - throw new IllegalStateException( - "Failed to delete " + path + " as it is not a symbolic link or a Windows junction."); + exists = exists || Files.exists(path); // since broken junctions are not detected by Files.exists(brokenJunction) + boolean isSymlink = exists && Files.isSymbolicLink(path); + + assert !(isSymlink && isJunction); + + if (exists) { + if (isJunction || isSymlink) { + this.context.info("Deleting previous " + (isJunction ? "junction" : "symlink") + " at " + path); + Files.delete(path); + } else { + throw new IllegalStateException( + "The file at " + path + " was not deleted since it is not a symlink or a Windows junction"); + } + } } /** * Adapts the given {@link Path} to be relative or absolute depending on the given {@code relative} flag. - * + * * @param source the {@link Path} to adapt. * @param targetLink the {@link Path} used to calculate the relative path to the {@code source} if {@code relative} is - * set to {@code true}. + * set to {@code true}. * @param relative the {@code relative} flag. * @return the adapted {@link Path}. */ private Path adaptPath(Path source, Path targetLink, boolean relative) { if (relative && source.isAbsolute()) { - return source.toAbsolutePath(); - } - if (!relative && !source.isAbsolute()) { source = targetLink.getParent().relativize(source); // to make relative links like this work: dir/link -> dir return (source.toString().isEmpty()) ? Paths.get(".") : source; } + if (!relative && !source.isAbsolute()) { + return source.toAbsolutePath(); + + } return source; } @@ -386,8 +392,9 @@ public void symlink(Path source, Path targetLink, boolean relative) { throw new RuntimeException(e); } } catch (IOException e) { - throw new IllegalStateException("Failed to create a " + (adaptedSource.isAbsolute() ? "" : "relative") - + "symbolic link " + targetLink + " pointing to " + source, e); + throw new IllegalStateException( + "Failed to create a " + (adaptedSource.isAbsolute() ? "" : "relative") + "symbolic link " + targetLink + + " pointing to " + source, e); } } diff --git a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java index 6b2831447..345a439d3 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java @@ -235,9 +235,8 @@ private void assertSymlinkIsBroken(Path link, boolean readLinks) throws IOExcept } } - /* - * only pass readLinks = true when junctions are not used. - */ + + // only pass readLinks = true when junctions are not used. private void assertSymlinksWork(Path dir, boolean readLinks) { assertSymlinkToRealPath(dir.resolve("d1/d11/link1"), dir.resolve("d1")); From 304c48cfe93485601872b63ee8424e981abec343 Mon Sep 17 00:00:00 2001 From: "CORP\\mmrzik" Date: Mon, 11 Dec 2023 22:22:39 +0100 Subject: [PATCH 13/17] #139: changed fallback in case of windows junctions --- .../devonfw/tools/ide/io/FileAccessImpl.java | 54 +++++++++++-------- 1 file changed, 33 insertions(+), 21 deletions(-) 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 index 785b71450..59f0bbda0 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java +++ b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java @@ -335,7 +335,7 @@ private void deleteLinkIfExists(Path path) throws IOException { * * @param source the {@link Path} to adapt. * @param targetLink the {@link Path} used to calculate the relative path to the {@code source} if {@code relative} is - * set to {@code true}. + * set to {@code true}. * @param relative the {@code relative} flag. * @return the adapted {@link Path}. */ @@ -353,6 +353,33 @@ private Path adaptPath(Path source, Path targetLink, boolean relative) { return source; } + private void createWindowsJunction(Path source, Path targetLink) { + + Path fallbackPath = null; + if (!source.isAbsolute()) { + try { + fallbackPath = targetLink.resolveSibling(source).toRealPath(); + } catch (IOException ioe) { + throw new IllegalStateException("Failed to create fallback symlink at " + fallbackPath, ioe); + } + this.context.warning( + "You are on Windows and you do not have permissions to create symbolic links. Junctions are used as an " + + "alternative, however, these can not point to relative paths. So the source (" + source + + ") is interpreted as " + "absolute path (" + fallbackPath + ")."); + + } else { + fallbackPath = source; + } + if (!Files.isDirectory(fallbackPath)) { // if source is a junction. This returns true as well. + // TODO this if does not recognize broken junctions + throw new IllegalStateException( + "These junctions can only point to directories or other junctions. Please make sure that the source (" + + source.toAbsolutePath() + ") is one of these."); + } + this.context.newProcess().executable("cmd") + .addArgs("/c", "mklink", "/d", "/j", targetLink.toString(), fallbackPath.toString()).run(); + } + @Override public void symlink(Path source, Path targetLink, boolean relative) { @@ -370,31 +397,16 @@ public void symlink(Path source, Path targetLink, boolean relative) { Files.createSymbolicLink(targetLink, adaptedSource); } catch (FileSystemException e) { if (this.context.getSystemInfo().isWindows()) { - String infoMsg = "Due to lack of permissions, Microsoft's mklink with junction had to be used to create " + this.context.info("Due to lack of permissions, Microsoft's mklink with junction had to be used to create " + "a Symlink. See https://github.com/devonfw/IDEasy/blob/main/documentation/symlinks.asciidoc for " - + "further details. Error was: " + e.getMessage(); - this.context.info(infoMsg); - if (!adaptedSource.isAbsolute()) { - this.context.warning( - "You are on Windows and you do not have permissions to create symbolic links. Junctions are used as an " - + "alternative, however, these can not point to relative paths. So the source (" + adaptedSource - + ") is interpreted as " + "absolute path (" + adaptedSource.toAbsolutePath() + ")."); - } - if (!Files.isDirectory(adaptedSource.toAbsolutePath())) { // if source is a junction. This returns true as well. - throw new IllegalStateException(infoMsg - + "\\n These junctions can only point to directories or other junctions. Please make sure that the source (" - + adaptedSource.toAbsolutePath() + ") is one of these."); - } - this.context.newProcess().executable("cmd") - .addArgs("/c", "mklink", "/d", "/j", targetLink.toString(), adaptedSource.toAbsolutePath().toString()) - .run(); + + "further details. Error was: " + e.getMessage()); + createWindowsJunction(adaptedSource, targetLink); } else { throw new RuntimeException(e); } } catch (IOException e) { - throw new IllegalStateException( - "Failed to create a " + (adaptedSource.isAbsolute() ? "" : "relative") + "symbolic link " + targetLink - + " pointing to " + source, e); + throw new IllegalStateException("Failed to create a " + (adaptedSource.isAbsolute() ? "" : "relative") + + "symbolic link " + targetLink + " pointing to " + source, e); } } From f8dfe79fd5dfeed18744a1957428e25f42ab243c Mon Sep 17 00:00:00 2001 From: "CORP\\mmrzik" Date: Mon, 11 Dec 2023 23:14:06 +0100 Subject: [PATCH 14/17] #139: fixed toAbsolute, which was wrong, more tests --- .../devonfw/tools/ide/io/FileAccessImpl.java | 8 +- .../tools/ide/io/FileAccessImplTest.java | 82 +++++++++++++++++-- 2 files changed, 84 insertions(+), 6 deletions(-) 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 index 59f0bbda0..76a1c188e 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java +++ b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java @@ -347,7 +347,13 @@ private Path adaptPath(Path source, Path targetLink, boolean relative) { return (source.toString().isEmpty()) ? Paths.get(".") : source; } if (!relative && !source.isAbsolute()) { - return source.toAbsolutePath(); + try { + source = targetLink.resolveSibling(source).toRealPath(); + } catch (IOException e) { + throw new IllegalStateException( + "Failed to create fallback symlink from " + source + " with target link " + targetLink, e); + } + // TODO maybe in the two off cases also call toRealPath to collapse paths like ../d1/../d2 } return source; diff --git a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java index 345a439d3..8030d5f9d 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java @@ -34,7 +34,8 @@ private boolean windowsJunctionsAreUsed(IdeContext context, Path dir) { } /** - * Test of {@link FileAccessImpl#symlink(Path, Path, boolean)} with "relative = false". + * Test of {@link FileAccessImpl#symlink(Path, Path, boolean)} with "relative = false". Passing absolute paths as + * source */ @Test public void testSymlinkNotRelative(@TempDir Path tempDir) { @@ -57,6 +58,31 @@ public void testSymlinkNotRelative(@TempDir Path tempDir) { assertSymlinksWork(dir, readLinks); } + /** + * Test of {@link FileAccessImpl#symlink(Path, Path, boolean)} with "relative = false". Passing relative paths as + * source + */ + @Test + public void testSymlinkNotRelativeWithRelativeSource(@TempDir Path tempDir) { + + // relative links are checked in testRelativeLinksWorkAfterMoving + + // arrange + IdeContext context = IdeTestContextMock.get(); + FileAccess fileAccess = new FileAccessImpl(context); + Path dir = tempDir.resolve("parent"); + createDirs(fileAccess, dir); + boolean readLinks = !windowsJunctionsAreUsed(context, tempDir); + boolean relative = false; + + // act + createSymlinksByPassingRelativeSource(fileAccess, dir, relative); + + // assert + assertSymlinksExist(dir); + assertSymlinksWork(dir, readLinks); + } + /** * Test of {@link FileAccessImpl#symlink(Path, Path, boolean)} with "relative = true". But Windows junctions are used * and therefore the fallback from relative to absolute paths is tested. @@ -111,6 +137,35 @@ public void testAbsoluteLinksBreakAfterMoving(@TempDir Path tempDir) throws IOEx assertSymlinksAreBroken(sibling, readLinks); } + /** + * Test of {@link FileAccessImpl#symlink(Path, Path, boolean)} with "relative = true". Furthermore, it is tested that + * the links still work after moving them. Passing relative paths as source. + */ + @Test + public void testRelativeLinksWorkAfterMovingWithRelativeSource(@TempDir Path tempDir) { + + // arrange + IdeContext context = IdeTestContextMock.get(); + if (windowsJunctionsAreUsed(context, tempDir)) { + context.info("Can not check the Test: testRelativeLinksWorkAfterMoving since windows junctions are used."); + return; + } + FileAccess fileAccess = new FileAccessImpl(context); + Path dir = tempDir.resolve("parent"); + createDirs(fileAccess, dir); + boolean relative = true; + createSymlinksByPassingRelativeSource(fileAccess, dir, relative); + boolean readLinks = true; // junctions are not used, so links can be read with Files.readSymbolicLink(link); + + // act + Path sibling = dir.resolveSibling("parent2"); + fileAccess.move(dir, sibling); + + // assert + assertSymlinksExist(sibling); + assertSymlinksWork(sibling, readLinks); + } + /** * Test of {@link FileAccessImpl#symlink(Path, Path, boolean)} with "relative = true". Furthermore, it is tested that * the links still work after moving them. @@ -150,8 +205,8 @@ public void testWindowsJunctionsCanNotPointToFiles(@TempDir Path tempDir) throws // arrange IdeContext context = IdeTestContextMock.get(); if (!windowsJunctionsAreUsed(context, tempDir)) { - context.info( - "Can not check the Test: testWindowsJunctionsCanNotPointToFiles since windows junctions are not used."); + context + .info("Can not check the Test: testWindowsJunctionsCanNotPointToFiles since windows junctions are not used."); return; } Path file = tempDir.resolve("file"); @@ -171,6 +226,24 @@ private void createDirs(FileAccess fileAccess, Path dir) { fileAccess.mkdirs(dir.resolve("d2/d22/d222")); } + private void createSymlinksByPassingRelativeSource(FileAccess fa, Path dir, boolean relative) { + + fa.symlink(Path.of("."), dir.resolve("d1/d11/link1"), relative); + // test if symbolic links or junctions can be overwritten with symlink() + fa.symlink(Path.of(".."), dir.resolve("d1/d11/link1"), relative); + + fa.symlink(Path.of("."), dir.resolve("d1/d11/link2"), relative); + fa.symlink(Path.of("d111"), dir.resolve("d1/d11/link3"), relative); + fa.symlink(Path.of("d111/d1111"), dir.resolve("d1/d11/link4"), relative); + fa.symlink(Path.of("../../d2"), dir.resolve("d1/d11/link5"), relative); + fa.symlink(Path.of("../../d2/d22"), dir.resolve("d1/d11/link6"), relative); + fa.symlink(Path.of("../../d2/d22/d222"), dir.resolve("d1/d11/link7"), relative); + + fa.symlink(Path.of("../../d1/d11/link1"), dir.resolve("d2/d22/link8"), relative); + fa.symlink(Path.of("../d1/d11/link1"), dir.resolve("d2/link9"), relative); + fa.symlink(Path.of("d2/link9"), dir.resolve("link10"), relative); + } + private void createSymlinks(FileAccess fa, Path dir, boolean relative) { fa.symlink(dir.resolve("d1/d11"), dir.resolve("d1/d11/link1"), relative); @@ -235,8 +308,7 @@ private void assertSymlinkIsBroken(Path link, boolean readLinks) throws IOExcept } } - - // only pass readLinks = true when junctions are not used. + // only pass readLinks = true when junctions are not used. private void assertSymlinksWork(Path dir, boolean readLinks) { assertSymlinkToRealPath(dir.resolve("d1/d11/link1"), dir.resolve("d1")); From b3994c85d00943b232be5d92d523fcc3906b039f Mon Sep 17 00:00:00 2001 From: "CORP\\mmrzik" Date: Tue, 12 Dec 2023 11:51:05 +0100 Subject: [PATCH 15/17] #139: modified tests and adaptPath --- .../devonfw/tools/ide/io/FileAccessImpl.java | 4 +- .../tools/ide/io/FileAccessImplTest.java | 146 +++++++++--------- 2 files changed, 75 insertions(+), 75 deletions(-) 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 index 76a1c188e..9f9344f56 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java +++ b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java @@ -348,13 +348,11 @@ private Path adaptPath(Path source, Path targetLink, boolean relative) { } if (!relative && !source.isAbsolute()) { try { - source = targetLink.resolveSibling(source).toRealPath(); + source = targetLink.resolveSibling(source).toRealPath(LinkOption.NOFOLLOW_LINKS); } catch (IOException e) { throw new IllegalStateException( "Failed to create fallback symlink from " + source + " with target link " + targetLink, e); } - // TODO maybe in the two off cases also call toRealPath to collapse paths like ../d1/../d2 - } return source; } diff --git a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java index 8030d5f9d..8aa54f824 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java @@ -38,7 +38,7 @@ private boolean windowsJunctionsAreUsed(IdeContext context, Path dir) { * source */ @Test - public void testSymlinkNotRelative(@TempDir Path tempDir) { + public void testSymlinkAbsoluteLinks(@TempDir Path tempDir) { // relative links are checked in testRelativeLinksWorkAfterMoving @@ -63,7 +63,7 @@ public void testSymlinkNotRelative(@TempDir Path tempDir) { * source */ @Test - public void testSymlinkNotRelativeWithRelativeSource(@TempDir Path tempDir) { + public void testSymlinkAbsolutePassingRelativeSource(@TempDir Path tempDir) { // relative links are checked in testRelativeLinksWorkAfterMoving @@ -117,7 +117,7 @@ public void testSymlinkAbsoluteAsFallback(@TempDir Path tempDir) { * the links are broken after moving them. */ @Test - public void testAbsoluteLinksBreakAfterMoving(@TempDir Path tempDir) throws IOException { + public void testSymlinkAbsoluteBreakAfterMoving(@TempDir Path tempDir) throws IOException { // arrange IdeContext context = IdeTestContextMock.get(); @@ -142,7 +142,7 @@ public void testAbsoluteLinksBreakAfterMoving(@TempDir Path tempDir) throws IOEx * the links still work after moving them. Passing relative paths as source. */ @Test - public void testRelativeLinksWorkAfterMovingWithRelativeSource(@TempDir Path tempDir) { + public void testSymlinkRelativeWorkAfterMovingPassingRelativeSource(@TempDir Path tempDir) { // arrange IdeContext context = IdeTestContextMock.get(); @@ -171,7 +171,7 @@ public void testRelativeLinksWorkAfterMovingWithRelativeSource(@TempDir Path tem * the links still work after moving them. */ @Test - public void testRelativeLinksWorkAfterMoving(@TempDir Path tempDir) { + public void testSymlinkRelativeWorkAfterMoving(@TempDir Path tempDir) { // arrange IdeContext context = IdeTestContextMock.get(); @@ -200,7 +200,7 @@ public void testRelativeLinksWorkAfterMoving(@TempDir Path tempDir) { * file. */ @Test - public void testWindowsJunctionsCanNotPointToFiles(@TempDir Path tempDir) throws IOException { + public void testSymlinkWindowsJunctionsCanNotPointToFiles(@TempDir Path tempDir) throws IOException { // arrange IdeContext context = IdeTestContextMock.get(); @@ -228,66 +228,67 @@ private void createDirs(FileAccess fileAccess, Path dir) { private void createSymlinksByPassingRelativeSource(FileAccess fa, Path dir, boolean relative) { - fa.symlink(Path.of("."), dir.resolve("d1/d11/link1"), relative); + fa.symlink(Path.of("."), dir.resolve("d1/d11/link_to_d1"), relative); // test if symbolic links or junctions can be overwritten with symlink() - fa.symlink(Path.of(".."), dir.resolve("d1/d11/link1"), relative); - - fa.symlink(Path.of("."), dir.resolve("d1/d11/link2"), relative); - fa.symlink(Path.of("d111"), dir.resolve("d1/d11/link3"), relative); - fa.symlink(Path.of("d111/d1111"), dir.resolve("d1/d11/link4"), relative); - fa.symlink(Path.of("../../d2"), dir.resolve("d1/d11/link5"), relative); - fa.symlink(Path.of("../../d2/d22"), dir.resolve("d1/d11/link6"), relative); - fa.symlink(Path.of("../../d2/d22/d222"), dir.resolve("d1/d11/link7"), relative); - - fa.symlink(Path.of("../../d1/d11/link1"), dir.resolve("d2/d22/link8"), relative); - fa.symlink(Path.of("../d1/d11/link1"), dir.resolve("d2/link9"), relative); - fa.symlink(Path.of("d2/link9"), dir.resolve("link10"), relative); + fa.symlink(Path.of(".."), dir.resolve("d1/d11/link_to_d1"), relative); + + fa.symlink(Path.of("."), dir.resolve("d1/d11/link_to_d11"), relative); + fa.symlink(Path.of("d111"), dir.resolve("d1/d11/link_to_d111"), relative); + fa.symlink(Path.of("d111/d1111"), dir.resolve("d1/d11/link_to_d1111"), relative); + fa.symlink(Path.of("../nonExistingDir/../../d2"), dir.resolve("d1/d11/link_to_d2"), relative); + fa.symlink(Path.of("../../d2/d22"), dir.resolve("d1/d11/link_to_d22"), relative); + fa.symlink(Path.of("../../d2/d22/d222"), dir.resolve("d1/d11/link_to_d222"), relative); + + fa.symlink(Path.of("../../d1/d11/link_to_d1"), dir.resolve("d2/d22/link_to_link_to_d1"), relative); + fa.symlink(Path.of("../d1/d11/link_to_d1"), dir.resolve("d2/another_link_to_link_to_d1"), relative); + fa.symlink(Path.of("d2/another_link_to_link_to_d1"), dir.resolve("link_to_another_link_to_link_to_d1"), relative); } private void createSymlinks(FileAccess fa, Path dir, boolean relative) { - fa.symlink(dir.resolve("d1/d11"), dir.resolve("d1/d11/link1"), relative); + fa.symlink(dir.resolve("d1/d11"), dir.resolve("d1/d11/link_to_d1"), relative); // test if symbolic links or junctions can be overwritten with symlink() - fa.symlink(dir.resolve("d1"), dir.resolve("d1/d11/link1"), relative); - - fa.symlink(dir.resolve("d1/d11"), dir.resolve("d1/d11/link2"), relative); - fa.symlink(dir.resolve("d1/d11/d111"), dir.resolve("d1/d11/link3"), relative); - fa.symlink(dir.resolve("d1/d11/d111/d1111"), dir.resolve("d1/d11/link4"), relative); - fa.symlink(dir.resolve("d2"), dir.resolve("d1/d11/link5"), relative); - fa.symlink(dir.resolve("d2/d22"), dir.resolve("d1/d11/link6"), relative); - fa.symlink(dir.resolve("d2/d22/d222"), dir.resolve("d1/d11/link7"), relative); - - fa.symlink(dir.resolve("d1/d11/link1"), dir.resolve("d2/d22/link8"), relative); - fa.symlink(dir.resolve("d1/d11/link1"), dir.resolve("d2/link9"), relative); - fa.symlink(dir.resolve("d2/link9"), dir.resolve("link10"), relative); + fa.symlink(dir.resolve("d1"), dir.resolve("d1/d11/link_to_d1"), relative); + + fa.symlink(dir.resolve("d1/d11"), dir.resolve("d1/d11/link_to_d11"), relative); + fa.symlink(dir.resolve("d1/d11/d111"), dir.resolve("d1/d11/link_to_d111"), relative); + fa.symlink(dir.resolve("d1/d11/d111/d1111"), dir.resolve("d1/d11/link_to_d1111"), relative); + fa.symlink(dir.resolve("nonExistingDir/../d2"), dir.resolve("d1/d11/link_to_d2"), relative); + fa.symlink(dir.resolve("d2/d22"), dir.resolve("d1/d11/link_to_d22"), relative); + fa.symlink(dir.resolve("d2/d22/d222"), dir.resolve("d1/d11/link_to_d222"), relative); + + fa.symlink(dir.resolve("d1/d11/link_to_d1"), dir.resolve("d2/d22/link_to_link_to_d1"), relative); + fa.symlink(dir.resolve("d1/d11/link_to_d1"), dir.resolve("d2/another_link_to_link_to_d1"), relative); + fa.symlink(dir.resolve("d2/another_link_to_link_to_d1"), dir.resolve("link_to_another_link_to_link_to_d1"), + relative); } private void assertSymlinksExist(Path dir) { - assertThat(dir.resolve("d1/d11/link1")).existsNoFollowLinks(); - assertThat(dir.resolve("d1/d11/link2")).existsNoFollowLinks(); - assertThat(dir.resolve("d1/d11/link3")).existsNoFollowLinks(); - assertThat(dir.resolve("d1/d11/link4")).existsNoFollowLinks(); - assertThat(dir.resolve("d1/d11/link5")).existsNoFollowLinks(); - assertThat(dir.resolve("d1/d11/link6")).existsNoFollowLinks(); - assertThat(dir.resolve("d1/d11/link7")).existsNoFollowLinks(); - assertThat(dir.resolve("d2/d22/link8")).existsNoFollowLinks(); - assertThat(dir.resolve("d2/link9")).existsNoFollowLinks(); - assertThat(dir.resolve("link10")).existsNoFollowLinks(); + assertThat(dir.resolve("d1/d11/link_to_d1")).existsNoFollowLinks(); + assertThat(dir.resolve("d1/d11/link_to_d11")).existsNoFollowLinks(); + assertThat(dir.resolve("d1/d11/link_to_d111")).existsNoFollowLinks(); + assertThat(dir.resolve("d1/d11/link_to_d1111")).existsNoFollowLinks(); + assertThat(dir.resolve("d1/d11/link_to_d2")).existsNoFollowLinks(); + assertThat(dir.resolve("d1/d11/link_to_d22")).existsNoFollowLinks(); + assertThat(dir.resolve("d1/d11/link_to_d222")).existsNoFollowLinks(); + assertThat(dir.resolve("d2/d22/link_to_link_to_d1")).existsNoFollowLinks(); + assertThat(dir.resolve("d2/another_link_to_link_to_d1")).existsNoFollowLinks(); + assertThat(dir.resolve("link_to_another_link_to_link_to_d1")).existsNoFollowLinks(); } private void assertSymlinksAreBroken(Path dir, boolean readLinks) throws IOException { - assertSymlinkIsBroken(dir.resolve("d1/d11/link1"), readLinks); - assertSymlinkIsBroken(dir.resolve("d1/d11/link2"), readLinks); - assertSymlinkIsBroken(dir.resolve("d1/d11/link3"), readLinks); - assertSymlinkIsBroken(dir.resolve("d1/d11/link4"), readLinks); - assertSymlinkIsBroken(dir.resolve("d1/d11/link5"), readLinks); - assertSymlinkIsBroken(dir.resolve("d1/d11/link6"), readLinks); - assertSymlinkIsBroken(dir.resolve("d1/d11/link7"), readLinks); - assertSymlinkIsBroken(dir.resolve("d2/d22/link8"), readLinks); - assertSymlinkIsBroken(dir.resolve("d2/link9"), readLinks); - assertSymlinkIsBroken(dir.resolve("link10"), readLinks); + assertSymlinkIsBroken(dir.resolve("d1/d11/link_to_d1"), readLinks); + assertSymlinkIsBroken(dir.resolve("d1/d11/link_to_d11"), readLinks); + assertSymlinkIsBroken(dir.resolve("d1/d11/link_to_d111"), readLinks); + assertSymlinkIsBroken(dir.resolve("d1/d11/link_to_d1111"), readLinks); + assertSymlinkIsBroken(dir.resolve("d1/d11/link_to_d2"), readLinks); + assertSymlinkIsBroken(dir.resolve("d1/d11/link_to_d22"), readLinks); + assertSymlinkIsBroken(dir.resolve("d1/d11/link_to_d222"), readLinks); + assertSymlinkIsBroken(dir.resolve("d2/d22/link_to_link_to_d1"), readLinks); + assertSymlinkIsBroken(dir.resolve("d2/another_link_to_link_to_d1"), readLinks); + assertSymlinkIsBroken(dir.resolve("link_to_another_link_to_link_to_d1"), readLinks); } private void assertSymlinkIsBroken(Path link, boolean readLinks) throws IOException { @@ -311,28 +312,29 @@ private void assertSymlinkIsBroken(Path link, boolean readLinks) throws IOExcept // only pass readLinks = true when junctions are not used. private void assertSymlinksWork(Path dir, boolean readLinks) { - assertSymlinkToRealPath(dir.resolve("d1/d11/link1"), dir.resolve("d1")); - assertSymlinkToRealPath(dir.resolve("d1/d11/link2"), dir.resolve("d1/d11")); - assertSymlinkToRealPath(dir.resolve("d1/d11/link3"), dir.resolve("d1/d11/d111")); - assertSymlinkToRealPath(dir.resolve("d1/d11/link4"), dir.resolve("d1/d11/d111/d1111")); - assertSymlinkToRealPath(dir.resolve("d1/d11/link5"), dir.resolve("d2")); - assertSymlinkToRealPath(dir.resolve("d1/d11/link6"), dir.resolve("d2/d22")); - assertSymlinkToRealPath(dir.resolve("d1/d11/link7"), dir.resolve("d2/d22/d222")); - assertSymlinkToRealPath(dir.resolve("d2/d22/link8"), dir.resolve("d1")); - assertSymlinkToRealPath(dir.resolve("d2/link9"), dir.resolve("d1")); - assertSymlinkToRealPath(dir.resolve("link10"), dir.resolve("d1")); + assertSymlinkToRealPath(dir.resolve("d1/d11/link_to_d1"), dir.resolve("d1")); + assertSymlinkToRealPath(dir.resolve("d1/d11/link_to_d11"), dir.resolve("d1/d11")); + assertSymlinkToRealPath(dir.resolve("d1/d11/link_to_d111"), dir.resolve("d1/d11/d111")); + assertSymlinkToRealPath(dir.resolve("d1/d11/link_to_d1111"), dir.resolve("d1/d11/d111/d1111")); + assertSymlinkToRealPath(dir.resolve("d1/d11/link_to_d2"), dir.resolve("d2")); + assertSymlinkToRealPath(dir.resolve("d1/d11/link_to_d22"), dir.resolve("d2/d22")); + assertSymlinkToRealPath(dir.resolve("d1/d11/link_to_d222"), dir.resolve("d2/d22/d222")); + assertSymlinkToRealPath(dir.resolve("d2/d22/link_to_link_to_d1"), dir.resolve("d1")); + assertSymlinkToRealPath(dir.resolve("d2/another_link_to_link_to_d1"), dir.resolve("d1")); + assertSymlinkToRealPath(dir.resolve("link_to_another_link_to_link_to_d1"), dir.resolve("d1")); if (readLinks) { - assertSymlinkRead(dir.resolve("d1/d11/link1"), dir.resolve("d1")); - assertSymlinkRead(dir.resolve("d1/d11/link2"), dir.resolve("d1/d11")); - assertSymlinkRead(dir.resolve("d1/d11/link3"), dir.resolve("d1/d11/d111")); - assertSymlinkRead(dir.resolve("d1/d11/link4"), dir.resolve("d1/d11/d111/d1111")); - assertSymlinkRead(dir.resolve("d1/d11/link5"), dir.resolve("d2")); - assertSymlinkRead(dir.resolve("d1/d11/link6"), dir.resolve("d2/d22")); - assertSymlinkRead(dir.resolve("d1/d11/link7"), dir.resolve("d2/d22/d222")); - assertSymlinkRead(dir.resolve("d2/d22/link8"), dir.resolve("d1/d11/link1")); - assertSymlinkRead(dir.resolve("d2/link9"), dir.resolve("d1/d11/link1")); - assertSymlinkRead(dir.resolve("link10"), dir.resolve("d2/link9")); + assertSymlinkRead(dir.resolve("d1/d11/link_to_d1"), dir.resolve("d1")); + assertSymlinkRead(dir.resolve("d1/d11/link_to_d11"), dir.resolve("d1/d11")); + assertSymlinkRead(dir.resolve("d1/d11/link_to_d111"), dir.resolve("d1/d11/d111")); + assertSymlinkRead(dir.resolve("d1/d11/link_to_d1111"), dir.resolve("d1/d11/d111/d1111")); + assertSymlinkRead(dir.resolve("d1/d11/link_to_d2"), dir.resolve("d2")); + assertSymlinkRead(dir.resolve("d1/d11/link_to_d22"), dir.resolve("d2/d22")); + assertSymlinkRead(dir.resolve("d1/d11/link_to_d222"), dir.resolve("d2/d22/d222")); + assertSymlinkRead(dir.resolve("d2/d22/link_to_link_to_d1"), dir.resolve("d1/d11/link_to_d1")); + assertSymlinkRead(dir.resolve("d2/another_link_to_link_to_d1"), dir.resolve("d1/d11/link_to_d1")); + assertSymlinkRead(dir.resolve("link_to_another_link_to_link_to_d1"), + dir.resolve("d2/another_link_to_link_to_d1")); } } From 7683f5b7db505d492a949605faccf50edd8ee422 Mon Sep 17 00:00:00 2001 From: MattesMrzik Date: Tue, 12 Dec 2023 13:23:24 +0100 Subject: [PATCH 16/17] #139: always use toRealPath in symlink --- .../devonfw/tools/ide/io/FileAccessImpl.java | 46 ++++++++++++++----- .../tools/ide/io/FileAccessImplTest.java | 35 +++++++++++++- 2 files changed, 67 insertions(+), 14 deletions(-) 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 index 9f9344f56..7cb51aa91 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java +++ b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java @@ -339,19 +339,35 @@ private void deleteLinkIfExists(Path path) throws IOException { * @param relative the {@code relative} flag. * @return the adapted {@link Path}. */ - private Path adaptPath(Path source, Path targetLink, boolean relative) { + private Path adaptPath(Path source, Path targetLink, boolean relative) throws IOException { - if (relative && source.isAbsolute()) { - source = targetLink.getParent().relativize(source); - // to make relative links like this work: dir/link -> dir - return (source.toString().isEmpty()) ? Paths.get(".") : source; - } - if (!relative && !source.isAbsolute()) { + if (source.isAbsolute()) { try { - source = targetLink.resolveSibling(source).toRealPath(LinkOption.NOFOLLOW_LINKS); + source = source.toRealPath(LinkOption.NOFOLLOW_LINKS); // to transform ../d1/../d2 to ../d2 } catch (IOException e) { - throw new IllegalStateException( - "Failed to create fallback symlink from " + source + " with target link " + targetLink, e); + throw new IOException("source.toRealPath() failed for source " + source, e); + } + if (relative) { + source = targetLink.getParent().relativize(source); + // to make relative links like this work: dir/link -> dir + source = (source.toString().isEmpty()) ? Paths.get(".") : source; + } + } else { // source is relative + if (relative) { + // even though the source is already relative, toRealPath should be called to transform paths like + // this ../d1/../d2 to ../d2 + source = targetLink.getParent() + .relativize(targetLink.resolveSibling(source).toRealPath(LinkOption.NOFOLLOW_LINKS)); + source = (source.toString().isEmpty()) ? Paths.get(".") : source; + } else { // !relative + try { + source = targetLink.resolveSibling(source).toRealPath(LinkOption.NOFOLLOW_LINKS); + } catch (IOException e) { + throw new IOException( + "targetLink.resolveSibling(source).toRealPath(LinkOption.NOFOLLOW_LINKS) failed for source " + source + + " and target link " + targetLink, + e); + } } } return source; @@ -362,7 +378,7 @@ private void createWindowsJunction(Path source, Path targetLink) { Path fallbackPath = null; if (!source.isAbsolute()) { try { - fallbackPath = targetLink.resolveSibling(source).toRealPath(); + fallbackPath = targetLink.resolveSibling(source).toRealPath(LinkOption.NOFOLLOW_LINKS); } catch (IOException ioe) { throw new IllegalStateException("Failed to create fallback symlink at " + fallbackPath, ioe); } @@ -387,7 +403,13 @@ private void createWindowsJunction(Path source, Path targetLink) { @Override public void symlink(Path source, Path targetLink, boolean relative) { - Path adaptedSource = adaptPath(source, targetLink, relative); + Path adaptedSource = null; + try { + adaptedSource = adaptPath(source, targetLink, relative); + } catch (IOException e) { + throw new IllegalStateException("Failed to adapt source for source (" + source + ") target (" + targetLink + + ") and relative (" + relative + ")", e); + } this.context.trace("Creating {} symbolic link {} pointing to {}", adaptedSource.isAbsolute() ? "" : "relative", targetLink, adaptedSource); diff --git a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java index 8aa54f824..5d6cf9ab8 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java @@ -220,6 +220,37 @@ public void testSymlinkWindowsJunctionsCanNotPointToFiles(@TempDir Path tempDir) assertThat(e1).hasMessageContaining("These junctions can only point to directories or other junctions"); } + @Test + public void testSymlinkShortcutPaths(@TempDir Path tempDir) { + + // arrange + IdeContext context = IdeTestContextMock.get(); + FileAccess fileAccess = new FileAccessImpl(context); + Path dir = tempDir.resolve("parent"); + createDirs(fileAccess, dir); + fileAccess.mkdirs(dir.resolve("d3")); + boolean readLinks = !windowsJunctionsAreUsed(context, tempDir); + + // act + fileAccess.symlink(dir.resolve("d3/../d1"), dir.resolve("link1"), false); + fileAccess.symlink(Path.of("d3/../d1"), dir.resolve("link2"), false); + fileAccess.symlink(dir.resolve("d3/../d1"), dir.resolve("link3"), true); + fileAccess.symlink(Path.of("d3/../d1"), dir.resolve("link4"), true); + fileAccess.delete(dir.resolve("d3")); + + // assert + assertSymlinkToRealPath(dir.resolve("link1"), dir.resolve("d1")); + assertSymlinkToRealPath(dir.resolve("link2"), dir.resolve("d1")); + assertSymlinkToRealPath(dir.resolve("link3"), dir.resolve("d1")); + assertSymlinkToRealPath(dir.resolve("link4"), dir.resolve("d1")); + if (readLinks) { + assertSymlinkRead(dir.resolve("link1"), dir.resolve("d1")); + assertSymlinkRead(dir.resolve("link2"), dir.resolve("d1")); + assertSymlinkRead(dir.resolve("link3"), dir.resolve("d1")); + assertSymlinkRead(dir.resolve("link4"), dir.resolve("d1")); + } + } + private void createDirs(FileAccess fileAccess, Path dir) { fileAccess.mkdirs(dir.resolve("d1/d11/d111/d1111")); @@ -235,7 +266,7 @@ private void createSymlinksByPassingRelativeSource(FileAccess fa, Path dir, bool fa.symlink(Path.of("."), dir.resolve("d1/d11/link_to_d11"), relative); fa.symlink(Path.of("d111"), dir.resolve("d1/d11/link_to_d111"), relative); fa.symlink(Path.of("d111/d1111"), dir.resolve("d1/d11/link_to_d1111"), relative); - fa.symlink(Path.of("../nonExistingDir/../../d2"), dir.resolve("d1/d11/link_to_d2"), relative); + fa.symlink(Path.of("../../d1/../d2"), dir.resolve("d1/d11/link_to_d2"), relative); fa.symlink(Path.of("../../d2/d22"), dir.resolve("d1/d11/link_to_d22"), relative); fa.symlink(Path.of("../../d2/d22/d222"), dir.resolve("d1/d11/link_to_d222"), relative); @@ -253,7 +284,7 @@ private void createSymlinks(FileAccess fa, Path dir, boolean relative) { fa.symlink(dir.resolve("d1/d11"), dir.resolve("d1/d11/link_to_d11"), relative); fa.symlink(dir.resolve("d1/d11/d111"), dir.resolve("d1/d11/link_to_d111"), relative); fa.symlink(dir.resolve("d1/d11/d111/d1111"), dir.resolve("d1/d11/link_to_d1111"), relative); - fa.symlink(dir.resolve("nonExistingDir/../d2"), dir.resolve("d1/d11/link_to_d2"), relative); + fa.symlink(dir.resolve("d1/../d2"), dir.resolve("d1/d11/link_to_d2"), relative); fa.symlink(dir.resolve("d2/d22"), dir.resolve("d1/d11/link_to_d22"), relative); fa.symlink(dir.resolve("d2/d22/d222"), dir.resolve("d1/d11/link_to_d222"), relative); From be24f119de8752826b79551c426e72080929b952 Mon Sep 17 00:00:00 2001 From: "CORP\\mmrzik" Date: Tue, 12 Dec 2023 16:47:48 +0100 Subject: [PATCH 17/17] #139: clean up --- .../com/devonfw/tools/ide/io/FileAccess.java | 17 ++-- .../devonfw/tools/ide/io/FileAccessImpl.java | 42 +++++--- .../tools/ide/io/FileAccessImplTest.java | 95 ++++++++++++++++--- 3 files changed, 117 insertions(+), 37 deletions(-) diff --git a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java index 8d6387bd6..d20361128 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java +++ b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccess.java @@ -59,23 +59,22 @@ public interface FileAccess { void move(Path source, Path targetDir); /** - * Creates a symbolic link to the given {@link Path}. If the given {@code targetLink} already exists and is a symbolic - * link or a Windows junction, it will be replaced. In case of missing privileges, Windows Junctions may be used as fallback, - * which must point to absolute paths. The created link will therefore be absolute in such case. + * Creates a symbolic link. If the given {@code targetLink} already exists and is a symbolic link or a Windows + * junction, it will be replaced. In case of missing privileges, Windows Junctions may be used as fallback, which must + * point to absolute paths. Therefore, the created link will be absolute instead of relative. * - * @param source the source {@link Path} to link to. + * @param source the source {@link Path} to link to, may be relative or absolute. * @param targetLink the {@link Path} where the symbolic link shall be created pointing to {@code source}. * @param relative - {@code true} if the symbolic link shall be relative, {@code false} if it shall be absolute. */ void symlink(Path source, Path targetLink, boolean relative); /** - * Creates a relative symbolic link to the given {@link Path}. If the given {@code targetLink} already exists and is a - * symbolic link or a Windows junction, it will be replaced. In case of missing privileges, Windows Junctions may be - * used as fallback, which must point to absolute paths. Hence, the created link will be absolute instead of relative - * in such case. + * Creates a relative symbolic link. If the given {@code targetLink} already exists and is a symbolic link or a + * Windows junction, it will be replaced. In case of missing privileges, Windows Junctions may be used as fallback, + * which must point to absolute paths. Therefore, the created link will be absolute instead of relative. * - * @param source the source {@link Path file or folder} to link to. + * @param source the source {@link Path} to link to, may be relative or absolute. * @param targetLink the {@link Path} where the symbolic link shall be created pointing to {@code source}. */ default void symlink(Path source, Path targetLink) { 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 index 7cb51aa91..08985d6d5 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java +++ b/cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java @@ -312,9 +312,11 @@ private void deleteLinkIfExists(Path path) throws IOException { isJunction = attr.isOther() && attr.isDirectory(); } catch (NoSuchFileException e) { // ignore, since there is no previous file at the location, so nothing to delete + return; } } - exists = exists || Files.exists(path); // since broken junctions are not detected by Files.exists(brokenJunction) + exists = exists || Files.exists(path); // "||" since broken junctions are not detected by + // Files.exists(brokenJunction) boolean isSymlink = exists && Files.isSymbolicLink(path); assert !(isSymlink && isJunction); @@ -332,12 +334,14 @@ private void deleteLinkIfExists(Path path) throws IOException { /** * Adapts the given {@link Path} to be relative or absolute depending on the given {@code relative} flag. + * Additionally, {@link Path#toRealPath(LinkOption...)} is applied to {@code source}. * * @param source the {@link Path} to adapt. * @param targetLink the {@link Path} used to calculate the relative path to the {@code source} if {@code relative} is * set to {@code true}. * @param relative the {@code relative} flag. * @return the adapted {@link Path}. + * @see FileAccessImpl#symlink(Path, Path, boolean) */ private Path adaptPath(Path source, Path targetLink, boolean relative) throws IOException { @@ -345,7 +349,8 @@ private Path adaptPath(Path source, Path targetLink, boolean relative) throws IO try { source = source.toRealPath(LinkOption.NOFOLLOW_LINKS); // to transform ../d1/../d2 to ../d2 } catch (IOException e) { - throw new IOException("source.toRealPath() failed for source " + source, e); + throw new IOException( + "Calling toRealPath() on the source (" + source + ") in method FileAccessImpl.adaptPath() failed.", e); } if (relative) { source = targetLink.getParent().relativize(source); @@ -363,38 +368,45 @@ private Path adaptPath(Path source, Path targetLink, boolean relative) throws IO try { source = targetLink.resolveSibling(source).toRealPath(LinkOption.NOFOLLOW_LINKS); } catch (IOException e) { - throw new IOException( - "targetLink.resolveSibling(source).toRealPath(LinkOption.NOFOLLOW_LINKS) failed for source " + source - + " and target link " + targetLink, - e); + throw new IOException("Calling toRealPath() on " + targetLink + ".resolveSibling(" + source + + ") in method FileAccessImpl.adaptPath() failed.", e); } } } return source; } + /** + * Creates a Windows junction at {@code targetLink} pointing to {@code source}. + * + * @param source must be another Windows junction or a directory. + * @param targetLink the location of the Windows junction. + */ private void createWindowsJunction(Path source, Path targetLink) { - Path fallbackPath = null; + this.context.trace("Creating a Windows junction at " + targetLink + " with " + source + " as source."); + Path fallbackPath; if (!source.isAbsolute()) { - try { - fallbackPath = targetLink.resolveSibling(source).toRealPath(LinkOption.NOFOLLOW_LINKS); - } catch (IOException ioe) { - throw new IllegalStateException("Failed to create fallback symlink at " + fallbackPath, ioe); - } this.context.warning( "You are on Windows and you do not have permissions to create symbolic links. Junctions are used as an " + "alternative, however, these can not point to relative paths. So the source (" + source - + ") is interpreted as " + "absolute path (" + fallbackPath + ")."); + + ") is interpreted as an absolute path."); + try { + fallbackPath = targetLink.resolveSibling(source).toRealPath(LinkOption.NOFOLLOW_LINKS); + } catch (IOException e) { + throw new IllegalStateException( + "Since Windows junctions are used, the source must be an absolute path. The transformation of the passed " + + "source (" + source + ") to an absolute path failed.", + e); + } } else { fallbackPath = source; } if (!Files.isDirectory(fallbackPath)) { // if source is a junction. This returns true as well. - // TODO this if does not recognize broken junctions throw new IllegalStateException( "These junctions can only point to directories or other junctions. Please make sure that the source (" - + source.toAbsolutePath() + ") is one of these."); + + fallbackPath + ") is one of these."); } this.context.newProcess().executable("cmd") .addArgs("/c", "mklink", "/d", "/j", targetLink.toString(), fallbackPath.toString()).run(); diff --git a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java index 5d6cf9ab8..c2f0bae8c 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/io/FileAccessImplTest.java @@ -20,25 +20,36 @@ */ public class FileAccessImplTest extends AbstractIdeContextTest { + /** + * Checks if Windows junctions are used. + * + * @param context the {@link IdeContext} to get system info and file access from. + * @param dir the {@link Path} to the directory which is used as temp directory. + * @return {@code true} if Windows junctions are used, {@code false} otherwise. + */ private boolean windowsJunctionsAreUsed(IdeContext context, Path dir) { + if (!context.getSystemInfo().isWindows()) { + return false; + } + Path source = dir.resolve("checkIfWindowsJunctionsAreUsed"); Path link = dir.resolve("checkIfWindowsJunctionsAreUsedLink"); context.getFileAccess().mkdirs(source); try { Files.createSymbolicLink(link, source); + return false; } catch (IOException e) { - return context.getSystemInfo().isWindows(); + return true; } - return false; } /** * Test of {@link FileAccessImpl#symlink(Path, Path, boolean)} with "relative = false". Passing absolute paths as - * source + * source. */ @Test - public void testSymlinkAbsoluteLinks(@TempDir Path tempDir) { + public void testSymlinkAbsolute(@TempDir Path tempDir) { // relative links are checked in testRelativeLinksWorkAfterMoving @@ -60,13 +71,11 @@ public void testSymlinkAbsoluteLinks(@TempDir Path tempDir) { /** * Test of {@link FileAccessImpl#symlink(Path, Path, boolean)} with "relative = false". Passing relative paths as - * source + * source. */ @Test public void testSymlinkAbsolutePassingRelativeSource(@TempDir Path tempDir) { - // relative links are checked in testRelativeLinksWorkAfterMoving - // arrange IdeContext context = IdeTestContextMock.get(); FileAccess fileAccess = new FileAccessImpl(context); @@ -220,6 +229,10 @@ public void testSymlinkWindowsJunctionsCanNotPointToFiles(@TempDir Path tempDir) assertThat(e1).hasMessageContaining("These junctions can only point to directories or other junctions"); } + /** + * Test of {@link FileAccessImpl#symlink(Path, Path, boolean)} and whether the source paths are simplified correctly + * by {@link Path#toRealPath(LinkOption...)}. + */ @Test public void testSymlinkShortcutPaths(@TempDir Path tempDir) { @@ -257,6 +270,14 @@ private void createDirs(FileAccess fileAccess, Path dir) { fileAccess.mkdirs(dir.resolve("d2/d22/d222")); } + /** + * Creates the symlinks with passing relative paths as source. This is used by the tests of + * {@link FileAccessImpl#symlink(Path, Path, boolean)}. + * + * @param fa the {@link FileAccess} to use. + * @param dir the {@link Path} to the directory where the symlinks shall be created. + * @param relative - {@code true} if the symbolic link shall be relative, {@code false} if it shall be absolute. + */ private void createSymlinksByPassingRelativeSource(FileAccess fa, Path dir, boolean relative) { fa.symlink(Path.of("."), dir.resolve("d1/d11/link_to_d1"), relative); @@ -275,6 +296,14 @@ private void createSymlinksByPassingRelativeSource(FileAccess fa, Path dir, bool fa.symlink(Path.of("d2/another_link_to_link_to_d1"), dir.resolve("link_to_another_link_to_link_to_d1"), relative); } + /** + * Creates the symlinks with passing absolute paths as source. This is used by the tests of + * {@link FileAccessImpl#symlink(Path, Path, boolean)}. + * + * @param fa the {@link FileAccess} to use. + * @param dir the {@link Path} to the directory where the symlinks shall be created. + * @param relative - {@code true} if the symbolic link shall be relative, {@code false} if it shall be absolute. + */ private void createSymlinks(FileAccess fa, Path dir, boolean relative) { fa.symlink(dir.resolve("d1/d11"), dir.resolve("d1/d11/link_to_d1"), relative); @@ -294,6 +323,11 @@ private void createSymlinks(FileAccess fa, Path dir, boolean relative) { relative); } + /** + * Checks if the symlinks exist. This is used by the tests of {@link FileAccessImpl#symlink(Path, Path, boolean)}. + * + * @param dir the {@link Path} to the directory where the symlinks are expected. + */ private void assertSymlinksExist(Path dir) { assertThat(dir.resolve("d1/d11/link_to_d1")).existsNoFollowLinks(); @@ -308,6 +342,14 @@ private void assertSymlinksExist(Path dir) { assertThat(dir.resolve("link_to_another_link_to_link_to_d1")).existsNoFollowLinks(); } + /** + * Checks if the symlinks are broken. This is used by the tests of + * {@link FileAccessImpl#symlink(Path, Path, boolean)}. + * + * @param dir the {@link Path} to the directory where the symlinks are expected. + * @param readLinks - {@code true} if the symbolic link shall be read with {@link Files#readSymbolicLink(Path)}, this + * does not work for Windows junctions. + */ private void assertSymlinksAreBroken(Path dir, boolean readLinks) throws IOException { assertSymlinkIsBroken(dir.resolve("d1/d11/link_to_d1"), readLinks); @@ -322,6 +364,13 @@ private void assertSymlinksAreBroken(Path dir, boolean readLinks) throws IOExcep assertSymlinkIsBroken(dir.resolve("link_to_another_link_to_link_to_d1"), readLinks); } + /** + * Checks if the symlink is broken. This is used by the tests of {@link FileAccessImpl#symlink(Path, Path, boolean)}. + * + * @param link the {@link Path} to the link. + * @param readLinks - {@code true} if the symbolic link shall be read with {@link Files#readSymbolicLink(Path)}, this + * does not work for Windows junctions. + */ private void assertSymlinkIsBroken(Path link, boolean readLinks) throws IOException { try { @@ -340,7 +389,13 @@ private void assertSymlinkIsBroken(Path link, boolean readLinks) throws IOExcept } } - // only pass readLinks = true when junctions are not used. + /** + * Checks if the symlinks work. This is used by the tests of {@link FileAccessImpl#symlink(Path, Path, boolean)}. + * + * @param dir the {@link Path} to the directory where the symlinks are expected. + * @param readLinks - {@code true} if the symbolic link shall be read with {@link Files#readSymbolicLink(Path)}, this + * does not work for Windows junctions. + */ private void assertSymlinksWork(Path dir, boolean readLinks) { assertSymlinkToRealPath(dir.resolve("d1/d11/link_to_d1"), dir.resolve("d1")); @@ -369,35 +424,49 @@ private void assertSymlinksWork(Path dir, boolean readLinks) { } } + /** + * Checks if the symlink works by checking {@link Path#toRealPath(LinkOption...)}} against the {@code trueTarget}. . + * This is used by the tests of {@link FileAccessImpl#symlink(Path, Path, boolean)}. + * + * @param link the {@link Path} to the link. + * @param trueTarget the {@link Path} to the true target. + */ private void assertSymlinkToRealPath(Path link, Path trueTarget) { Path realPath = null; try { realPath = link.toRealPath(); } catch (IOException e) { - fail("Could not call toRealPath on link: " + link, e); + fail("In method assertSymlinkToRealPath() could not call toRealPath() on link " + link, e); } assertThat(realPath).exists(); assertThat(realPath).existsNoFollowLinks(); assertThat(realPath).isEqualTo(trueTarget); } + /** + * Checks if the symlink works by checking {@link Files#readSymbolicLink(Path)} against the {@code trueTarget}. This + * is used by the tests of {@link FileAccessImpl#symlink(Path, Path, boolean)}. Only call this method if junctions are + * not used, since junctions can not be read with {@link Files#readSymbolicLink(Path)}. + * + * @param link the {@link Path} to the link. + * @param trueTarget the {@link Path} to the true target. + */ private void assertSymlinkRead(Path link, Path trueTarget) { - // only call this method if junctions are not used - Path readPath = null; try { readPath = Files.readSymbolicLink(link); } catch (IOException e) { - fail("Could not call Files.readSymbolicLink on link: " + link, e); + fail("In method assertSymlinkRead() could not call readSymbolicLink() on link " + link, e); } assertThat(link.resolveSibling(readPath)).existsNoFollowLinks(); assertThat(link.resolveSibling(readPath)).exists(); try { assertThat(link.resolveSibling(readPath).toRealPath(LinkOption.NOFOLLOW_LINKS)).isEqualTo(trueTarget); } catch (IOException e) { - fail("Couldn't link.resolveSibling(readPath).toRealPath() in assertSymlinkRead:", e); + fail("In method assertSymlinkRead() could not call toRealPath() on link.resolveSibling(readPath) for link " + link + + " and readPath " + readPath, e); } } }