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

[PATCH v3 6/6] wt-status.c: handle worktree renames

From
Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Date
Dec 27, 2017, 10:18 UTC
Message-ID
<20171227101839.26427-7-pclouds@gmail.com>
In-Reply-To
<20171227101839.26427-1-pclouds@gmail.com>

Before 425a28e0a4 (diff-lib: allow ita entries treated as "not yet exist in index" - 2016-10-24) there are never "new files" in the index, which essentially disables rename detection because we only detect renames when a new file appears in a diff pair.

After that commit, an i-t-a entry can appear as a new file in "git diff-files". But the diff callback function in wt-status.c does not handle this case and produces incorrect status output.

PS. The reader may notice that this patch adds a new xstrdup() but not a free(). Yes we leak memory (the same for head_path). But wt_status so far has been short lived, this leak should not matter in practice.

Noticed-by: Alex Vandiver <alexmv@dropbox.com>
Helped-by: Igor Djordjevic <igor.d.djordjevic@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-status.txt | 23 +++++++++--------
 t/t2203-add-intent.sh        | 60 ++++++++++++++++++++++++++++++++++++++++++++
 wt-status.c                  | 22 +++++++++++++---
 3 files changed, 92 insertions(+), 13 deletions(-)
diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 81cab9aefb..72bfb87f66 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -149,14 +149,15 @@ the status.relativePaths config option below.
 Short Format
 ~~~~~~~~~~~~
 
-In the short-format, the status of each path is shown as
+In the short-format, the status of each path is shown as one of these
+forms
 
-	XY PATH1 -> PATH2
+	XY PATH
+	XY ORIG_PATH -> PATH
 
-where `PATH1` is the path in the `HEAD`, and the " `-> PATH2`" part is
-shown only when `PATH1` corresponds to a different path in the
-index/worktree (i.e. the file is renamed). The `XY` is a two-letter
-status code.
+where `ORIG_PATH` is where the renamed/copied contents came
+from. `ORIG_PATH` is only shown when the entry is renamed or
+copied. The `XY` is a two-letter status code.
 
 The fields (including the `->`) are separated from each other by a
 single space. If a filename contains whitespace or other nonprintable
@@ -192,6 +193,8 @@ in which case `XY` are `!!`.
     [MARC]           index and work tree matches
     [ MARC]     M    work tree changed since index
     [ MARC]     D    deleted in work tree
+    [ D]        R    renamed in work tree
+    [ D]        C    copied in work tree
     -------------------------------------------------
     D           D    unmerged, both deleted
     A           U    unmerged, added by us
@@ -309,13 +312,13 @@ Renamed or copied entries have the following format:
 		of similarity between the source and target of the
 		move or copy). For example "R100" or "C75".
     <path>      The pathname.  In a renamed/copied entry, this
-		is the path in the index and in the working tree.
+		is the target path.
     <sep>       When the `-z` option is used, the 2 pathnames are separated
 		with a NUL (ASCII 0x00) byte; otherwise, a tab (ASCII 0x09)
 		byte separates them.
-    <origPath>  The pathname in the commit at HEAD.  This is only
-		present in a renamed/copied entry, and tells
-		where the renamed/copied contents came from.
+    <origPath>  The pathname in the commit at HEAD or in the index.
+		This is only present in a renamed/copied entry, and
+		tells where the renamed/copied contents came from.
     --------------------------------------------------------
 
 Unmerged entries have the following format; the first character is
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 878e73fe98..78236dc7d8 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -162,5 +162,65 @@ test_expect_success 'commit: ita entries ignored in empty commit check' '
 	)
 '
 
