Re: new platform & S_IFGITLINK problem

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: May 4, 2010, 03:52 UTC
Message-ID: <alpine.LFD.2.00.1005032042470.5478@i5.linux-foundation.org>
In-Reply-To: <alpine.LFD.2.00.1005032035310.5478@i5.linux-foundation.org>

On Mon, 3 May 2010, Linus Torvalds wrote:

Show 16 quoted lines

> On Sat, 1 May 2010, Junio C Hamano wrote:
> > 
> >  (3) write MODE_SYSTEM_TO_GIT() macro to convert from S_IF{REG,DIR,LNK} we
> >      read from struct stat to the "canonical" GIT_S_IF{REG,DIR,LNK}
> >      values; and
> > 
> >  (4) change all the code that read mode from struct stat and use it to
> >      first use MODE_SYSTEM_TO_GIT().
> > 
> > Currently 'git grep -e "S_IF[A-Z]" -e "struct stat"' reports around 250
> > hits, so it is not infeasible amount of work, but it is not a trivial and
> > mechanical replacement, either.  I or somebody need to set aside a block
> > of time to do this clean-up and audit the result.
> 
> Ugh. And since nobody sane has different values from the system ones, if 
> we miss some case we'll never notice on any sane platform ;(

As to your (3) and (4), I actually think that the best option would be to stop using '[lf]stat()' directly for "working tree stat", and instead just introduce a 'git_[lf]stat()' function that just always does the conversion (when necessary - the "conversion" can be a no-op on sane architectures) for us.

Right now, a _lot_ of the functions work either directly on 'st->st_mode' _or_ on random index entries, and that would be a major pain if they might ever be in "different number domains". So the easiest way to make sure that we _always_ use the GIT_IFxyz numbers would be to never ever have anything that uses the native ones even by mistake.

I've actually wanted to have a 'git_lstat()' wrapper for other reasons: it would have made it _so_ much easier to do breakpoints etc when doing the whole name lookup optimizations.

Note: there are various cases of '[lf]stat()' being used not for working 
tree entries, but for things like the pack files etc internal git files. 
Those should _not_ do the conversion - they should use the "native" stat() 
functions. It's only the working tree accesses we should need to do any 
conversion on, since those are the ones that are relevant for the index 
(and tree) comparisons.

			Linus