Re: [PATCH] attr: do not mark queried macros as unset
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 22, 2019, 22:00 UTC
- Message-ID
- <xmqqef94e4kc.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20190118214626.GC28808@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 7 quoted lines
> And here's a resurrection of the optimization that _seems_ to work, but > I'm not 100% confident in. > > In particular, it does not care about macros at all. It simply asks: is > this queried attribute a thing which was ever mentioned in the > attributes files (either as a path match or as a possible macro > expansion). If not, then we know we do not need to look further for it.
So, if we are looking for 'diff' and we know no .gitattributes (or $GIT_DIR/info/attributes) entry for 'diff' or any macro that expands to touch 'diff' (e.g. 'binary') is in use, we know for any path governed by the current attr-stack 'diff' attribute is unspecified. But if we see an entry, say, "*.exe binary", then we do need to be aware of the possibility that 'diff' may be unset for some paths.
Makes sense.
Show 9 quoted lines
> I guess maybe what I'm missing is that asking for "diff" means that we > need to care about: > > - whether "diff" was mentioned in the stack > > - whether "binary" was mentioned in the stack > > But just "binary" mentioning "diff" is not interesting without somebody > actually mentioning "binary".
Yeah, that matches my understanding (which mostly comes from the original design before even Duy's optimization).
Show 7 quoted lines
> I'm not sure how to do it robustly without being able to reverse-map all > of the macros after we've resolved them (i.e., to know that "diff" gets > mentioned by "binary", and then check if "binary" is actually > mentioned). I think that would be possible now, as we should know that > after determine_macros(). But I also wonder if we are hitting > diminishing returns (after all, determine_macros() is already walking > the attr stack).