threads / patch / 29561

patch, 4 partsDeprecate "not allow as-is commit with i-t-a entries"

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

## tl;dr

20 messages between Feb 6, 2012 and Feb 7, 2012. Diffs are folded; open one to read it.

replies: 19people: 3as markdown or json

Nguyễn Thái Ngọc Duy· Feb 6, 2012, 10:57 UTC · lore

By the end of this series "git commit -m foo" will proceed regardless intent-to-add (aka "git add -N") entries. commit.ignoreIntentToAdd is used as transitiono config key.

The plan is in 2/4. I set 1.8.0 as the first deprecation date, but of course it's just a suggestion. The second deprecation date might be 1.8.5, long enough for users to adapt to the new behavior.

..or we switch back half way because we find current behavior does make more sense.

Nguyễn Thái Ngọc Duy (4):
  cache-tree: update API to take abitrary flags
  commit: introduce a config key to allow as-is commit with i-t-a
    entries
  commit: turn commit.ignoreIntentToAdd to true by default
  commit: remove commit.ignoreIntentToAdd, assume it's always true
 Documentation/git-add.txt |   12 ++++++++++--
 builtin/commit.c          |    9 ++++++---
 cache-tree.c              |   35 +++++++++++++++++------------------
 cache-tree.h              |    5 ++++-
 merge-recursive.c         |    2 +-
 t/t2203-add-intent.sh     |   21 ++++++++++++++++++++-
 test-dump-cache-tree.c    |    2 +-
 7 files changed, 59 insertions(+), 27 deletions(-)
-- 
1.7.8.36.g69ee2
Nguyễn Thái Ngọc Duy· Feb 6, 2012, 10:57 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH 1/4] cache-tree: update API to take abitrary flags

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/commit.c       |    4 ++--
 cache-tree.c           |   27 ++++++++++++---------------
 cache-tree.h           |    4 +++-
 merge-recursive.c      |    2 +-
 test-dump-cache-tree.c |    2 +-
 5 files changed, 19 insertions(+), 20 deletions(-)
Show changes to 5 files +19 −20

builtin/commit.c, cache-tree.c, cache-tree.h, merge-recursive.c, test-dump-cache-tree.c

diff --git a/builtin/commit.c b/builtin/commit.c
index eba1377..bf42bb3 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -400,7 +400,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 		fd = hold_locked_index(&index_lock, 1);
 		add_files_to_cache(also ? prefix : NULL, pathspec, 0);
 		refresh_cache_or_die(refresh_flags);
-		update_main_cache_tree(1);
+		update_main_cache_tree(WRITE_TREE_SILENT);
 		if (write_cache(fd, active_cache, active_nr) ||
 		    close_lock_file(&index_lock))
 			die(_("unable to write new_index file"));
@@ -421,7 +421,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 		fd = hold_locked_index(&index_lock, 1);
 		refresh_cache_or_die(refresh_flags);
 		if (active_cache_changed) {
-			update_main_cache_tree(1);
+			update_main_cache_tree(WRITE_TREE_SILENT);
 			if (write_cache(fd, active_cache, active_nr) ||
 			    commit_locked_index(&index_lock))
 				die(_("unable to write new_index file"));
diff --git a/cache-tree.c b/cache-tree.c
index 8de3959..16355d6 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -150,9 +150,10 @@ void cache_tree_invalidate_path(struct cache_tree *it, const char *path)
 }
 
 static int verify_cache(struct cache_entry **cache,
-			int entries, int silent)
+			int entries, int flags)
 {
 	int i, funny;
+	int silent = flags & WRITE_TREE_SILENT;
 
 	/* Verify that the tree is merged */
 	funny = 0;
@@ -241,10 +242,11 @@ static int update_one(struct cache_tree *it,
 		      int entries,
 		      const char *base,
 		      int baselen,
-		      int missing_ok,
-		      int dryrun)
+		      int flags)
 {
 	struct strbuf buffer;
+	int missing_ok = flags & WRITE_TREE_MISSING_OK;
+	int dryrun = flags & WRITE_TREE_DRY_RUN;
 	int i;
 
 	if (0 <= it->entry_count && has_sha1_file(it->sha1))
@@ -288,8 +290,7 @@ static int update_one(struct cache_tree *it,
 				    cache + i, entries - i,
 				    path,
 				    baselen + sublen + 1,
-				    missing_ok,
-				    dryrun);
+				    flags);
 		if (subcnt < 0)
 			return subcnt;
 		i += subcnt - 1;
@@ -371,15 +372,13 @@ static int update_one(struct cache_tree *it,
 int cache_tree_update(struct cache_tree *it,
 		      struct cache_entry **cache,
 		      int entries,
-		      int missing_ok,
-		      int dryrun,
-		      int silent)
+		      int flags)
 {
 	int i;
-	i = verify_cache(cache, entries, silent);
+	i = verify_cache(cache, entries, flags);
 	if (i)
 		return i;
-	i = update_one(it, cache, entries, "", 0, missing_ok, dryrun);
+	i = update_one(it, cache, entries, "", 0, flags);
 	if (i < 0)
 		return i;
 	return 0;
@@ -572,11 +571,9 @@ int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix)
 
 	was_valid = cache_tree_fully_valid(active_cache_tree);
 	if (!was_valid) {
-		int missing_ok = flags & WRITE_TREE_MISSING_OK;
-
 		if (cache_tree_update(active_cache_tree,
 				      active_cache, active_nr,
-				      missing_ok, 0, 0) < 0)
+				      flags) < 0)
 			return WRITE_TREE_UNMERGED_INDEX;
 		if (0 <= newfd) {
 			if (!write_cache(newfd, active_cache, active_nr) &&
@@ -672,10 +669,10 @@ int cache_tree_matches_traversal(struct cache_tree *root,
 	return 0;
 }
 
-int update_main_cache_tree (int silent)
+int update_main_cache_tree (int flags)
 {
 	if (!the_index.cache_tree)
 		the_index.cache_tree = cache_tree();
 	return cache_tree_update(the_index.cache_tree,
-				 the_index.cache, the_index.cache_nr, 0, 0, silent);
+				 the_index.cache, the_index.cache_nr, flags);
 }
diff --git a/cache-tree.h b/cache-tree.h
index 0ec0b2a..d8cb2e9 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -29,13 +29,15 @@ void cache_tree_write(struct strbuf *, struct cache_tree *root);
 struct cache_tree *cache_tree_read(const char *buffer, unsigned long size);
 
 int cache_tree_fully_valid(struct cache_tree *);
-int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int, int, int);
+int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int);
 
 int update_main_cache_tree(int);
 
 /* bitmasks to write_cache_as_tree flags */
 #define WRITE_TREE_MISSING_OK 1
 #define WRITE_TREE_IGNORE_CACHE_TREE 2
