git status / git diff -C not detecting file copy

git status / git diff -C not detecting file copy

[Pol Online]

Hi,

The documentation for git status at http://git-scm.com/docs/git-status
implies that it should be able to detect both renames and copies (with
the R and C states). The command git diff -C should do it as well.

However I can't get either to detect copies in this simple test case -
what is happening?


mkdir test
cd test/
git init
echo 'Hello World!' > hello.txt
echo 'Goodbye World!' > goodbye.txt
git add -A
git commit -m "Initial commit"

cp hello.txt copied.txt
mv goodbye.txt moved.txt
git add -A

$ git status --short
A  copied.txt  <------------ NO COPY DETECTED
R  goodbye.txt -> moved.txt

$ git diff -M -C --summary --cached
 create mode 100644 copied.txt  <------------ NO COPY DETECTED
 rename goodbye.txt => moved.txt (100%)

$ git commit -m Test
$ git diff -M -C --summary HEAD~
  create mode 100644 copied.txt  <------------ NO COPY DETECTED
  rename goodbye.txt => moved.txt (100%)


-Pol

Re: git status / git diff -C not detecting file copy

[Bryan Turner]

Pol,

By default, -C only finds copies when the source file was also
modified in the same commit. Since you did not modify hello.txt in the
same commit where you copied it to copied.txt, it will not be
considered.

If you pass -C -C (twice), or use --find-copies-harder, Git will
consider all files in the repository. Note that this can be slower,
which is the reason why it's not the default.

The documentation for git diff describes the -C (--find-copies) and
--find-copies-harder flags and their limitations.

Hope this helps,
Bryan Turner

On Sun, Nov 30, 2014 at 11:35 AM, Pol Online <info@pol-online.net> wrote:
> Hi,
>
> The documentation for git status at http://git-scm.com/docs/git-status
> implies that it should be able to detect both renames and copies (with
> the R and C states). The command git diff -C should do it as well.
>
> However I can't get either to detect copies in this simple test case -
> what is happening?
>
>
> mkdir test
> cd test/
> git init
> echo 'Hello World!' > hello.txt
> echo 'Goodbye World!' > goodbye.txt
> git add -A
> git commit -m "Initial commit"
>
> cp hello.txt copied.txt
> mv goodbye.txt moved.txt
> git add -A
>
> $ git status --short
> A  copied.txt  <------------ NO COPY DETECTED
> R  goodbye.txt -> moved.txt
>
> $ git diff -M -C --summary --cached
>  create mode 100644 copied.txt  <------------ NO COPY DETECTED
>  rename goodbye.txt => moved.txt (100%)
>
> $ git commit -m Test
> $ git diff -M -C --summary HEAD~
>   create mode 100644 copied.txt  <------------ NO COPY DETECTED
>   rename goodbye.txt => moved.txt (100%)
>
>
> -Pol
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git status / git diff -C not detecting file copy

[Pol Online]

Hi Bryan,

OK that explains the behavior of git diff, but what about git status?
The doc implies it should be able to detect copies in the index /
staging area since it has a "C" state.

- Pol

On Sun, Nov 30, 2014 at 10:03 AM, Bryan Turner <bturner@atlassian.com> wrote:
> Pol,
>
> By default, -C only finds copies when the source file was also
> modified in the same commit. Since you did not modify hello.txt in the
> same commit where you copied it to copied.txt, it will not be
> considered.
>
> If you pass -C -C (twice), or use --find-copies-harder, Git will
> consider all files in the repository. Note that this can be slower,
> which is the reason why it's not the default.
>
> The documentation for git diff describes the -C (--find-copies) and
> --find-copies-harder flags and their limitations.
>
> Hope this helps,
> Bryan Turner
>
> On Sun, Nov 30, 2014 at 11:35 AM, Pol Online <info@pol-online.net> wrote:
>> Hi,
>>
>> The documentation for git status at http://git-scm.com/docs/git-status
>> implies that it should be able to detect both renames and copies (with
>> the R and C states). The command git diff -C should do it as well.
>>
>> However I can't get either to detect copies in this simple test case -
>> what is happening?
>>
>>
>> mkdir test
>> cd test/
>> git init
>> echo 'Hello World!' > hello.txt
>> echo 'Goodbye World!' > goodbye.txt
>> git add -A
>> git commit -m "Initial commit"
>>
>> cp hello.txt copied.txt
>> mv goodbye.txt moved.txt
>> git add -A
>>
>> $ git status --short
>> A  copied.txt  <------------ NO COPY DETECTED
>> R  goodbye.txt -> moved.txt
>>
>> $ git diff -M -C --summary --cached
>>  create mode 100644 copied.txt  <------------ NO COPY DETECTED
>>  rename goodbye.txt => moved.txt (100%)
>>
>> $ git commit -m Test
>> $ git diff -M -C --summary HEAD~
>>   create mode 100644 copied.txt  <------------ NO COPY DETECTED
>>   rename goodbye.txt => moved.txt (100%)
>>
>>
>> -Pol
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git status / git diff -C not detecting file copy

