sepia
sepia copied to clipboard
Fixed EINVAL error with fs.utimesSync() due to incorrect Unix timestamp
This should address issue #17
@mseminatore: Thanks for the PR. Two things:
-
Since I don't work at LinkedIn anymore, we'll have to track down someone who can merge into this repositor. Or, you can open up a PR on my fork of this project.
-
I'm not sure I understand why this problem just became apparent now. Can you provide a test case that demonstrates the problem by failing before your change and passing afterwards?
@avik-das I can reproduce this using the following code:
// utimes-test.js
var fs = require('fs');
var now = Date.now();
fs.utimesSync('a.txt', now, now);
console.log("Success!");
c:\sepia-test>echo hello > a.txt c:\sepia-test>node utimes-test.js
I am not certain why this wasn't an issue either. Maybe the prior code works on other OS's or different Node versions. In any case the current NodeJS documentation is clear that Date.now() can't be used as is for atime and mtime.
Snipped:
Note: the arguments atime and mtime of the following related functions follow these rules:
- The value should be a Unix timestamp in seconds. For example, Date.now() returns milliseconds, so it should be divided by 1000 before passing it in.
- If the value is a numeric string like '123456789', the value will get converted to the corresponding number.
- If the value is NaN or Infinity, the value will get converted to Date.now() / 1000.
@avik-das OK, PR to your fork sent as well.
@avik-das Do you have any contact with anyone who can accept this PR? It would be nice to have this fix integrated.
@mseminatore - This repo appears to be abandoned. A couple of us created a fork called "replayer" to start accepting commits. You'll need to rename sepia=>replayer.
Could you submit your PR there?
@aneilbaboo I've not done this before. Can you share the specific steps that I would need to take? I assume it is more than simply renaming my forked repository. That won't connect my fork to your repo though will it?
@avik-das Have you had any luck contacting the LinkedIn owners?
@mseminatore - That won't work.
I think this is right, but haven't checked it:
-
fork replayer & clone replayer
-
add a remote pointing at your sepia repo
git remote add mysepia [email protected]:msemiatore/sepia.git
-
checkout your sepia master branch to a new local branch in your local replayer repo, giving it a new name, to avoid confusing it with the replayer master branch. E.g., call it the einval branch.
git fetch mysepia && git checkout -b einval --track mysepia/master
-
checkout the replayer master branch
git checkout master
-
merge your branch into master and enjoy fixing the merge conflicts
git merge einval
This should mostly be renamings of sepia => replayer in your code. -
issue a pull request
If you're having trouble or would just prefer me to do it, that's fine too.
I took a look at the changes. They seem fine.