+#define WRITE_TREE_DRY_RUN 4
+#define WRITE_TREE_SILENT 8
 
 /* error return codes */
 #define WRITE_TREE_UNREADABLE_INDEX (-1)
diff --git a/merge-recursive.c b/merge-recursive.c
index d83cd6c..6479a60 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -264,7 +264,7 @@ struct tree *write_tree_from_memory(struct merge_options *o)
 
 	if (!cache_tree_fully_valid(active_cache_tree) &&
 	    cache_tree_update(active_cache_tree,
-			      active_cache, active_nr, 0, 0, 0) < 0)
+			      active_cache, active_nr, 0) < 0)
 		die("error building trees");
 
 	result = lookup_tree(active_cache_tree->sha1);
diff --git a/test-dump-cache-tree.c b/test-dump-cache-tree.c
index e6c2923..a6ffdf3 100644
--- a/test-dump-cache-tree.c
+++ b/test-dump-cache-tree.c
@@ -59,6 +59,6 @@ int main(int ac, char **av)
 	struct cache_tree *another = cache_tree();
 	if (read_cache() < 0)
 		die("unable to read index file");
-	cache_tree_update(another, active_cache, active_nr, 0, 1, 0);
+	cache_tree_update(another, active_cache, active_nr, WRITE_TREE_DRY_RUN);
 	return dump_cache_tree(active_cache_tree, another, "");
 }
-- 
1.7.8.36.g69ee2
Junio C Hamano· Feb 6, 2012, 19:51 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 1/4] cache-tree: update API to take abitrary flags

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
Show 7 quoted lines
>  int cache_tree_update(struct cache_tree *it,
>  		      struct cache_entry **cache,
>  		      int entries,
> -		      int missing_ok,
> -		      int dryrun,
> -		      int silent)
> +		      int flags)
Very nice ;-)
Nguyễn Thái Ngọc Duy· Feb 6, 2012, 10:57 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH 2/4] commit: introduce a config key to allow as-is commit with i-t-a entries

This is the start of deprecating current commit behavior (do not allow commit as-is when there are i-t-a entries in index). Users will be annoyed by a warning saying the behavior will change in 1.8.0 and asked to set commit.ignoreIntentToAdd properly.

The plan after that is:
 - Keep it this way until 1.8.0
 - In 1.8.0, we consider the lack of commit.ignoreIntentToAdd means
   "true". A deprecation date is set. The warning message is updated
   stating that from that day, this config key will be ignored. The
   behavior will be as if this config key is always "true".
 - In the chosen release, remove support for this config key.
   WRITE_TREE_IGNORE_INTENT_TO_ADD is set unconditionally.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/config.txt  |   10 ++++++++++
 Documentation/git-add.txt |   12 ++++++++++--
 builtin/commit.c          |   28 +++++++++++++++++++++++++---
 cache-tree.c              |    8 +++++---
 cache-tree.h              |    1 +
 t/t2203-add-intent.sh     |   21 ++++++++++++++++++++-
 6 files changed, 71 insertions(+), 9 deletions(-)
Show changes to 6 files +71 −9

Documentation/config.txt, Documentation/git-add.txt, builtin/commit.c, cache-tree.c, cache-tree.h, t/t2203-add-intent.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index abeb82b..6ec81a8 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -831,6 +831,16 @@ commit.template::
 	"{tilde}/" is expanded to the value of `$HOME` and "{tilde}user/" to the
 	specified user's home directory.
 
+commit.ignoreIntentToAdd::
+	Allow to commit the index as-is even if there are
+	intent-to-add entries (see option `-N` in linkgit:git-add[1])
+	in index. Set to `false` to disallow commit in this acase, or `true`
+	to allow it.
++
+By default, `git commit` refuses to commit as-is when you have intent-to-add
+entries. This will change in 1.8.0, where `git commit` allows it. If you
+prefer current behavior, please set it to `false`.
+
 credential.helper::
 	Specify an external helper to be called when a username or
 	password credential is needed; the helper may consult external
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 9c1d395..ec548ea 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -123,8 +123,16 @@ subdirectories.
 	Record only the fact that the path will be added later. An entry
 	for the path is placed in the index with no content. This is
 	useful for, among other things, showing the unstaged content of
-	such files with `git diff` and committing them with `git commit
-	-a`.
+	such files with `git diff`.
++
+Paths added with this option have intent-to-add flag in index. The
+flag is removed once real content is added or updated. By default you
+cannot commit the index as-is from until this flag is removed from all
+entries (i.e. all entries have real content). See commit.ignoreIntentToAdd
+regardless the flag.
++
+Committing with `git commit -a` or with selected paths works
+regardless the config key and the flag.
 
 --refresh::
 	Don't add the file(s), but only refresh their stat()
diff --git a/builtin/commit.c b/builtin/commit.c
index bf42bb3..af3250c 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -86,6 +86,7 @@ static int all, also, interactive, patch_interactive, only, amend, signoff;
 static int edit_flag = -1; /* unspecified */
 static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
 static int no_post_rewrite, allow_empty_message;