[Bryan Turner]

Pol,

It's the same set of limitations. Git does not track renames or copies
as such. It uses heuristics to compute a similarity index and then
decide. All the tree stores for your copy is a file add, and that's
the status you're seeing. I don't think there is any way to turn on
aggressive copy detection for git status. However, before you run git
commit, you could run git diff --find-copies-harder --cached instead
and it should show the copy.

I'll let someone a little more intimately familiar with the internals
of git status comment on why the documentation for that mentions
copies.

Hope this helps,
Bryan Turner

On Sun, Nov 30, 2014 at 12:30 PM, Pol Online <info@pol-online.net> wrote:
> Hi Bryan,
>
> OK that explains the behavior of git diff, but what about git status?
> The doc implies it should be able to detect copies in the index /
> staging area since it has a "C" state.
>
> - Pol
>
> On Sun, Nov 30, 2014 at 10:03 AM, Bryan Turner <bturner@atlassian.com> wrote:
>> Pol,
>>
>> By default, -C only finds copies when the source file was also
>> modified in the same commit. Since you did not modify hello.txt in the
>> same commit where you copied it to copied.txt, it will not be
>> considered.
>>
>> If you pass -C -C (twice), or use --find-copies-harder, Git will
>> consider all files in the repository. Note that this can be slower,
>> which is the reason why it's not the default.
>>
>> The documentation for git diff describes the -C (--find-copies) and
>> --find-copies-harder flags and their limitations.
>>
>> Hope this helps,
>> Bryan Turner
>>
>> On Sun, Nov 30, 2014 at 11:35 AM, Pol Online <info@pol-online.net> wrote:
>>> Hi,
>>>
>>> The documentation for git status at http://git-scm.com/docs/git-status
>>> implies that it should be able to detect both renames and copies (with
>>> the R and C states). The command git diff -C should do it as well.
>>>
>>> However I can't get either to detect copies in this simple test case -
>>> what is happening?
>>>
>>>
>>> mkdir test
>>> cd test/
>>> git init
>>> echo 'Hello World!' > hello.txt
>>> echo 'Goodbye World!' > goodbye.txt
>>> git add -A
>>> git commit -m "Initial commit"
>>>
>>> cp hello.txt copied.txt
>>> mv goodbye.txt moved.txt
>>> git add -A
>>>
>>> $ git status --short
>>> A  copied.txt  <------------ NO COPY DETECTED
>>> R  goodbye.txt -> moved.txt
>>>
>>> $ git diff -M -C --summary --cached
>>>  create mode 100644 copied.txt  <------------ NO COPY DETECTED
>>>  rename goodbye.txt => moved.txt (100%)
>>>
>>> $ git commit -m Test
>>> $ git diff -M -C --summary HEAD~
>>>   create mode 100644 copied.txt  <------------ NO COPY DETECTED
>>>   rename goodbye.txt => moved.txt (100%)
>>>
>>>
>>> -Pol
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe git" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git status / git diff -C not detecting file copy

[Jeff King]

On Sun, Nov 30, 2014 at 12:54:53PM +1100, Bryan Turner wrote:

> I'll let someone a little more intimately familiar with the internals
> of git status comment on why the documentation for that mentions
> copies.

