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

Changed to use futimes instead of utimes #1

Merged
merged 5 commits into from
Aug 17, 2016

Conversation

ibobo
Copy link
Contributor

@ibobo ibobo commented Aug 11, 2016

As stated here utimes has 1 second resolution, whereas futimes has 1 microsecond resolution.
Since stat also has a 1 microsecond resolution, this caused conditional builds (with gulp-newer for instance) to always rebuild files on platforms with 1 microsecond timestamps for files (Linux with ext2/3).

As stated here: nodejs/node-v0.x-archive#7000
utimes has 1 second resolution, whereas futimes has 1 microsecond resolution.
Since stat also has a 1 microsecond resolution, this caused conditional
builds (with gulp-newer for instance) to always rebuild files on
platforms with 1 microsecond timestamps for files (Linux with ext2/3).
@richardlawley
Copy link
Owner

The idea sounds fine, but the tests fail on Windows. If you can fix it so it works on both, I'll merge it in.

The atime check was failing because the atime of the source files was
updated when the first test copied the files; by recreating the test
environment for each test the problem is solved
@ibobo
Copy link
Contributor Author

ibobo commented Aug 11, 2016

I fixed the tests on my machine: they were failing because when the first test copied the files their atime was updated also, causing a difference with the reference.
I tried sending some more changes, but I cannot understand what's wrong with appVeyor, sorry. Hope you can take a look.
I tested the branch on Mac and Linux: the test goes fine and the code run correctly; the problems on appVeyor seem related to file permissions.

@richardlawley
Copy link
Owner

I don't think the problem is related to file permissions, but the tests fail on my Windows machine in the same way that they do on appveyor (I believe appveyor is running on Windows). I've spent a little time this morning trying to fix it on Windows, and I can't get it to work.

@ibobo
Copy link
Contributor Author

ibobo commented Aug 17, 2016

I found the time to make it work on Windows, the file had to be opened for writing (so I used 'a' mode instead of 'r')

@richardlawley richardlawley merged commit a597a3c into richardlawley:master Aug 17, 2016
@richardlawley
Copy link
Owner

Thanks, that makes sense! Published to NPM as 1.1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants