fs: preserve directory timestamps in cp - #65540
Conversation
This fixes an issue where fs.cp and fs.cpSync (when a filter is provided) correctly restore mtime and atime on copied files when preserveTimestamps: true is passed, but skip restoring them for directories. The fix applies the existing setDestTimestamps helper to directories, ensuring it is called after directory creation/copying but before any modes are restored (since restoring a read-only mode would prevent timestamp modification). Note: This fix covers fs.cp() (async, all cases) and fs.cpSync() when a filter is provided. The native fast path used by cpSync() without a filter (CpSyncCopyDir in src/node_file.cc) has the identical gap and is left for a separate follow-up contribution, since it requires native code changes and a different review path. Signed-off-by: Abhinandan Kumar <abhi128618@gmail.com>
70b223c to
321146b
Compare
|
Could someone please check the result of Jenkins CI job 76579? I’m unable to access the job because Jenkins shows “abhi128nandan is missing the Overall/Read permission”, so I can’t see whether there’s an actual test failure or an infrastructure issue. Thanks! |
I woldn't normally see a request like this but I happened to be looking at the job (randomly) for something else so I'll reply. The jobs is still running. The message with the link was only posted 40 minutes ago and it takes longer than that to run through the tests. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #65540 +/- ##
=======================================
Coverage 90.13% 90.13%
=======================================
Files 751 751
Lines 253656 253644 -12
Branches 47787 47800 +13
=======================================
- Hits 228631 228626 -5
Misses 16258 16258
+ Partials 8767 8760 -7
🚀 New features to boost your workflow:
|
|
Could a collaborator please approve/re-run the GitHub Actions and start Jenkins CI for the updated HEAD |
Was there a reason to sync? AFAICT there were no conflicts with current |
fs: preserve directory timestamps in cp
This fixes an issue where fs.cp and fs.cpSync (when a filter is
provided) correctly restore mtime and atime on copied files when
preserveTimestamps: true is passed, but skip restoring them for
directories.
The fix applies the existing setDestTimestamps helper to directories,
ensuring it is called after directory creation/copying but before any
modes are restored (since restoring a read-only mode would prevent
timestamp modification).
Note: This fix covers fs.cp() (async, all cases) and fs.cpSync()
when a filter is provided. The native fast path used by cpSync()
without a filter (CpSyncCopyDir in src/node_file.cc) has the
identical gap and is left for a separate follow-up contribution, since
it requires native code changes and a different review path.