Re: [PATCH 2/2] checkout: fix attribute handling in checkout all

On Aug 12, 2007, at 11:50 PM, Junio C Hamano wrote:

Steffen Prohaska [off-list ref] writes:

quoted

We need to check out .gitattributes files first to have
them in place when we check out the remaining files. This
is needed to get the right attributes during checkout,
for example having the right crlf conversion on the first
checkout if crlf is controlled by a .gitattribute file.

This works only together with the commit

'attr: fix attribute handling if .gitattributes is involved'

While I think it is _one_ good approach to make things two-pass,
I do not know if this is enough.  A logic similar to this should
be made available to the codepath that switches branches,
shouldn't it?

I think so. I played a bit more and I am pretty sure that
switching branches suffers from the same problem. After I
understood the problem I now remember that I wondered why
I needed '-f' now and then to convince git to do things
that normally just work.

It feels somewhat bogus to treat only the files that contain
".gitattributes" as substring.  Don't you want to at least say
"is .gitattributes or ends with /.gitattributes"?

Yes, if someone really want to construct a case, he can break
my code. Where should I place a helper function, such as
ends_with_gitattributes()?

I am not 100% convinced that it is "unexpected" that
these two sequences give different results.

 (1) rm -f .gitattributes other
     git-checkout-index -f .gitattributes
     git-checkout-index -f other

 (2) rm -f .gitattributes other
     git-checkout-index -f other
     git-checkout-index -f .gitattributes

Yeah, it's not obvious to me either.

My feeling it that the working tree should match the current
content of .gitattributes. That is after you modified
.gittattributes by whatever means (checkout, editor, ...),
or you modified the global default for autocrlf, you should
have a way to update your working tree. One way would be to
force a fresh checkout of all files in the working tree.
How can I do that?

I'm pretty certain that all files in the working tree resulting
from a single command should match the .gitattributes that
were modified by the same command. This is true for initial
checkout, but also for branch switching.

And if this is mostly to work around the chicken-and-egg problem
of the initial checkout, I do not know if we would want to
complicate checkout_all() nor prepare_attr_stack().  Perhaps the
_initial_ checkout can do something like:

 * look at index, checkout .gitattributes and */.gitattributes;
 * checkout -f -a

_at the Porcelain level_, without complicating the plumbing?

I'm not convinced that it's only the initial checkout. As you
already mentioned above, branch switching suffers from the
same problem. But it could perhaps be handled on a porcelain
level.

Maybe we should start with some test cases first?

Both patches are seriously out of existing coding style, by the
way.  Extra spaces after called function names everywhere, etc.

Hmm, I see, ... my daytime coding style got burnt into my brain.
I'd rework if needed. But let's first find out what's needed.

	Steffen