You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by cs...@apache.org on 2024/10/12 16:11:59 UTC

(maven-resolver) branch master updated: [MRESOLVER-605] Linking support in file transporter (#578)

This is an automated email from the ASF dual-hosted git repository.

cstamas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-resolver.git


The following commit(s) were added to refs/heads/master by this push:
     new 8b0c7ff8 [MRESOLVER-605] Linking support in file transporter (#578)
8b0c7ff8 is described below

commit 8b0c7ff87886f395aab509ee13e094d2faaca455
Author: Tamas Cservenak <ta...@cservenak.net>
AuthorDate: Sat Oct 12 18:11:54 2024 +0200

    [MRESOLVER-605] Linking support in file transporter (#578)
    
    Support linking for FileTransport. Still, as FileTransport is remote transport, due checksumming (via Listener) file does have to be read.
    
    Big fat note: this change will cause that symlinks may land in local repository. This PR does NOT enforce rest of Maven plugins knows how to handle them, is out of scope for this PR.
    
    This feature is EXPERIMENTAL.
    
    ---
    
    https://issues.apache.org/jira/browse/MRESOLVER-605
---
 .github/workflows/maven-verify.yml                 |  1 +
 .../aether/transport/file/FileTransporter.java     | 69 ++++++++++++++++++----
 .../transport/file/FileTransporterFactory.java     | 20 ++++++-
 .../aether/transport/file/FileTransporterTest.java | 60 ++++++++++++++-----
 .../transport/file/RecordingTransportListener.java | 10 +++-
 5 files changed, 132 insertions(+), 28 deletions(-)

diff --git a/.github/workflows/maven-verify.yml b/.github/workflows/maven-verify.yml
index 1c7c89a6..35fef6e4 100644
--- a/.github/workflows/maven-verify.yml
+++ b/.github/workflows/maven-verify.yml
@@ -30,4 +30,5 @@ jobs:
       ff-site-run: false
       jdk-matrix: '[ "21" ]'
       maven-matrix: '[ "3.9.8" ]'
+      verify-fail-fast: false
 
diff --git a/maven-resolver-transport-file/src/main/java/org/eclipse/aether/transport/file/FileTransporter.java b/maven-resolver-transport-file/src/main/java/org/eclipse/aether/transport/file/FileTransporter.java
index 0ad2475d..631630b9 100644
--- a/maven-resolver-transport-file/src/main/java/org/eclipse/aether/transport/file/FileTransporter.java
+++ b/maven-resolver-transport-file/src/main/java/org/eclipse/aether/transport/file/FileTransporter.java
@@ -18,13 +18,11 @@
  */
 package org.eclipse.aether.transport.file;
 
-import java.nio.file.FileSystemNotFoundException;
+import java.nio.ByteBuffer;
+import java.nio.channels.FileChannel;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 
-import org.eclipse.aether.repository.RemoteRepository;
-import org.eclipse.aether.repository.RepositoryUriUtils;
 import org.eclipse.aether.spi.connector.transport.AbstractTransporter;
 import org.eclipse.aether.spi.connector.transport.GetTask;
 import org.eclipse.aether.spi.connector.transport.PeekTask;
@@ -36,15 +34,23 @@ import org.eclipse.aether.transfer.NoTransporterException;
  * A transporter using {@link java.io.File}.
  */
 final class FileTransporter extends AbstractTransporter {
+    /**
+     * The file op transport can use.
+     *
+     * @since 2.0.2
+     */
+    enum FileOp {
+        COPY,
+        SYMLINK,
+        HARDLINK;
+    }
 
     private final Path basePath;
+    private final FileOp fileOp;
 
-    FileTransporter(RemoteRepository repository) throws NoTransporterException {
-        try {
-            basePath = Paths.get(RepositoryUriUtils.toUri(repository.getUrl())).toAbsolutePath();
-        } catch (FileSystemNotFoundException | IllegalArgumentException e) {
-            throw new NoTransporterException(repository, e);
-        }
+    FileTransporter(Path basePath, FileOp fileOp) throws NoTransporterException {
+        this.basePath = basePath;
+        this.fileOp = fileOp;
     }
 
     Path getBasePath() {
@@ -59,6 +65,14 @@ final class FileTransporter extends AbstractTransporter {
         return ERROR_OTHER;
     }
 
+    private FileOp effectiveFileOp(FileOp wanted, GetTask task) {
+        if (task.getDataPath() != null) {
+            return wanted;
+        }
+        // task carries no path, caller wants in-memory read, so COPY must be used
+        return FileOp.COPY;
+    }
+
     @Override
     protected void implPeek(PeekTask task) throws Exception {
         getPath(task, true);
@@ -67,7 +81,40 @@ final class FileTransporter extends AbstractTransporter {
     @Override
     protected void implGet(GetTask task) throws Exception {
         Path path = getPath(task, true);
-        utilGet(task, Files.newInputStream(path), true, Files.size(path), false);
+        long size = Files.size(path);
+        FileOp effective = effectiveFileOp(fileOp, task);
+        switch (effective) {
+            case COPY:
+                utilGet(task, Files.newInputStream(path), true, size, false);
+                break;
+            case SYMLINK:
+            case HARDLINK:
+                Files.deleteIfExists(task.getDataPath());
+                task.getListener().transportStarted(0L, size);
+                if (effective == FileOp.HARDLINK) {
+                    Files.createLink(task.getDataPath(), path);
+                } else {
+                    Files.createSymbolicLink(task.getDataPath(), path);
+                }
+                if (size > 0) {
+                    try (FileChannel fc = FileChannel.open(path)) {
+                        try {
+                            task.getListener().transportProgressed(fc.map(FileChannel.MapMode.READ_ONLY, 0, fc.size()));
+                        } catch (UnsupportedOperationException e) {
+                            // not all FS support mmap: fallback to "plain read loop"
+                            ByteBuffer byteBuffer = ByteBuffer.allocate(1024 * 32);
+                            while (fc.read(byteBuffer) != -1) {
+                                byteBuffer.flip();
+                                task.getListener().transportProgressed(byteBuffer);
+                                byteBuffer.clear();
+                            }
+                        }
+                    }
+                }
+                break;
+            default:
+                throw new IllegalStateException("Unknown fileOp" + fileOp);
+        }
     }
 
     @Override
diff --git a/maven-resolver-transport-file/src/main/java/org/eclipse/aether/transport/file/FileTransporterFactory.java b/maven-resolver-transport-file/src/main/java/org/eclipse/aether/transport/file/FileTransporterFactory.java
index 168cb973..313be701 100644
--- a/maven-resolver-transport-file/src/main/java/org/eclipse/aether/transport/file/FileTransporterFactory.java
+++ b/maven-resolver-transport-file/src/main/java/org/eclipse/aether/transport/file/FileTransporterFactory.java
@@ -20,8 +20,12 @@ package org.eclipse.aether.transport.file;
 
 import javax.inject.Named;
 
+import java.nio.file.FileSystemNotFoundException;
+import java.nio.file.Paths;
+
 import org.eclipse.aether.RepositorySystemSession;
 import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.repository.RepositoryUriUtils;
 import org.eclipse.aether.spi.connector.transport.Transporter;
 import org.eclipse.aether.spi.connector.transport.TransporterFactory;
 import org.eclipse.aether.transfer.NoTransporterException;
@@ -66,6 +70,20 @@ public final class FileTransporterFactory implements TransporterFactory {
         requireNonNull(session, "session cannot be null");
         requireNonNull(repository, "repository cannot be null");
 
-        return new FileTransporter(repository);
+        FileTransporter.FileOp fileOp = FileTransporter.FileOp.COPY;
+        String repositoryUrl = repository.getUrl();
+        if (repositoryUrl.startsWith("symlink+")) {
+            fileOp = FileTransporter.FileOp.SYMLINK;
+            repositoryUrl = repositoryUrl.substring("symlink+".length());
+        } else if (repositoryUrl.startsWith("hardlink+")) {
+            fileOp = FileTransporter.FileOp.HARDLINK;
+            repositoryUrl = repositoryUrl.substring("hardlink+".length());
+        }
+        try {
+            return new FileTransporter(
+                    Paths.get(RepositoryUriUtils.toUri(repositoryUrl)).toAbsolutePath(), fileOp);
+        } catch (FileSystemNotFoundException | IllegalArgumentException e) {
+            throw new NoTransporterException(repository, e);
+        }
     }
 }
diff --git a/maven-resolver-transport-file/src/test/java/org/eclipse/aether/transport/file/FileTransporterTest.java b/maven-resolver-transport-file/src/test/java/org/eclipse/aether/transport/file/FileTransporterTest.java
index a6455fe2..9232e013 100644
--- a/maven-resolver-transport-file/src/test/java/org/eclipse/aether/transport/file/FileTransporterTest.java
+++ b/maven-resolver-transport-file/src/test/java/org/eclipse/aether/transport/file/FileTransporterTest.java
@@ -59,11 +59,23 @@ public class FileTransporterTest {
 
     private Path repoDir;
 
+    private Path tempDir;
+
     private FileSystem fileSystem;
 
     enum FS {
-        DEFAULT,
-        JIMFS
+        DEFAULT(""),
+        DEFAULT_SL("symlink+"),
+        DEFAULT_HL("hardlink+"),
+        JIMFS(""),
+        JIMFS_SL("symlink+"),
+        JIMFS_HL("hardlink+");
+
+        final String uriPrefix;
+
+        FS(String uriPrefix) {
+            this.uriPrefix = uriPrefix;
+        }
     }
 
     private RemoteRepository newRepo(String url) {
@@ -86,12 +98,15 @@ public class FileTransporterTest {
 
     void setUp(FS fs) {
         try {
-            fileSystem = fs == FS.JIMFS ? Jimfs.newFileSystem() : null;
-            repoDir = fileSystem == null ? TestFileUtils.createTempDir().toPath() : fileSystem.getPath(".");
+            fileSystem = fs.name().startsWith("JIMFS") ? Jimfs.newFileSystem() : null;
+            repoDir = fileSystem == null ? TestFileUtils.createTempDir().toPath() : fileSystem.getPath("repo");
+            Files.createDirectories(repoDir);
+            tempDir = fileSystem == null ? TestFileUtils.createTempDir().toPath() : fileSystem.getPath("tmp");
+            Files.createDirectories(tempDir);
             Files.write(repoDir.resolve("file.txt"), "test".getBytes(StandardCharsets.UTF_8));
             Files.write(repoDir.resolve("empty.txt"), "".getBytes(StandardCharsets.UTF_8));
             Files.write(repoDir.resolve("some space.txt"), "space".getBytes(StandardCharsets.UTF_8));
-            newTransporter(repoDir.toUri().toASCIIString());
+            newTransporter(fs.uriPrefix + repoDir.toUri().toASCIIString());
         } catch (Exception e) {
             Assertions.fail(e);
         }
@@ -169,11 +184,12 @@ public class FileTransporterTest {
     @EnumSource(FS.class)
     void testGet_ToFile(FS fs) throws Exception {
         setUp(fs);
-        Path file = TestFileUtils.createTempFile("failure").toPath();
+        Path file = tempDir.resolve("testGet_ToFile");
+        Files.write(file, "whatever".getBytes(StandardCharsets.UTF_8));
         RecordingTransportListener listener = new RecordingTransportListener();
         GetTask task = new GetTask(URI.create("file.txt")).setDataPath(file).setListener(listener);
         transporter.get(task);
-        assertEquals("test", TestFileUtils.readString(file.toFile()));
+        assertEquals("test", new String(Files.readAllBytes(file), StandardCharsets.UTF_8));
         assertEquals(0L, listener.dataOffset);
         assertEquals(4L, listener.dataLength);
         assertEquals(1, listener.startedCount);
@@ -185,11 +201,12 @@ public class FileTransporterTest {
     @EnumSource(FS.class)
     void testGet_EmptyResource(FS fs) throws Exception {
         setUp(fs);
-        Path file = TestFileUtils.createTempFile("failure").toPath();
+        Path file = tempDir.resolve("testGet_EmptyResource");
+        Files.write(file, "".getBytes(StandardCharsets.UTF_8));
         RecordingTransportListener listener = new RecordingTransportListener();
         GetTask task = new GetTask(URI.create("empty.txt")).setDataPath(file).setListener(listener);
         transporter.get(task);
-        assertEquals("", TestFileUtils.readString(file.toFile()));
+        assertEquals("", new String(Files.readAllBytes(file), StandardCharsets.UTF_8));
         assertEquals(0L, listener.dataOffset);
         assertEquals(0L, listener.dataLength);
         assertEquals(1, listener.startedCount);
@@ -229,9 +246,22 @@ public class FileTransporterTest {
     void testGet_FileHandleLeak(FS fs) throws Exception {
         setUp(fs);
         for (int i = 0; i < 100; i++) {
-            Path file = TestFileUtils.createTempFile("failure").toPath();
+            Path file = tempDir.resolve("testGet_FileHandleLeak" + i);
             transporter.get(new GetTask(URI.create("file.txt")).setDataPath(file));
-            assertTrue(file.toFile().delete(), i + ", " + file.toFile().getAbsolutePath());
+            if (fs.uriPrefix.startsWith("symlink+")) {
+                assertTrue(Files.isSymbolicLink(file));
+                assertTrue(Files.deleteIfExists(file), i + ", " + file.toAbsolutePath());
+            } else if (fs.uriPrefix.startsWith("hardlink+")) {
+                assertTrue(Files.isRegularFile(file));
+                // Doing this on windows FS is not possible (immediately create then delete link) due windows lock
+                // semantics. While other OS do perform this test OK, it fails on Windows with AccessDeniedEx.
+                // The file becomes deletable on Windows after some arbitrary time, but let's not fiddle with that in
+                // this UT.
+                // assertTrue(Files.deleteIfExists(file), i + ", " + file.toAbsolutePath());
+            } else {
+                assertTrue(Files.isRegularFile(file));
+                assertTrue(Files.deleteIfExists(file), i + ", " + file.toAbsolutePath());
+            }
         }
     }
 
@@ -316,7 +346,8 @@ public class FileTransporterTest {
     @EnumSource(FS.class)
     void testPut_FromFile(FS fs) throws Exception {
         setUp(fs);
-        Path file = TestFileUtils.createTempFile("upload").toPath();
+        Path file = tempDir.resolve("upload");
+        Files.write(file, "upload".getBytes(StandardCharsets.UTF_8));
         RecordingTransportListener listener = new RecordingTransportListener();
         PutTask task = new PutTask(URI.create("file.txt")).setListener(listener).setDataPath(file);
         transporter.put(task);
@@ -380,10 +411,11 @@ public class FileTransporterTest {
     void testPut_FileHandleLeak(FS fs) throws Exception {
         setUp(fs);
         for (int i = 0; i < 100; i++) {
-            Path src = TestFileUtils.createTempFile("upload").toPath();
+            Path src = tempDir.resolve("upload");
+            Files.write(src, "upload".getBytes(StandardCharsets.UTF_8));
             Path dst = repoDir.resolve("file.txt");
             transporter.put(new PutTask(URI.create("file.txt")).setDataPath(src));
-            assertTrue(src.toFile().delete(), i + ", " + src.toFile().getAbsolutePath());
+            assertTrue(Files.deleteIfExists(src), i + ", " + src.toAbsolutePath());
             assertTrue(Files.deleteIfExists(dst), i + ", " + dst.toAbsolutePath());
         }
     }
diff --git a/maven-resolver-transport-file/src/test/java/org/eclipse/aether/transport/file/RecordingTransportListener.java b/maven-resolver-transport-file/src/test/java/org/eclipse/aether/transport/file/RecordingTransportListener.java
index 03d1060c..54738e5a 100644
--- a/maven-resolver-transport-file/src/test/java/org/eclipse/aether/transport/file/RecordingTransportListener.java
+++ b/maven-resolver-transport-file/src/test/java/org/eclipse/aether/transport/file/RecordingTransportListener.java
@@ -19,8 +19,10 @@
 package org.eclipse.aether.transport.file;
 
 import java.io.ByteArrayOutputStream;
-import java.nio.Buffer;
+import java.io.IOException;
+import java.io.UncheckedIOException;
 import java.nio.ByteBuffer;
+import java.nio.channels.Channels;
 
 import org.eclipse.aether.spi.connector.transport.TransportListener;
 import org.eclipse.aether.transfer.TransferCancelledException;
@@ -56,7 +58,11 @@ class RecordingTransportListener extends TransportListener {
     @Override
     public void transportProgressed(ByteBuffer data) throws TransferCancelledException {
         progressedCount++;
-        baos.write(data.array(), data.arrayOffset() + ((Buffer) data).position(), data.remaining());
+        try {
+            Channels.newChannel(baos).write(data);
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
         if (cancelProgress) {
             throw new TransferCancelledException();
         }