git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/2] [RFC] Only show bulkmoves in output.

From
Yann Dirson <ydirson@altern.org>
Date
Oct 10, 2010, 19:59 UTC
Message-ID
<1286740795-7929-2-git-send-email-ydirson@altern.org>
In-Reply-To
<1286740795-7929-1-git-send-email-ydirson@altern.org>

At display time, append a "*" suffix to bulk-move sources. Later we will be more selective and not display it for bulk moves that are indeed a directory rename.

Open question: "foo/*" is a valid name for a file, so if such a file is part of a bulk move, we will see one "foo/*" entry for the bulk move (with zeroed sha1's), and one "foo/*" for the individual file (with its own sha1). Is that seen as a problem ? After all, the zeroed sha1's are probably here to stay (or do we want to add tree sha1's where possible ?), so it is pretty easy to tell which is which. An alternative would be to have another way of telling it is a bulk-move, such as a flag to M or C (eg. M100B), but there is no precedent for this, so I can't tell for myself.

After this patch only one of the expected failures can be considered
innocuous.
---
 diff.c                           |    9 ++++++++-
 diffcore-rename.c                |    4 ++--
 diffcore.h                       |    1 +
 t/t4046-diff-rename-factorize.sh |   22 +++++++++++-----------
 4 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/diff.c b/diff.c
index 64e01b3..4de43d6 100644
--- a/diff.c
+++ b/diff.c
@@ -3471,7 +3471,14 @@ static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt)
 	if (p->status == DIFF_STATUS_COPIED ||
 	    p->status == DIFF_STATUS_RENAMED) {
 		const char *name_a, *name_b;
-		name_a = p->one->path;
+		if (p->is_bulkmove) {
+			/* append "*" to the first dirname */
+			char buf[PATH_MAX];
+			char* next = memccpy(buf, p->one->path, '\0', PATH_MAX);
+			next[-1] = '*'; *next = '\0';
+			name_a = buf;
+		} else
+			name_a = p->one->path;
 		name_b = p->two->path;
 		strip_prefix(opt->prefix_length, &name_a, &name_b);
 		write_name_quoted(name_a, opt->file, inter_name_termination);
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 18a0605..8de0d57 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -603,8 +603,7 @@ static void check_one_bulk_move(struct diff_filepair *dstpair)
 	/* already considered ? */
 	for (seen=bulkmove_candidates; seen; seen = seen->next)
 		if (!strcmp(seen->one->path, one_parent_path)) break;
-	if (!seen) {
-		/* record potential dir rename */
+	if (!seen) { /* record potential dir rename */
 		seen = xmalloc(sizeof(*seen));
 		seen->one = alloc_filespec(one_parent_path);
 		fill_filespec(seen->one, null_sha1 /*FIXME*/, S_IFDIR);
@@ -830,6 +829,7 @@ void diffcore_rename(struct diff_options *options)
 		pair = diff_queue(&outq, candidate->one, candidate->two);
 		pair->score = MAX_SCORE;
 		pair->renamed_pair = 1;
+		pair->is_bulkmove = 1;
 	}
 
 	for (i = 0; i < q->nr; i++) {
diff --git a/diffcore.h b/diffcore.h
index b8f1fde..6dab95b 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -69,6 +69,7 @@ struct diff_filepair {
 	unsigned broken_pair : 1;
 	unsigned renamed_pair : 1;
 	unsigned is_unmerged : 1;
+	unsigned is_bulkmove : 1;
 };
 #define DIFF_PAIR_UNMERGED(p) ((p)->is_unmerged)
 
diff --git a/t/t4046-diff-rename-factorize.sh b/t/t4046-diff-rename-factorize.sh
index d063e25..9353929 100755
--- a/t/t4046-diff-rename-factorize.sh
+++ b/t/t4046-diff-rename-factorize.sh
@@ -37,7 +37,7 @@ test_expect_success 'setup' '
 
 test_expect_success 'diff-index --detect-bulk-moves after directory move.' '
 	cat >expected <<-EOF &&
-	:040000 040000 X X R#	a/	b/
+	:040000 040000 X X R#	a/*	b/
 	:100644 100644 X X R#	a/path0	b/path0
 	:100644 100644 X X R#	a/path1	b/path1
 	:100644 100644 X X R#	a/path2	b/path2
@@ -58,7 +58,7 @@ test_expect_success 'setup non-100% rename' '
 
 test_expect_success 'diff-index --detect-bulk-moves after content changes.' '
 	cat >expected <<-EOF &&
-	:040000 040000 X X R#	a/	b/
+	:040000 040000 X X R#	a/*	b/
 	:100644 000000 X X D#	a/path3
 	:100644 100644 X X R#	a/path2	b/2path
 	:100644 100644 X X R#	a/path0	b/path0
@@ -87,7 +87,7 @@ test_expect_success 'setup bulk move that is not directory move' '
 	git update-index --add --remove a/* c/apath0 c/apath1 c/apath2
 '
 
-test_expect_failure 'diff-index --detect-bulk-moves without full-dir rename.' '
+test_expect_success 'diff-index --detect-bulk-moves without full-dir rename.' '
 	cat >expected <<-EOF &&
 	:040000 040000 X X R#	c/*	a/
 	:100644 100644 X X R#	c/apath0	a/apath0
@@ -105,7 +105,7 @@ test_expect_success 'setup bulk move to toplevel' '
 	git update-index --add --remove apath* c/apath0 c/apath1 c/apath2
 '
 
-test_expect_failure 'diff-index --detect-bulk-moves bulk move to toplevel.' '
+test_expect_success 'diff-index --detect-bulk-moves bulk move to toplevel.' '
 	cat >expected <<-EOF &&
 	:040000 040000 X X R#	c/*	./
 	:100644 100644 X X R#	c/apath0	apath0
@@ -132,7 +132,7 @@ test_expect_success 'setup move including a subdir, with some content changes' '
 
 test_expect_failure 'diff-index --detect-bulk-moves on a move including a subdir.' '
 	cat >expected <<-EOF &&
-	:040000 040000 X X R#	a/	b/
+	:040000 040000 X X R#	a/*	b/
 	:100644 100644 X X R#	a/c/apath0	b/c/apath0
 	:100644 100644 X X R#	a/c/apath1	b/c/apath1
 	:100644 100644 X X R#	a/c/apath2	b/c/apath2
@@ -176,7 +176,7 @@ test_expect_success 'setup move of files and subdirs to different places' '
 
 test_expect_failure 'moving subdirs into one dir and files into another is not mistaken for dir move' '
 	cat >expected <<-EOF &&
-	:040000 040000 X X R#	a/c/	b/
+	:040000 040000 X X R#	a/c/*	b/
 	:100644 100644 X X R#	a/c/apath0	b/apath0
 	:100644 100644 X X R#	a/c/apath1	b/apath1
 	:100644 100644 X X R#	a/c/apath2	b/apath2
@@ -201,11 +201,11 @@ test_expect_success 'setup move of dir with only subdirs' '
 	git mv a z
 '
 
-test_expect_failure 'moving a dir with no files' '
+test_expect_failure 'moving a dir with no direct children files' '
 	cat >expected <<-EOF &&
-	:040000 040000 X X R#	a/	z/
-	:040000 040000 X X R#	a/b/	z/b/
-	:040000 040000 X X R#	a/c/	z/c/
+	:040000 040000 X X R#	a/*	z/
+	:040000 040000 X X R#	a/b/*	z/b/
+	:040000 040000 X X R#	a/c/*	z/c/
 	:100644 100644 X X R#	a/b/path0	z/b/path0
 	:100644 100644 X X R#	a/b/path1	z/b/path1
 	:100644 100644 X X R#	a/b/path2	z/b/path2
@@ -235,7 +235,7 @@ test_expect_success 'setup move from toplevel to subdir' '
 	git update-index --add --remove path0 path1 path2 path3 z/path*
 '
 
-test_expect_failure '--detect-bulk-moves everything from toplevel.' '
+test_expect_success '--detect-bulk-moves everything from toplevel.' '
 	cat >expected <<-EOF &&
 	:040000 040000 X X R#	./*	z/
 	:100644 100644 X X R#	path0	z/path0

-- 
1.7.2.3
Previous: Yann DirsonNext: Yann Dirson
Message 8 of 9 in “Detection of directory renames”
  1. 0/4 Detection of directory renamesYann Dirson, Oct 9, 2010
  2. 1/4 Introduce bulk-move detection in diffcore.Yann Dirson, Oct 9, 2010
  3. 2/4 Add testcases for the --detect-bulk-moves diffcore flag.Yann Dirson, Oct 9, 2010
  4. 3/4 [RFC] Only show bulkmoves in output.Yann Dirson, Oct 9, 2010
  5. 4/4 [RFC] Allow hiding renames of individual files involved in a directory rename.Yann Dirson, Oct 9, 2010
  6. Yann DirsonOct 10, 2010
  7. 0/2 Re: [PATCH v5 3/4] [RFC] Only show bulkmoves in output.Yann Dirson, Oct 10, 2010
  8. 1/2 [RFC] Only show bulkmoves in output.Yann Dirson, Oct 10, 2010
  9. 2/2 [RFC] Transfer special display of toplevel dir to display-time.Yann Dirson, Oct 10, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.