Re: [PATCH v4 2/2] apply: reload .gitattributes after patching it

[Phillip Wood <phillip.wood123@gmail.com>]

On 20/08/2019 04:05, brian m. carlson wrote:
> On 2019-08-19 at 09:55:27, Phillip Wood wrote:
>> On 19/08/2019 10:41, Phillip Wood wrote:
>>> [...]
>>>> diff --git a/convert.c b/convert.c
>>>> index 94ff837649..030e9b81b9 100644
>>>> --- a/convert.c
>>>> +++ b/convert.c
>>>> @@ -1293,10 +1293,11 @@ struct conv_attrs {
>>>>        const char *working_tree_encoding; /* Supported encoding or
>>>> default encoding if NULL */
>>>>    };
>>>> +static struct attr_check *check;
>>>
>>> I was concerned about the impact adding a file global if we ever want to
>>> multi-thread this for submodules, but looking through the file there are
>>> a couple of others already so this isn't creating a new problem.
>>
>> Doh, I've just realized it was static already - ignore that.
> 
> And I just realized that I didn't read the entire thread before
> responding.  Sorry about that.
> 
>> One thing did occur to me though - does this patch reset attributes like the
>> merge marker length (they're less critical though if there is a conflict
>> after an attribute change it would be nice to have the correct length) or
>> just the ones for filtering files?
> 
> It resets "crlf", "ident", "filter", "eol", "text", and
> "working-tree-encoding".  Things it doesn't reset include "whitespace",
> "export-ignore", "export-subst", "merge", and "conflict-marker-size".
> Of these, I think only the latter two are relevant.
> 
> I'll update that in v5.

Thanks, one other thought I had was that this is really a bug in 'am' 
rather than 'rebase'. There has been some talk of switching the default 
rabase backend to the sequencer so maybe it would be better to test 'am' 
rather than 'rebase' to ensure we still check 'am' is working properly 
after any switch in the rebase backend. (perhaps we should be testing 
rebase and rebase --interactive separately as well to prevent any future 
regressions I'm not sure)

Best Wishes

Phillip