All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Altmanninger <aclopte@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: rhodges@cisco.com, git@vger.kernel.org, rphodges@gmail.com,
	Johannes Altmanninger <aclopte@gmail.com>
Subject: [PATCH v3] apply: --intent-to-add should imply --index
Date: Sat,  6 Nov 2021 12:42:02 +0100	[thread overview]
Message-ID: <20211106114202.3486969-1-aclopte@gmail.com> (raw)
In-Reply-To: <20211106112439.iw7asj4bq6uwcb3l@gmail.com>

Otherwise we do not read the current index, and more importantly, we
do not check with the current index, losing all the safety.

And the worst part of the story is that we still write the result
out to the index, which loses all the files that are not mentioned
in the incoming patch.

Make --intent-to-add imply --index. This means that apply
--intent-to-add will error without a repo, and refuse to modify
untracked files (except for added files). Add a test for the latter, and
another one to make sure that combinations like "--cached -N" keep working.
as documented (-N is ignored, otherwise it would do weird things to the index).

Use --intent-to-add instead of -N because we don't document -N in
git-apply.txt, which might be because it's much more obscure than "add -N".

Reported-by: Ryan Hodges <rhodges@cisco.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johannes Altmanninger <aclopte@gmail.com>
---

Not sure about the log message, it feels a bit stitched together.

Most importantly this adds a test to show the difference between v2 (where
ita did not imply check_index).

I wrapped the doc changes to 80 columns, but not the entire paragraph,
since we are inconsistent about that.

 Documentation/git-apply.txt |  7 +++----
 apply.c                     |  9 +++++++--
 t/t2203-add-intent.sh       |  2 +-
 t/t4140-apply-ita.sh        | 29 +++++++++++++++++++++++++++++
 4 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index aa1ae56a25..18ddb4cf8a 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -77,10 +77,9 @@ OPTIONS
 --intent-to-add::
 	When applying the patch only to the working tree, mark new
 	files to be added to the index later (see `--intent-to-add`
-	option in linkgit:git-add[1]). This option is ignored unless
-	running in a Git repository and `--index` is not specified.
-	Note that `--index` could be implied by other options such
-	as `--cached` or `--3way`.
+	option in linkgit:git-add[1]). This option has is ignored if `--index`
+	is specified.  Note that `--index` could be implied by other options
+	such as `--cached` or `--3way`.
 
 -3::
 --3way::
diff --git a/apply.c b/apply.c
index 43a0aebf4e..b0239b7482 100644
--- a/apply.c
+++ b/apply.c
@@ -153,8 +153,13 @@ int check_apply_state(struct apply_state *state, int force_apply)
 			return error(_("--cached outside a repository"));
 		state->check_index = 1;
 	}
-	if (state->ita_only && (state->check_index || is_not_gitdir))
+	if (state->ita_only && state->check_index)
 		state->ita_only = 0;
+	if (state->ita_only) {
+		if (is_not_gitdir)
+			return error(_("--intent-to-add outside a repository"));
+		state->check_index = 1;
+	}
 	if (state->check_index)
 		state->unsafe_paths = 0;
 
@@ -4760,7 +4765,7 @@ static int apply_patch(struct apply_state *state,
 	if (state->whitespace_error && (state->ws_error_action == die_on_ws_error))
 		state->apply = 0;
 
-	state->update_index = (state->check_index || state->ita_only) && state->apply;
+	state->update_index = state->check_index && state->apply;
 	if (state->update_index && !is_lock_file_locked(&state->lock_file)) {
 		if (state->index_file)
 			hold_lock_file_for_update(&state->lock_file,
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index cf0175ad6e..035ce3a2b9 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -307,7 +307,7 @@ test_expect_success 'apply --intent-to-add' '
 	grep "new file" expected &&
 	git reset --hard &&
 	git apply --intent-to-add expected &&
-	git diff >actual &&
+	(git diff && git diff --cached) >actual &&
 	test_cmp expected actual
 '
 
diff --git a/t/t4140-apply-ita.sh b/t/t4140-apply-ita.sh
index c614eaf04c..4db1ae4e7e 100755
--- a/t/t4140-apply-ita.sh
+++ b/t/t4140-apply-ita.sh
@@ -53,4 +53,33 @@ test_expect_success 'apply deletion patch to ita path (--index)' '
 	git ls-files --stage --error-unmatch test-file
 '
 
+test_expect_success 'apply --intent-to-add is not allowed to modify untracked file' '
+	echo version1 >file &&
+	! git apply --intent-to-add <<-EOF
+	diff --git a/file b/file
+	index 1234567..89abcde 100644
+	--- b/file
+	+++ b/file
+	@@ -1 +1 @@
+	-version1
+	+version2
+	EOF
+'
+
+test_expect_success 'apply --index --intent-to-add ignores --intent-to-add, so it does not set i-t-a bit of touched file' '
+	echo >file &&
+	git add file &&
+	git apply --index --intent-to-add <<-EOF &&
+	diff --git a/file b/file
+	deleted file mode 100644
+	index 1234567..89abcde 100644
+	--- a/file
+	+++ /dev/null
+	@@ -1 +0,0 @@
+	-
+	EOF
+	git ls-files file >actual &&
+	test_must_be_empty actual
+'
+
 test_done
-- 
2.33.1


  reply	other threads:[~2021-11-06 11:46 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-26 15:11 git apply --indent-to-add deletes other files from the index Ryan Hodges (rhodges)
2021-10-30 20:39 ` git apply --intent-to-add " Johannes Altmanninger
2021-10-30 21:42   ` Ryan Hodges
2021-10-31  6:43     ` Johannes Altmanninger
2021-10-30 20:41 ` [PATCH] apply: make --intent-to-add not stomp index Johannes Altmanninger
2021-10-30 20:51   ` [PATCH v2] " Johannes Altmanninger
2021-11-01  6:40     ` Junio C Hamano
2021-11-01  7:07       ` Re* " Junio C Hamano
2021-11-06 11:24         ` Johannes Altmanninger
2021-11-06 11:42           ` Johannes Altmanninger [this message]
2021-11-06 11:47             ` [PATCH v3] apply: --intent-to-add should imply --index Johannes Altmanninger
2021-11-06 11:24       ` [PATCH v2] apply: make --intent-to-add not stomp index Johannes Altmanninger

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=20211106114202.3486969-1-aclopte@gmail.com \
    --to=aclopte@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=rhodges@cisco.com \
    --cc=rphodges@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.