Access and Modify time stats are preserved from source files

[tad-lispy]

If you run following gulpfile with Gulp 4 (current HEAD) you will get dest/file.txt with Access and Modification time preserved from source, (file.txt; January 1st 1970). I believe this is wrong - the modification time should reflect the time when each destination file was written by Gulp.

This is mostly problematic with plugins that only read one file, but this file depends on other files to produce a single output, e.g. gulp-stylus. In my case I pass only main.styl file to gulp.src and inside of it I have some @import statements. Changes to imported files trigger a task, but final CSS file has modification time of main.styl which seldom changes. This is causing issues with browsers cache - I get 304 Not Modified from my webserver.

Here is a sample gulpfile.js to reporduce the issue:

var gulp = require("gulp");
var exec = require("child_process").exec

gulp.task ("create", function(done) {
  exec("touch -d 1970-01-01 file.txt", done)
});

gulp.task ("process", function() {
  return gulp
    .src("file.txt")
    .pipe(gulp.dest("dest/"))
});

gulp.task ("check", function(done) {
  exec("stat -c %y dest/file.txt", function(error, stdout) {
    if (new Date(stdout).getFullYear() === 1970) {
      console.log("It's wrong.");
    }
    else {
      console.log("It is OK in this version.");
    }
    done(null);
  });
});

gulp.task ("default", gulp.series([
  "create",
  "process",
  "check"
]));

Equivalent file executed with v. 3 produces correct (IMO) output.

edit: Wrong argument was passed to stat command (%x instead of %y). Fixed.

[phated]

Thanks for filing this. It is on our radar over at gulpjs/vinyl-fs#128 but changes need to be made in the content setter of https://github.com/gulpjs/vinyl to update the stat times when contents are changed.

[tad-lispy]

Thanks for pointing me there. I've read the discussion and it seems to reach rather deep into vinyl internals. I'd love to help there, but I don't think I'm up to it ATM.

In case anybody bumps into this superficial issue of gulp-4 here is my quick and dirty workaround until it gets resolved in vinyl-fs.

// ... 
var hrough = require('through2');
var utimes  = require('fs').utimes;

var touch = through.obj(function(file, enc, done) {
  var now = new Date;
  utimes(file.path, now, now, done);
});

// ...

gulp.task ("process", function() {
  return gulp
    .src("file.txt")
    .pipe(gulp.dest("dest/"))
    .pipe(touch)
});

[phated]

@lzrski you can actually modify the stat object on each vinyl file before the gulp.dest and it will modify the times to the ones you set (avoid another fs call) which was actually the whole goal that caused this problem in the first place 😛

[tad-lispy]

@phated works, thanks :)

This may be a very lame question so excuse my ignorance:

Can't gulp.dest just do it for every file it writes?

Maybe at least before you guys come up with more clever solution in vinyl / vinyl-fs you can cut one corner here and then take time necessary to engineer it better. You said it's the last roadblock and I'm sure there are lots of people who would love to see Gulp 4 released.

In case my question is too dumb to answer just say so and I'll shut up :) Anyway Gulp is absolutely great and I use it every day. Thanks for that.

[phated]

I've been digging into *nix commands (namely cp) and it seems that the "preserve timestamps" mode is not default. I think we are going to end up putting this functionality behind a flag similar to cp -p

[tad-lispy]

+1

The only use case for preserving mtime I can think of is for static assets, i.e. files that get copied from src to dest without modification. Here preserving is good for browsers cache. But I would argue that this is an exception, so flag is a good idea.

On the other hand using getters and setters in vinyl object is way more elegant. It mimics real filesystem behaviour. How hard do you think that would be? Should I try a PR there or will I drown in complexity?

[phated]

@lzrski vinyl is a much simpler library than vinyl-fs. Most code, expect some utility functions, is in the single index.js file. If you have an understanding of how real filesystems adjust the atime/mtime, I'd love for you to attempt an implementation.

[erikkemperman]

