undef/git-commit-vandalism
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[PATCH] Rework -B output.

Browse Source 
Patch for a completely rewritten file detected by the -B flag
was shown as a pair of creation followed by deletion in earlier
versions.  This was an misguided attempt to make reviewing such
a complete rewrite easier, and unnecessarily ended up confusing
git-apply.  Instead, show the entire contents of old version
prefixed with '-', followed by the entire contents of new
version prefixed with '+'.  This gives the same easy-to-review
for human consumer while keeping it a single, regular
modification patch for machine consumption, something that even
GNU patch can grok.

Signed-off-by: Junio C Hamano <junkio@cox.net>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
This commit is contained in:
Junio C Hamano 2005-06-19 13:17:50 -07:00 committed by Linus Torvalds
parent 232b75ab3d
commit 366175ef8c
4 changed files with 186 additions and 95 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
Documentation/diffcore.txt
View File

@ -191,6 +191,15 @@ like these:
-B/60 (the same as above, since diffcore-break defautls to -B/60 (the same as above, since diffcore-break defautls to
50%). 50%).
Note that earlier implementation left a broken pair as a separate
creation and deletion patches. This was unnecessary hack and
the latest implementation always merges all the broken pairs
back into modifications, but the resulting patch output is
formatted differently to still let the reviewing easier for such
a complete rewrite by showing the entire contents of old version
prefixed with '-', followed by the entire contents of new
version prefixed with '+'.
diffcore-pickaxe diffcore-pickaxe
---------------- ----------------

229
diff.c
View File

