Re: Re: git log -M -- filename is not working?

From: Jeff King <peff@peff.net>
Date: May 14, 2010, 04:55 UTC
Message-ID: <20100514045522.GE6075@coredump.intra.peff.net>
In-Reply-To: <19434.48308.815673.263230@winooski.ccs.neu.edu>

On Wed, May 12, 2010 at 10:35:32AM -0400, Eli Barzilay wrote:

Show 7 quoted lines

> But with `-p' it was doing something confusing: I used two files that
> were recently renamed, and the result was the correct log history, but
> the first patch that was shown (the rename) showed the two files as
> added.  (That's even when I added `-C' and `-M'.)  This happens even
> with a single path.  OTOH, using `--follow' with `-p' and a single
> path without your patch produces the expected result where the first
> patch is a rename (even without `-C'/`-M').

Ah, yeah, I see. The problem is that my code is doing something like:

  1. Do a sha1-only diff with our current path list.

  2. If there were any created files, they might be renames.
     Put aside the old diff results. Do a new diff, looking for renames.

  3. If there are renames, add them to our path list.

  4. Restore the old diff results.

  5. Proceed with other desired diff options (rename detection, showing
     patches, etc).

But the during step (5), remember that we are still working with the old diff results, which will not include the expanded path. Thus we won't consider the new path as a rename source, and will fail to find the rename.

The naive right way would be to re-do step (1) with the expanded path. But there is an optimization, since we can use the diff results from (2) directly, including avoiding re-doing the rename detection.

The only "downside" is that it means --follow actually impacts the diff generation by implying --find-copies-harder. And I put downside in quotes because it is probably not a big deal. We have already spent the CPU time to find the answer, so it is silly not to show it. I can't imagine why somebody would want --follow, but would _not_ want rename detection in the resulting diff.

Bo's version of the patch does that optimization. When I clean up the patch (probably sometime next week), I'll take those changes.

-Peff