Re: [PATCH/RFC][GSoC] diff-no-index: transform "$directory $file" args to "$directory/$file $file"

From: Yurii Shevtsov <ungetch@gmail.com>
Date: Mar 21, 2015, 19:08 UTC
Message-ID: <CAHLaBN+q8T6XJ-aJNu1HsNdnKDrR_5PFeY20vmj8RUDTX8PNew@mail.gmail.com>
In-Reply-To: <xmqqsicyciz2.fsf@gitster.dls.corp.google.com>

Show 35 quoted lines

>> diff --git a/diff-no-index.c b/diff-no-index.c
>> index 265709b..9a3439a 100644
>> --- a/diff-no-index.c
>> +++ b/diff-no-index.c
>> @@ -97,8 +97,39 @@ static int queue_diff(struct diff_options *o,
>>      if (get_mode(name1, &mode1) || get_mode(name2, &mode2))
>>          return -1;
>>
>> -    if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2))
>> -        return error("file/directory conflict: %s, %s", name1, name2);
>> +    if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2)) {
>> +        struct strbuf path;
>> +        const char *dir, *file;
>> +        char *filename, *dirname = 0;
>> +        int i, ret = 0;
>
> If you have two directories, a and b, under which there are two
> files a/sub/file and b/sub (i.e. 'sub' in a/ is a directory and b/
> is a file), and if you say "git diff --no-index a b", what happens?
>
>  - the caller of this function gives a and b in name1 and name2;
>
>  - we do not come in this codepath as both are directories;
>
>  - we read from a/ and b/ and fill p1 and p2 with names of paths in
>    the directories -- p1 and p2 will both have 'sub';
>
>  - queue_diff() is recursively called to compare a/sub and b/sub;
>
>    - now we have name1 = a/sub and name2 = b/sub;
>
>    - we come in this codepath, and they are turned into comparison
>      between a/sub/sub and b/sub.
>
> The last step is simply crazy.

It won't try to compare a/sub/sub and b/sub, since git diff with this patch falls before with error message, as I planned (if it did compare I would get 'Couldn't access a/sub/sub'). I tested my code! Original git diff also stops comparing when hits folder and file with same names. So my patch allows to pass 'D F' args, and doesn't affect other logic.

Show 6 quoted lines

> Hmmmm, is vger reinjecting an old message, or you sent an older and
> wrong version of a patch by mistake?  We discussed why doing this in
> queue_diff() is wrong in the thread that has $gmane/265543 in it,
> and I was expecting to see a logic like this in the caller.
>
> Puzzled...

Yes, we discussed it, but, as I mentioned in commit message, I think my solution much better fits in queue_diff()