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

@@ -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"));

@@ -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);
 }

@@ -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)

@@ -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);

@@ -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, "");
 }

@@ -831,6 +831,22 @@ commit.template::
 	"{tilde}/" is expanded to the value of `$HOME` and "{tilde}user/" to the
 	specified user's home directory.
 
+commit.ignoreIntentToAdd::
+	When set to `false`, 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 `true` 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 future releases of git. 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`.
+
 credential.helper::
 	Specify an external helper to be called when a username or
 	password credential is needed; the helper may consult external

@@ -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,21 @@ 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) {
+				error(_("you intended to add \"%s\" but did not add it; not committing\n"
+					"hint: to commit all changes to tracked files, use \"commit -a\"\n"
+					"hint: to commit anyway without adding, set commit.ignoreIntentToAdd to true"),
+				      active_cache[i]->name);
+				exit(128); /* die() */
+			}
+		}
 		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 +886,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 +1354,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)

@@ -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');

@@ -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)

@@ -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' '

@@ -839,13 +839,10 @@ commit.ignoreIntentToAdd::
 	these entries.  Setting this to `true` 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 future releases of git. 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`.
+The default for this variable is `true`. You are discouraged to set it
+to `false` to keep the old behaviour a bit longer because support
+setting this to `false` will be removed in future releases without
+warning.
 
 credential.helper::
 	Specify an external helper to be called when a username or

@@ -423,15 +423,16 @@ 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) {
 				error(_("you intended to add \"%s\" but did not add it; not committing\n"
-					"hint: to commit all changes to tracked files, use \"commit -a\"\n"
-					"hint: to commit anyway without adding, set commit.ignoreIntentToAdd to true"),
+					"this behavior is deprecated, please set commit.ignoreIntentToAdd\n"
+					"to true or remove the configuration variable. See the configuration\n"
+					"variable documentation for more information."),
 				      active_cache[i]->name);
 				exit(128); /* die() */
 			}

@@ -1424,6 +1425,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 {

@@ -1583,5 +1587,14 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	if (!quiet)
 		print_summary(prefix, sha1, !current_head);
 
+	if (set_commit_ignoreintenttoadd) {
+		if (cache_tree_flags & WRITE_TREE_IGNORE_INTENT_TO_ADD)
+			warning(_("commit.ignoreIntentToAdd = true is not needed anymore.\n"
+				  "Please remove it."));
+		else
+			warning(_("commit.ignoreIntentToAdd = false is deprecated.\n"
+				  "Please see the commit.ignoreIntentToAdd documentation for\n"
+				  "more information and remove the configuration variable."));
+	}
 	return 0;
 }

@@ -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 &&

@@ -831,19 +831,6 @@ commit.template::
 	"{tilde}/" is expanded to the value of `$HOME` and "{tilde}user/" to the
 	specified user's home directory.
 
-commit.ignoreIntentToAdd::
-	When set to `false`, 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 `true` makes `git commit`
-	pretend as if these entries do not exist in the index.
-+
-The default for this variable is `true`. You are discouraged to set it
-to `false` to keep the old behaviour a bit longer because support
-setting this to `false` will be removed in future releases without
-warning.
-
 credential.helper::
 	Specify an external helper to be called when a username or
 	password credential is needed; the helper may consult external

@@ -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,20 +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) {
-				error(_("you intended to add \"%s\" but did not add it; not committing\n"
-					"this behavior is deprecated, please set commit.ignoreIntentToAdd\n"
-					"to true or remove the configuration variable. See the configuration\n"
-					"variable documentation for more information."),
-				      active_cache[i]->name);
-				exit(128); /* die() */
-			}
-		}
 		if (active_cache_changed) {
 			update_main_cache_tree(cache_tree_flags | WRITE_TREE_SILENT);
 			if (write_cache(fd, active_cache, active_nr) ||

@@ -1355,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)

@@ -1425,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;

@@ -1587,14 +1563,5 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	if (!quiet)
 		print_summary(prefix, sha1, !current_head);
 
-	if (set_commit_ignoreintenttoadd) {
-		if (cache_tree_flags & WRITE_TREE_IGNORE_INTENT_TO_ADD)
-			warning(_("commit.ignoreIntentToAdd = true is not needed anymore.\n"
-				  "Please remove it."));
-		else
-			warning(_("commit.ignoreIntentToAdd = false is deprecated.\n"
-				  "Please see the commit.ignoreIntentToAdd documentation for\n"
-				  "more information and remove the configuration variable."));
-	}
 	return 0;
 }

@@ -44,13 +44,13 @@ test_expect_success 'cannot commit with i-t-a entry' '
 	git commit -minitial
 '
 
-test_expect_success 'can commit tree with i-t-a entry' '
+test_expect_success 'commit.ignoreIntentToAdd = false is ignored' '
 	git reset --hard HEAD^ &&
 	echo xyzzy >rezrov &&
 	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 &&