+test_expect_success 'rename detection finds the right names' '
+	git init rename-detection &&
+	(
+		cd rename-detection &&
+		echo contents >first &&
+		git add first &&
+		git commit -m first &&
+		mv first third &&
+		git add -N third &&
+
+		git status | grep -v "^?" >actual.1 &&
+		test_i18ngrep "renamed: *first -> third" actual.1 &&
+
+		git status --porcelain | grep -v "^?" >actual.2 &&
+		cat >expected.2 <<-\EOF &&
+		 R first -> third
+		EOF
+		test_cmp expected.2 actual.2 &&
+
+		hash=12f00e90b6ef79117ce6e650416b8cf517099b78 &&
+		git status --porcelain=v2 | grep -v "^?" >actual.3 &&
+		cat >expected.3 <<-EOF &&
+		2 .R N... 100644 100644 100644 $hash $hash R100 third	first
+		EOF
+		test_cmp expected.3 actual.3
+	)
+'
+
+test_expect_success 'double rename detection in status' '
+	git init rename-detection-2 &&
+	(
+		cd rename-detection-2 &&
+		echo contents >first &&
+		git add first &&
+		git commit -m first &&
+		git mv first second &&
+		mv second third &&
+		git add -N third &&
+
+		git status | grep -v "^?" >actual.1 &&
+		test_i18ngrep "renamed: *first -> second" actual.1 &&
+		test_i18ngrep "renamed: *second -> third" actual.1 &&
+
+		git status --porcelain | grep -v "^?" >actual.2 &&
+		cat >expected.2 <<-\EOF &&
+		R  first -> second
+		 R second -> third
+		EOF
+		test_cmp expected.2 actual.2 &&
+
+		hash=12f00e90b6ef79117ce6e650416b8cf517099b78 &&
+		git status --porcelain=v2 | grep -v "^?" >actual.3 &&
+		cat >expected.3 <<-EOF &&
+		2 R. N... 100644 100644 100644 $hash $hash R100 second	first
+		2 .R N... 100644 100644 100644 $hash $hash R100 third	second
+		EOF
+		test_cmp expected.3 actual.3
+	)
+'
+
 test_done
 
