On Tue, Apr 14, 2020 at 03:55:35PM -0400, Jeff King wrote:
> On Tue, Apr 14, 2020 at 04:27:26PM +0200, luciano.rocha@xxxxxxxxxxx wrote:
>
> > Update freshen_file() to use a NULL `times', semantically equivalent to
> > the currently setup, with an explicit `actime' and `modtime' set to the
> > "current time", but with the advantage that it works with other files
> > not owned by the current user.
> >
> > Fixes an issue on shared repos with a split index, where eventually a
> > user's operation creates a shared index, and another user will later do
> > an operation that will try to update its freshness, but will instead
> > raise a warning:
> > $ git status
> > warning: could not freshen shared index '.git/sharedindex.bd736fa10e0519593fefdb2aec253534470865b2'
>
> Thanks, this makes sense. And as a bonus, it's less code. ;)
>
> I don't recall having any particular reason not to use NULL in the
> original (I may simply not have been aware it was an option), and I
> can't find any discussion either way.
>
> One observation:
>
> > static int freshen_file(const char *fn)
> > {
> > - struct utimbuf t;
> > - t.actime = t.modtime = time(NULL);
> > - return !utime(fn, &t);
> > + return !utime(fn, NULL);
> > }
>
> The old code was setting the time based on the system time from time().
> We've occasionally hit cases where the filesystem time and the system
> time do not match exactly (this might be true on an NFS mount, for
> example).
>
> It's not clear to me whether utime(NULL) would be using the system or
> filesystem time in such a case. If the former, then there's no change in
> behavior. If the latter, I'd argue that it's probably an _improvement_,
> since we're simulating the case that we wrote a new file with a new
> mtime.
I'm not that familiar with kernel code, so can't say for sure. From a
cursory look, it doesn't seem like it uses the remote server's time.
But it does seem to have a higher precision, for filesystems that
support it.
In Linux, it ends up calling current_time(inode), which says:
* Return the current time truncated to the time granularity supported by
* the fs.
And uses ktime_get_coarse_real_ts64(). That could explain any previous
case where utime() of time(NULL) was different that just utime() or an
effect from writing to a file.
Arguably, even more of an improvement if it gives higher resolution.
Regards,
--
Luciano Rocha