I don't think there is a good reason. git-status has used renames since
mid-2005. The documentation mentioning copies was added much later,
along with the short and porcelain formats. That code handles whatever
the diff engine throws at it.  I don't think anybody considered at that
time the fact that you cannot actually provoke status to look for
copies.

Interestingly, the rename behavior dates all the way back to:

  commit 753fd78458b6d7d0e65ce0ebe7b62e1bc55f3992
  Author: Linus Torvalds <torvalds@ppc970.osdl.org>
  Date:   Fri Jun 17 15:34:19 2005 -0700

  Use "-M" instead of "-C" for "git diff" and "git status"
  
  The "C" in "-C" may stand for "Cool", but it's also pretty slow, since
  right now it leaves all unmodified files to be tested even if there are
  no new files at all.  That just ends up being unacceptably slow for big
  projects, especially if it's not all in the cache.

I suspect that the copy code may be much faster these days (it sounds
like we did not even have the find-copies-harder distinction then, and
these days we certainly take the quick return if there are no copy
destination candidates).

To get a rough sense of how much effort is entailed in the various
options, here are "git log --raw" timings for git.git (all timings are
warm cache, best-of-five, wall clock time):

  log --raw:       0m2.311s
  log --raw -M:    0m2.362s
  log --raw -C:    0m2.576s
  log --raw -C -C: 1m4.462s

You can see that rename detection adds a little, and copy detections
adds a little more.  That makes sense; it's rare for new files to appear
at the same that old files are going away (renames), so most of the time
it does nothing. Copies introduce a bit more work; we have to compare
against any changed files, and there are typically several in each
commit. find-copies-harder is...well, very expensive.

These timings are of diffs between commits and their parents, of course.
But if we assume that "git status" will show diffs roughly similar to
what gets committed, then this should be comparable. There are about 30K
non-merge commits we traversed there, so adding 200ms is an average of
not very much per commit. Of course the cost is disproportionately borne
by diffs which have an actual file come into being. There are ~2000
commits that introduce a file, so it's probably accurate to say that it
either adds nothing in most cases, or ~1/10th of a millisecond in
others.

Note this is also doing inexact detection, which involves actually
looking at the contents of candidate blobs (whereas exact detection can
be done by comparing sha1s, which is very fast). If you set
diff.renamelimit to "1", then we do only exact detections. Here are
timings there:

  log --raw:       0m02.311s    (for reference)
  log --raw -M:    0m02.337s
  log --raw -C:    0m02.347s
  log --raw -C -C: 0m24.419s

That speeds things up a fair bit, even for "-C" (we don't have to access
the blobs anymore, so I suspect the time is going to just accessing all
of the trees; normally diff does not descend into subtrees that have the
same sha1). Of course, you probably wouldn't want to turn off inexact
renames completely. I suspect what you'd want is a --find-copies-moderately
where we look for cheap copies using "-C", and then follow up with "-C
-C" only using exact renames.

So from these timings, I'd conclude that:

  1. It's probably fine to turn on copies for "git status".

  2. It's probably even OK to use "-C -C" for some projects. Even though
     22s looks scary there, that's only 11ms for git.git (remember,
     spread across 2000 commits). For linux.git, it's much, much worse.
     I killed my "-C -C" run after 10 minutes, and it had only gone
     through 1/20th of the commits. Extrapolating, you're looking at
     500ms or so added to a "git status" run.

     So you'd almost certainly want this to be configurable.

Does either of you want to try your hand at a patch? Just enabling
copies should be a one-liner. Making it configurable is more involved,
but should also be pretty straightforward.

-Peff

Re: git status / git diff -C not detecting file copy

[Pol Online]

Jeff,

Thanks much for the detailed answer and analysis.

> Does either of you want to try your hand at a patch? Just enabling
> copies should be a one-liner. Making it configurable is more involved,
> but should also be pretty straightforward.

I'll have to pass on this. I'm absolutely not familiar with the Git
source code nor with the contributing guidelines and process at this
time. I'm currently working on libgit2 and trying to clone the git
status behavior and that's how I discovered this file copy detection
issue.

In the meantime, shouldn't we patch the docs? That should be trivial for sure.

- Pol