diff --git a/wt-status.c b/wt-status.c
index fab6951573..f5debcd2b4 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -360,8 +360,6 @@ static void wt_longstatus_print_change_data(struct wt_status *s,
 	switch (change_type) {
 	case WT_STATUS_UPDATED:
 		status = d->index_status;
-		if (d->rename_source)
-			one_name = d->rename_source;
 		break;
 	case WT_STATUS_CHANGED:
 		if (d->new_submodule_commits || d->dirty_submodule) {
@@ -382,6 +380,14 @@ static void wt_longstatus_print_change_data(struct wt_status *s,
 		    change_type);
 	}
 
+	/*
+	 * Only pick up the rename it's relevant. If the rename is for
+	 * the changed section and we're printing the updated section,
+	 * ignore it.
+	 */
+	if (d->rename_status == status)
+		one_name = d->rename_source;
+
 	one = quote_path(one_name, s->prefix, &onebuf);
 	two = quote_path(two_name, s->prefix, &twobuf);
 
@@ -433,7 +439,7 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,
 		struct wt_status_change_data *d;
 
 		p = q->queue[i];
-		it = string_list_insert(&s->change, p->one->path);
+		it = string_list_insert(&s->change, p->two->path);
 		d = it->util;
 		if (!d) {
 			d = xcalloc(1, sizeof(*d));
@@ -460,6 +466,14 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,
 			/* mode_worktree is zero for a delete. */
 			break;
 
+		case DIFF_STATUS_COPIED:
+		case DIFF_STATUS_RENAMED:
+			if (d->rename_status)
+				die("BUG: multiple renames on the same target? how?");
+			d->rename_source = xstrdup(p->one->path);
+			d->rename_score = p->score * 100 / MAX_SCORE;
+			d->rename_status = p->status;
+			/* fallthru */
 		case DIFF_STATUS_MODIFIED:
 		case DIFF_STATUS_TYPE_CHANGED:
 		case DIFF_STATUS_UNMERGED:
@@ -531,6 +545,8 @@ static void wt_status_collect_updated_cb(struct diff_queue_struct *q,
 
 		case DIFF_STATUS_COPIED:
 		case DIFF_STATUS_RENAMED:
+			if (d->rename_status)
+				die("BUG: multiple renames on the same target? how?");
 			d->rename_source = xstrdup(p->one->path);
 			d->rename_score = p->score * 100 / MAX_SCORE;
 			d->rename_status = p->status;

-- 
2.15.0.320.g0453912d77
Previous: Nguyễn Thái Ngọc DuyNext: Igor Djordjevic
Message 34 of 37 in “[BUG] File move with `add -N` shows as rename to same name”
  1. Alex VandiverDec 23, 2017
  2. Duy NguyenDec 25, 2017
  3. status: handle worktree renamesNguyễn Thái Ngọc Duy, Dec 25, 2017
  4. Igor DjordjevicDec 25, 2017
  5. Igor DjordjevicDec 25, 2017
  6. Igor DjordjevicDec 25, 2017
  7. Duy NguyenDec 26, 2017
  8. Duy NguyenDec 26, 2017
  9. Junio C HamanoDec 27, 2017
  10. Junio C HamanoDec 27, 2017
  11. Jeff HostetlerJan 2, 2018
  12. Duy NguyenJan 10, 2018
  13. 0/7 Renames in git-status "changed not staged" sectionNguyễn Thái Ngọc Duy, Dec 26, 2017
  14. 1/7 t2203: test status output with porcelain v2 formatNguyễn Thái Ngọc Duy, Dec 26, 2017
  15. 2/7 Use DIFF_DETECT_RENAME for detect_rename assignmentsNguyễn Thái Ngọc Duy, Dec 26, 2017
  16. 3/7 wt-status.c: coding style fixNguyễn Thái Ngọc Duy, Dec 26, 2017
  17. 4/7 wt-status.c: rename wt_status_change_data::scoreNguyễn Thái Ngọc Duy, Dec 26, 2017
  18. 5/7 wt-status.c: catch unhandled diff status codesNguyễn Thái Ngọc Duy, Dec 26, 2017
  19. 6/7 wt-status.c: handle worktree renamesNguyễn Thái Ngọc Duy, Dec 26, 2017
  20. Igor DjordjevicDec 26, 2017
  21. Duy NguyenDec 27, 2017
  22. Igor DjordjevicDec 28, 2017
  23. Igor DjordjevicDec 28, 2017
  24. 7/7 wt-status.c: avoid double renames in short/porcelain formatNguyễn Thái Ngọc Duy, Dec 26, 2017
  25. Igor DjordjevicDec 26, 2017
  26. Duy NguyenDec 27, 2017
  27. Igor DjordjevicDec 27, 2017
  28. 0/6 Renames in git-status "changed not staged" sectionNguyễn Thái Ngọc Duy, Dec 27, 2017
  29. 1/6 t2203: test status output with porcelain v2 formatNguyễn Thái Ngọc Duy, Dec 27, 2017
  30. 2/6 Use DIFF_DETECT_RENAME for detect_rename assignmentsNguyễn Thái Ngọc Duy, Dec 27, 2017
  31. 3/6 wt-status.c: coding style fixNguyễn Thái Ngọc Duy, Dec 27, 2017
  32. 4/6 wt-status.c: catch unhandled diff status codesNguyễn Thái Ngọc Duy, Dec 27, 2017
  33. 5/6 wt-status.c: rename rename-related fields in wt_status_change_dataNguyễn Thái Ngọc Duy, Dec 27, 2017
  34. 6/6 wt-status.c: handle worktree renamesNguyễn Thái Ngọc Duy, Dec 27, 2017
  35. Igor DjordjevicDec 28, 2017
  36. Jeff HostetlerJan 2, 2018
  37. Torsten BögershausenDec 26, 2017

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.