gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1305645105
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
+ String message;
+ if (!source.exists()) {
+ message = "File " + source + " does not exist";
+ throw new IOException(message);
+ } else if
(!source.getCanonicalPath().equals(destination.getCanonicalPath())) {
+ File parentFile = destination.getParentFile();
+ if (parentFile != null && !parentFile.exists()) {
+ parentFile.mkdirs();
+ }
+ Files.copy(
+ source.toPath(),
+ destination.toPath(),
+ StandardCopyOption.REPLACE_EXISTING,
+ StandardCopyOption.COPY_ATTRIBUTES);
+ if (source.length() != destination.length()) {
Review Comment:
maven-compat is deprecated and should be gone in Maven 4.1, there's no point
in enhancing the code there. Quite the opposite, any change may introduce
regressions in legacy / untested code. Untested, because all the ITs have now
been updated and that code path isn't used anymore, but in very plugins. If
you want to modify a code that has been proven to work, please provide a unit
test for that then.
Also, the following test simply exhibits that the check has an effect and is
not a no-op. The documentation does in no way guarantee that the copy is
atomic, it's actually specifically written that it's not. I agree there's no
way simple way to ensure that in this code, and I certainly would not want to
go in that direction for the reason that code is bound to be removed soon.
```
public static void main(String[] args) throws Exception {
Path dir = Files.createTempDirectory("test-");
Path source = dir.resolve("source.txt");
try (Writer w = Files.newBufferedWriter(source,
StandardCharsets.UTF_8)) {
for (int i = 0; i < 1024 * 1024; i++) {
w.append("01234567890\n");
}
}
Path dest = dir.resolve("dest.txt");
for (int t = 0; t < 4; t++) {
new Thread(() -> {
try {
Files.copy(source, dest,
StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES);
if (Files.size(source) != Files.size(dest)) {
throw new IOException("Failed to copy full contents
from " + source + " to " + dest);
}
} catch (IOException e) {
throw new IOError(e);
}
}).start();
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
