Subject: Re: [PATCH/RFC v2 2/3] make USE_NSEC work as expected

From: Junio C Hamano <[email-masked]>
Date: Fri, 20 Feb 2009 21:42:25 -0800

Kjetil Barvik <[email-masked]> writes:

> Junio C Hamano <[email-masked]> writes:
>
>>> +	ce->ce_ctime.sec = (unsigned int)st->st_ctime;
>>> +	ce->ce_mtime.sec = (unsigned int)st->st_mtime;
>>> +#ifdef USE_NSEC
>>> +	ce->ce_ctime.nsec = (unsigned int)st->st_ctim.tv_nsec;
>>> +	ce->ce_mtime.nsec = (unsigned int)st->st_mtim.tv_nsec;
>>> +#else
>>> +	ce->ce_ctime.nsec = 0;
>>> +	ce->ce_mtime.nsec = 0;
>>> +#endif
>>
>> How does this affect a use case where the same index file used with two 
>> instances of git (one compiled with and another without USE_NSEC)?
>
>   OK, I admit that I was thinking safe here,...

It was not a veiled objection in the guise of a rhetoric question.  I just
wanted to know what happens, when you have "/usr/bin/git" compiled without
NSEC and "/usr/local/bin/git" compiled with NSEC, and tried to use the two
interchangeably.  A NSEC enabled one will leave nsec in the index entry,
and the normal one reads from the index file (truncating the nsec from
both file timestamp and on-disk index entries), and it is unclear what it
does to the racy-git algorithm.  If there is no adverse effect, that is
great.  Otherwise, even though the on-disk format might be compatible, we
need to somehow tell people not to use two gits on the same index file.

>>> diff --git a/unpack-trees.c b/unpack-trees.c
>>> index e547282..44714cc 100644
>>> --- a/unpack-trees.c
>>> +++ b/unpack-trees.c
>>> @@ -380,8 +380,12 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
>>>  
>>>  	memset(&o->result, 0, sizeof(o->result));
>>>  	o->result.initialized = 1;
>>> -	if (o->src_index)
>>> -		o->result.timestamp = o->src_index->timestamp;
>>> +	if (o->src_index) {
>>> +		o->result.timestamp.sec = o->src_index->timestamp.sec;
>>> +#ifdef USE_NSEC
>>> +		o->result.timestamp.nsec = o->src_index->timestamp.nsec;
>>> +#endif
>>> +	}
>>
>> Do we need this hunk?
>
>   Since timestamp is now a 'struct cache_time' member, I converted the
>   usage of this if-test to be in line with the USE_NSEC usage.

My question was if it is wrong to leave this as "a->timestamp = b->timestamp"
which now becomes structure assignment.  You would need to move extra 4
(or 8) bytes if you are not using NSEC, but this is not per index entry
but one assignment for the whole index file, so...