Re: [PATCH 1/1] diffcore: add a filter to find a specific blob

From: Jeff King <peff@peff.net>
Date: Dec 8, 2017, 21:38 UTC
Message-ID: <20171208213822.GD7355@sigill.intra.peff.net>
In-Reply-To: <CAGZ79kaos7cmQe3bmR5gCVXbUjBBQdSYYOE11egnDEMaX-7xSA@mail.gmail.com>

On Fri, Dec 08, 2017 at 12:39:55PM -0800, Stefan Beller wrote:

Show 8 quoted lines

> > If you add --raw, you can see that both commits introduce that blob, and
> > it never "goes away". That's because that happened in a merge, which we
> > don't diff in a default log invocation.
> 
> We should when --raw is given.
> --raw is documented as  "For each commit, show a summary of changes
> using the raw diff format." and I would argue that 'each commit' includes
> merges. Though I guess this may have implications for long time users.

And "--patch" is documented as "generate patch", but it also does nothing for merges by default. I think it has little to do with "--raw". It is simply that the default for "log" is none of "-c", "--cc", or "-m".

We _could_ change that default ("--cc" is already the default for git-show), but I would not be surprised if that has fallouts (certainly it makes git-log much slower).

Show 7 quoted lines

> > So I think this one is tricky because of the revert. In the same way
> > that pathspec-limiting is often tricky in the face of a revert, because
> > the merges "hide" interesting things happening.
> 
> yup, hidden merges are unfortunate. Is there an easy way to find out
> about merges? (Junio hints at having tests around merges, which I'll do
> next)

If you find such an easy way, let me know. :)

One of the few really manual types of query I remember having done in recent years is trying to pinpoint a bad merge. I.e., somebody during merge resolution accidentally does "git checkout --ours foo.c", blowing away changes which they didn't mean to. And then later you want to figure out which merge did it.

If you use "-c" or "--cc", that isn't an "interesting" change, because it resolves to one side of the merge. If you use "-m", you get way too many changes and have to comb through them manually. I've resorted to "-m --first-parent", but then you frequently have to dig down several layers (e.g., the bad merge is a merge from "master" onto a topic branch, and your first "--first-parent" attempt will just find the bad topic being merged back into master).

I think the most promising tool I've seen there is to redo the merge and show the diff between the auto-merge (including conflicts) and the committed tree. It's just another definition of "is this hunk interesting" that's different from "--cc".

I'm not sure how that would interact with something like "--blobfind", though. For that matter, I'm not quite sure how your patch would interact with "--cc". I think you may need to special-case it.

-Peff