Getters/Setters should update atime & mtime on file.stat

[phated]

Currently the file.stat.atime and file.stat.mtime aren't handled like they would be on a file system. I think the getters and setters should update them accordingly.

[phated]

Ref gulpjs/gulp#1461

[phated]

Closed by #75

[erikkemperman]

@phated The more I think about this, the more it seems to me this may have been merged prematurely. Well, submitted prematurely, to be completely honest. That is, the patch more or less does what this issue asked for, but:

1. I am still convinced it does it at the wrong level -- we're not really interested in when something gets the contents property itself, but rather when its value is accessed or modified via methods of Buffer and Stream.
2. It doesn't take into account changes to stat.mode which should update stat.ctime.
3. It makes things more strict as to the order in which plugins that use, say, mtime are piped.

To illustrate the last point:

gulp.task('test', function() {
  return gulp.src('in.js')
    .pipe(gulp-header('/* head */\n'))
    .pipe(gulp-changed('out.js'))
    .pipe(gulp.dest('out.js'));
});

The old behaviour would be for this to only (over)write out.js if it doesn't exist or if in.js has a later modification stamp. The new behaviour would be for this to always (over)write, since gulp-header modified the file contents and thus the mtime will have been updated.

So I guess I am saying that the new behaviour is conceptually correct, but we must now switch around gulp-changed and gulp-header to get the pipeline to do what we want.

[phated]

Changes have been rebased out of master to discuss edge case problems.

[erikkemperman]

@phated Thanks, yes I think this should probably be debated a bit more.

My feeling is your suggestion that started this issue is sound, but needs a slightly more delicate implementation to avoid some weird side-effects (as mentioned on the PR). And even then it will change the results of some fairly basic scenarios, as in the above.

But on the other hand perhaps that's ok, as long as it's documented clearly. I'm not in a position to judge, really.

Disregarding practical inconveniences, conceptually I might even argue to go all the way and just say that vinyl objects deal with stat changes in much the same way as an actual file would. So file.stat.mode = ... is effectively a chmod, for instance. Possibly it would enforce things like ownership on that as well.

Or is that really too far out?

I've been thinking about how to detect reads and writes on file.contents at the level of Stream and Buffer methods, which would avoid the quirks we saw earlier. Patching those methods would work but is messy... Proxies would be ideal for that, but probably it's out of the question to rely on experimental features like --harmony-proxies.

[Marak]

It seems to me there are two unique pieces of information that need to be tracked.

1. atime and mtime of the original source file
2. atime and mtime of the virtual vinyl File representation

The legacy behavior is such that the ecosystem depends on #1, so changing this behavior will be problematic.

Tracking the new information #2 ( the reason for this issue ) now needs to be done in a non-destructive way.

If there was no legacy API, I would make #2 the primary values returned from stat. I'd keep the original atime and ctime as separate values on the File object.

Since a lot of tools use #1, it would probably be best to add new properties and a File.vstat method for atime and ctime of vinyl File instance.

[phated]

Closing in favor of #105