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

Re: reproducible unexpected behavior for 'git reset'

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 14, 2011, 06:01 UTC
Message-ID
<7vzkkh2yfu.fsf@alter.siamese.dyndns.org>
In-Reply-To
<7voc10a1a2.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> This patch is probably a wrong way to fix this issue...
Yes, it was a wrong way. Here is the hopefully right one ;-)

Funny thing is that this is a rather ancient bug, dating back to d1f2d7e (Make run_diff_index() use unpack_trees(), not read_tree(), 2008-01-19) and survived a fix for that commit at 29796c6 (diff-index: report unmerged new entries, 2009-08-04). The reason it survived is probably because people care about the fact that the path is unmerged, but not the exact object name on the LHS of the diff (which comes from the tree).

-- >8 --
Subject: [PATCH] reset [<commit>] paths...: do not mishandle unmerged paths

Because "diff --cached HEAD" showed an incorrect blob object name on the LHS of the diff, we ended up updating the index entry with bogus value, not what we read from the tree.

Noticed by John Nowak.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/reset.c  |    2 +-
 diff-lib.c       |    3 ++-
 t/t7102-reset.sh |   15 +++++++++++++++
 3 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 98bca04..777e7c6 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -162,7 +162,7 @@ static void update_index_from_diff(struct diff_queue_struct *q,
 
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filespec *one = q->queue[i]->one;
-		if (one->mode) {
+		if (one->mode && !is_null_sha1(one->sha1)) {
 			struct cache_entry *ce;
 			ce = make_cache_entry(one->mode, one->sha1, one->path,
 				0, 0);
diff --git a/diff-lib.c b/diff-lib.c
index 9c29293..fd61acb 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -379,7 +379,8 @@ static void do_oneway_diff(struct unpack_trees_options *o,
 	if (cached && idx && ce_stage(idx)) {
 		struct diff_filepair *pair;
 		pair = diff_unmerge(&revs->diffopt, idx->name);
-		fill_filespec(pair->one, idx->sha1, idx->ce_mode);
+		if (tree)
+			fill_filespec(pair->one, tree->sha1, tree->ce_mode);
 		return;
 	}
 
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index f1cfc9a..b096dc8 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -429,6 +429,21 @@ test_expect_success '--mixed refreshes the index' '
 	test_i18ncmp expect output
 '
 
+test_expect_success 'resetting specific path that is unmerged' '
+	git rm --cached file2 &&
+	F1=$(git rev-parse HEAD:file1) &&
+	F2=$(git rev-parse HEAD:file2) &&
+	F3=$(git rev-parse HEAD:secondfile) &&
+	{
+		echo "100644 $F1 1	file2" &&
+		echo "100644 $F2 2	file2" &&
+		echo "100644 $F3 3	file2"
+	} | git update-index --index-info &&
+	git ls-files -u &&
+	test_must_fail git reset HEAD file2 &&
+	git diff-index --exit-code --cached HEAD
+'
+
 test_expect_success 'disambiguation (1)' '
 
 	git reset --hard &&
-- 
1.7.6.178.g55272
Previous: Junio C HamanoNext: Junio C Hamano
Message 4 of 9 in “reproducible unexpected behavior for 'git reset'”
  1. John NowakJul 10, 2011
  2. Junio C HamanoJul 11, 2011
  3. Junio C HamanoJul 11, 2011
  4. Junio C HamanoJul 14, 2011
  5. 0/2 refactoring diff-indexJunio C Hamano, Jul 19, 2011
  6. 1/2 diff-lib: simplify do_diff_cache()Junio C Hamano, Jul 19, 2011
  7. Jeff KingJul 19, 2011
  8. Junio C HamanoJul 19, 2011
  9. 2/2 diff-lib: refactor run_diff_index() and do_diff_cache()Junio C Hamano, Jul 19, 2011

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.