All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Stefan Beller <sbeller@google.com>
Cc: Ramsay Jones <ramsay@ramsayjones.plus.com>, git <git@vger.kernel.org>
Subject: Re: [PATCH 1/1] diffcore: add a filter to find a specific blob
Date: Fri, 8 Dec 2017 16:38:22 -0500	[thread overview]
Message-ID: <20171208213822.GD7355@sigill.intra.peff.net> (raw)
In-Reply-To: <CAGZ79kaos7cmQe3bmR5gCVXbUjBBQdSYYOE11egnDEMaX-7xSA@mail.gmail.com>

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

> > 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).

> > 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

  reply	other threads:[~2017-12-08 21:38 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-08  0:24 [PATCH 0/1] diffcore-blobfind Stefan Beller
2017-12-08  0:24 ` [PATCH 1/1] diffcore: add a filter to find a specific blob Stefan Beller
2017-12-08  9:34   ` Jeff King
2017-12-08 16:28     ` Ramsay Jones
2017-12-08 20:19       ` Jeff King
2017-12-08 20:39         ` Stefan Beller
2017-12-08 21:38           ` Jeff King [this message]
2017-12-08 15:04   ` Junio C Hamano
2017-12-08 17:21     ` Junio C Hamano
2017-12-08 21:11     ` Stefan Beller
2017-12-08 21:15       ` Junio C Hamano
2017-12-11 19:58 ` [PATCH 0/1] diff-core blobfind Stefan Beller
2017-12-11 19:58   ` [PATCH 1/1] diffcore: add a filter to find a specific blob Stefan Beller
2017-12-11 23:17     ` Junio C Hamano
2017-12-12  0:21       ` Stefan Beller
2017-12-12  1:24         ` [PATCH] " Stefan Beller
2017-12-12 18:36           ` Junio C Hamano
2017-12-14 21:22           ` Jonathan Nieder
2017-12-14 22:30             ` Stefan Beller
2017-12-14 22:52               ` Jonathan Nieder
2017-12-15  2:18                 ` Junio C Hamano
2017-12-27 18:49                   ` Stefan Beller
2017-12-27 18:59                     ` Jonathan Nieder
2017-12-27 19:57                       ` Junio C Hamano
2017-12-14 22:44             ` Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2017-11-20 22:25 [PATCH 0/1] Teaching the diff machinery about blobfind [WAS: git describe <blob>] Stefan Beller
2017-11-20 22:25 ` [PATCH 1/1] diffcore: add a filter to find a specific blob Stefan Beller
2017-11-24  7:43   ` Junio C Hamano
2017-11-25  4:59     ` Junio C Hamano
2017-12-07 21:40     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171208213822.GD7355@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=ramsay@ramsayjones.plus.com \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.