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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 12, 2007, 21:50 UTC
Message-ID: <7veji8ifs2.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <118695087531-git-send-email-prohaska@zib.de>

Steffen Prohaska <prohaska@zib.de> writes:

Show 9 quoted lines

> 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?

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"?

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

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?

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