[GitHub] [commons-io] kinow commented on a diff in pull request #377: IO-769: FileUtils copyDirectory() should not use COPY_ATTRIBUTES

[GitBox <gi...@apache.org>]

kinow commented on code in PR #377:
URL: https://github.com/apache/commons-io/pull/377#discussion_r961252278


##########
src/main/java/org/apache/commons/io/FileUtils.java:
##########
@@ -2840,37 +2836,30 @@ private static File requireFileIfExists(final File file, final String name) {
     }
 
     /**
-     * Sets the given {@code targetFile}'s last modified date to the value from {@code sourceFile}.
+     * Set file lastModifiedTime, lastAccessTime and creationTime to match source file
      *
      * @param sourceFile The source file to query.
      * @param targetFile The target file or directory to set.
      * @throws NullPointerException if sourceFile is {@code null}.
      * @throws NullPointerException if targetFile is {@code null}.
      * @throws IOException if setting the last-modified time failed.
      */
-    private static void setLastModified(final File sourceFile, final File targetFile) throws IOException {
+    private static void setTimes(final File sourceFile, final File targetFile) throws IOException {
         Objects.requireNonNull(sourceFile, "sourceFile");
         Objects.requireNonNull(targetFile, "targetFile");
-        if (targetFile.isFile()) {
-            PathUtils.setLastModifiedTime(targetFile.toPath(), sourceFile.toPath());
-        } else {
-            setLastModified(targetFile, lastModified(sourceFile));
-        }
-    }
-
-    /**
-     * Sets the given {@code targetFile}'s last modified date to the given value.
-     *
-     * @param file The source file to query.
-     * @param timeMillis The new last-modified time, measured in milliseconds since the epoch 01-01-1970 GMT.
-     * @throws NullPointerException if file is {@code null}.
-     * @throws IOException if setting the last-modified time failed.
-     */
-    private static void setLastModified(final File file, final long timeMillis) throws IOException {
-        Objects.requireNonNull(file, "file");
-        if (!file.setLastModified(timeMillis)) {
-            throw new IOException(String.format("Failed setLastModified(%s) on '%s'", timeMillis, file));
-        }
+        try {
+            // Set creation, modified, last accessed to match source file
+            final BasicFileAttributes srcAttr = Files.readAttributes(sourceFile.toPath(), BasicFileAttributes.class);
+            final BasicFileAttributeView destAttrView = Files.getFileAttributeView(targetFile.toPath(), BasicFileAttributeView.class);
+            // null guards are not needed; BasicFileAttributes.setTimes(...) is null safe
+            destAttrView.setTimes(srcAttr.lastModifiedTime(), srcAttr.lastAccessTime(), srcAttr.creationTime());
+        } catch(IOException unused) {
+            // Fallback: Only set modified time to match source file
+            targetFile.setLastModified(sourceFile.lastModified());
+        }
+
+        // TODO: (Help!) Determine historically why setLastModified(File, File) needed PathUtils.setLastModifiedTime() if
+        //  sourceFile.isFile() was true, but needed setLastModifiedTime(File, long) if sourceFile.isFile() was false

Review Comment:
   Thanks for adding the note. No idea why that changed, didn't have time to dig into `git`'s history, sorry.



-- 
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-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org