public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Jeff King <peff@peff•net>
Cc: "Nguyễn Thái Ngọc Duy" <pclouds@gmail•com>, git@vger•kernel.org
Subject: [PATCH] add: make "add -u/-A" update full tree without pathspec
Date: Tue, 22 Mar 2011 17:02:32 -0700	[thread overview]
Message-ID: <7vtyeuiu07.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: 20110321111643.GE16334@sigill.intra.peff.net

When -u was introduced in dfdac5d (git-add -u: match the index with
working tree., 2007-04-20), "add -u" (without pathspec) added
everything. Shortly after, 2ed2c22 (git-add -u paths... now works from
subdirectory, 2007-08-16) broke it while fixing something related.

This makes -u and -A inconsistent with some other options, namely -p.
It's been four years since the unintentional breakage and people are
probably used to "git add -u" updating only current directory.

Let's plan in 1.8.0 to change its behaviour in such a way that does
not hurt existing users too badly during the transition period.

 - A new add.treewideupdate configuration variable can be set to
   "true" to make "add -u/-A" that is ran without any pathspec from
   a subdirectory to affect the whole tree.  When the variable is
   set to "false", the operation is limited to the current working
   directory.

 - Missing configuration variable means the same thing as setting it
   to "false" for now, but the user will be given a warning about
   the transition plan, and an advise to either set the variable or
   to say "."

 - In 1.8.0, the warning message needs to be rephrased, the added
   test needs to be updated, and the default value for the variable
   needs to be flipped to "true".  In a few releases after that, we
   would remove the warning message.

Signed-off-by: Junio C Hamano <gitster@pobox•com>
---

 * So how about an alternative with a bit saner migration path?
   The message would need rewording, though.

 builtin/add.c         |   47 ++++++++++++++++++++++++++++++++++++++++++-----
 t/t2200-add-update.sh |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 5 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index e127d5a..595f5cc 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -310,6 +310,7 @@ static const char ignore_error[] =
 
 static int verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0;
 static int ignore_add_errors, addremove, intent_to_add, ignore_missing = 0;
+static int default_tree_wide_update = -1;
 
 static struct option builtin_add_options[] = {
 	OPT__DRY_RUN(&show_only, "dry run"),
@@ -335,6 +336,10 @@ static int add_config(const char *var, const char *value, void *cb)
 		ignore_add_errors = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcasecmp(var, "add.treewideupdate")) {
+		default_tree_wide_update = git_config_bool(var, value);
+		return 0;
+	}
 	return git_default_config(var, value, cb);
 }
 
@@ -359,6 +364,29 @@ static int add_files(struct dir_struct *dir, int flags)
 	return exit_status;
 }
 
+static const char *warn_add_uA_180_migration_msg[] = {
+	"In release 1.8.0, running 'git add -u' (or 'git add -A') from",
+	"a subdirectory without giving any pathspec WILL take effect",
+	"on the whole working tree, not just the part under the current",
+	"directory. You can set add.treewideupdate configuration variable",
+	"to 'false' to keep the current behaviour.",
+	"You can set the configuration variable to 'true' to make the",
+	"'git add -u/-A' command without pathspec take effect on the whole",
+	"working tree now. If you do so, you can use '.' at the end of",
+	"the command, e.g. 'git add -u .' when you want to limit the",
+	"operation to the current directory.",
+	"This warning will be issued until you set the configuration variable",
+	"to either 'true' or 'false'."
+};
+
+static int warn_180_migration(void)
+{
+	int i;
+	for (i = 0; i < ARRAY_SIZE(warn_add_uA_180_migration_msg); i++)
+		warning("%s", warn_add_uA_180_migration_msg[i]);
+	return 0; /* default to "no" (not tree-wide, i.e. local) */
+}
+
 int cmd_add(int argc, const char **argv, const char *prefix)
 {
 	int exit_status = 0;
@@ -368,6 +396,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	int flags;
 	int add_new_files;
 	int require_pathspec;
+	int whole_tree_add = 0;
 	char *seen = NULL;
 
 	git_config(add_config, NULL);
@@ -389,9 +418,13 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	if (!show_only && ignore_missing)
 		die("Option --ignore-missing can only be used together with --dry-run");
 	if ((addremove || take_worktree_changes) && !argc) {
-		static const char *here[2] = { ".", NULL };
-		argc = 1;
-		argv = here;
+		whole_tree_add = 1;
+		if (prefix) {
+			if (default_tree_wide_update < 0)
+				default_tree_wide_update = warn_180_migration();
+			if (!default_tree_wide_update)
+				whole_tree_add = 0;
+		}
 	}
 
 	add_new_files = !take_worktree_changes && !refresh_only;
@@ -406,12 +439,16 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		 (!(addremove || take_worktree_changes)
 		  ? ADD_CACHE_IGNORE_REMOVAL : 0));
 
