On Mon, Sep 19, 2016 at 11:04 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Brandon Williams <bmwill@xxxxxxxxxx> writes:
>
>>> Again, what do we have in "name" and "item" at this point? If we
>>> have a submodule at "sub/" and we are checking a pathspec element
>>> "sub/dir1/*", what is the non-wildcard part of the pathspec and what
>>> is the "string"? Aren't they "sub/dir1/" and "sub/" respectively,
>>> which would not pass ps_strncmp() and produce a (false) negative?
>>
>> item will be the pathspec_item struct that we are trying to match against.
>
> ... which would mean "sub/dir1/" in the above example (which is
> followed by '*' that is wildcard).
>
>> name will be the file we are trying to match, which should already have the
>> 'prefix' cut off (this is the prefix that is used as an optimization
>> in the common
>> case, which isn't used in the submodule case).
>
> ... which would be "sub/" in the above example, because we disable
> the common-prefix optimization.
>
> So in short, the answer to the last questions in the first quoted
> paragraph are yes, yes, and "no they do not pass ps_strncmp()"?
Yes in that case it wouldn't have passed ps_strncmp()...but we should have never
made it there in the first place due to a piece of logic in match_pathspec_item:
@@ -283,6 +308,24 @@ static int match_pathspec_item(const struct
pathspec_item *item, int prefix,
item->nowildcard_len - prefix))
return MATCHED_FNMATCH;
+ /* Perform checks to see if "name" is a super set of the pathspec */
+ if (flags & DO_MATCH_SUBMODULE) {
+ int matched = 0;
+
+ /* Check if the name is a literal prefix of the pathspec */
+ if ((item->match[namelen] == '/') &&
+ !ps_strncmp(item, match, name, namelen)) {
+ matched = MATCHED_RECURSIVELY;
+ /* Check if the name wildmatches to the pathspec */
+ } else if (item->nowildcard_len < item->len &&
+ !prefix_fnmatch(item, match, name,
+ item->nowildcard_len - prefix)) {
+ matched = MATCHED_FNMATCH;
+ }
+
+ return matched;
+ }
Perhaps the call structure and code organization could be changes a bit to make
a little more sense.