Re: [PATCH 1/3] name-hash: refactor polymorphic index_name_exists()

[Eric Sunshine <sunshine@sunshineco.com>]

On Fri, Sep 13, 2013 at 2:40 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>> Depending upon the absence or presence of a trailing '/' on the incoming
>> pathname, index_name_exists() checks either if a file is present in the
>> index or if a directory is represented within the index.  Each caller
>> explicitly chooses the mode of operation by adding or removing a
>> trailing '/' before invoking index_name_exists().
>>
>> Since these two modes of operations are disjoint and have no code in
>> common (one searches index_state.name_hash; the other dir_hash), they
>> can be represented more naturally as distinct functions: one to search
>> for a file, and one for a directory.
>>
>> Splitting index searching into two functions relieves callers of the
>> artificial burden of having to add or remove a slash to select the mode
>> of operation; instead they just call the desired function. A subsequent
>> patch will take advantage of this benefit in order to eliminate the
>> requirement that the incoming pathname for a directory search must have
>> a trailing slash.
>
> Good thinking.  Thanks for working on this; I agree with the general
> direction this series is going.
>
> I however wonder if it would be a good idea to rename the other one
> to {cache|index}_file_exists(), and even keep the *_name_exists()
> variant that switches between the two based on the trailing slash,
> to avoid breaking other topics in flight that may have added new
> callsites to *_name_exists().  Otherwise the new callsites will feed
> a path with a trailing slash to *_name_exists() and get a false
> result, no?

An assert("no trailing /") was added to index_name_exists() precisely
for the purpose of catching cases of code incorrectly calling
index_name_exists() to search for a directory. No existing code in
'master' trips the assertion (at least according to the tests),
however, the assertion may be annoying for topics in flight which do.

Other than plopping these patches atop 'pu' and seeing if anything
breaks, what would be the "git way" of detecting in-flight topics
which add callers of index_name_exists()? (Excuse my git ignorance.)