--dirstat-by-file: Make it faster and more correct

Currently, when using --dirstat-by-file, it first does the full --dirstat
analysis (using diffcore_count_changes()), and then resets 'damage' to 1,
if any damage was found by diffcore_count_changes().

But --dirstat-by-file is not interested in the file damage per se. It only
cares if the file changed at all. In that sense it only cares if the blob
object for a file has changed. We therefore only need to compare the
object names of each file pair in the diff queue and we can skip the
entire --dirstat analysis and simply set 'damage' to 1 for each entry
where the object name has changed.

This makes --dirstat-by-file faster, and also bypasses --dirstat's practice
of ignoring rearranged lines within a file.

The patch also contains an added testcase verifying that --dirstat-by-file
now detects changes that only rearrange lines within a file.

Signed-off-by: Johan Herland <johan@herland.net>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Johan Herland 2011-04-11 00:48:51 +02:00 committed by Junio C Hamano
parent 204f01a2f7
commit 0133dab75d
3 changed files with 25 additions and 5 deletions

25
diff.c
View file

@ -1539,9 +1539,27 @@ static void show_dirstat(struct diff_options *options)
struct diff_filepair *p = q->queue[i];
const char *name;
unsigned long copied, added, damage;
int content_changed;
name = p->one->path ? p->one->path : p->two->path;
if (p->one->sha1_valid && p->two->sha1_valid)
content_changed = hashcmp(p->one->sha1, p->two->sha1);
else
content_changed = 1;
if (DIFF_OPT_TST(options, DIRSTAT_BY_FILE)) {
/*
* In --dirstat-by-file mode, we don't really need to
* look at the actual file contents at all.
* The fact that the SHA1 changed is enough for us to
* add this file to the list of results
* (with each file contributing equal damage).
*/
damage = content_changed ? 1 : 0;
goto found_damage;
}
if (DIFF_FILE_VALID(p->one) && DIFF_FILE_VALID(p->two)) {
diff_populate_filespec(p->one, 0);
diff_populate_filespec(p->two, 0);
@ -1564,14 +1582,11 @@ static void show_dirstat(struct diff_options *options)
/*
* Original minus copied is the removed material,
* added is the new material. They are both damages
* made to the preimage. In --dirstat-by-file mode, count
* damaged files, not damaged lines. This is done by
* counting only a single damaged line per file.
* made to the preimage.
*/
damage = (p->one->size - copied) + added;
if (DIFF_OPT_TST(options, DIRSTAT_BY_FILE) && damage > 0)
damage = 1;
found_damage:
ALLOC_GROW(dir.files, dir.nr + 1, dir.alloc);
dir.files[dir.nr].name = name;
dir.files[dir.nr].changed = damage;

View file

@ -302,6 +302,8 @@ diff master master^ side
diff --dirstat master~1 master~2
# --dirstat doesn't notice changes that simply rearrange existing lines
diff --dirstat initial rearrange
# ...but --dirstat-by-file does notice changes that only rearrange lines
diff --dirstat-by-file initial rearrange
EOF
test_expect_success 'log -S requires an argument' '

View file

@ -0,0 +1,3 @@
$ git diff --dirstat-by-file initial rearrange
100.0% dir/
$