+static int cache_tree_flags;
 static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
 static char *sign_commit;
 
@@ -117,6 +118,8 @@ static enum {
 } status_format = STATUS_FORMAT_LONG;
 static int status_show_branch;
 
+static int set_commit_ignoreintenttoadd;
+
 static int opt_parse_m(const struct option *opt, const char *arg, int unset)
 {
 	struct strbuf *buf = opt->value;
@@ -400,7 +403,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 		fd = hold_locked_index(&index_lock, 1);
 		add_files_to_cache(also ? prefix : NULL, pathspec, 0);
 		refresh_cache_or_die(refresh_flags);
-		update_main_cache_tree(WRITE_TREE_SILENT);
+		update_main_cache_tree(cache_tree_flags | WRITE_TREE_SILENT);
 		if (write_cache(fd, active_cache, active_nr) ||
 		    close_lock_file(&index_lock))
 			die(_("unable to write new_index file"));
@@ -420,8 +423,20 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 	if (!pathspec || !*pathspec) {
 		fd = hold_locked_index(&index_lock, 1);
 		refresh_cache_or_die(refresh_flags);
+		if (!set_commit_ignoreintenttoadd) {
+			int i;
+			for (i = 0; i < active_nr; i++)
+				if (active_cache[i]->ce_flags & CE_INTENT_TO_ADD)
+					break;
+			if (i < active_nr)
+				warning(_("You are committing as-is with intent-to-add entries as the result of\n"
+					  "\"git add -N\". Git currently forbids this case. This will change in\n"
+					  "1.8.0 where intent-to-add entries are simply ignored when committing\n"
+					  "as-is. Please look up document and set commit.ignoreIntentToAdd\n"
+					  "properly to stop this warning."));
+		}
 		if (active_cache_changed) {
-			update_main_cache_tree(WRITE_TREE_SILENT);
+			update_main_cache_tree(cache_tree_flags | WRITE_TREE_SILENT);
 			if (write_cache(fd, active_cache, active_nr) ||
 			    commit_locked_index(&index_lock))
 				die(_("unable to write new_index file"));
@@ -870,7 +885,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	 */
 	discard_cache();
 	read_cache_from(index_file);
-	if (update_main_cache_tree(0)) {
+	if (update_main_cache_tree(cache_tree_flags)) {
 		error(_("Error building trees"));
 		return 0;
 	}
@@ -1338,6 +1353,13 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 		include_status = git_config_bool(k, v);
 		return 0;
 	}
+	if (!strcmp(k, "commit.ignoreintenttoadd")) {
+		set_commit_ignoreintenttoadd = 1;
+		if (git_config_bool(k, v))
+			cache_tree_flags |= WRITE_TREE_IGNORE_INTENT_TO_ADD;
+		else
+			cache_tree_flags &= ~WRITE_TREE_IGNORE_INTENT_TO_ADD;
+	}
 
 	status = git_gpg_config(k, v, NULL);
 	if (status)
diff --git a/cache-tree.c b/cache-tree.c
index 16355d6..d0be159 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -159,7 +159,9 @@ 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) ||
+		    ((flags & WRITE_TREE_IGNORE_INTENT_TO_ADD) == 0 &&
+		     (ce->ce_flags & CE_INTENT_TO_ADD))) {
 			if (silent)
 				return -1;
 			if (10 < ++funny) {
@@ -339,8 +341,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/cache-tree.h b/cache-tree.h
index d8cb2e9..af3b917 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -38,6 +38,7 @@ int update_main_cache_tree(int);
 #define WRITE_TREE_IGNORE_CACHE_TREE 2
 #define WRITE_TREE_DRY_RUN 4
 #define WRITE_TREE_SILENT 8
+#define WRITE_TREE_IGNORE_INTENT_TO_ADD 16
 
 /* error return codes */
 #define WRITE_TREE_UNREADABLE_INDEX (-1)
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 58a3299..88a508e 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -41,7 +41,26 @@ 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
+	test_must_fail git commit -minitial
+'
+
+test_expect_success 'can commit tree with i-t-a entry' '
+	git reset --hard &&
+	echo xyzzy >rezrov &&
+	echo frotz >nitfol &&
+	git add rezrov &&
+	git add -N nitfol &&
+	git config commit.ignoreIntentToAdd true &&
+	git commit -m initial &&
+	git ls-tree -r HEAD >actual &&
+	cat >expected <<EOF &&
+100644 blob ce013625030ba8dba906f756967f9e9ca394464a	elif
+100644 blob ce013625030ba8dba906f756967f9e9ca394464a	file
+100644 blob cf7711b63209d0dbc2d030f7fe3513745a9880e4	rezrov
+EOF
+	test_cmp expected actual &&
+	git config commit.ignoreIntentToAdd false &&
+	git reset HEAD^
 '
 
 test_expect_success 'can commit with an unrelated i-t-a entry in index' '
-- 
1.7.8.36.g69ee2
Junio C Hamano· Feb 6, 2012, 19:50 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 2/4] commit: introduce a config key to allow as-is commit with i-t-a entries

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
Show 9 quoted lines
> +commit.ignoreIntentToAdd::
> +	Allow to commit the index as-is even if there are
> +	intent-to-add entries (see option `-N` in linkgit:git-add[1])
> +	in index. Set to `false` to disallow commit in this acase, or `true`
> +	to allow it.
> ++
> +By default, `git commit` refuses to commit as-is when you have intent-to-add
> +entries. This will change in 1.8.0, where `git commit` allows it. If you
> +prefer current behavior, please set it to `false`.
s/acase/case/;

More importantly, if we allow it, what would be included and what would be omitted from the final commit? What happens if we allow it is much more important information than "we allow it even if there are".

	When set to `true`, prevent `git commit` from creating a commit
	from an index that has entries that were added with `git add -N`
	but have not been updated with real contents, as the user may have
	forgotten to tell the final contents for these entries.  Setting
	this to `false` makes `git commit` pretend as if these entries do
	not exist in the index.
	The default for this variable is `false`, but it will change to
	`true` in later releases of git (perhaps 1.8.0). To ease the
	transition, you may want to set it to `true` now and get used to
	the new behaviour early, or you may want to set it to `false` to
	keep the old behaviour a bit longer. We however expect to support
	setting this to `false` (to keep the current behaviour) only for a
	limited time after the default is changed to `true`.

Now, I removed the "intent to add" jargon from the above paragraph, the description below can lose it as well.

Show 17 quoted lines
> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
> index 9c1d395..ec548ea 100644
> --- a/Documentation/git-add.txt
> +++ b/Documentation/git-add.txt
> @@ -123,8 +123,16 @@ subdirectories.
>  	Record only the fact that the path will be added later. An entry
>  	for the path is placed in the index with no content. This is
>  	useful for, among other things, showing the unstaged content of
> -	such files with `git diff` and committing them with `git commit
> -	-a`.
> +	such files with `git diff`.
> ++
> +Paths added with this option have intent-to-add flag in index. The
> +flag is removed once real content is added or updated. By default you
> +cannot commit the index as-is from until this flag is removed from all
> +entries (i.e. all entries have real content). See commit.ignoreIntentToAdd
> +regardless the flag.
I cannot understand "regardlessthe flag" in the last sentence.
> ++
> +Committing with `git commit -a` or with selected paths works
> +regardless the config key and the flag.
Just an aside, not suggesting to change anything to the final text.

This is interesting in that the reason why "selected paths" works is a bit subtle. You may have entries added with `-N` that are covered by the pathspec, and those that are not covered by the pathspec. For the former (i.e. those the user said "I'll decide the final contents later" but are covered by pathspec), the user is telling the final contents to be in the commit, so we know the final contents for them. For the latter (i.e. those outside the pathspec), they are excluded and made to match the version in the HEAD commit, so we know the final contents for them, too.

> +			if (i < active_nr)
> +				warning(_("You are committing as-is with intent-to-add entries as the result of\n"
> +					  "\"git add -N\". Git currently forbids this case.
Can we phrase this a bit better?

It is not like "forbids", but is "giving up because you didn't tell me what content to include in the commit, even though you said you will tell me later".

Nguyen Thai Ngoc Duy· Feb 7, 2012, 00:43 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/4] commit: introduce a config key to allow as-is commit with i-t-a entries

2012/2/7 Junio C Hamano <gitster@pobox.com>:
Show 9 quoted lines
>> +                     if (i < active_nr)
>> +                             warning(_("You are committing as-is with intent-to-add entries as the result of\n"
>> +                                       "\"git add -N\". Git currently forbids this case.
>
> Can we phrase this a bit better?
>
> It is not like "forbids", but is "giving up because you didn't tell me
> what content to include in the commit, even though you said you will tell
> me later".

"rejects"? I would rather say "see `git add -N` man page for more explanation" than putting it here. The warning is quite long as it is right now.

-- 
Duy
Jonathan Nieder· Feb 7, 2012, 00:59 UTC · re: Nguyen Thai Ngoc Duy · lore

Re: [PATCH 2/4] commit: introduce a config key to allow as-is commit with i-t-a entries

Nguyen Thai Ngoc Duy wrote:
> 2012/2/7 Junio C Hamano <gitster@pobox.com>:
Show 13 quoted lines
>>> +                     if (i < active_nr)
>>> +                             warning(_("You are committing as-is with intent-to-add entries as the result of\n"
>>> +                                       "\"git add -N\". Git currently forbids this case.
>>
>> Can we phrase this a bit better?
>>
>> It is not like "forbids", but is "giving up because you didn't tell me
>> what content to include in the commit, even though you said you will tell
>> me later".
>
> "rejects"? I would rather say "see `git add -N` man page for more
> explanation" than putting it here. The warning is quite long as it is
> right now.
If I ruled the world, it would say something like this:
	error: you intended to add "foo.c" but did not add it; not committing
	hint: to commit all changes to tracked files, use "commit -a"
	hint: to commit without adding "foo.c", use "commit --ignore-intent-to-add", which may become the default in future versions of git
But without the long line. ;-)
Junio C Hamano· Feb 6, 2012, 21:10 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 2/4] commit: introduce a config key to allow as-is commit with i-t-a entries

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
Show 20 quoted lines
> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
> index 9c1d395..ec548ea 100644
> --- a/Documentation/git-add.txt
> +++ b/Documentation/git-add.txt
> @@ -123,8 +123,16 @@ subdirectories.
>  	Record only the fact that the path will be added later. An entry
>  	for the path is placed in the index with no content. This is
>  	useful for, among other things, showing the unstaged content of
> -	such files with `git diff` and committing them with `git commit
> -	-a`.
> +	such files with `git diff`.
> ++
> +Paths added with this option have intent-to-add flag in index. The
> +flag is removed once real content is added or updated. By default you
> +cannot commit the index as-is from until this flag is removed from all
> +entries (i.e. all entries have real content). See commit.ignoreIntentToAdd
> +regardless the flag.
> ++
> +Committing with `git commit -a` or with selected paths works
> +regardless the config key and the flag.

It still is not clear to me how best to sell this change to the end-user community.

IIRC, the original motivation of intent-to-add "add -N" was in response to users who curse Git because they often forget to add new files before committing, and they wanted to say "Here I have a new file, it is not complete yet, but I do not want it left out of the commit. While my memory is fresh, let me tell Git to keep an eye on it, so that it can remind me if I forget to add the final contents." For them, the current "did you forget to add them? If so tell me the final contents for at least the paths you will be changing with this commit" error was a perfect safety solution.

It turned out that the benefits described we see above in the context, "This is useful, among other things, ...", were of more value, and for these use cases, i-t-a entries ceased to mean "I may forget, so I am telling you now, please remind me when I say commit." And "did you forget?" error is hinderance for them.

But does that mean nobody will ever need "please remind me lest I forget"? Just the original requestor of the "add -N" feature may still be using git, but more importantly, isn't it the case that those who have been using it merely for the other side effect (e.g. 'git diff') sometimes want the "please remind me" safety?

I suspect that some among 1 million Git users would want the "please remind me", so a solution with configuration variable without command line override is not ideal (command line without any configurability is fine as long as we have a good default).

I am beginning to think "safety by default, which can be turned off by learned users, but still can be turned on on demand" may be a lot easier to sell this. That is:

 - commit.ignoreIntentToAdd defaults to `false`; the default will never
   change.  The users can set it to `true`.
 - "commit --ignore-intent-to-add" can be used without setting the
   configuration or to defeat an explicit `false`, for a one-shot request.
 - "commit --honor-intent-to-add" can be used to defeat an explicit
   `true`, for a one-shot request.

The third one is a bit funny, as it is a way to bring back safety when the user earlier decided that he does not need that kind of safety (i.e. "I only say 'add -N' for `diff` and stuff, I will never forget to add real contents before committing"), so it will almost never be used, because these users who set 'ignoreIntentToAdd = true' do _not_ expect Git to help them in remembering to add the real contents. And having to add a funny option just for the sake of completeness is often an indication that there is something fundamentally wrong in the system that the option tries to express an interface into it.

Without conclusion... Sigh...
Jonathan Nieder· Feb 6, 2012, 21:13 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/4] commit: introduce a config key to allow as-is commit with i-t-a entries

Hi,
Junio C Hamano wrote:
Show 6 quoted lines
> IIRC, the original motivation of intent-to-add "add -N" was in response to
> users who curse Git because they often forget to add new files before
> committing, and they wanted to say "Here I have a new file, it is not
> complete yet, but I do not want it left out of the commit. While my memory
> is fresh, let me tell Git to keep an eye on it, so that it can remind me
> if I forget to add the final contents."

I agree with everything up to here. But I believe these people were _already_ paying attention to "git status" output from the commandline and in the editor window when they run "git commit", to notice other changes they forgot to add, too. I don't think this series would inconvenience them.

Hoping I can find time to look over the other changes soon.

Thanks, Jonathan

Junio C Hamano· Feb 6, 2012, 21:48 UTC · re: Jonathan Nieder · lore

Re: [PATCH 2/4] commit: introduce a config key to allow as-is commit with i-t-a entries

Jonathan Nieder <jrnieder@gmail.com> writes:
Show 14 quoted lines
> Junio C Hamano wrote:
>
>> IIRC, the original motivation of intent-to-add "add -N" was in response to
>> users who curse Git because they often forget to add new files before
>> committing, and they wanted to say "Here I have a new file, it is not
>> complete yet, but I do not want it left out of the commit. While my memory
>> is fresh, let me tell Git to keep an eye on it, so that it can remind me
>> if I forget to add the final contents."
>
> I agree with everything up to here.  But I believe these people were
> _already_ paying attention to "git status" output from the commandline
> and in the editor window when they run "git commit", to notice other
> changes they forgot to add, too.  I don't think this series would
> inconvenience them.

That means that you are willing to declare that nobody will ever need "please remind me lest I forget". Not just the original requestor of the "add -N" feature, but absolutely nobody else.

Then the deprecation sequence presented in this series is fine. The wording to sell that "removal of misfeature" to the end user community needs to be well thought out, though.

Nguyen Thai Ngoc Duy· Feb 7, 2012, 00:58 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/4] commit: introduce a config key to allow as-is commit with i-t-a entries

2012/2/7 Junio C Hamano <gitster@pobox.com>:
Show 29 quoted lines
> It still is not clear to me how best to sell this change to the end-user
> community.
>
> IIRC, the original motivation of intent-to-add "add -N" was in response to
> users who curse Git because they often forget to add new files before
> committing, and they wanted to say "Here I have a new file, it is not
> complete yet, but I do not want it left out of the commit. While my memory
> is fresh, let me tell Git to keep an eye on it, so that it can remind me
> if I forget to add the final contents."  For them, the current "did you
> forget to add them?  If so tell me the final contents for at least the
> paths you will be changing with this commit" error was a perfect safety
> solution.
>
> It turned out that the benefits described we see above in the context,
> "This is useful, among other things, ...", were of more value, and for
> these use cases, i-t-a entries ceased to mean "I may forget, so I am
> telling you now, please remind me when I say commit."  And "did you
> forget?" error is hinderance for them.
>
> But does that mean nobody will ever need "please remind me lest I forget"?
> Just the original requestor of the "add -N" feature may still be using
> git, but more importantly, isn't it the case that those who have been
> using it merely for the other side effect (e.g. 'git diff') sometimes want
> the "please remind me" safety?
>
> I suspect that some among 1 million Git users would want the "please
> remind me", so a solution with configuration variable without command line
> override is not ideal (command line without any configurability is fine as
> long as we have a good default).

Which is why I prefer adding a new configuration variable (and optionally a command line option) instead of deprecating current behavior, because (being lazy) I never be able to find "some among 1 million" so I'm fine with assuming there are some among 1 million that favors safety over convenience.

Show 22 quoted lines
> I am beginning to think "safety by default, which can be turned off by
> learned users, but still can be turned on on demand" may be a lot easier
> to sell this.  That is:
>
>  - commit.ignoreIntentToAdd defaults to `false`; the default will never
>   change.  The users can set it to `true`.
>
>  - "commit --ignore-intent-to-add" can be used without setting the
>   configuration or to defeat an explicit `false`, for a one-shot request.
>
>  - "commit --honor-intent-to-add" can be used to defeat an explicit
>   `true`, for a one-shot request.
>
> The third one is a bit funny, as it is a way to bring back safety when the
> user earlier decided that he does not need that kind of safety (i.e. "I
> only say 'add -N' for `diff` and stuff, I will never forget to add real
> contents before committing"), so it will almost never be used, because
> these users who set 'ignoreIntentToAdd = true' do _not_ expect Git to help
> them in remembering to add the real contents.  And having to add a funny
> option just for the sake of completeness is often an indication that there
> is something fundamentally wrong in the system that the option tries to
> express an interface into it.

Well, that --honor-intent-to-add could be renamed as --no-ignore-intent-to-add. The --[no-]ignore-intent-to-add pair functions as a way to override default behavior/config var. No extra could required. "git commit -h" just does not show it.

We need better option/config names though, --ignore-intent-to-add looks way too long to type and it's not clear what it does without looking up "git add -N".

-- 
Duy
Junio C Hamano· Feb 7, 2012, 01:13 UTC · re: Nguyen Thai Ngoc Duy · lore

Re: [PATCH 2/4] commit: introduce a config key to allow as-is commit with i-t-a entries

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
Show 12 quoted lines
> 2012/2/7 Junio C Hamano <gitster@pobox.com>:
>
>> I suspect that some among 1 million Git users would want the "please
>> remind me", so a solution with configuration variable without command line
>> override is not ideal (command line without any configurability is fine as
>> long as we have a good default).
>
> Which is why I prefer adding a new configuration variable (and
> optionally a command line option) instead of deprecating current
> behavior, because (being lazy) I never be able to find "some among 1
> million" so I'm fine with assuming there are some among 1 million that
> favors safety over convenience.

If there is one thing I want to absolutely avoid, it is to split the userbase into many pieces by giving sticky configuration variables, so the above argument is not a good starting point.

Show 8 quoted lines
>> The third one is a bit funny, as it is a way to bring back safety when the
>> ....  And having to add a funny
>> option just for the sake of completeness is often an indication that there
>> is something fundamentally wrong in the system that the option tries to
>> express an interface into it.
>
> Well, that --honor-intent-to-add could be renamed as
> --no-ignore-intent-to-add.
I wasn't talking about the name at all.  What is _funny_ is the semantics.

A "by default unsafe" configuration introduces a need for an option to be extra careful only when matters, but "an option" to be extra careful by definition is easy to forget, so it is no longer a safety at all, iow, people who want "by default unsafe" will get "always unsafe". And it probably is perfectly fine because to them, forgetting to add 'add -N' entries is not a mistake at all, but always is a deliberate act.

Another thing I am somewhat worried about is if there are existing scripts that create commits and relies on the current "we cannot commit because the final contents is not known yet". I didn't check but for example how well does "git stash" work when the default is flipped to "just ignore"?

Junio C Hamano· Feb 7, 2012, 06:10 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/4] commit: introduce a config key to allow as-is commit with i-t-a entries

Junio C Hamano <gitster@pobox.com> writes:
> It still is not clear to me how best to sell this change to the end-user
> community.
OK, sorry for wavering.

Unless you are doing "commit -a" or "commit pathspec", you are responsible for adding all contents you want to have in the commit before you run the "git commit" command (and for the purpose of this statement, "add -N" to tell Git to keey an eye on a path does _not_ add contents). A change to the file in the working tree that is left unadded is what you decided to deliberately leave out of the commit, be it a change to a path already in HEAD, or a path marked with "add -N". Forgetting to add modified file and forgetting to add a file you earlier used "add -N" amount to the same kind of risk, and "git status" is the way to make sure your partial commit has exactly what you want (if you are not worried about partial commit, you would be doing "commit -a", so the "safety" is a moot point).

I was worried too much about backward compatibility and was blind to the above, and was mistakenly defending a false "safety" that did not exist at all. Sorry for wasting everybody's time.

So let's bite the bullet and admit in the Release Notes that the current behaviour was a UI mistake based on the misguided assumption that we can give some kind of "safety" by committing when there are "add -N" entries in the index, which is untrue, and we are fixing it in the new release. We do not need configuration nor command line options.

Let me try to reroll your patch tomorrow (unless you beat me to it) and see if I can come up with an easy-to-understand explanation to it.

Thanks.
Nguyen Thai Ngoc Duy· Feb 7, 2012, 06:26 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/4] commit: introduce a config key to allow as-is commit with i-t-a entries

2012/2/7 Junio C Hamano <gitster@pobox.com>:
Show 11 quoted lines
> Unless you are doing "commit -a" or "commit pathspec", you are responsible
> for adding all contents you want to have in the commit before you run the
> "git commit" command (and for the purpose of this statement, "add -N" to
> tell Git to keey an eye on a path does _not_ add contents). A change to
> the file in the working tree that is left unadded is what you decided to
> deliberately leave out of the commit, be it a change to a path already in
> HEAD, or a path marked with "add -N".  Forgetting to add modified file and
> forgetting to add a file you earlier used "add -N" amount to the same kind
> of risk, and "git status" is the way to make sure your partial commit has
> exactly what you want (if you are not worried about partial commit, you
> would be doing "commit -a", so the "safety" is a moot point).

We need something in the commit message so that 5 years from now, when someone raises the issue again, (s)he does not have to search the mail archive. May I steal the above paragraph, maybe rephrase a little bit, for commit message?

-- 
Duy
Junio C Hamano· Feb 7, 2012, 07:57 UTC · re: Nguyen Thai Ngoc Duy · lore

Re: [PATCH 2/4] commit: introduce a config key to allow as-is commit with i-t-a entries

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
Show 17 quoted lines
> 2012/2/7 Junio C Hamano <gitster@pobox.com>:
>> Unless you are doing "commit -a" or "commit pathspec", you are responsible
>> for adding all contents you want to have in the commit before you run the
>> "git commit" command (and for the purpose of this statement, "add -N" to
>> tell Git to keey an eye on a path does _not_ add contents). A change to
>> the file in the working tree that is left unadded is what you decided to
>> deliberately leave out of the commit, be it a change to a path already in
>> HEAD, or a path marked with "add -N".  Forgetting to add modified file and
>> forgetting to add a file you earlier used "add -N" amount to the same kind
>> of risk, and "git status" is the way to make sure your partial commit has
>> exactly what you want (if you are not worried about partial commit, you
>> would be doing "commit -a", so the "safety" is a moot point).
>
> We need something in the commit message so that 5 years from now, when
> someone raises the issue again, (s)he does not have to search the mail
> archive. May I steal the above paragraph, maybe rephrase a little bit,
> for commit message?
Surely, and thanks.
Nguyễn Thái Ngọc Duy· Feb 6, 2012, 10:57 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH 3/4] commit: turn commit.ignoreIntentToAdd to true by default

From now on, those who has not set commit.ignoreIntentToAdd can commit as-is even if there are intent-to-add entries. Users are advised/annoyed to switch to new behavior.

Support for "commit.ignoreIntentToAdd = true" will be dropped in future. The placeholder "FIXME" needs to be replaced when it's decided what release will drop config support.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/config.txt |    5 ++---
 builtin/commit.c         |   13 ++++++++-----
 t/t2203-add-intent.sh    |    4 ++--
 3 files changed, 12 insertions(+), 10 deletions(-)
Show changes to 3 files +12 −10

Documentation/config.txt, builtin/commit.c, t/t2203-add-intent.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6ec81a8..f9a05ac 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -837,9 +837,8 @@ commit.ignoreIntentToAdd::
 	in index. Set to `false` to disallow commit in this acase, or `true`
 	to allow it.
 +
-By default, `git commit` refuses to commit as-is when you have intent-to-add
-entries. This will change in 1.8.0, where `git commit` allows it. If you
-prefer current behavior, please set it to `false`.
+By default, `git commit` allows to commit as-is when you have intent-to-add
+entries. Support for this configuration variable will be dropped in FIXME.
 
 credential.helper::
 	Specify an external helper to be called when a username or
diff --git a/builtin/commit.c b/builtin/commit.c
index af3250c..eb0ca49 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -423,17 +423,17 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 	if (!pathspec || !*pathspec) {
 		fd = hold_locked_index(&index_lock, 1);
 		refresh_cache_or_die(refresh_flags);
-		if (!set_commit_ignoreintenttoadd) {
+		if (!(cache_tree_flags & WRITE_TREE_IGNORE_INTENT_TO_ADD)) {
 			int i;
 			for (i = 0; i < active_nr; i++)
 				if (active_cache[i]->ce_flags & CE_INTENT_TO_ADD)
 					break;
 			if (i < active_nr)
 				warning(_("You are committing as-is with intent-to-add entries as the result of\n"
-					  "\"git add -N\". Git currently forbids this case. This will change in\n"
-					  "1.8.0 where intent-to-add entries are simply ignored when committing\n"
-					  "as-is. Please look up document and set commit.ignoreIntentToAdd\n"
-					  "properly to stop this warning."));
+					  "\"git add -N\". Git currently forbids this case. But this is deprecated\n"
+					  "support for this behavior will be dropped in FIXME.\n"
+					  "Please look up document and set commit.ignoreIntentToAdd to true\n"
+					  "or remove it."));
 		}
 		if (active_cache_changed) {
 			update_main_cache_tree(cache_tree_flags | WRITE_TREE_SILENT);
@@ -1423,6 +1423,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	git_config(git_commit_config, &s);
 	determine_whence(&s);
 
+	if (!set_commit_ignoreintenttoadd)
+		cache_tree_flags |= WRITE_TREE_IGNORE_INTENT_TO_ADD;
+
 	if (get_sha1("HEAD", sha1))
 		current_head = NULL;
 	else {
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 88a508e..09b8bbf 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -41,11 +41,11 @@ 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 -minitial
+	git commit -minitial
 '
 
 test_expect_success 'can commit tree with i-t-a entry' '
-	git reset --hard &&
+	git reset --hard HEAD^ &&
 	echo xyzzy >rezrov &&
 	echo frotz >nitfol &&
 	git add rezrov &&
-- 
1.7.8.36.g69ee2
Junio C Hamano· Feb 6, 2012, 20:03 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 3/4] commit: turn commit.ignoreIntentToAdd to true by default

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> -		if (!set_commit_ignoreintenttoadd) {

Now no warning() is associated with testing of this flag, which means that people who didn't get around to read the doc and react to the warnings in the earlier releases will get _nothing_ when the real change hits them?

That sounds strangely bad.  What am I missing?
Show 15 quoted lines
> +		if (!(cache_tree_flags & WRITE_TREE_IGNORE_INTENT_TO_ADD)) {
>  			int i;
>  			for (i = 0; i < active_nr; i++)
>  				if (active_cache[i]->ce_flags & CE_INTENT_TO_ADD)
>  					break;
>  			if (i < active_nr)
>  				warning(_("You are committing as-is with intent-to-add entries as the result of\n"
> -					  "\"git add -N\". Git currently forbids this case. This will change in\n"
> -					  "1.8.0 where intent-to-add entries are simply ignored when committing\n"
> -					  "as-is. Please look up document and set commit.ignoreIntentToAdd\n"
> -					  "properly to stop this warning."));
> +					  "\"git add -N\". Git currently forbids this case. But this is deprecated\n"
> +					  "support for this behavior will be dropped in FIXME.\n"
> +					  "Please look up document and set commit.ignoreIntentToAdd to true\n"
> +					  "or remove it."));
As this is marked with FIXME ;-)

At this point in the deprecation cycle, "currently forbids this case." is not true at all. "You asked me to punt when you forgot to tell me the final contents for i-t-a entries, so I am honoring your wish" is what is happening here. Perhaps...

	Git currently allows you to set commit.ignoreIntentToAdd to false
        to remind you about the paths you may have forgotten to add the
        real contents to before committing, but this support will be
        removed in future versions of Git.  Set commit.ignoreIntentToAdd
        to `false` or remove the variable, and get used to the new
        behaviour, to squelch this message.
Nguyen Thai Ngoc Duy· Feb 7, 2012, 01:03 UTC · re: Junio C Hamano · lore

Re: [PATCH 3/4] commit: turn commit.ignoreIntentToAdd to true by default

2012/2/7 Junio C Hamano <gitster@pobox.com>:
Show 9 quoted lines
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> -             if (!set_commit_ignoreintenttoadd) {
>
> Now no warning() is associated with testing of this flag, which means that
> people who didn't get around to read the doc and react to the warnings in
> the earlier releases will get _nothing_ when the real change hits them?
>
> That sounds strangely bad.  What am I missing?

Well if they stumble upon this case during the previous deprecation phase, they ought to set commit.ignoreIntentToAdd. If it's not set, they probably do not have prior experience with this feature. With their fresh mind, they hopefully learn the new default behavior via to-be-updated git-add.txt so no warnings for them.

-- 
Duy
Nguyễn Thái Ngọc Duy· Feb 6, 2012, 10:57 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH 4/4] commit: remove commit.ignoreIntentToAdd, assume it's always true

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/config.txt |    9 ---------
 builtin/commit.c         |   24 +-----------------------
 t/t2203-add-intent.sh    |    2 +-
 3 files changed, 2 insertions(+), 33 deletions(-)
Show changes to 3 files +2 −33

Documentation/config.txt, builtin/commit.c, t/t2203-add-intent.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f9a05ac..abeb82b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -831,15 +831,6 @@ commit.template::
 	"{tilde}/" is expanded to the value of `$HOME` and "{tilde}user/" to the
 	specified user's home directory.
 
-commit.ignoreIntentToAdd::
-	Allow to commit the index as-is even if there are
-	intent-to-add entries (see option `-N` in linkgit:git-add[1])
-	in index. Set to `false` to disallow commit in this acase, or `true`
-	to allow it.
-+
-By default, `git commit` allows to commit as-is when you have intent-to-add
-entries. Support for this configuration variable will be dropped in FIXME.
-
 credential.helper::
 	Specify an external helper to be called when a username or
 	password credential is needed; the helper may consult external
diff --git a/builtin/commit.c b/builtin/commit.c
index eb0ca49..491cae1 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -118,8 +118,6 @@ static enum {
 } status_format = STATUS_FORMAT_LONG;
 static int status_show_branch;
 
-static int set_commit_ignoreintenttoadd;
-
 static int opt_parse_m(const struct option *opt, const char *arg, int unset)
 {
 	struct strbuf *buf = opt->value;
@@ -423,18 +421,6 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 	if (!pathspec || !*pathspec) {
 		fd = hold_locked_index(&index_lock, 1);
 		refresh_cache_or_die(refresh_flags);
-		if (!(cache_tree_flags & WRITE_TREE_IGNORE_INTENT_TO_ADD)) {
-			int i;
-			for (i = 0; i < active_nr; i++)
-				if (active_cache[i]->ce_flags & CE_INTENT_TO_ADD)
-					break;
-			if (i < active_nr)
-				warning(_("You are committing as-is with intent-to-add entries as the result of\n"
-					  "\"git add -N\". Git currently forbids this case. But this is deprecated\n"
-					  "support for this behavior will be dropped in FIXME.\n"
-					  "Please look up document and set commit.ignoreIntentToAdd to true\n"
-					  "or remove it."));
-		}
 		if (active_cache_changed) {
 			update_main_cache_tree(cache_tree_flags | WRITE_TREE_SILENT);
 			if (write_cache(fd, active_cache, active_nr) ||
@@ -1353,13 +1339,6 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 		include_status = git_config_bool(k, v);
 		return 0;
 	}
-	if (!strcmp(k, "commit.ignoreintenttoadd")) {
-		set_commit_ignoreintenttoadd = 1;
-		if (git_config_bool(k, v))
-			cache_tree_flags |= WRITE_TREE_IGNORE_INTENT_TO_ADD;
-		else
-			cache_tree_flags &= ~WRITE_TREE_IGNORE_INTENT_TO_ADD;
-	}
 
 	status = git_gpg_config(k, v, NULL);
 	if (status)
@@ -1423,8 +1402,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	git_config(git_commit_config, &s);
 	determine_whence(&s);
 
-	if (!set_commit_ignoreintenttoadd)
-		cache_tree_flags |= WRITE_TREE_IGNORE_INTENT_TO_ADD;
+	cache_tree_flags |= WRITE_TREE_IGNORE_INTENT_TO_ADD;
 
 	if (get_sha1("HEAD", sha1))
 		current_head = NULL;
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 09b8bbf..7c7ab54 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -50,7 +50,7 @@ test_expect_success 'can commit tree with i-t-a entry' '
 	echo frotz >nitfol &&
 	git add rezrov &&
 	git add -N nitfol &&
-	git config commit.ignoreIntentToAdd true &&
+	git config commit.ignoreIntentToAdd false &&
 	git commit -m initial &&
 	git ls-tree -r HEAD >actual &&
 	cat >expected <<EOF &&
-- 
1.7.8.36.g69ee2
Junio C Hamano· Feb 6, 2012, 20:05 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 4/4] commit: remove commit.ignoreIntentToAdd, assume it's always true

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
Show 10 quoted lines
> diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
> index 09b8bbf..7c7ab54 100755
> --- a/t/t2203-add-intent.sh
> +++ b/t/t2203-add-intent.sh
> @@ -50,7 +50,7 @@ test_expect_success 'can commit tree with i-t-a entry' '
>  	echo frotz >nitfol &&
>  	git add rezrov &&
>  	git add -N nitfol &&
> -	git config commit.ignoreIntentToAdd true &&
> +	git config commit.ignoreIntentToAdd false &&
This deserves a comment to the effect that the variable is now *ignored*.
>  	git commit -m initial &&
>  	git ls-tree -r HEAD >actual &&
>  	cat >expected <<EOF &&

← back to recent threads