Re: git status / git diff -C not detecting file copy

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Interestingly, the rename behavior dates all the way back to:
>
>   commit 753fd78458b6d7d0e65ce0ebe7b62e1bc55f3992
>   Author: Linus Torvalds <torvalds@ppc970.osdl.org>
>   Date:   Fri Jun 17 15:34:19 2005 -0700
>
>   Use "-M" instead of "-C" for "git diff" and "git status"
>   
>   The "C" in "-C" may stand for "Cool", but it's also pretty slow, since
>   right now it leaves all unmodified files to be tested even if there are
>   no new files at all.  That just ends up being unacceptably slow for big
>   projects, especially if it's not all in the cache.
> ...
> To get a rough sense of how much effort is entailed in the various
> options, here are "git log --raw" timings for git.git (all timings are
> warm cache, best-of-five, wall clock time):

The rationale of the change talks about "big projects" and your
analysis and argument to advocate reversion of that change is based
on "git.git"?  What is going on here?

Also our history is fairly unusual in that we do not have a lot of
renames (there was one large "s|builtin-|builtin/|" rename event,
but that is about it), which may not be a good example to base such
a design decision on.

It is a fine idea to make this configurable ("status.renames = -C"
or something, perhaps?), though.

Re: git status / git diff -C not detecting file copy

[Jeff King]

On Tue, Dec 02, 2014 at 09:57:07AM -0800, Junio C Hamano wrote:

> > To get a rough sense of how much effort is entailed in the various
> > options, here are "git log --raw" timings for git.git (all timings are
> > warm cache, best-of-five, wall clock time):
> 
> The rationale of the change talks about "big projects" and your
> analysis and argument to advocate reversion of that change is based
> on "git.git"?  What is going on here?

I find that git.git is often a useful and easy thing to time to
extrapolate to other projects. It's 1/10th-1/20th the size of the kernel
(both in tree size and commit depth), which I do consider a "big
project" (and I have a feeling is what Linus was talking about).

Of course, performance numbers do not always scale linearly with repo
size. I didn't show the full numbers for the kernel, but they are:

  log --raw:       0m53.587s
  log --raw -M:    0m55.424s
  log --raw -C:    1m02.733s
  log --raw -C -C: <killed after 10 minutes>

There are ~20K commits that introduce files in the kernel (about 10x
what git.git had). So renames add well under a millisecond to each of
those diffs, and a single "-C" adds a third of a millisecond.

Which is pretty in-line with the git.git findings (it is not linear
here, but actually fairly constant. This makes sense, as it scales with
the size of the commit, not the size of the tree).

And as I noted, "-C -C" is rather expensive (I gave some estimated
timings earlier; you could probably come up with something more accurate
by doing smarter sampling).

> Also our history is fairly unusual in that we do not have a lot of
> renames (there was one large "s|builtin-|builtin/|" rename event,
> but that is about it), which may not be a good example to base such
> a design decision on.

I think the work scales not with the number of actual renames, but with
the number of commits where we even bother to look at renames at all
(i.e., ones with an 'A' diff-status). And my estimates assume that we
pay zero cost for other diffs, and attribute all of the extra time to
those diffs. So I think frequency of rename (or 'A') events does not
impact the estimate of the impact on a single "git status" run.

What does impact it is the _size_ of each commit. If you quite
frequently add a new file while touching tens of thousands of other
files, then the performance will be a lot more noticeable. And both
git.git and linux.git are probably better than some other projects about
having small commits.

Still, though. I stand by my earlier conclusions. Even with commits ten
times as large as the kernel's, you are talking about 3ms added to a
"status" run (and again, only when you hit such a gigantic commit _and_
it has an 'A' file).

> It is a fine idea to make this configurable ("status.renames = -C"
> or something, perhaps?), though.

I think it would be OK to move to "-C" as a default, but I agree it is
nicer if it is configurable, as it gives a safety hatch for people in
pathological repos to drop back to the old behavior (or even turn off
renames altogether).

-Peff

Re: git status / git diff -C not detecting file copy

[Bryan Turner]

