From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Wed, 11 Jan 2012 03:30:11 GMT
Subject: Re: leaky cherry-pick
Message-ID: <CALkWK0kDnxjtQ+ihH_dif_7yivHLd=pibao4KPs_PDXfc2UMOA@mail.gmail.com>
In-Reply-To: <20120110195017.GA19961@sigill.intra.peff.net>

Hi Jeff,

Jeff King wrote:
> On Tue, Jan 10, 2012 at 10:58:45AM +0530, Ramkumar Ramachandra wrote:
>> diff --git a/attr.c b/attr.c
>> index 76b079f..12e3824 100644
>> --- a/attr.c
>> +++ b/attr.c
>> @@ -745,6 +745,7 @@ int git_check_attr(const char *path, int num,
>> struct git_attr_check *check)
>>               check[i].value = value;
>>       }
>>
>> +     drop_attr_stack();
>>       return 0;
>>  }
>
> I don't think this is right.

Yeah, I figured it wasn't right by running the testsuite.  I was
struggling to figure out why.

> The attr_stack is intentionally kept in
> place after a lookup as a cache, because callers are very likely to
> lookup nearby filenames. The first thing we do is pop unrelated parts of
> the stack, keep the early bits, and then push any new needed
> directories.
>
> So if you do a lookup for "foo/bar/baz/file1", the stack afterwards would
> be:
>
>  $GIT_DIR/info/attributes
>  foo/bar/baz/.gitattributes
>  foo/bar/.gitattributes
>  foo/.gitattributes
>  .gitattributes
>  [builtins]
>
> If you then do a lookup for "foo/bar/baz/file2", it can use the exact
> same stack without looking for or reparsing the attribute files. If you
> then do a lookup for "foo/bar/bleep/file", it pops only the entry for
> "foo/bar/baz/.gitattributes", and pushes only the entry for
> "foo/bar/bleep/.gitattributes".

I see.  Thanks for the excellent explanation-  I'll try implementing
this scheme.

-- Ram