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

[PATCH v2 7/7] wt-status.c: avoid double renames in short/porcelain format

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

The presence of worktree rename leads to an interesting situation, what if the same index entry is renamed twice, compared to HEAD and to worktree? We can have that with this setup

    echo first > first && git add first && git commit -m first
    git mv first second  # rename reported in "diff --cached"
    mv second third      # rename reported in "diff-files"

For the long format this is fine because we print two "->" rename lines, one in the "updated" section, one in "changed" one.

For other output formats, it gets tricky because they combine both diffs in one line but can only display one rename per line. The result "XY" column of short format, for example, would be "RR" in that case.

This case either needs some extension in short/porcelain format to show something crazy like

    RR first -> second -> third

or we could show renames as two lines instead of one, for example something like this for short form:

    R  first -> second
     R second -> third

But for now it's safer and simpler to just break the "second -> third" rename pair and show

    RD first -> second
     A third
like we have been showing until now.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/t2203-add-intent.sh | 33 +++++++++++++++++++++++++++++
 wt-status.c           | 58 ++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 84 insertions(+), 7 deletions(-)
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index e5bfda1853..79aca93810 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -190,5 +190,38 @@ test_expect_success 'rename detection finds the right names' '
 	)
 '
 
+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 &&
+		RD first -> second
+		 A third
+		EOF
+		test_cmp expected.2 actual.2 &&
+
+		oid=12f00e90b6ef79117ce6e650416b8cf517099b78 &&
+		git status --porcelain=v2 | grep -v "^?" >actual.3 &&
+		cat >expected.3 <<-EOF &&
+		2 RD N... 100644 100644 000000 $oid $oid R100 second	first
+		1 .A N... 000000 000000 100644 $_z40 $_z40 third
+		EOF
+		test_cmp expected.3 actual.3
+	)
+'
+
 test_done
 
diff --git a/wt-status.c b/wt-status.c
index d5bdf4c2e9..e62853f748 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -419,6 +419,47 @@ static char short_submodule_status(struct wt_status_change_data *d)
 	return d->worktree_status;
 }
 
+static struct string_list_item * break_double_rename(
+		struct wt_status *s, struct string_list_item *it,
+		int *status, struct diff_filepair *p)
+{
+	struct wt_status_change_data *d;
+	struct string_list_item *new_it;
+
+	d = it->util;
+	/*
+	 * _collect_index_changes() must have been called or
+	 * d->head_path does not contain a real value.
+	 */
+	if (!d || !d->head_path)
+		return it;
+
+	switch (s->status_format) {
+	case STATUS_FORMAT_SHORT:
+	case STATUS_FORMAT_PORCELAIN:
+	case STATUS_FORMAT_PORCELAIN_V2:
+		break;
+	case STATUS_FORMAT_LONG:
+	case STATUS_FORMAT_NONE:
+		/* this output can handle double renames ok */
+		return it;
+	default:
+		die("BUG: finalize_deferred_config() should have been called");
+	}
+
+	switch (*status) {
+	case DIFF_STATUS_RENAMED:
+		d->worktree_status = DIFF_STATUS_DELETED;
+		/* fallthru */
+	case DIFF_STATUS_COPIED:
+		*status = DIFF_STATUS_ADDED;
+		new_it = string_list_insert(&s->change, p->two->path);
+		return new_it;
+	}
+
+	return it;
+}
+
 static void wt_status_collect_changed_cb(struct diff_queue_struct *q,
 					 struct diff_options *options,
 					 void *data)
@@ -433,16 +474,19 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,
 		struct diff_filepair *p;
 		struct string_list_item *it;
 		struct wt_status_change_data *d;
+		int status;
 
 		p = q->queue[i];
+		status = p->status;
 		it = string_list_insert(&s->change, p->one->path);
+		it = break_double_rename(s, it, &status, p);
 		d = it->util;
 		if (!d) {
 			d = xcalloc(1, sizeof(*d));
 			it->util = d;
 		}
 		if (!d->worktree_status)
-			d->worktree_status = p->status;
+			d->worktree_status = status;
 		if (S_ISGITLINK(p->two->mode)) {
 			d->dirty_submodule = p->two->dirty_submodule;
 			d->new_submodule_commits = !!oidcmp(&p->one->oid,
@@ -451,7 +495,7 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,
 				d->worktree_status = short_submodule_status(d);
 		}
 
-		switch (p->status) {
+		switch (status) {
 		case DIFF_STATUS_ADDED:
 			d->mode_worktree = p->two->mode;
 			break;
@@ -477,7 +521,7 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,
 			break;
 
 		default:
-			die("BUG: unhandled diff-files status '%c'", p->status);
+			die("BUG: unhandled diff-files status '%c'", status);
 			break;
 		}
 
@@ -710,12 +754,12 @@ static void wt_status_collect_untracked(struct wt_status *s)
 
 void wt_status_collect(struct wt_status *s)
 {
-	wt_status_collect_changes_worktree(s);
-
 	if (s->is_initial)
 		wt_status_collect_changes_initial(s);
 	else
+		/* must be called before _collect_changes_worktree() */
 		wt_status_collect_changes_index(s);
+	wt_status_collect_changes_worktree(s);
 	wt_status_collect_untracked(s);
 }
 
@@ -1733,7 +1777,7 @@ static void wt_shortstatus_status(struct string_list_item *it,
 	putchar(' ');
 
 	if (d->head_path && d->worktree_path)
-		die("BUG: to be addressed in the next patch");
+		die("BUG: break_double_rename() fails to break this pair");
 
 	if (d->head_path) {
 		from = d->head_path;
@@ -2077,7 +2121,7 @@ static void wt_porcelain_v2_print_changed_entry(
 	key[2] = 0;
 
 	if (d->head_path && d->worktree_path)
-		die("BUG: to be addressed in the next patch");
+		die("BUG: break_double_rename() fails to break this pair");
 
 	if (d->head_path) {
 		path_other = d->head_path;

-- 
2.15.0.320.g0453912d77
Previous: Igor DjordjevicNext: Igor Djordjevic
Message 24 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.