Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs.copyFile - add option to keep timestamps #15793

Closed
metabench opened this issue Oct 6, 2017 · 9 comments
Closed

fs.copyFile - add option to keep timestamps #15793

metabench opened this issue Oct 6, 2017 · 9 comments
Labels
feature request Issues that request new features to be added to Node.js. fs Issues and PRs related to the fs subsystem / file system.

Comments

@metabench
Copy link

fs.copyFile - It would be useful to have options to keep date created and date modified timestamps.

Possibly this would be beyond trivial to implement, but in my opinion it would be an excellent extension to node's cross-platform system capabilities.

@mscdex mscdex added feature request Issues that request new features to be added to Node.js. fs Issues and PRs related to the fs subsystem / file system. labels Oct 6, 2017
@tniessen
Copy link
Member

tniessen commented Oct 6, 2017

I like the idea, many libraries / runtimes already have such options. There are some things to consider though: If I remember this correctly, Unix inodes support change, modification and access timestamps, so there is no guarantee that creation timestamps exist on Unix filesystems, whereas Windows uses creation, modification and access timestamps.

If we decide to support such an option, this will need to be implemented upstream (in libuv).

@bnoordhuis
Copy link
Member

Libuv in general cannot do better than call futimes() afterwards. That's something node.js could do so there is no reason for it to live in libuv. I don't think there are platform-specific APIs that would justify that.

Of note is that futimes() won't preserve the change time or birth time.

@bnoordhuis
Copy link
Member

It's been a month with no action, and in light of my previous comment, I'm closing this out. Reopen if you feel this is worth pursuing (but please motivate why.)

@System-Arch
Copy link

System-Arch commented Dec 12, 2017

@bnoordhuis @metabench

I am not sure how to reopen this issue, but I would like to advocate for it being re-opened. At present the behavior on Linux differs from that on Windows and Mac in an unexpected manner. While all three platforms preserve file mode metadata, only Windows and Mac preserve the modification time. This discrepancy could be rectified by having node/deps/uv/src/unix/fs.c:uv__fs_copyfile() call futimes() similar to the way it calls fchmod(). If the existing behavior is deemed desirable under some circumstances, an additional flag could be added, e.g., COPYFILE_STAT (following the existing Mac-inspired paradigm of COPYFILE_EXCL) to control this behavior.

As for motivation, currently one must add as explicit call to fs.utimes() in their application code to achieve platform parity, and one must test their code on all three platforms to discover this undocumented variation in behavior.

@bnoordhuis
Copy link
Member

@System-Arch I'm not sure if that is a change we'd accept it libuv because of the race window between the copy and the timestamp update.

Any way you slice it, there will be a window where another process can manipulate the file and cause the timestamps to be off. Better to be consistently wrong all the time than flaky wrong some of the time.

However, if you want to pursue this, please open an issue or pull request over at libuv/libuv.

@MylesBorins
Copy link
Member

It looks like not having accurate time stamps on fs.copy has caused fs-extra to ship broken code... seems like expectations are not being met.

@bnoordhuis would you say this is worth exploring?

@bnoordhuis
Copy link
Member

The race condition is intrinsic, not much to explore there. Likewise the birth time issue since you can't change that after the fact.

If someone still thinks it's an excellent idea, he or she should open a pull request and we'll see where the discussion leads.

@BYK
Copy link

BYK commented Mar 9, 2018

@bnoordhuis this also hurt us at Yarn. Do you think we can at least add something to the docs regarding this in the meantime? (not sure if this is going to be addressed but it would have been great to know about it at before we got burnt :))

If you agree with the docs, I'll submit a PR for that.

@cjihrig
Copy link
Contributor

cjihrig commented Mar 16, 2018

@BYK I think a docs update calling out this corner case would be good.

tonywoode added a commit to tonywoode/quickPlayNode that referenced this issue Aug 29, 2019
...nix will grow dest file, windows will allocate full size on write
that's fine, we can check mdate on windows, but mdate on nix will always
be updated on successful write. To ensure mdate is the same as source
file on all platforms, write it! see
nodejs/node#15793 - in this use case the file
is unlikely to change between the two operations!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

No branches or pull requests

8 participants