-	if (require_pathspec && argc == 0) {
+	if (require_pathspec && !(argc || whole_tree_add)) {
 		fprintf(stderr, "Nothing specified, nothing added.\n");
 		fprintf(stderr, "Maybe you wanted to say 'git add .'?\n");
 		return 0;
 	}
-	pathspec = validate_pathspec(argc, argv, prefix);
+
+	if (whole_tree_add)
+		pathspec = NULL;
+	else
+		pathspec = validate_pathspec(argc, argv, prefix);
 
 	if (read_cache() < 0)
 		die("index file corrupt");
diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
index 0692427..7ac8b70 100755
--- a/t/t2200-add-update.sh
+++ b/t/t2200-add-update.sh
@@ -80,6 +80,55 @@ test_expect_success 'change gets noticed' '
 
 '
 
+test_expect_success 'update from a subdirectory without pathspec (no config)' '
+	# This test needs to be updated to expect the whole tree
+	# update after 1.8.0 migration.
+	test_might_fail git config --remove add.treewideupdate &&
+	test_might_fail git reset check dir1 &&
+	echo changed >check &&
+	(
+		cd dir1 &&
+		echo even more >sub2 &&
+		git add -u 2>../expect.warning
+	) &&
+	git diff-files --name-only dir1 check >actual &&
+	echo check >expect &&
+	test_cmp expect actual &&
+	grep warning expect.warning
+'
+
+test_expect_success 'update from a subdirectory without pathspec (local)' '
+	test_when_finished "git config --remove add.treewideupdate; :" &&
+	git config add.treewideupdate false &&
+	test_might_fail git reset check dir1 &&
+	echo changed >check &&
+	(
+		cd dir1 &&
+		echo even more >sub2 &&
+		git add -u 2>../expect.warning
+	) &&
+	git diff-files --name-only dir1 check >actual &&
+	echo check >expect &&
+	test_cmp expect actual &&
+	! grep warning expect.warning
+'
+
+test_expect_success 'update from a subdirectory without pathspec (global)' '
+	test_when_finished "git config --remove add.treewideupdate; :" &&
+	git config add.treewideupdate true &&
+	test_might_fail git reset check dir1 &&
+	echo changed >check &&
+	(
+		cd dir1 &&
+		echo even more >sub2 &&
+		git add -u 2>../expect.warning
+	) &&
+	git diff-files --name-only dir1 check >actual &&
+	: >expect &&
+	test_cmp expect actual &&
+	! grep warning expect.warning
+'
+
 test_expect_success SYMLINKS 'replace a file with a symlink' '
 
 	rm foo &&
-- 
1.7.4.1.559.ga7cf60f

  parent reply	other threads:[~2011-03-23  0:03 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-20 19:49 What's cooking in git.git (Mar 2011, #02; Sun, 20) Junio C Hamano
2011-03-20 20:43 ` Jens Lehmann
2011-03-20 21:06 ` Junio C Hamano
2011-03-20 21:22   ` Junio C Hamano
2011-03-20 21:36     ` Matthieu Moy
2011-03-21  0:06       ` Junio C Hamano
2011-03-21 11:16         ` Jeff King
2011-03-21 14:28           ` Junio C Hamano
2011-03-23  0:02           ` Junio C Hamano [this message]
2011-03-23  5:17             ` [PATCH] add: make "add -u/-A" update full tree without pathspec (step 2) Junio C Hamano
2011-03-23  5:19               ` [PATCH] add: make "add -u/-A" update full tree without pathspec (step 3) Junio C Hamano
2011-03-23 14:02                 ` Nguyen Thai Ngoc Duy
2011-03-23 15:54                   ` Junio C Hamano
2011-03-22 10:57 ` What's cooking in git.git (Mar 2011, #02; Sun, 20) Ævar Arnfjörð Bjarmason
2011-03-22 18:00   ` Junio C Hamano

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=7vtyeuiu07.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=pclouds@gmail$(echo .)com \
    --cc=peff@peff$(echo .)net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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