On Tue, Dec 2, 2014 at 5:55 PM, Jeff King <peff@peff.net> wrote:
>
> So from these timings, I'd conclude that:
>
>   1. It's probably fine to turn on copies for "git status".
>
>   2. It's probably even OK to use "-C -C" for some projects. Even though
>      22s looks scary there, that's only 11ms for git.git (remember,
>      spread across 2000 commits). For linux.git, it's much, much worse.
>      I killed my "-C -C" run after 10 minutes, and it had only gone
>      through 1/20th of the commits. Extrapolating, you're looking at
>      500ms or so added to a "git status" run.
>
>      So you'd almost certainly want this to be configurable.
>
> Does either of you want to try your hand at a patch? Just enabling
> copies should be a one-liner. Making it configurable is more involved,
> but should also be pretty straightforward.

I'm interested in taking a stab at a patch, but I'd like to confirm
which way to go. Based on Junio's reply I'm not certain the simple
patch could get accepted (assuming I do all the submission parts
properly and the implementation itself passes review). Does that mean
the only real option is the configurable patch?

>
> -Peff

Re: git status / git diff -C not detecting file copy

[Jeff King]

On Wed, Dec 03, 2014 at 08:40:47AM +1100, Bryan Turner wrote:

> On Tue, Dec 2, 2014 at 5:55 PM, Jeff King <peff@peff.net> wrote:
> >
> > So from these timings, I'd conclude that:
> >
> >   1. It's probably fine to turn on copies for "git status".
> >
> >   2. It's probably even OK to use "-C -C" for some projects. Even though
> >      22s looks scary there, that's only 11ms for git.git (remember,
> >      spread across 2000 commits). For linux.git, it's much, much worse.
> >      I killed my "-C -C" run after 10 minutes, and it had only gone
> >      through 1/20th of the commits. Extrapolating, you're looking at
> >      500ms or so added to a "git status" run.
> >
> >      So you'd almost certainly want this to be configurable.
> >
> > Does either of you want to try your hand at a patch? Just enabling
> > copies should be a one-liner. Making it configurable is more involved,
> > but should also be pretty straightforward.
> 
> I'm interested in taking a stab at a patch, but I'd like to confirm
> which way to go. Based on Junio's reply I'm not certain the simple
> patch could get accepted (assuming I do all the submission parts
> properly and the implementation itself passes review). Does that mean
> the only real option is the configurable patch?

I think this is the part where you get to use your judgement, and decide
what you think the maintainer will take. :)

Personally, I would probably go for the configurable version, as it is
not that much harder, and is a nicer end-point.

Junio gave an example elsewhere in which the config option value would
look like "-C -C". I'd consider trying to match diff.renames instead,
which takes false/true/copies for its three levels. It may make sense to
teach both places "copies-harder" or something similar, for
completeness.

-Peff

Re: git status / git diff -C not detecting file copy

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I find that git.git is often a useful and easy thing to time to
> extrapolate to other projects. It's 1/10th-1/20th the size of the kernel
> (both in tree size and commit depth), which I do consider a "big
> project" (and I have a feeling is what Linus was talking about).
>
> Of course, performance numbers do not always scale linearly with repo
> size.
> ...
> What does impact it is the _size_ of each commit. If you quite
> frequently add a new file while touching tens of thousands of other
> files, then the performance will be a lot more noticeable. And both
> git.git and linux.git are probably better than some other projects about
> having small commits.

Yes, the number of paths touched per-commit in git.git may not be
typical.  If it is unusually lower, that would skew the result, as
the cost of rename detection is proportional to it, and the cost of
-C -C is that number times the number of total paths in the project.

> Still, though. I stand by my earlier conclusions. Even with commits ten
> times as large as the kernel's, you are talking about 3ms added to a
> "status" run (and again, only when you hit such a gigantic commit _and_
> it has an 'A' file).
>
>> It is a fine idea to make this configurable ("status.renames = -C"
>> or something, perhaps?), though.
>
> I think it would be OK to move to "-C" as a default, but I agree it is
> nicer if it is configurable, as it gives a safety hatch for people in
> pathological repos to drop back to the old behavior (or even turn off
> renames altogether).

Yeah I am OK with that, too.

Re: git status / git diff -C not detecting file copy

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> ... I'd consider trying to match diff.renames instead,
> which takes false/true/copies for its three levels. It may make sense to
> teach both places "copies-harder" or something similar, for
> completeness.

Yeah, I think that is a very sensible thing to do.