@phated @lzrski FYI I had already started on this a few days back, in the getter/setter of vinyl. Just need to add some tests. Maybe I should have indicated that I was doing this... Hope we didn't duplicate too much effort!

I didn't add a preserve flag though. How would that work? An option to src and then pass it along as an attribute on the vinyl object? Frankly I don't see a huge benefit in having such a flag, wouldn't using a filtering plugin like gulp-newer or gulp-changed be sufficient? Or am I missing some obvious scenario?

By the way, if we're looking at command line primitives for inspiration, wouldn't it be a better match with how gulp works to consider something like cat srcfile > dstfile instead of cp?

[phated]

@erikkemperman can you submit a PR with your work to vinyl so anyone that wants to work on this has a starting point? I think you are right that the comparison is cat srcfile > dstfile instead of cp and don't need a preserve option flag if we fix vinyl properly.

[erikkemperman]

@phated Yeah, couple of problems here unfortunately... Laptop died during the holidays, so I will need to re-do what I'd done earlier. But in the meantime it also occurred to me that I might have been approaching this too shallow.

What I'd done is just to set the atime from within the contents getter, and mtime from the setter. But this doesn't cover the case where plugins might modify an existing Buffer, for instance. And if it is a stream, I suppose that reading it should update atime and writing it should update mtime.

Then there is ctime, which I can't imagine being very interesting to a plugin, and we can't actually reflect it on disk via fs.futimes -- but technically I guess it should be updated whenever mtime is, and additionally when mode or uid/gid is set?

Thing is I don't see how to do any of this without ugly monkeypatching stream and buffer methods.

So I could make a PR of the simple case pretty quickly but I think that might not be sufficient. Any ideas?

[phated]

@erikkemperman that's terrible about your laptop. I think the shallow case might be fine along with some documentation updates. We've always recommended the following patterns:

// Buffers
var contents = file.contents.toString();
// modify contents
file.contents = new Buffer(contents);

// Streams
file.contents = file.contents.pipe(someTransform);

If the documentation was updated to explain that the reassignment always needs to happen and why, we can get away with the shallow implementation. I had thought about the mode, uid and gid which is why I started https://github.com/gulpjs/better-stats but got hung up on some details and where to go with it.

[erikkemperman]

@phated Appreciate the sentiment -- actually it was a small miracle that it didn't happen before, it was pretty old. Back to desktop, for now.

Anyway, I've been thinking about the "deep" approach a little more, and having noticed this issue about stat-mode earlier, I played around with a little class today which I now realize much resembles your better-stats, except that it would also maintain the time attributes internally.

Probably you're right that the shallow approach would cover a pretty large area of use cases. I'm having a hard time thinking of anything but contrived exceptions.

I'll make a PR for that shortly.

But then I'll likely still want to try finding a more comprehensive approach... This metadata stuff somehow feels like it deserves a pretty thorough design. Our talk of mimicking cat infile > outfile got me to thinking it should be easy to come up with a matrix of outcomes, and thus a complete picture of what vinyl/vinyl-fs ought to do in different circumstances. Difficult to unit test, though.

I get that people want something out there, but would the @next tag mean that more or less fundamental things like the stat object can no longer be messed with?

[phated]

Thanks for following up so quickly. Once we publish 4.0.0 to npm with the next tag, we can't do anything breaking, just bug fixes and features. I think we need to land the basic functionality in vinyl so vinyl-fs is outputting correct timestamps.

[JacobDB]

Just upgraded to gulp 4, and I'm still seeing this (2+ years later). Fixed with gulp-touch-fd, but are there any plans to really fix this?

[phated]

@JacobDB this behavior is due to supporting gulp.src().pipe(gulp.dest()) to be the same behavior as the cp operator on Mac and Linux. There's a fundamental flaw in the way Vinyl works and requires a ton of work to fix. If you'd like to contribute, further information is available at gulpjs/vinyl#105 (comment) - otherwise you'll need to use the module you linked.