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

[PATCH] Be careful with symlinks when detecting renames and copies.

From
Junio C Hamano <junkio@cox.net>
Date
May 23, 2005, 04:24 UTC
Message-ID
<7vhdgubmcu.fsf_-_@assigned-by-dhcp.cox.net>
In-Reply-To
<7vekbzfaze.fsf@assigned-by-dhcp.cox.net>

Linus, this is *not* based on the fixed rename/copy fix I am going to send out shortly, but comes before that due to patch dependency (the order I happened to have done things, that is).

Please apply this one before the rename/copy fix.

------------ Earlier round was not treating symbolic links carefully enough, and would have produced diff output that renamed/copied then edited the contents of a symbolic link, which made no practical sense. Change it to detect only pure renames.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diffcore-rename.c | 24 ++++++++------ t/t4004-diff-rename-symlink.sh | 66 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 10 deletions(-) new file (100755): t/t4004-diff-rename-symlink.sh

diff --git a/diffcore-rename.c b/diffcore-rename.c
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -20,7 +20,7 @@ static void diff_rename_pool_add(struct 
 				 struct diff_filespec *s)
 {
 	if (S_ISDIR(s->mode))
-		return;  /* rename/copy patch for tree does not make sense. */
+		return;  /* no trees, please */
 
 	if (pool->alloc <= pool->nr) {
 		pool->alloc = alloc_nr(pool->alloc);
@@ -71,6 +71,13 @@ static int estimate_similarity(struct di
 	unsigned long delta_size, base_size;
 	int score;
 
+	/* We deal only with regular files.  Symlink renames are handled
+	 * only when they are exact matches --- in other words, no edits
+	 * after renaming.
+	 */
+	if (!S_ISREG(src->mode) || !S_ISREG(dst->mode))
+		return 0;
+
 	delta_size = ((src->size < dst->size) ?
 		      (dst->size - src->size) : (src->size - dst->size));
 	base_size = ((src->size < dst->size) ? src->size : dst->size);
@@ -268,7 +275,7 @@ void diffcore_rename(int detect_rename, 
 		struct diff_filepair *p = q->queue[i];
 		if (!DIFF_FILE_VALID(p->one))
 			if (!DIFF_FILE_VALID(p->two))
-				continue; /* ignore nonsense */
+				continue; /* unmerged */
 			else
 				diff_rename_pool_add(&created, p->two);
 		else if (!DIFF_FILE_VALID(p->two))
@@ -360,12 +367,9 @@ void diffcore_rename(int detect_rename, 
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *dp, *p = q->queue[i];
 		if (!DIFF_FILE_VALID(p->one)) {
-			if (DIFF_FILE_VALID(p->two)) {
-				/* creation */
-				dp = diff_queue(&outq, p->one, p->two);
-				dp->xfrm_work = 4;
-			}
-			/* otherwise it is a nonsense; just ignore it */
+			/* creation or unmerged entries */
+			dp = diff_queue(&outq, p->one, p->two);
+			dp->xfrm_work = 4;
 		}
 		else if (!DIFF_FILE_VALID(p->two)) {
 			/* deletion */
@@ -394,7 +398,7 @@ void diffcore_rename(int detect_rename, 
 	for (i = 0; i < outq.nr; i++) {
 		struct diff_filepair *p = outq.queue[i];
 		if (!DIFF_FILE_VALID(p->one)) {
-			/* created */
+			/* created or unmerged */
 			if (p->two->xfrm_flags & RENAME_DST_MATCHED)
 				; /* rename/copy created it already */
 			else
@@ -443,7 +447,7 @@ void diffcore_rename(int detect_rename, 
 		else
 			/* otherwise it is a modified (or stayed) entry */
 			diff_queue(q, p->one, p->two);
-		free(p);
+		diff_free_filepair(p);
 	}
 
 	free(outq.queue);
diff --git a/t/t4004-diff-rename-symlink.sh b/t/t4004-diff-rename-symlink.sh
new file mode 100755
--- /dev/null
+++ b/t/t4004-diff-rename-symlink.sh
@@ -0,0 +1,66 @@
+#!/bin/sh
+#
+# Copyright (c) 2005 Junio C Hamano
+#
+
+test_description='More rename detection tests.
+
+The rename detection logic should be able to detect pure rename or
+copy of symbolic links, but should not produce rename/copy followed
+by an edit for them.
+'
+. ./test-lib.sh
+
+test_expect_success \
+    'prepare reference tree' \
+    'echo xyzzy | tr -d '\\\\'012 >yomin &&
+     ln -s xyzzy frotz &&
+    git-update-cache --add frotz yomin &&
+    tree=$(git-write-tree) &&
+    echo $tree'
+
+test_expect_success \
+    'prepare work tree' \
+    'mv frotz rezrov &&
+     rm -f yomin &&
+     ln -s xyzzy nitfol &&
+     ln -s xzzzy bozbar &&
+    git-update-cache --add --remove frotz rezrov nitfol bozbar yomin'
+
+# tree has frotz pointing at xyzzy, and yomin that contains xyzzy to
+# confuse things.  work tree has rezrov (xyzzy) nitfol (xyzzy) and
+# bozbar (xzzzy).
+# rezrov and nitfol are rename/copy of frotz and bozbar should be
+# a new creation.
+
+GIT_DIFF_OPTS=--unified=0 git-diff-cache -M -p $tree >current
+cat >expected <<\EOF
+diff --git a/frotz b/nitfol
+similarity index 100%
+copy from frotz
+copy to nitfol
+diff --git a/frotz b/rezrov
+similarity index 100%
+rename old frotz
+rename new rezrov
+diff --git a/yomin b/yomin
+deleted file mode 100644
+--- a/yomin
++++ /dev/null
+@@ -1 +0,0 @@
+-xyzzy
+\ No newline at end of file
+diff --git a/bozbar b/bozbar
+new file mode 120000
+--- /dev/null
++++ b/bozbar
+@@ -0,0 +1 @@
++xzzzy
+\ No newline at end of file
+EOF
+
+test_expect_success \
+    'validate diff output' \
+    'diff -u current expected'
+
+test_done
------------------------------------------------
Previous: Junio C Hamano
Message 33 of 33 in “updated design for the diff-raw format.”
  1. Junio C HamanoMay 21, 2005
  2. Junio C HamanoMay 21, 2005
  3. Junio C HamanoMay 21, 2005
  4. Junio C HamanoMay 21, 2005
  5. Junio C HamanoMay 21, 2005
  6. Prepare diffcore interface for diff-tree header supression.Junio C Hamano, May 22, 2005
  7. The diff-raw format updates.Junio C Hamano, May 22, 2005
  8. Linus TorvaldsMay 22, 2005
  9. Junio C HamanoMay 22, 2005
  10. Junio C HamanoMay 22, 2005
  11. Fix tweak in similarity estimator.Junio C Hamano, May 22, 2005
  12. Linus TorvaldsMay 22, 2005
  13. Niklas HoglundMay 22, 2005
  14. Junio C HamanoMay 22, 2005
  15. Thomas GlanzmannMay 22, 2005
  16. Linus TorvaldsMay 22, 2005
  17. Thomas GlanzmannMay 22, 2005
  18. Junio C HamanoMay 22, 2005
  19. Junio C HamanoMay 22, 2005
  20. Linus TorvaldsMay 22, 2005
  21. Junio C HamanoMay 22, 2005
  22. Linus TorvaldsMay 22, 2005
  23. Junio C HamanoMay 23, 2005
  24. Linus TorvaldsMay 23, 2005
  25. Junio C HamanoMay 23, 2005
  26. Rename/copy detection fix.Junio C Hamano, May 23, 2005
  27. Comments on "Rename/copy detection fix."Junio C Hamano, May 23, 2005
  28. Junio C HamanoMay 22, 2005
  29. Diffcore updates.Junio C Hamano, May 22, 2005
  30. Linus TorvaldsMay 22, 2005
  31. Junio C HamanoMay 22, 2005
  32. Diffcore updates.Junio C Hamano, May 22, 2005
  33. Be careful with symlinks when detecting renames and copies.Junio C Hamano, May 23, 2005

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.