Re: [RFC PATCH] checkout: Force matching mtime between files

From: Michał Górny <mgorny@gentoo.org>
Date: Apr 25, 2018, 07:13 UTC
Message-ID: <1524640391.1104.26.camel@gentoo.org>
In-Reply-To: <robbat2-20180425T065446-231760424Z@orbis-terrarum.net>

W dniu śro, 25.04.2018 o godzinie 06∶58 +0000, użytkownik Robin H. Johnson napisał:

Show 13 quoted lines

> On Fri, Apr 13, 2018 at 07:01:29PM +0200, Michał Górny wrote:
> > --- a/entry.c
> > +++ b/entry.c
> > @@ -411,6 +411,7 @@ int checkout_entry(struct cache_entry *ce,
> >  {
> >  	static struct strbuf path = STRBUF_INIT;
> >  	struct stat st;
> > +	int ret;
> >  
> >  	if (topath)
> >  		return write_entry(ce, topath, state, 1);
> 
> mgorny: Should the topath case trigger utime as well?

I don't think so. AFAIU topath case is only used by 'git checkout-index --all', and it implies that data is written to temporary files rather than actual working copy files, so mtimes do not really matter there.

> 
> Other questions:
> - Would there be be any value in hoisting the utime change into
>   write_entry's finish block rather than having it in checkout_entry?

I've attempted to reduce the scope of my changes to minimum, therefore checkout_entry() seemed like the 'closest' thing to where it is set and where it could be implemented. I see no problem in doing that in write_entry(), though. In fact, it might be useful to do that before filling the stat cache.

> - Should mtimes on directories be set if the directory is explicitly
>   created?

I don't think there's really a purpose in that. I can't think of any reason why anybody would be able to use directory mtimes reliably, so maybe keeping their standard behavior is better here.

> - Maybe using futimens on supported platforms?

I'm all for that. However, this is something the git maintainers should decide as it probably implies some maintenance burden.

-- 
Best regards,
Michał Górny