Re: [PATCH v2 0/4] Deprecate "not allow as-is commit with i-t-a entries"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Ahh, thanks.
>
> But when I said "let's admit that this is just fixing an UI mistake, no
> configuration, no options", I really meant it.  Without the backward
> compatiblity "For now please do not fix this bug for me and keep being
> buggy until I get used to the non-buggy behaviour" fuss, which we never do
> to any bugfix.

Given that a patch to do so (with or without your 1/4 which is an
independently good change) on top of an ancient v1.7.6 codebase looks like
this, I am inclined to think this is the way to go.

I still need to review the existing documentation to see if anything needs
to be fixed up, though.

-- >8 --
From: Junio C Hamano <gitster@xxxxxxxxx>
Subject: [PATCH] commit: ignore intent-to-add entries instead of refusing

Originally, "git add -N" was introduced to help users from forgetting to
add new files to the index before they ran "git commit -a".  As an attempt
to help them further so that they do not forget to say "-a", "git commit"
to commit the index as-is was taught to error out, reminding the user that
they may have forgotten to add the final contents of the paths before
running the command.

This turned out to be a false "safety" that is useless.  If the user made
changes to already tracked paths and paths added with "git add -N", and
then ran "git add" to register the final contents of the paths added with
"git add -N", "git commit" will happily create a commit out of the index,
without including the local changes made to the already tracked paths. It
was not a useful "safety" measure to prevent "forgetful" mistakes from
happening.

It turns out that this behaviour is not just a useless false "safety", but
actively hurts use cases of "git add -N" that were discovered later and
have become popular, namely, to tell Git to be aware of these paths added
by "git add -N", so that commands like "git status" and "git diff" would
include them in their output, even though the user is not interested in
including them in the next commit they are going to make.

Fix this ancient UI mistake, and instead make a commit from the index
ignoring the paths added by "git add -N" without adding real contents.

Based on the work by Nguyễn Thái Ngọc Duy, and helped by injection of
sanity from Jonathan Nieder and others on the Git mailing list.

Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
 * The last hunk of t2203 is because the test assumed the previous one
   failed to make a commit and "rezrov" in HEAD does not have "xyzzy".
   Now that the previous test creates a commit and records "xyzzy" in
   "rezrov", this test will fail with "Nothing to commit" without it.

 cache-tree.c          |    6 +++---
 t/t2203-add-intent.sh |    8 +++++---
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/cache-tree.c b/cache-tree.c
index f755590..ce0d0e3 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -158,7 +158,7 @@ static int verify_cache(struct cache_entry **cache,
 	funny = 0;
 	for (i = 0; i < entries; i++) {
 		struct cache_entry *ce = cache[i];
-		if (ce_stage(ce) || (ce->ce_flags & CE_INTENT_TO_ADD)) {
+		if (ce_stage(ce)) {
 			if (10 < ++funny) {
 				fprintf(stderr, "...\n");
 				break;
@@ -336,8 +336,8 @@ static int update_one(struct cache_tree *it,
 				mode, sha1_to_hex(sha1), entlen+baselen, path);
 		}
 
-		if (ce->ce_flags & CE_REMOVE)
-			continue; /* entry being removed */
+		if (ce->ce_flags & (CE_REMOVE | CE_INTENT_TO_ADD))
+			continue; /* entry being removed or placeholder */
 
 		strbuf_grow(&buffer, entlen + 100);
 		strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0');
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 2543529..ec35409 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -32,7 +32,7 @@ test_expect_success 'intent to add does not clobber existing paths' '
 	! grep "$empty" actual
 '
 
-test_expect_success 'cannot commit with i-t-a entry' '
+test_expect_success 'i-t-a entry is simply ignored' '
 	test_tick &&
 	git commit -a -m initial &&
 	git reset --hard &&
@@ -41,12 +41,14 @@ test_expect_success 'cannot commit with i-t-a entry' '
 	echo frotz >nitfol &&
 	git add rezrov &&
 	git add -N nitfol &&
-	test_must_fail git commit -m initial
+	git commit -m second &&
+	test $(git ls-tree HEAD -- nitfol | wc -l) = 0 &&
+	test $(git diff --name-only HEAD -- nitfol | wc -l) = 1
 '
 
 test_expect_success 'can commit with an unrelated i-t-a entry in index' '
 	git reset --hard &&
-	echo xyzzy >rezrov &&
+	echo bozbar >rezrov &&
 	echo frotz >nitfol &&
 	git add rezrov &&
 	git add -N nitfol &&
-- 
1.7.9.231.g87173

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]