@ -83,10 +83,88 @@ static struct diff_tempfile {
char tmp_path[50]; char tmp_path[50];
} diff_temp[2]; } diff_temp[2];
static int count_lines(const char *filename)
{
FILE *in;
int count, ch, completely_empty = 1, nl_just_seen = 0;
in = fopen(filename, "r");
count = 0;
while ((ch = fgetc(in)) != EOF)
if (ch == '\n') {
count++;
nl_just_seen = 1;
completely_empty = 0;
}
else {
nl_just_seen = 0;
completely_empty = 0;
}
fclose(in);
if (completely_empty)
return 0;
if (!nl_just_seen)
count++; /* no trailing newline */
return count;
}
static void print_line_count(int count)
{
switch (count) {
case 0:
printf("0,0");
break;
case 1:
printf("1");
break;
default:
printf("1,%d", count);
break;
}
}
static void copy_file(int prefix, const char *filename)
{
FILE *in;
int ch, nl_just_seen = 1;
in = fopen(filename, "r");
while ((ch = fgetc(in)) != EOF) {
if (nl_just_seen)
putchar(prefix);
putchar(ch);
if (ch == '\n')
nl_just_seen = 1;
else
nl_just_seen = 0;
}
fclose(in);
if (!nl_just_seen)
printf("\n\\ No newline at end of file\n");
}
static void emit_rewrite_diff(const char *name_a,
const char *name_b,
struct diff_tempfile *temp)
{
/* Use temp[i].name as input, name_a and name_b as labels */
int lc_a, lc_b;
lc_a = count_lines(temp[0].name);
lc_b = count_lines(temp[1].name);
printf("--- %s\n+++ %s\n@@ -", name_a, name_b);
print_line_count(lc_a);
printf(" +");
print_line_count(lc_b);
printf(" @@\n");
if (lc_a)
copy_file('-', temp[0].name);
if (lc_b)
copy_file('+', temp[1].name);
}
static void builtin_diff(const char *name_a, static void builtin_diff(const char *name_a,
const char *name_b, const char *name_b,
struct diff_tempfile *temp, struct diff_tempfile *temp,
const char *xfrm_msg) const char *xfrm_msg,
int complete_rewrite)
{ {
int i, next_at, cmd_size; int i, next_at, cmd_size;
const char *diff_cmd = "diff -L'%s%s' -L'%s%s'"; const char *diff_cmd = "diff -L'%s%s' -L'%s%s'";
@ -149,12 +227,16 @@ static void builtin_diff(const char *name_a,
} }
if (xfrm_msg && xfrm_msg[0]) if (xfrm_msg && xfrm_msg[0])
puts(xfrm_msg); puts(xfrm_msg);
if (strncmp(temp[0].mode, temp[1].mode, 3)) if (strncmp(temp[0].mode, temp[1].mode, 3))
/* we do not run diff between different kind /* we do not run diff between different kind
* of objects. * of objects.
*/ */
exit(0); exit(0);
if (complete_rewrite) {
fflush(NULL);
emit_rewrite_diff(name_a, name_b, temp);
exit(0);
}
} }
fflush(NULL); fflush(NULL);
execlp("/bin/sh","sh", "-c", cmd, NULL); execlp("/bin/sh","sh", "-c", cmd, NULL);
@ -474,7 +556,8 @@ static void run_external_diff(const char *pgm,
const char *other, const char *other,
struct diff_filespec *one, struct diff_filespec *one,
struct diff_filespec *two, struct diff_filespec *two,
const char *xfrm_msg) const char *xfrm_msg,
int complete_rewrite)
{ {
struct diff_tempfile *temp = diff_temp; struct diff_tempfile *temp = diff_temp;
pid_t pid; pid_t pid;
@ -524,7 +607,8 @@ static void run_external_diff(const char *pgm,
* otherwise we use the built-in one. * otherwise we use the built-in one.
*/ */
if (one && two) if (one && two)
builtin_diff(name, other ? : name, temp, xfrm_msg); builtin_diff(name, other ? : name, temp, xfrm_msg,
complete_rewrite);
else else
printf("* Unmerged path %s\n", name); printf("* Unmerged path %s\n", name);
exit(0); exit(0);
@ -547,29 +631,75 @@ static void run_external_diff(const char *pgm,
remove_tempfile(); remove_tempfile();
} }
static void run_diff(const char *name, static void run_diff(struct diff_filepair *p)
const char *other,
struct diff_filespec *one,
struct diff_filespec *two,
const char *xfrm_msg)
{ {
const char *pgm = external_diff(); const char *pgm = external_diff();
char msg_[PATH_MAX*2+200], *xfrm_msg;
struct diff_filespec *one;
struct diff_filespec *two;
const char *name;
const char *other;
int complete_rewrite = 0;
if (DIFF_PAIR_UNMERGED(p)) {
/* unmerged */
run_external_diff(pgm, p->one->path, NULL, NULL, NULL, NULL,
0);
return;
}
name = p->one->path;
other = (strcmp(name, p->two->path) ? p->two->path : NULL);
one = p->one; two = p->two;
switch (p->status) {
case 'C':
sprintf(msg_,
"similarity index %d%%\n"
"copy from %s\n"
"copy to %s",
(int)(0.5 + p->score * 100.0/MAX_SCORE),
name, other);
xfrm_msg = msg_;
break;
case 'R':
sprintf(msg_,
"similarity index %d%%\n"
"rename from %s\n"
"rename to %s",
(int)(0.5 + p->score * 100.0/MAX_SCORE),
name, other);
xfrm_msg = msg_;
break;
case 'M':
if (p->score) {
sprintf(msg_,
"dissimilarity index %d%%",
(int)(0.5 + p->score * 100.0/MAX_SCORE));
xfrm_msg = msg_;
complete_rewrite = 1;
break;
}
/* fallthru */
default:
xfrm_msg = NULL;
}
if (!pgm && if (!pgm &&
one && two &&
DIFF_FILE_VALID(one) && DIFF_FILE_VALID(two) && DIFF_FILE_VALID(one) && DIFF_FILE_VALID(two) &&
(S_IFMT & one->mode) != (S_IFMT & two->mode)) { (S_IFMT & one->mode) != (S_IFMT & two->mode)) {
/* a filepair that changes between file and symlink /* a filepair that changes between file and symlink
* needs to be split into deletion and creation. * needs to be split into deletion and creation.
*/ */
struct diff_filespec *null = alloc_filespec(two->path); struct diff_filespec *null = alloc_filespec(two->path);
run_external_diff(NULL, name, other, one, null, xfrm_msg); run_external_diff(NULL, name, other, one, null, xfrm_msg, 0);
free(null); free(null);
null = alloc_filespec(one->path); null = alloc_filespec(one->path);
run_external_diff(NULL, name, other, null, two, xfrm_msg); run_external_diff(NULL, name, other, null, two, xfrm_msg, 0);
free(null); free(null);
} }
else else
run_external_diff(pgm, name, other, one, two, xfrm_msg); run_external_diff(pgm, name, other, one, two, xfrm_msg,
complete_rewrite);
} }
void diff_setup(int flags) void diff_setup(int flags)
@ -693,26 +823,22 @@ static void diff_flush_raw(struct diff_filepair *p,
die(err, p->two->path); die(err, p->two->path);
} }
if (p->score)
sprintf(status, "%c%03d", p->status,
(int)(0.5 + p->score * 100.0/MAX_SCORE));
else {
status[0] = p->status;
status[1] = 0;
}
switch (p->status) { switch (p->status) {
case 'C': case 'R': case 'C': case 'R':
two_paths = 1; two_paths = 1;
sprintf(status, "%c%03d", p->status,
(int)(0.5 + p->score * 100.0/MAX_SCORE));
break; break;
case 'N': case 'D': case 'N': case 'D':
two_paths = 0; two_paths = 0;
if (p->score)
sprintf(status, "%c%03d", p->status,
(int)(0.5 + p->score * 100.0/MAX_SCORE));
else {
status[0] = p->status;
status[1] = 0;
}
break; break;
default: default:
two_paths = 0; two_paths = 0;
status[0] = p->status;
status[1] = 0;
break; break;
} }
printf(":%06o %06o %s ", printf(":%06o %06o %s ",
@ -763,55 +889,14 @@ int diff_unmodified_pair(struct diff_filepair *p)
static void diff_flush_patch(struct diff_filepair *p) static void diff_flush_patch(struct diff_filepair *p)
{ {
const char *name, *other;
char msg_[PATH_MAX*2+200], *msg;
if (diff_unmodified_pair(p)) if (diff_unmodified_pair(p))
return; return;
name = p->one->path;
other = (strcmp(name, p->two->path) ? p->two->path : NULL);
if ((DIFF_FILE_VALID(p->one) && S_ISDIR(p->one->mode)) || if ((DIFF_FILE_VALID(p->one) && S_ISDIR(p->one->mode)) ||
(DIFF_FILE_VALID(p->two) && S_ISDIR(p->two->mode))) (DIFF_FILE_VALID(p->two) && S_ISDIR(p->two->mode)))
return; /* no tree diffs in patch format */ return; /* no tree diffs in patch format */
switch (p->status) { run_diff(p);
case 'C':
sprintf(msg_,
"similarity index %d%%\n"
"copy from %s\n"
"copy to %s",
(int)(0.5 + p->score * 100.0/MAX_SCORE),
p->one->path, p->two->path);
msg = msg_;
break;
case 'R':
sprintf(msg_,
"similarity index %d%%\n"
"rename from %s\n"
"rename to %s",
(int)(0.5 + p->score * 100.0/MAX_SCORE),
p->one->path, p->two->path);
msg = msg_;
break;
case 'D': case 'N':
if (DIFF_PAIR_BROKEN(p)) {
sprintf(msg_,
"dissimilarity index %d%%",
(int)(0.5 + p->score * 100.0/MAX_SCORE));
msg = msg_;
}
else
msg = NULL;
break;
default:
msg = NULL;
}
if (DIFF_PAIR_UNMERGED(p))
run_diff(name, NULL, NULL, NULL, NULL);
else
run_diff(name, other, p->one, p->two, msg);
} }
int diff_queue_is_empty(void) int diff_queue_is_empty(void)
@ -972,8 +1057,10 @@ static void diffcore_apply_filter(const char *filter)
int found; int found;
for (i = found = 0; !found && i < q->nr; i++) { for (i = found = 0; !found && i < q->nr; i++) {
struct diff_filepair *p = q->queue[i]; struct diff_filepair *p = q->queue[i];
if ((p->broken_pair && strchr(filter, 'B')) || if (((p->status == 'M') &&
(!p->broken_pair && strchr(filter, p->status))) ((p->score && strchr(filter, 'B')) ||
(!p->score && strchr(filter, 'M')))) ||
((p->status != 'M') && strchr(filter, p->status)))
found++; found++;
} }
if (found) if (found)
@ -991,8 +1078,10 @@ static void diffcore_apply_filter(const char *filter)
/* Only the matching ones */ /* Only the matching ones */
for (i = 0; i < q->nr; i++) { for (i = 0; i < q->nr; i++) {
struct diff_filepair *p = q->queue[i]; struct diff_filepair *p = q->queue[i];
if ((p->broken_pair && strchr(filter, 'B')) || if (((p->status == 'M') &&
(!p->broken_pair && strchr(filter, p->status))) ((p->score && strchr(filter, 'B')) ||
(!p->score && strchr(filter, 'M')))) ||
((p->status != 'M') && strchr(filter, p->status)))
diff_q(&outq, p); diff_q(&outq, p);
else else
diff_free_filepair(p); diff_free_filepair(p);

7
diffcore-break.c
View File

@ -214,7 +214,7 @@ static void merge_broken(struct diff_filepair *p,
struct diff_queue_struct *outq) struct diff_queue_struct *outq)
{ {
/* p and pp are broken pairs we want to merge */ /* p and pp are broken pairs we want to merge */
struct diff_filepair *c = p, *d = pp; struct diff_filepair *c = p, *d = pp, *dp;
if (DIFF_FILE_VALID(p->one)) { if (DIFF_FILE_VALID(p->one)) {
/* this must be a delete half */ /* this must be a delete half */
d = p; c = pp; d = p; c = pp;
@ -229,7 +229,8 @@ static void merge_broken(struct diff_filepair *p,
if (!DIFF_FILE_VALID(c->two)) if (!DIFF_FILE_VALID(c->two))
die("internal error in merge #4"); die("internal error in merge #4");
diff_queue(outq, d->one, c->two); dp = diff_queue(outq, d->one, c->two);
dp->score = p->score;
diff_free_filespec_data(d->two); diff_free_filespec_data(d->two);
diff_free_filespec_data(c->one); diff_free_filespec_data(c->one);
free(d); free(d);
@ -251,7 +252,6 @@ void diffcore_merge_broken(void)
/* we already merged this with its peer */ /* we already merged this with its peer */
continue; continue;
else if (p->broken_pair && else if (p->broken_pair &&
p->score == 0 &&
!strcmp(p->one->path, p->two->path)) { !strcmp(p->one->path, p->two->path)) {
/* If the peer also survived rename/copy, then /* If the peer also survived rename/copy, then
* we merge them back together. * we merge them back together.
@ -259,7 +259,6 @@ void diffcore_merge_broken(void)
for (j = i + 1; j < q->nr; j++) { for (j = i + 1; j < q->nr; j++) {
struct diff_filepair *pp = q->queue[j]; struct diff_filepair *pp = q->queue[j];
if (pp->broken_pair && if (pp->broken_pair &&
p->score == 0 &&
!strcmp(pp->one->path, pp->two->path) && !strcmp(pp->one->path, pp->two->path) &&
!strcmp(p->one->path, pp->two->path)) { !strcmp(p->one->path, pp->two->path)) {
/* Peer survived. Merge them */ /* Peer survived. Merge them */

36
t/t4008-diff-break-rewrite.sh
View File

@ -44,13 +44,12 @@ test_expect_success \
cat >expected <<\EOF cat >expected <<\EOF
:100644 000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 0000000000000000000000000000000000000000 D file0 :100644 000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 0000000000000000000000000000000000000000 D file0
:100644 000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0000000000000000000000000000000000000000 D100 file1 :100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 11e331465a89c394dc25c780de230043750c1ec8 M100 file1
:000000 100644 0000000000000000000000000000000000000000 11e331465a89c394dc25c780de230043750c1ec8 N100 file1
EOF EOF
test_expect_success \ test_expect_success \
'validate result of -B (#1)' \ 'validate result of -B (#1)' \
'compare_diff_raw current expected' 'compare_diff_raw expected current'
test_expect_success \ test_expect_success \
'run diff with -B and -M' \ 'run diff with -B and -M' \
@ -62,7 +61,7 @@ EOF
test_expect_success \ test_expect_success \
'validate result of -B -M (#2)' \ 'validate result of -B -M (#2)' \
'compare_diff_raw current expected' 'compare_diff_raw expected current'
test_expect_success \ test_expect_success \
'swap file0 and file1' \ 'swap file0 and file1' \
@ -79,15 +78,13 @@ test_expect_success \
'git-diff-cache -B "$tree" >current' 'git-diff-cache -B "$tree" >current'
cat >expected <<\EOF cat >expected <<\EOF
:100644 000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 0000000000000000000000000000000000000000 D100 file0 :100644 100644 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 6ff87c4664981e4397625791c8ea3bbb5f2279a3 M100 file0
:000000 100644 0000000000000000000000000000000000000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 N100 file0 :100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 M100 file1
:100644 000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0000000000000000000000000000000000000000 D100 file1
:000000 100644 0000000000000000000000000000000000000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 N100 file1
EOF EOF
test_expect_success \ test_expect_success \
'validate result of -B (#3)' \ 'validate result of -B (#3)' \
'compare_diff_raw current expected' 'compare_diff_raw expected current'
test_expect_success \ test_expect_success \
'run diff with -B and -M' \ 'run diff with -B and -M' \
@ -100,7 +97,7 @@ EOF
test_expect_success \ test_expect_success \
'validate result of -B -M (#4)' \ 'validate result of -B -M (#4)' \
'compare_diff_raw current expected' 'compare_diff_raw expected current'
test_expect_success \ test_expect_success \
'make file0 into something completely different' \ 'make file0 into something completely different' \
@ -114,13 +111,12 @@ test_expect_success \
cat >expected <<\EOF cat >expected <<\EOF
:100644 120000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 67be421f88824578857624f7b3dc75e99a8a1481 T file0 :100644 120000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 67be421f88824578857624f7b3dc75e99a8a1481 T file0
:100644 000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0000000000000000000000000000000000000000 D100 file1 :100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 M100 file1
:000000 100644 0000000000000000000000000000000000000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 N100 file1
EOF EOF
test_expect_success \ test_expect_success \
'validate result of -B (#5)' \ 'validate result of -B (#5)' \
'compare_diff_raw current expected' 'compare_diff_raw expected current'
test_expect_success \ test_expect_success \
'run diff with -B' \ 'run diff with -B' \
@ -130,13 +126,12 @@ test_expect_success \
# due to type differences. # due to type differences.
cat >expected <<\EOF cat >expected <<\EOF
:100644 120000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 67be421f88824578857624f7b3dc75e99a8a1481 T file0 :100644 120000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 67be421f88824578857624f7b3dc75e99a8a1481 T file0
:100644 000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0000000000000000000000000000000000000000 D100 file1 :100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 M100 file1
:000000 100644 0000000000000000000000000000000000000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 N100 file1
EOF EOF
test_expect_success \ test_expect_success \
'validate result of -B -M (#6)' \ 'validate result of -B -M (#6)' \
'compare_diff_raw current expected' 'compare_diff_raw expected current'
test_expect_success \ test_expect_success \
'run diff with -M' \ 'run diff with -M' \
@ -151,7 +146,7 @@ EOF
test_expect_success \ test_expect_success \
'validate result of -M (#7)' \ 'validate result of -M (#7)' \
'compare_diff_raw current expected' 'compare_diff_raw expected current'
test_expect_success \ test_expect_success \
'file1 edited to look like file0 and file0 rename-edited to file2' \ 'file1 edited to look like file0 and file0 rename-edited to file2' \
@ -169,14 +164,13 @@ test_expect_success \
cat >expected <<\EOF cat >expected <<\EOF
:100644 000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 0000000000000000000000000000000000000000 D file0 :100644 000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 0000000000000000000000000000000000000000 D file0
:100644 000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0000000000000000000000000000000000000000 D100 file1 :100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 08bb2fb671deff4c03a4d4a0a1315dff98d5732c M100 file1
:000000 100644 0000000000000000000000000000000000000000 08bb2fb671deff4c03a4d4a0a1315dff98d5732c N100 file1
:000000 100644 0000000000000000000000000000000000000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 N file2 :000000 100644 0000000000000000000000000000000000000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 N file2
EOF EOF
test_expect_success \ test_expect_success \
'validate result of -B (#8)' \ 'validate result of -B (#8)' \
'compare_diff_raw current expected' 'compare_diff_raw expected current'
test_expect_success \ test_expect_success \
'run diff with -B -M' \ 'run diff with -B -M' \
@ -189,6 +183,6 @@ EOF
test_expect_success \ test_expect_success \
'validate result of -B -M (#9)' \ 'validate result of -B -M (#9)' \
'compare_diff_raw current expected' 'compare_diff_raw expected current'
test_done test_done