From 8b0c7ff87886f395aab509ee13e094d2faaca455 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Sat, 12 Oct 2024 18:11:54 +0200 Subject: [PATCH] [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 + .../transport/file/FileTransporter.java | 69 ++++++++++++++++--- .../file/FileTransporterFactory.java | 20 +++++- .../transport/file/FileTransporterTest.java | 60 ++++++++++++---- .../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 1c7c89a65..35fef6e4f 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 0ad2475db..631630b99 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 @@ * 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 @@ public int classify(Throwable error) { 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 @@ protected void implPeek(PeekTask task) throws Exception { @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 168cb9734..313be701d 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 @@ 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 Transporter newInstance(RepositorySystemSession session, RemoteRepository 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 a6455fe21..9232e0136 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 @@ private void newTransporter(String url) throws Exception { 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 @@ void testGet_ToMemory(FS fs) throws Exception { @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 @@ void testGet_ToFile(FS fs) throws Exception { @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 @@ void testGet_Query(FS fs) throws Exception { 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 @@ void testPut_FromMemory(FS fs) throws Exception { @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 @@ void testPut_EncodedResourcePath(FS fs) throws Exception { 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 03d1060c2..54738e5a8 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 @@ public void transportStarted(long dataOffset, long dataLength) throws TransferCa @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(); }