git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Varun Naik <vcnaik94@gmail.com>
To: vcnaik94@gmail.com
Cc: git@vger.kernel.org, gitster@pobox.com, pclouds@gmail.com
Subject: [PATCH v3] diff-lib.c: handle empty deleted ita files
Date: Thu,  1 Aug 2019 09:15:58 -0700	[thread overview]
Message-ID: <20190801161558.12838-1-vcnaik94@gmail.com> (raw)
In-Reply-To: <20190712150235.12633-1-vcnaik94@gmail.com>

It is possible to delete a committed file from the index and then add it
as intent-to-add. Certain forms of `git diff` should show the file.
After `git reset HEAD`, the file should be identical in the index and
HEAD. The commands already work correctly if the file has contents in
HEAD. This patch provides the desired behavior even when the file is
empty in HEAD.

The affected "diff" commands and the "reset" command call
diff-lib.c:do_oneway_diff() with a cache entry in the index and a cache
entry in HEAD. An ita file is represented in the index by a cache entry
with the same hash as an empty file. For a nonempty deleted ita file,
do_oneway_diff() calls show_modified(), which detects a diff between the
cache entry in the index and the cache entry in HEAD and therefore deems
the file "modified". However, for an empty deleted ita file,
do_oneway_diff() previously detected no such diff between the two cache
entries and therefore deemed the file "not modified". After this fix,
for any deleted ita file, do_oneway_diff() calls diff_index_show_file()
and deems the file "deleted".

`git diff-index --cached HEAD` prints a row of output for both a
"modified" and a "deleted" file, although the output differs slightly.
`git reset HEAD` treats a "modified" and a "deleted" file similarly,
resurrecting the file in the index from HEAD.

This change should not affect newly added ita files. For those, the
"tree" cache entry is NULL, so the changed code is not executed.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Varun Naik <vcnaik94@gmail.com>
---
I tried to limit the "blast radius" of affected commands as much as
possible. To find commands that were affected, I ran the following:
    git diff
    git diff HEAD
    git diff --cached HEAD
    git diff-index HEAD
    git diff-index --cached HEAD
    git diff-files

I also ran each command with the option "--ita-visible-in-index" and the
option "--ita-invisible-in-index". Of these 18 commands, the following
three showed a diff for nonempty deleted ita files, but no diff for
empty deleted ita files. All three commands now work correctly, but I
chose the first one for the test case because the option
"--ita-visible-in-index" is still marked as experimental.
    git diff-index --cached HEAD
    git diff-index --cached --ita-visible-in-index HEAD
    git diff --cached --ita-visible-in-index HEAD

The `git add` at the end of the "diff-index" test case is necessary
because `git reset --hard HEAD` at the beginning of the next test case
_also_ breaks for empty deleted ita files. That command goes into
unpack-trees.c:oneway_merge() rather than diff-lib.c:do_oneway_diff().
I plan to create a separate patch to fix that, after I figure out which
commands are part of its blast radius.

 diff-lib.c            |  5 ++++-
 t/t2203-add-intent.sh | 13 +++++++++++++
 t/t7102-reset.sh      | 11 +++++++++++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/diff-lib.c b/diff-lib.c
index 61812f48c2..29dba467d5 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -433,8 +433,11 @@ static void do_oneway_diff(struct unpack_trees_options *o,
 
 	/*
 	 * Something removed from the tree?
+	 * Consider a file deleted from the index and added as ita to be "deleted",
+	 * even though it should arguably be "modified", because we want empty
+	 * deleted ita files to appear in the diff.
 	 */
-	if (!idx) {
+	if (!idx || (cached && ce_intent_to_add(idx))) {
 		diff_index_show_file(revs, "-", tree, &tree->oid, 1,
 				     tree->ce_mode, 0);
 		return;
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 68e54d5c44..4e4a972921 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -261,6 +261,19 @@ test_expect_success '"diff HEAD" includes ita as new files' '
 	test_cmp expected actual
 '
 
+test_expect_success '"diff-index --cached HEAD" detects diff for deleted intent-to-add file' '
+	git reset --hard &&
+	echo "nonempty" >nonempty &&
+	>empty &&
+	git add nonempty empty &&
+	git commit -m "create files to be deleted" &&
+	git rm --cached nonempty empty &&
+	git add -N nonempty empty &&
+	test_expect_code 1 git diff-index --cached --exit-code HEAD nonempty &&
+	test_expect_code 1 git diff-index --cached --exit-code HEAD empty &&
+	git add nonempty empty
+'
+
 test_expect_success 'apply --intent-to-add' '
 	git reset --hard &&
 	echo new >new-ita &&
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 97be0d968d..7b79502f7d 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -566,4 +566,15 @@ test_expect_success 'reset --mixed sets up work tree' '
 	test_must_be_empty actual
 '
 
+test_expect_success 'reset --mixed removes deleted intent-to-add file from index' '
+	echo "nonempty" >nonempty &&
+	>empty &&
+	git add nonempty empty &&
+	git commit -m "create files to be deleted" &&
+	git rm --cached nonempty empty &&
+	git add -N nonempty empty &&
+	git reset HEAD nonempty empty &&
+	git diff --cached --exit-code nonempty empty
+'
+
 test_done
-- 
2.22.0


  parent reply	other threads:[~2019-08-01 16:16 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-12 15:02 [PATCH] reset: unstage empty deleted ita files Varun Naik
2019-07-26  4:48 ` [PATCH v2] " Varun Naik
2019-07-26 18:19   ` Junio C Hamano
2019-07-29  6:52     ` Varun Naik
2019-07-29 16:07       ` Junio C Hamano
2019-08-01 16:15 ` Varun Naik [this message]
2019-08-15 16:26   ` [PATCH v3] diff-lib.c: handle " Varun Naik
2019-08-15 19:06   ` Junio C Hamano
2019-08-19 15:42     ` Varun Naik
2019-08-19 20:15       ` Junio C Hamano
2020-02-14 17:12         ` Varun Naik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190801161558.12838-1-vcnaik94@gmail.com \
    --to=vcnaik94@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).