Re: [PATCH 1/2] diff: Fix modified lines stats with --stat and --numstat

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Sep 23, 2020, 19:16 UTC
Message-ID: <nycvar.QRO.7.76.6.2009231709340.5061@tvgsbejvaqbjf.bet>
In-Reply-To: <xmqq363fm02a.fsf@gitster.c.googlers.com>

Hi,

On Fri, 18 Sep 2020, Junio C Hamano wrote:

Show 19 quoted lines

> Thomas Guyot-Sionnest <tguyot@gmail.com> writes:
>
> >> > -     same_contents = oideq(&one->oid, &two->oid);
> >> > +     if (one->is_stdin && two->is_stdin)
> >> > +             same_contents = !strcmp(one->data, two->data);
> >> > +     else
> >> > +             same_contents = oideq(&one->oid, &two->oid);
> >>
> >> ...should this actually be checking the oid_valid flag in each filespec?
> >> That would presumably cover the is_stdin case, too. I also wonder
> >> whether range-diff ought to be using that flag instead of is_stdin.
> >
> > I considered that, but IIRC when run under a debugger oid_valid was
> > set to 0 - it seemed to be used for something different that i'm not
> > familiar with, maybe it's an indication the object is in git datastore
> > (whereas with --no-index outside files will only be hashed for
> > comparison).
>
> If it says !oid_valid, I think you are getting what you do want.

I suspect the same.

Show 7 quoted lines

> The contents from the outside world, be it what was read from the
> standard input or a pipe, a regular file that is not up-to-date with
> the index, may not have a usable oid computed for it, and oid_valid
> being false signals you that you need byte-for-byte comparison.  As
> suggested by Peff in another message, you can take that signal and
> compare the size and then the contents with memcmp() to see if they
> are the same.

To complete the information: `struct diff_filespec`'s first attribute is `oid`, the object ID of the data. If it is left uninitialized (as is the case in `range-diff`'s case), `oid_valid` has to be 0 to prevent it from being used.

I believe that that is exactly the reason why we want this:

-	same_contents = oideq(&one->oid, &two->oid);
+	same_contents = one->oid_valid && two->oid_valid ?
		oideq(&one->oid, &two->oid) : !strcmp(one->data, two->data);

Ciao, Dscho