Re: [PATCH] add core.pathspecGlob config option
- From
Jeff King <peff@peff.net>
- Date
- Dec 20, 2012, 04:06 UTC
- Message-ID
- <20121220040602.GA15172@sigill.intra.peff.net>
- In-Reply-To
- <20121220035543.GA14965@sigill.intra.peff.net>
On Wed, Dec 19, 2012 at 10:55:43PM -0500, Jeff King wrote:
Show 22 quoted lines
> On Wed, Dec 19, 2012 at 07:51:16PM -0800, Junio C Hamano wrote:
>
> > > ++ if (limit_pathspec_to_literal())
> > > ++ item->nowildcard_len = item->len;
> > > ++ else {
> > > ++ item->nowildcard_len = simple_length(path);
> > > ++ if (item->nowildcard_len < item->len) {
> > > ++ pathspec->has_wildcard = 1;
> > > ++ if (path[item->nowildcard_len] == '*' &&
> > > ++ no_wildcard(path + item->nowildcard_len + 1))
> > > ++ item->flags |= PATHSPEC_ONESTAR;
> > > ++ }
> > > + }
> >
> > Hmph. I thought that returning the length without any "stop at glob
> > special" trick from simple_length() would be a simpler resolution.
> >
> > That is what is queued at the tip of 'pu', anyway.
>
> I don't think we can make a change in simple_length. It gets used not
> only for pathspecs, but also for parsing exclude patterns, which I do
> not think should be affected by this option.Our test suite wouldn't catch such a misfeature, of course, because the feature is not turned on by default. But I found it instructive to run all of the tests with GIT_LITERAL_PATHSPECS on. There are failures, of course, but by inspecting each failure you can see that it is an intended effect of the patch (i.e., each tries to use a wildcard pathspec, which no longer works).
When you suggested changing common_prefix, I ran such a test both with and without the change[1] and confirmed that it did not change the set of failure sites. I did not try it, but I suspect running such a test with the tip of pu would reveal new failures in the .gitignore tests.
-Peff
[1] This is in addition to reading and reasoning about the code, of
course. I would not consider this a very robust form of testing,
though a test failure which cannot be easily explained would be a
good starting point for investigation.