Re: [PATCH 8/8] pathspec: convert parse_pathspec to take an index

From: Junio C Hamano <gitster@pobox.com>
Date: May 10, 2017, 06:04 UTC
Message-ID: <xmqq1srxxn72.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170509191805.176266-9-bmwill@google.com>

Brandon Williams <bmwill@google.com> writes:

Show 12 quoted lines

> Convert 'parse_pathspec()' to take an index parameter.
>
> Since the index is only needed when the PATHSPEC_SUBMODULE_LEADING_PATH
> and PATHSPEC_STRIP_SUBMODULE_SLASH flags are given, add a check
> requiring a non-NULL index when either of these flags are given.
> Convert callers which use these two flags to pass in an index while
> having other callers pass in NULL.
>
> Now that pathspec.c does not use any cache macros and has no references
> to 'the_index', mark it by defining NO_THE_INDEX_COMPATIBILITY_MACROS.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>

The same comment as 5/8 applies to this change, but it is a bit easier to judge, because it has so many callers, and for some builtins, especially manipulator commands like add, checkout, and commit, there may be a good reason why they want to keep the primary index while playing with an additional in-core index in a distant future.

Does a pathspec parsed using one instance of index_state expected to work when matching against a path in another instance of index_state? Otherwise, passing a non-NULL istate to parse_pathspec() would tie the resulting pathspec to a particular index_state in some way and there may need a mechanism to catch an attempt to match paths in another index_state with such a pathspec as an error. Just speculating out loud...