Skip to content

Commit f78c79c

Browse files
Junio C HamanoLinus Torvalds
Junio C Hamano
authored and
Linus Torvalds
committed
[PATCH] diffcore-break.c: various fixes.
This fixes three bugs in the -B heuristics. - Although it was advertised that the initial break criteria used was the same as what diffcore-rename uses, it was using something different. Instead of using smaller of src and dst size to compare with "edit" size, (insertion and deletion), it was using larger of src and dst, unlike the rename/copy detection logic. This caused the parameter to -B to mean something different from the one to -M and -C. To compensate for this change, the default break score is also changed to match that of the default for rename/copy. - The code would have crashed with division by zero when trying to break an originally empty file. - Contrary to what the comment said, the algorithm was breaking small files, only to later merge them together. Signed-off-by: Junio C Hamano <junkio@cox.net> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
1 parent 49d9e85 commit f78c79c

File tree

2 files changed

+9
-15
lines changed

2 files changed

+9
-15
lines changed

diffcore-break.c

+8-14
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,7 @@ static int should_break(struct diff_filespec *src,
6161
if (diff_populate_filespec(src, 0) || diff_populate_filespec(dst, 0))
6262
return 0; /* error but caught downstream */
6363

64-
delta_size = ((src->size < dst->size) ?
65-
(dst->size - src->size) : (src->size - dst->size));
66-
67-
/* Notice that we use max of src and dst as the base size,
68-
* unlike rename similarity detection. This is so that we do
69-
* not mistake a large addition as a complete rewrite.
70-
*/
71-
base_size = ((src->size < dst->size) ? dst->size : src->size);
64+
base_size = ((src->size < dst->size) ? src->size : dst->size);
7265

7366
delta = diff_delta(src->data, src->size,
7467
dst->data, dst->size,
@@ -88,10 +81,11 @@ static int should_break(struct diff_filespec *src,
8881
* less than the minimum, after rename/copy runs.
8982
*/
9083
if (src->size <= src_copied)
91-
delta_size = 0; /* avoid wrapping around */
92-
else
84+
; /* all copied, nothing removed */
85+
else {
9386
delta_size = src->size - src_copied;
94-
*merge_score_p = delta_size * MAX_SCORE / src->size;
87+
*merge_score_p = delta_size * MAX_SCORE / src->size;
88+
}
9589

9690
/* Extent of damage, which counts both inserts and
9791
* deletes.
@@ -174,7 +168,8 @@ void diffcore_break(int break_score)
174168
!S_ISDIR(p->one->mode) && !S_ISDIR(p->two->mode) &&
175169
!strcmp(p->one->path, p->two->path)) {
176170
if (should_break(p->one, p->two,
177-
break_score, &score)) {
171+
break_score, &score) &&
172+
MINIMUM_BREAK_SIZE <= p->one->size) {
178173
/* Split this into delete and create */
179174
struct diff_filespec *null_one, *null_two;
180175
struct diff_filepair *dp;
@@ -185,8 +180,7 @@ void diffcore_break(int break_score)
185180
* Also we do not want to break very
186181
* small files.
187182
*/
188-
if ((score < merge_score) ||
189-
(p->one->size < MINIMUM_BREAK_SIZE))
183+
if (score < merge_score)
190184
score = 0;
191185

192186
/* deletion of one */

diffcore.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
*/
1818
#define MAX_SCORE 60000
1919
#define DEFAULT_RENAME_SCORE 30000 /* rename/copy similarity minimum (50%) */
20-
#define DEFAULT_BREAK_SCORE 59400 /* minimum for break to happen (99%)*/
20+
#define DEFAULT_BREAK_SCORE 30000 /* minimum for break to happen (50%)*/
2121
#define DEFAULT_MERGE_SCORE 48000 /* maximum for break-merge to happen (80%)*/
2222

2323
#define MINIMUM_BREAK_SIZE 400 /* do not break a file smaller than this */

0 commit comments

Comments
 (0)