[
https://issues.apache.org/jira/browse/MRESOLVER-372?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17787082#comment-17787082
]
ASF GitHub Bot commented on MRESOLVER-372:
------------------------------------------
michael-o commented on code in PR #364:
URL: https://github.com/apache/maven-resolver/pull/364#discussion_r1396865129
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java:
##########
@@ -33,6 +39,10 @@
* @since 1.9.0
*/
public final class FileUtils {
+ // Logic borrowed from Commons-Lang3: we really need only this, to decide
do we fsync on directories or not
+ private static final boolean IS_WINDOWS =
+ System.getProperty("os.name", "unknown").startsWith("Windows");
+
Review Comment:
I believe we have plexus utils code for this, no?
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/FileUtils.java:
##########
@@ -98,23 +111,79 @@ public static CollocatedTempFile newTempFile(Path file)
throws IOException {
Path tempFile = parent.resolve(file.getFileName() + "."
+
Long.toUnsignedString(ThreadLocalRandom.current().nextLong()) + ".tmp");
return new CollocatedTempFile() {
+ private final AtomicBoolean wantsMove = new AtomicBoolean(false);
+
@Override
public Path getPath() {
return tempFile;
}
@Override
- public void move() throws IOException {
- Files.move(tempFile, file, StandardCopyOption.ATOMIC_MOVE);
+ public void move() {
+ wantsMove.set(true);
}
@Override
public void close() throws IOException {
+ if (wantsMove.get() && Files.isReadable(tempFile)) {
+ if (IS_WINDOWS) {
+ copy(tempFile, file);
+ } else {
+ fsyncFile(tempFile);
+ Files.move(tempFile, file,
StandardCopyOption.ATOMIC_MOVE);
+ fsyncParent(tempFile);
+ }
+ }
Files.deleteIfExists(tempFile);
}
};
}
+ /**
+ * On Windows we use pre-NIO2 way to copy files, as for some reason it
works. Beat me why.
Review Comment:
Beat, why?
> Download fails if file is currently in use under windows
> --------------------------------------------------------
>
> Key: MRESOLVER-372
> URL: https://issues.apache.org/jira/browse/MRESOLVER-372
> Project: Maven Resolver
> Issue Type: Bug
> Components: Resolver
> Reporter: Christoph Läubrich
> Assignee: Tamas Cservenak
> Priority: Major
> Fix For: 2.0.0, 1.9.17
>
>
> With the new file-locking in maven-resolver there is a problem under windows
> if the file is currently used by another process (this can for example happen
> in an IDE ...) and resolver likes to move the file:
>
> {code:java}
> Caused by: java.nio.file.AccessDeniedException:
> xxx-SNAPSHOT.jar.15463549870494779429.tmp -> xxxx-SNAPSHOT.jar
> at
> java.base/sun.nio.fs.WindowsException.translateToIOException(WindowsException.java:89)
> at
> java.base/sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:103)
> at java.base/sun.nio.fs.WindowsFileCopy.move(WindowsFileCopy.java:317)
> at
> java.base/sun.nio.fs.WindowsFileSystemProvider.move(WindowsFileSystemProvider.java:293)
> at java.base/java.nio.file.Files.move(Files.java:1432)
> at org.eclipse.aether.util.FileUtils$2.move(FileUtils.java:108)
> at
> org.eclipse.aether.internal.impl.DefaultFileProcessor.copy(DefaultFileProcessor.java:96)
> at
> org.eclipse.aether.internal.impl.DefaultFileProcessor.copy(DefaultFileProcessor.java:88)
> at
> org.eclipse.aether.internal.impl.DefaultArtifactResolver.getFile(DefaultArtifactResolver.java:490)
> ... 30 more{code}
>
> My suggestion would be that resolver simply uses the temp file if it can't be
> moved to final location and marks it as delete on exit. Even though this is
> not optimal, it at least ensures the the build does not fail to the cost that
> next time the file needs to be downloaded again.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
