Re: [PATCH] Define a version of lstat(2) specially for copy operation

[Junio C Hamano <gitster@pobox.com>]

Johannes Sixt <j.sixt@viscovery.net> writes:

> Sorry that I skip your elaboration on this. First, I barely understand
> read-cache.c; and second, I have the feeling that this is
> over-engineering: It introduces a genericity that we don't need in
> practice.

That's Ok.  The whole speculation depended on your answer to (2) being
"yes", and because it is not the case, you can safely skip it.

I just wanted to see if we can sprinkle a handful of lstat_remainder()
calls that disappear on POSIX side without changing anything else, which
would be the least impact solution from my point of view.

> But that wouldn't be different in principle from Alex's patch that
> introduced lstat_for_copy(), would it? Since it would be used in exactly
> the same strategic places.

Hm, I sort of agree.

If you imagine somebody with POSIX background who is new to git's codebase
and is trying understand the resulting code, I thought lstat() followed by
lstat_remainder() is conceptually slightly easier to explain ("'remainder'
is needed on platforms whose lstat() do not get the x-bit right, and we
use it only where it matters"), but I do not think the difference is that
great.

You need to explain when to add the "remainder" one after an existing
lstat() call in this API, or you need to explain when to replace an
existing lstat() with "for copy" in the Alex's API.  Either way you need
to contaminate the generic codepath with the distasteful concept that "On
some platforms, you lstat() may not do what you asked it to do".

And that was what I was unhappy about Alex's patch to begin with.  Not the
"patch" nor "Alex", but the fact that we have to deal with the "issue".

And lstat() followed by lstat_remainder() does not solve that issue at
all.  So let's scrap this lstat_fast()/lstat_remainder().