summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--diff.c145
-rw-r--r--diffcore-pathspec.c2
-rw-r--r--diffcore-pickaxe.c2
-rw-r--r--diffcore-rename.c112
-rw-r--r--diffcore.h13
-rw-r--r--t/t4003-diff-rename-1.sh26
-rw-r--r--t/t4005-diff-rename-2.sh82
7 files changed, 244 insertions, 138 deletions
diff --git a/diff.c b/diff.c
index 4b2d105..ababcaa 100644
--- a/diff.c
+++ b/diff.c
@@ -283,12 +283,6 @@ int diff_populate_filespec(struct diff_filespec *s)
return 0;
}
-void diff_free_filepair(struct diff_filepair *p)
-{
- free(p->xfrm_msg);
- free(p);
-}
-
void diff_free_filespec_data(struct diff_filespec *s)
{
if (s->should_free)
@@ -501,9 +495,9 @@ struct diff_filepair *diff_queue(struct diff_queue_struct *queue,
struct diff_filepair *dp = xmalloc(sizeof(*dp));
dp->one = one;
dp->two = two;
- dp->xfrm_msg = NULL;
+ dp->score = 0;
dp->orig_order = queue->nr;
- dp->xfrm_work = 0;
+ dp->rename_rank = 0;
diff_q(queue, dp);
return dp;
}
@@ -522,23 +516,7 @@ static void diff_flush_raw(struct diff_filepair *p)
p->two->path, line_termination);
}
-static void diff_flush_patch(struct diff_filepair *p)
-{
- const char *name, *other;
-
- 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)) ||
- (DIFF_FILE_VALID(p->two) && S_ISDIR(p->two->mode)))
- return; /* no tree diffs in patch format */
-
- if (DIFF_PAIR_UNMERGED(p))
- run_external_diff(name, NULL, NULL, NULL, NULL);
- else
- run_external_diff(name, other, p->one, p->two, p->xfrm_msg);
-}
-
-static int uninteresting(struct diff_filepair *p)
+int diff_unmodified_pair(struct diff_filepair *p)
{
/* This function is written stricter than necessary to support
* the currently implemented transformers, but the idea is to
@@ -570,12 +548,70 @@ static int uninteresting(struct diff_filepair *p)
return 0;
}
+static void diff_flush_patch(struct diff_filepair *p, const char *msg)
+{
+ const char *name, *other;
+
+ /* diffcore_prune() keeps "stay" entries for diff-raw
+ * copy/rename detection, but when we are generating
+ * patches we do not need them.
+ */
+ if (diff_unmodified_pair(p))
+ 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)) ||
+ (DIFF_FILE_VALID(p->two) && S_ISDIR(p->two->mode)))
+ return; /* no tree diffs in patch format */
+
+ if (DIFF_PAIR_UNMERGED(p))
+ run_external_diff(name, NULL, NULL, NULL, NULL);
+ else
+ run_external_diff(name, other, p->one, p->two, msg);
+}
+
+int diff_needs_to_stay(struct diff_queue_struct *q, int i,
+ struct diff_filespec *it)
+{
+ /* If it will be used in later entry (either stay or used
+ * as the source of rename/copy), we need to copy, not rename.
+ */
+ while (i < q->nr) {
+ struct diff_filepair *p = q->queue[i++];
+ if (!DIFF_FILE_VALID(p->two))
+ continue; /* removed is fine */
+ if (strcmp(p->one->path, it->path))
+ continue; /* not relevant */
+
+ /* p has its src set to *it and it is not a delete;
+ * it will be used for in-place change, rename/copy,
+ * or just stays there. We cannot rename it out.
+ */
+ return 1;
+ }
+ return 0;
+}
+
+static int diff_used_as_source(struct diff_queue_struct *q, int lim,
+ struct diff_filespec *it)
+{
+ int i;
+ for (i = 0; i < lim; i++) {
+ struct diff_filepair *p = q->queue[i++];
+ if (!strcmp(p->one->path, it->path))
+ return 1;
+ }
+ return 0;
+}
+
void diffcore_prune(void)
{
/*
* Although rename/copy detection wants to have "no-change"
* entries fed into them, the downstream do not need to see
- * them. This function removes such entries.
+ * them, unless we had rename/copy for the same path earlier.
+ * This function removes such entries.
*
* The applications that use rename/copy should:
*
@@ -585,6 +621,7 @@ void diffcore_prune(void)
* (3) call diffcore_prune
* (4) call other diffcore_xxx that do not need to see
* "no-change" entries.
+ * (5) call diff_flush().
*/
struct diff_queue_struct *q = &diff_queued_diff;
struct diff_queue_struct outq;
@@ -595,22 +632,21 @@ void diffcore_prune(void)
for (i = 0; i < q->nr; i++) {
struct diff_filepair *p = q->queue[i];
- if (!uninteresting(p))
+ if (!diff_unmodified_pair(p) ||
+ diff_used_as_source(q, i, p->one))
diff_q(&outq, p);
else
- diff_free_filepair(p);
+ free(p);
}
free(q->queue);
*q = outq;
return;
}
-static void diff_flush_one(struct diff_filepair *p)
+static void diff_flush_one(struct diff_filepair *p, const char *msg)
{
- if (uninteresting(p))
- return;
if (generate_patch)
- diff_flush_patch(p);
+ diff_flush_patch(p, msg);
else
diff_flush_raw(p);
}
@@ -618,14 +654,7 @@ static void diff_flush_one(struct diff_filepair *p)
int diff_queue_is_empty(void)
{
struct diff_queue_struct *q = &diff_queued_diff;
- int i;
-
- for (i = 0; i < q->nr; i++) {
- struct diff_filepair *p = q->queue[i];
- if (!uninteresting(p))
- return 0;
- }
- return 1;
+ return q->nr == 0;
}
void diff_flush(int diff_output_style)
@@ -646,13 +675,35 @@ void diff_flush(int diff_output_style)
generate_patch = 1;
break;
}
- for (i = 0; i < q->nr; i++)
- diff_flush_one(q->queue[i]);
+ for (i = 0; i < q->nr; i++) {
+ char msg_[PATH_MAX*2+200], *msg = NULL;
+ struct diff_filepair *p = q->queue[i];
+ if (strcmp(p->one->path, p->two->path)) {
+ /* This is rename or copy. Which one is it? */
+ if (diff_needs_to_stay(q, i+1, p->one)) {
+ sprintf(msg_,
+ "similarity index %d%%\n"
+ "copy from %s\n"
+ "copy to %s\n",
+ (int)(0.5 + p->score * 100/MAX_SCORE),
+ p->one->path, p->two->path);
+ }
+ else
+ sprintf(msg_,
+ "similarity index %d%%\n"
+ "rename old %s\n"
+ "rename new %s\n",
+ (int)(0.5 + p->score * 100/MAX_SCORE),
+ p->one->path, p->two->path);
+ msg = msg_;
+ }
+ diff_flush_one(p, msg);
+ }
+
for (i = 0; i < q->nr; i++) {
struct diff_filepair *p = q->queue[i];
diff_free_filespec_data(p->one);
diff_free_filespec_data(p->two);
- free(p->xfrm_msg);
free(p);
}
free(q->queue);
@@ -674,10 +725,10 @@ void diff_addremove(int addremove, unsigned mode,
* entries to the diff-core. They will be prefixed
* with something like '=' or '*' (I haven't decided
* which but should not make any difference).
- * Feeding the same new and old to diff_change() should
- * also have the same effect. diff_flush() should
- * filter uninteresting ones out at the final output
- * stage.
+ * Feeding the same new and old to diff_change()
+ * also has the same effect. diffcore_prune() should
+ * be used to filter uninteresting ones out before the
+ * final output happens.
*/
if (reverse_diff)
addremove = (addremove == '+' ? '-' :
diff --git a/diffcore-pathspec.c b/diffcore-pathspec.c
index 25ab9ed..4b7adc3 100644
--- a/diffcore-pathspec.c
+++ b/diffcore-pathspec.c
@@ -55,7 +55,7 @@ void diffcore_pathspec(const char **pathspec)
matches_pathspec(p->two->path, spec, speccnt))
diff_q(&outq, p);
else
- diff_free_filepair(p);
+ free(p);
}
free(q->queue);
*q = outq;
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 08d1bb7..be1bbcc 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -48,7 +48,7 @@ void diffcore_pickaxe(const char *needle)
contains(p->two, needle, len))
diff_q(&outq, p);
if (onum == outq.nr)
- diff_free_filepair(p);
+ free(p);
}
free(q->queue);
*q = outq;
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 52f09d2..e8de493 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -119,27 +119,34 @@ static void record_rename_pair(struct diff_queue_struct *outq,
int rank,
int score)
{
- /* The rank is used to sort the final output, because there
- * are certain dependencies.
- *
- * - rank #0 depends on deleted ones.
- * - rank #1 depends on kept files before they are modified.
- * - rank #2 depends on kept files after they are modified;
- * currently not used.
- *
- * Therefore, the final output order should be:
+ /*
+ * These ranks are used to sort the final output, because there
+ * are certain dependencies:
*
- * 1. rank #0 rename/copy diffs.
+ * 1. rename/copy that depends on deleted ones.
* 2. deletions in the original.
- * 3. rank #1 rename/copy diffs.
- * 4. additions and modifications in the original.
- * 5. rank #2 rename/copy diffs; currently not used.
+ * 3. rename/copy that depends on the pre-edit image of kept files.
+ * 4. additions, modifications and no-modifications in the original.
+ * 5. rename/copy that depends on the post-edit image of kept files
+ * (note that we currently do not detect such rename/copy).
+ *
+ * The downstream diffcore transformers are free to reorder
+ * the entries as long as they keep file pairs that has the
+ * same p->one->path in earlier rename_rank to appear before
+ * later ones. This ordering is used by the diff_flush()
+ * logic to tell renames from copies, and also used by the
+ * diffcore_prune() logic to omit unnecessary
+ * "no-modification" entries.
*
- * To achieve this sort order, we give xform_work the number
- * above.
+ * To the final output routine, and in the diff-raw format
+ * output, a rename/copy that is based on a path that has a
+ * later entry that shares the same p->one->path and is not a
+ * deletion is a copy. Otherwise it is a rename.
*/
+
struct diff_filepair *dp = diff_queue(outq, src, dst);
- dp->xfrm_work = (rank * 2 + 1) | (score<<RENAME_SCORE_SHIFT);
+ dp->rename_rank = rank * 2 + 1;
+ dp->score = score;
dst->xfrm_flags |= RENAME_DST_MATCHED;
}
@@ -161,10 +168,8 @@ static void debug_filepair(const struct diff_filepair *p, int i)
{
debug_filespec(p->one, i, "one");
debug_filespec(p->two, i, "two");
- fprintf(stderr, "pair flags %d, orig order %d, score %d\n",
- (p->xfrm_work & ((1<<RENAME_SCORE_SHIFT) - 1)),
- p->orig_order,
- (p->xfrm_work >> RENAME_SCORE_SHIFT));
+ fprintf(stderr, "pair rank %d, orig order %d, score %d\n",
+ p->rename_rank, p->orig_order, p->score);
}
static void debug_queue(const char *msg, struct diff_queue_struct *q)
@@ -191,8 +196,8 @@ static int rank_compare(const void *a_, const void *b_)
{
const struct diff_filepair *a = *(const struct diff_filepair **)a_;
const struct diff_filepair *b = *(const struct diff_filepair **)b_;
- int a_rank = a->xfrm_work & ((1<<RENAME_SCORE_SHIFT) - 1);
- int b_rank = b->xfrm_work & ((1<<RENAME_SCORE_SHIFT) - 1);
+ int a_rank = a->rename_rank;
+ int b_rank = b->rename_rank;
if (a_rank != b_rank)
return a_rank - b_rank;
@@ -209,28 +214,6 @@ static int score_compare(const void *a_, const void *b_)
return b->score - a->score;
}
-static int needs_to_stay(struct diff_queue_struct *q, int i,
- struct diff_filespec *it)
-{
- /* If it will be used in later entry (either stay or used
- * as the source of rename/copy), we need to copy, not rename.
- */
- while (i < q->nr) {
- struct diff_filepair *p = q->queue[i++];
- if (!DIFF_FILE_VALID(p->two))
- continue; /* removed is fine */
- if (strcmp(p->one->path, it->path))
- continue; /* not relevant */
-
- /* p has its src set to *it and it is not a delete;
- * it will be used for in-place change or rename/copy,
- * so we cannot rename it out.
- */
- return 1;
- }
- return 0;
-}
-
int diff_scoreopt_parse(const char *opt)
{
int diglen, num, scale, i;
@@ -359,27 +342,24 @@ void diffcore_rename(int detect_rename, int minimum_score)
* downstream, so we assign the sort keys in this loop.
*
* See comments at the top of record_rename_pair for numbers used
- * to assign xfrm_work.
- *
- * Note that we have not annotated the diff_filepair with any comment
- * so there is nothing other than p to free.
+ * to assign rename_rank.
*/
for (i = 0; i < q->nr; i++) {
struct diff_filepair *dp, *p = q->queue[i];
if (!DIFF_FILE_VALID(p->one)) {
/* creation or unmerged entries */
dp = diff_queue(&outq, p->one, p->two);
- dp->xfrm_work = 4;
+ dp->rename_rank = 4;
}
else if (!DIFF_FILE_VALID(p->two)) {
/* deletion */
dp = diff_queue(&outq, p->one, p->two);
- dp->xfrm_work = 2;
+ dp->rename_rank = 2;
}
else {
/* modification, or stay as is */
dp = diff_queue(&outq, p->one, p->two);
- dp->xfrm_work = 4;
+ dp->rename_rank = 4;
}
free(p);
}
@@ -415,39 +395,21 @@ void diffcore_rename(int detect_rename, int minimum_score)
/* rename or copy */
struct diff_filepair *dp =
diff_queue(q, p->one, p->two);
- int msglen = (strlen(p->one->path) +
- strlen(p->two->path) + 100);
- int score = (p->xfrm_work >> RENAME_SCORE_SHIFT);
- dp->xfrm_msg = xmalloc(msglen);
+ dp->score = p->score;
/* if we have a later entry that is a rename/copy
* that depends on p->one, then we copy here.
* otherwise we rename it.
*/
- if (needs_to_stay(&outq, i+1, p->one)) {
- /* copy it */
- sprintf(dp->xfrm_msg,
- "similarity index %d%%\n"
- "copy from %s\n"
- "copy to %s\n",
- (int)(0.5 + score * 100 / MAX_SCORE),
- p->one->path, p->two->path);
- }
- else {
- /* rename it, and mark it as gone. */
+ if (!diff_needs_to_stay(&outq, i+1, p->one))
+ /* this is the last one, so mark it as gone.
+ */
p->one->xfrm_flags |= RENAME_SRC_GONE;
- sprintf(dp->xfrm_msg,
- "similarity index %d%%\n"
- "rename old %s\n"
- "rename new %s\n",
- (int)(0.5 + score * 100 / MAX_SCORE),
- p->one->path, p->two->path);
- }
}
else
- /* otherwise it is a modified (or stayed) entry */
+ /* otherwise it is a modified (or "stay") entry */
diff_queue(q, p->one, p->two);
- diff_free_filepair(p);
+ free(p);
}
free(outq.queue);
diff --git a/diffcore.h b/diffcore.h
index d087e17..7b1cd4a 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -41,13 +41,18 @@ extern void diff_free_filespec_data(struct diff_filespec *);
struct diff_filepair {
struct diff_filespec *one;
struct diff_filespec *two;
- char *xfrm_msg;
+ int score; /* only valid when one and two are different paths */
int orig_order; /* the original order of insertion into the queue */
- int xfrm_work; /* for use by tramsformers, not by diffcore */
+ int rename_rank; /* rename/copy dependency needs to enforce
+ * certain ordering of patches that later
+ * diffcore transformations should not break.
+ */
};
#define DIFF_PAIR_UNMERGED(p) \
(!DIFF_FILE_VALID((p)->one) && !DIFF_FILE_VALID((p)->two))
+extern int diff_unmodified_pair(struct diff_filepair *);
+
struct diff_queue_struct {
struct diff_filepair **queue;
int alloc;
@@ -59,6 +64,8 @@ extern struct diff_filepair *diff_queue(struct diff_queue_struct *,
struct diff_filespec *,
struct diff_filespec *);
extern void diff_q(struct diff_queue_struct *, struct diff_filepair *);
-extern void diff_free_filepair(struct diff_filepair *);
+
+extern int diff_needs_to_stay(struct diff_queue_struct *, int,
+ struct diff_filespec *);
#endif
diff --git a/t/t4003-diff-rename-1.sh b/t/t4003-diff-rename-1.sh
index 167d0b9..999ee60 100644
--- a/t/t4003-diff-rename-1.sh
+++ b/t/t4003-diff-rename-1.sh
@@ -11,7 +11,8 @@ test_description='More rename detection
test_expect_success \
'prepare reference tree' \
'cat ../../COPYING >COPYING &&
- git-update-cache --add COPYING &&
+ echo frotz >rezrov &&
+ git-update-cache --add COPYING rezrov &&
tree=$(git-write-tree) &&
echo $tree'
@@ -22,9 +23,10 @@ test_expect_success \
rm -f COPYING &&
git-update-cache --add --remove COPYING COPYING.?'
-# tree has COPYING. work tree has COPYING.1 and COPYING.2,
-# both are slightly edited. So we say you copy-and-edit one,
-# and rename-and-edit the other.
+# tree has COPYING and rezrov. work tree has COPYING.1 and COPYING.2,
+# both are slightly edited, and unchanged rezrov. So we say you
+# copy-and-edit one, and rename-and-edit the other. We do not say
+# anything about rezrov.
GIT_DIFF_OPTS=--unified=0 git-diff-cache -M -p $tree |
sed -e 's/\([0-9][0-9]*\)/#/g' >current &&
@@ -64,9 +66,10 @@ test_expect_success \
'mv COPYING.2 COPYING &&
git-update-cache --add --remove COPYING COPYING.1 COPYING.2'
-# tree has COPYING. work tree has COPYING and COPYING.1,
-# both are slightly edited. So we say you edited one,
-# and copy-and-edit the other.
+# tree has COPYING and rezrov. work tree has COPYING and COPYING.1,
+# both are slightly edited, and unchanged rezrov. So we say you
+# edited one, and copy-and-edit the other. We do not say
+# anything about rezrov.
GIT_DIFF_OPTS=--unified=0 git-diff-cache -C -p $tree |
sed -e 's/\([0-9][0-9]*\)/#/g' >current
@@ -103,10 +106,11 @@ test_expect_success \
'cat ../../COPYING >COPYING &&
git-update-cache --add --remove COPYING COPYING.1'
-# tree has COPYING. work tree has the same COPYING and COPYING.1,
-# but COPYING is not edited. We say you copy-and-edit COPYING.1;
-# this is only possible because -C mode now reports the unmodified
-# file to the diff-core.
+# tree has COPYING and rezrov. work tree has COPYING and COPYING.1,
+# but COPYING is not edited. We say you copy-and-edit COPYING.1; this
+# is only possible because -C mode now reports the unmodified file to
+# the diff-core. Unchanged rezrov, although being fed to
+# git-diff-cache as well, should not be mentioned.
GIT_DIFF_OPTS=--unified=0 git-diff-cache -C -p $tree |
sed -e 's/\([0-9][0-9]*\)/#/g' >current
diff --git a/t/t4005-diff-rename-2.sh b/t/t4005-diff-rename-2.sh
new file mode 100644
index 0000000..90f4f4c
--- /dev/null
+++ b/t/t4005-diff-rename-2.sh
@@ -0,0 +1,82 @@
+#!/bin/sh
+#
+# Copyright (c) 2005 Junio C Hamano
+#
+
+test_description='Same rename detection as t4003 but testing diff-raw.
+
+'
+. ./test-lib.sh
+
+test_expect_success \
+ 'prepare reference tree' \
+ 'cat ../../COPYING >COPYING &&
+ echo frotz >rezrov &&
+ git-update-cache --add COPYING rezrov &&
+ tree=$(git-write-tree) &&
+ echo $tree'
+
+test_expect_success \
+ 'prepare work tree' \
+ 'sed -e 's/HOWEVER/However/' <COPYING >COPYING.1 &&
+ sed -e 's/GPL/G.P.L/g' <COPYING >COPYING.2 &&
+ rm -f COPYING &&
+ git-update-cache --add --remove COPYING COPYING.?'
+
+# tree has COPYING and rezrov. work tree has COPYING.1 and COPYING.2,
+# both are slightly edited, and unchanged rezrov. We say COPYING.1
+# and COPYING.2 are based on COPYING, and do not say anything about
+# rezrov.
+
+git-diff-cache -M $tree >current
+
+cat >expected <<\EOF
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 COPYING COPYING.1
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 06c67961bbaed34a127f76d261f4c0bf73eda471 COPYING COPYING.2
+EOF
+
+test_expect_success \
+ 'validate output from rename/copy detection' \
+ 'diff -u current expected'
+
+test_expect_success \
+ 'prepare work tree again' \
+ 'mv COPYING.2 COPYING &&
+ git-update-cache --add --remove COPYING COPYING.1 COPYING.2'
+
+# tree has COPYING and rezrov. work tree has COPYING and COPYING.1,
+# both are slightly edited, and unchanged rezrov. We say COPYING.1
+# is based on COPYING and COPYING is still there, and do not say anything
+# about rezrov.
+
+git-diff-cache -C $tree >current
+cat >expected <<\EOF
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 COPYING COPYING.1
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 06c67961bbaed34a127f76d261f4c0bf73eda471 COPYING COPYING
+EOF
+
+test_expect_success \
+ 'validate output from rename/copy detection' \
+ 'diff -u current expected'
+
+test_expect_success \
+ 'prepare work tree once again' \
+ 'cat ../../COPYING >COPYING &&
+ git-update-cache --add --remove COPYING COPYING.1'
+
+# tree has COPYING and rezrov. work tree has the same COPYING and
+# copy-edited COPYING.1, and unchanged rezrov. We should see
+# unmodified COPYING in the output, so that downstream diff-helper can
+# notice. We should not say anything about rezrov.
+
+git-diff-cache -C $tree >current
+cat >expected <<\EOF
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 COPYING COPYING.1
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 COPYING COPYING
+EOF
+
+test_expect_success \
+ 'validate output from rename/copy detection' \
+ 'diff -u current expected'
+
+test_done