{"thread":{"id":"33110","subject":"[PATCH 0/2] \"git add -u/-A\" from future","startedAt":"2013-03-08T23:54:47Z","lastAt":"2013-04-02T16:57:22Z","messageCount":51,"participants":["Junio C Hamano","Matthieu Moy","Jeff King","Jonathan Nieder","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"210861","messageId":"1362786889-28688-1-git-send-email-gitster@pobox.com","threadId":"33110","inReplyTo":null,"subject":"[PATCH 0/2] \"git add -u/-A\" from future","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-08T23:54:47Z","receivedAt":"2013-03-08T23:54:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Here are two future steps to update the behaviour of \"add -u/-A\" run\nwithout pathspec towards Git 2.0; the first step may probably be\noptional, but it is included for completeness.\n\nJunio C Hamano (2):\n require pathspec for \"git add -u/-A\"\n git add: -u/-A now affects the entire working tree\n\n Documentation/git-add.txt | 8 ++++----\n builtin/add.c | 49 ++++-------------------------------------------\n 2 files changed, 8 insertions(+), 49 deletions(-)\n\n-- \n1.8.2-rc3-196-gfcda97c\n"},{"id":"210862","messageId":"1362786889-28688-2-git-send-email-gitster@pobox.com","threadId":"33110","inReplyTo":"1362786889-28688-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/2] require pathspec for \"git add -u/-A\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-08T23:54:48Z","receivedAt":"2013-03-08T23:54:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"As promised in 0fa2eb530fb7 (add: warn when -u or -A is used without\npathspec, 2013-01-28), \"git add -u/-A\" that is run without pathspec\nin a subdirectory will stop working sometime before Git 2.0, to wean\nusers off of the old default, in preparation for adopting the new\ndefault in Git 2.0.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/git-add.txt | 12 ++++++++----\n builtin/add.c | 38 ++++++++++++++++++--------------------\n 2 files changed, 26 insertions(+), 24 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 388a225..1ea1d39 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -107,10 +107,14 @@ apply to the index. See EDITING PATCHES below.\n \tfrom the index if the corresponding files in the working tree\n \thave been removed.\n +\n-If no is given, the current version of Git defaults to\n-\".\"; in other words, update all tracked files in the current directory\n-and its subdirectories. This default will change in a future version\n-of Git, hence the form without should not be used.\n+If no is given when `-u` or `-A` option is used, Git used\n+to update all tracked files in the current directory and its\n+subdirectories. We would eventually want to change this to operate\n+on the entire working tree, not limiting it to the current\n+directory, to make it consistent with `git commit -a` and other\n+commands. In order to avoid harming users who are used to the old\n+default, Git *errors out* when no is given to train their\n+fingers to explicitly type `git add -u .` when they mean it.\n \n -A::\n --all::\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 0dd014e..4b9d57c 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -321,30 +321,28 @@ static int add_files(struct dir_struct *dir, int flags)\n \treturn exit_status;\n }\n \n-static void warn_pathless_add(const char *option_name, const char *short_name) {\n+static void die_on_pathless_add(const char *option_name, const char *short_name)\n+{\n \t/*\n \t * To be consistent with \"git add -p\" and most Git\n \t * commands, we should default to being tree-wide, but\n \t * this is not the original behavior and can't be\n \t * changed until users trained themselves not to type\n-\t * \"git add -u\" or \"git add -A\". For now, we warn and\n-\t * keep the old behavior. Later, this warning can be\n-\t * turned into a die(...), and eventually we may\n-\t * reallow the command with a new behavior.\n+\t * \"git add -u\" or \"git add -A\". In the previous release,\n+\t * we kept the old behavior but gave a big warning.\n \t */\n-\twarning(_(\"The behavior of 'git add %s (or %s)' with no path argument from a\\n\"\n-\t\t \"subdirectory of the tree will change in Git 2.0 and should not be used anymore.\\n\"\n-\t\t \"To add content for the whole tree, run:\\n\"\n-\t\t \"\\n\"\n-\t\t \" git add %s :/\\n\"\n-\t\t \" (or git add %s :/)\\n\"\n-\t\t \"\\n\"\n-\t\t \"To restrict the command to the current directory, run:\\n\"\n-\t\t \"\\n\"\n-\t\t \" git add %s .\\n\"\n-\t\t \" (or git add %s .)\\n\"\n-\t\t \"\\n\"\n-\t\t \"With the current Git version, the command is restricted to the current directory.\"),\n+\tdie(_(\"The behavior of 'git add %s (or %s)' with no path argument from a\\n\"\n+\t \"subdirectory of the tree will change in Git 2.0 and should not be \"\n+\t \"used anymore.\\n\"\n+\t \"To add content for the whole tree, run:\\n\"\n+\t \"\\n\"\n+\t \" git add %s :/\\n\"\n+\t \" (or git add %s :/)\\n\"\n+\t \"\\n\"\n+\t \"To restrict the command to the current directory, run:\\n\"\n+\t \"\\n\"\n+\t \" git add %s .\\n\"\n+\t \" (or git add %s .)\"),\n \t\toption_name, short_name,\n \t\toption_name, short_name,\n \t\toption_name, short_name);\n@@ -392,8 +390,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tif (option_with_implicit_dot && !argc) {\n \t\tstatic const char *here[2] = { \".\", NULL };\n \t\tif (prefix)\n-\t\t\twarn_pathless_add(option_with_implicit_dot,\n-\t\t\t\t\t short_option_with_implicit_dot);\n+\t\t\tdie_on_pathless_add(option_with_implicit_dot,\n+\t\t\t\t\t short_option_with_implicit_dot);\n \t\targc = 1;\n \t\targv = here;\n \t}\n-- \n1.8.2-rc3-196-gfcda97c\n"},{"id":"210863","messageId":"1362786889-28688-3-git-send-email-gitster@pobox.com","threadId":"33110","inReplyTo":"1362786889-28688-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/2] git add: -u/-A now affects the entire working tree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-08T23:54:49Z","receivedAt":"2013-03-08T23:54:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"As promised in 0fa2eb530fb7 (add: warn when -u or -A is used without\npathspec, 2013-01-28), in Git 2.0, \"git add -u/-A\" that is run\nwithout pathspec in a subdirectory updates all updated paths in the\nentire working tree, not just the current directory and its\nsubdirectories.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/git-add.txt | 12 ++++--------\n builtin/add.c | 47 ++++-------------------------------------------\n 2 files changed, 8 insertions(+), 51 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 1ea1d39..15a8859 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -107,14 +107,10 @@ apply to the index. See EDITING PATCHES below.\n \tfrom the index if the corresponding files in the working tree\n \thave been removed.\n +\n-If no is given when `-u` or `-A` option is used, Git used\n-to update all tracked files in the current directory and its\n-subdirectories. We would eventually want to change this to operate\n-on the entire working tree, not limiting it to the current\n-directory, to make it consistent with `git commit -a` and other\n-commands. In order to avoid harming users who are used to the old\n-default, Git *errors out* when no is given to train their\n-fingers to explicitly type `git add -u .` when they mean it.\n+If no is given when `-u` or `-A` option is used, all\n+tracked files in the entire working tree are updated (old versions\n+of Git used to limit the update to the current directory and its\n+subdirectories).\n \n -A::\n --all::\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 4b9d57c..6cd063f 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -321,33 +321,6 @@ static int add_files(struct dir_struct *dir, int flags)\n \treturn exit_status;\n }\n \n-static void die_on_pathless_add(const char *option_name, const char *short_name)\n-{\n-\t/*\n-\t * To be consistent with \"git add -p\" and most Git\n-\t * commands, we should default to being tree-wide, but\n-\t * this is not the original behavior and can't be\n-\t * changed until users trained themselves not to type\n-\t * \"git add -u\" or \"git add -A\". In the previous release,\n-\t * we kept the old behavior but gave a big warning.\n-\t */\n-\tdie(_(\"The behavior of 'git add %s (or %s)' with no path argument from a\\n\"\n-\t \"subdirectory of the tree will change in Git 2.0 and should not be \"\n-\t \"used anymore.\\n\"\n-\t \"To add content for the whole tree, run:\\n\"\n-\t \"\\n\"\n-\t \" git add %s :/\\n\"\n-\t \" (or git add %s :/)\\n\"\n-\t \"\\n\"\n-\t \"To restrict the command to the current directory, run:\\n\"\n-\t \"\\n\"\n-\t \" git add %s .\\n\"\n-\t \" (or git add %s .)\"),\n-\t\toption_name, short_name,\n-\t\toption_name, short_name,\n-\t\toption_name, short_name);\n-}\n-\n int cmd_add(int argc, const char **argv, const char *prefix)\n {\n \tint exit_status = 0;\n@@ -358,8 +331,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tint add_new_files;\n \tint require_pathspec;\n \tchar *seen = NULL;\n-\tconst char *option_with_implicit_dot = NULL;\n-\tconst char *short_option_with_implicit_dot = NULL;\n \n \tgit_config(add_config, NULL);\n \n@@ -379,21 +350,11 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"-A and -u are mutually incompatible\"));\n \tif (!show_only && ignore_missing)\n \t\tdie(_(\"Option --ignore-missing can only be used together with --dry-run\"));\n-\tif (addremove) {\n-\t\toption_with_implicit_dot = \"--all\";\n-\t\tshort_option_with_implicit_dot = \"-A\";\n-\t}\n-\tif (take_worktree_changes) {\n-\t\toption_with_implicit_dot = \"--update\";\n-\t\tshort_option_with_implicit_dot = \"-u\";\n-\t}\n-\tif (option_with_implicit_dot && !argc) {\n-\t\tstatic const char *here[2] = { \".\", NULL };\n-\t\tif (prefix)\n-\t\t\tdie_on_pathless_add(option_with_implicit_dot,\n-\t\t\t\t\t short_option_with_implicit_dot);\n+\n+\tif ((addremove || take_worktree_changes) && !argc) {\n+\t\tstatic const char *whole[2] = { \":/\", NULL };\n \t\targc = 1;\n-\t\targv = here;\n+\t\targv = whole;\n \t}\n \n \tadd_new_files = !take_worktree_changes && !refresh_only;\n-- \n1.8.2-rc3-196-gfcda97c\n"},{"id":"210978","messageId":"vpqmwubgsqy.fsf@grenoble-inp.fr","threadId":"33110","inReplyTo":"1362786889-28688-2-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 1/2] require pathspec for \"git add -u/-A\"","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-03-10T15:49:09Z","receivedAt":"2013-03-10T15:49:09Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> As promised in 0fa2eb530fb7 (add: warn when -u or -A is used without\n> pathspec, 2013-01-28), \"git add -u/-A\" that is run without pathspec\n> in a subdirectory will stop working sometime before Git 2.0, to wean\n> users off of the old default, in preparation for adopting the new\n> default in Git 2.0.\n\nI originally thought this step was necessary, but I changed my mind. The\nwarning is big enough and doesn't need to be turned into an error.\n\nIf this step is applied, it should be applied at 2.0, not before, as\nthis is the big incompatible change. Re-introducing a new behavior won't\nharm users OTOH.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"211025","messageId":"7vboaqbeou.fsf@alter.siamese.dyndns.org","threadId":"33110","inReplyTo":"vpqmwubgsqy.fsf@grenoble-inp.fr","subject":"Re: [PATCH 1/2] require pathspec for \"git add -u/-A\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-11T07:04:01Z","receivedAt":"2013-03-11T07:04:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Junio C Hamano writes:\n>\n>> As promised in 0fa2eb530fb7 (add: warn when -u or -A is used without\n>> pathspec, 2013-01-28), \"git add -u/-A\" that is run without pathspec\n>> in a subdirectory will stop working sometime before Git 2.0, to wean\n>> users off of the old default, in preparation for adopting the new\n>> default in Git 2.0.\n>\n> I originally thought this step was necessary, but I changed my mind. The\n> warning is big enough and doesn't need to be turned into an error.\n\nI tend to agree.\n\nThe plan requires the warning to be big enough and warning period to\nbe long enough so that by the time Git 2.0 is released, no existing\nusers will run \"git add -u/-A\" without pathspec expecting it to\nlimit the operation to the current directory, so an extra step to\nerror out such a command invocation is simply redundant. If it is\nnot redundant, that would only mean that the warning period was not\nlong enough. The only effect the extra \"error it out\" step would\nhave is to hurt the people who jump on Git bandwagon after such\nrelease ships, as they do not have any reason to retrain their\nfingers---they instead can just get used to the new behaviour right\naway.\n\nSo let's squash these two steps into one and keep that in 'next'\nuntil 2.0 ships.\n\nThanks for an injection of sanity.\n"},{"id":"211026","messageId":"vpqk3pefjs4.fsf@grenoble-inp.fr","threadId":"33110","inReplyTo":"7vboaqbeou.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] require pathspec for \"git add -u/-A\"","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-03-11T08:00:27Z","receivedAt":"2013-03-11T08:00:27Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> So let's squash these two steps into one and keep that in 'next'\n> until 2.0 ships.\n\nOK, then we may update the comment describing the plan (for people\ndigging in the code to find out what the plan is). Small patch serie\nfollows with this (will probably give conflict with your patch, feel\nfree to drop if resolving them is too painful given the benefit) and\nanother minor improvement.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"211027","messageId":"1362988893-27539-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"33110","inReplyTo":"vpqk3pefjs4.fsf@grenoble-inp.fr","subject":"[PATCH 1/2] add: update pathless 'add [-u|-A]' warning to reflect change of plan","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2013-03-11T08:01:32Z","receivedAt":"2013-03-11T08:01:32Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"We originally thought the transition would need a period where \"git add\n[-u|-A]\" without pathspec would be forbidden, but the warning is big\nenough to scare people and teach them not to use it (or, if so, to\nunderstand the consequences).\n\nSigned-off-by: Matthieu Moy \n---\n builtin/add.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 0dd014e..ab1c9e8 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -328,9 +328,9 @@ static void warn_pathless_add(const char *option_name, const char *short_name) {\n \t * this is not the original behavior and can't be\n \t * changed until users trained themselves not to type\n \t * \"git add -u\" or \"git add -A\". For now, we warn and\n-\t * keep the old behavior. Later, this warning can be\n-\t * turned into a die(...), and eventually we may\n-\t * reallow the command with a new behavior.\n+\t * keep the old behavior. Later, the behavior can be changed\n+\t * to tree-wide, keeping the warning for a while, and\n+\t * eventually we can drop the warning.\n \t */\n \twarning(_(\"The behavior of 'git add %s (or %s)' with no path argument from a\\n\"\n \t\t \"subdirectory of the tree will change in Git 2.0 and should not be used anymore.\\n\"\n-- \n1.8.2.rc3.16.g0a33571.dirty\n"},{"id":"211028","messageId":"1362988893-27539-2-git-send-email-Matthieu.Moy@imag.fr","threadId":"33110","inReplyTo":"1362988893-27539-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 2/2] add: add a newline at the end of pathless 'add [-u|-A]' warning","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2013-03-11T08:01:33Z","receivedAt":"2013-03-11T08:01:33Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"When the commands give an actual output (e.g. when ran with -v), the\noutput is visually mixed with the warning. The newline makes the actual\noutput more visible.\n\nSigned-off-by: Matthieu Moy \n---\n builtin/add.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex ab1c9e8..620bf00 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -344,7 +344,7 @@ static void warn_pathless_add(const char *option_name, const char *short_name) {\n \t\t \" git add %s .\\n\"\n \t\t \" (or git add %s .)\\n\"\n \t\t \"\\n\"\n-\t\t \"With the current Git version, the command is restricted to the current directory.\"),\n+\t\t \"With the current Git version, the command is restricted to the current directory.\\n\"),\n \t\toption_name, short_name,\n \t\toption_name, short_name,\n \t\toption_name, short_name);\n-- \n1.8.2.rc3.16.g0a33571.dirty\n"},{"id":"211051","messageId":"7v7gldc44p.fsf@alter.siamese.dyndns.org","threadId":"33110","inReplyTo":"1362988893-27539-2-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 2/2] add: add a newline at the end of pathless 'add [-u|-A]' warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-11T16:06:46Z","receivedAt":"2013-03-11T16:06:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> When the commands give an actual output (e.g. when ran with -v), the\n> output is visually mixed with the warning. The newline makes the actual\n> output more visible.\n>\n> Signed-off-by: Matthieu Moy \n> ---\n\nIt would have been easier to immediately understand what is going on\nif you said \"blank line\" instead of \"newline\" ;-)\n\nAn obvious issues is what if user does not run with \"-v\" or if \"-v\"\nproduces no results. We will be left with an extra blank line at\nthe end.\n\nI suspect that the true reason why the warning does not stand out\nand other output looks mixed in it may be because we only prefix the\nfirst line with the \"warning: \" label. In the longer term, I have a\nfeeling that we should be showing something like this instead:\n\n $ cd t && echo >>t0000*.sh && git add -u -v\n warning: The behavior of 'git add --update (or -u)' with no path ar...\n warning: subdirectory of the tree will change in Git 2.0 and should...\n warning: To add content for the whole tree, run:\n warning:\n warning: git add --update :/\n warning: (or git add -u :/)\n warning:\n warning: To restrict the command to the current directory, run:\n warning:\n warning: git add --update .\n warning: (or git add -u .)\n warning:\n warning: With the current Git version, the command is restricted to...\n add 't/t0000-basic.sh'\n\nusing a logic similar to what strbuf_add_commented_lines() and\nstrbuf_add_lines() use.\n\n> builtin/add.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/add.c b/builtin/add.c\n> index ab1c9e8..620bf00 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -344,7 +344,7 @@ static void warn_pathless_add(const char *option_name, const char *short_name) {\n> \t\t \" git add %s .\\n\"\n> \t\t \" (or git add %s .)\\n\"\n> \t\t \"\\n\"\n> -\t\t \"With the current Git version, the command is restricted to the current directory.\"),\n> +\t\t \"With the current Git version, the command is restricted to the current directory.\\n\"),\n> \t\toption_name, short_name,\n> \t\toption_name, short_name,\n> \t\toption_name, short_name);\n"},{"id":"211115","messageId":"20130312112840.GA13186@sigill.intra.peff.net","threadId":"33110","inReplyTo":"vpqmwubgsqy.fsf@grenoble-inp.fr","subject":"Re: [PATCH 1/2] require pathspec for \"git add -u/-A\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-12T11:28:40Z","receivedAt":"2013-03-12T11:28:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Mar 10, 2013 at 04:49:09PM +0100, Matthieu Moy wrote:\n\n> Junio C Hamano writes:\n> \n> > As promised in 0fa2eb530fb7 (add: warn when -u or -A is used without\n> > pathspec, 2013-01-28), \"git add -u/-A\" that is run without pathspec\n> > in a subdirectory will stop working sometime before Git 2.0, to wean\n> > users off of the old default, in preparation for adopting the new\n> > default in Git 2.0.\n> \n> I originally thought this step was necessary, but I changed my mind. The\n> warning is big enough and doesn't need to be turned into an error.\n> \n> If this step is applied, it should be applied at 2.0, not before, as\n> this is the big incompatible change. Re-introducing a new behavior won't\n> harm users OTOH.\n\nFWIW, I agree with this. The warning is enough without an error period\n(unless the intent was to switch to the error and stay there forever,\nbut I do not think that is the proposal).\n\n-Peff\n\nPS I wonder how others are finding the warning? I'm finding it slightly\n annoying. Perhaps I just haven't retrained my fingers yet. But one\n problem with that retraining is that I type \"git add -u\" from the\n root _most_ of the time, and it just works. But occasionally, I get\n the \"hey, do not do that\" warning, because I'm in a subdir, even\n though I would be happy to have the full-tree behavior. I guess we\n already rejected the idea of being able to shut off the warning and\n just get the new behavior, in favor of having people specify it\n manually each time?\n"},{"id":"211131","messageId":"vpqhakgiusr.fsf@grenoble-inp.fr","threadId":"33110","inReplyTo":"20130312112840.GA13186@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] require pathspec for \"git add -u/-A\"","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-03-12T13:58:44Z","receivedAt":"2013-03-12T13:58:44Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jeff King writes:\n\n> PS I wonder how others are finding the warning? I'm finding it slightly\n> annoying. Perhaps I just haven't retrained my fingers yet. But one\n> problem with that retraining is that I type \"git add -u\" from the\n> root _most_ of the time, and it just works. But occasionally, I get\n> the \"hey, do not do that\" warning, because I'm in a subdir, even\n> though I would be happy to have the full-tree behavior.\n\nSame here. Not terribly disturbing, but I did get the warning\noccasionally, even though I'm the author of the patch introducing\nit ;-).\n\n> I guess we already rejected the idea of being able to shut off the\n> warning and just get the new behavior, in favor of having people\n> specify it manually each time?\n\nSomehow, but we may find a way to do so, as long as it temporary (i.e.\nsomething that will have no effect after the transition period), and\nthat is is crystal clear that it's temporary.\n\nAll proposals up to now were rejected because of the risk of confusing\nusers (either shutting the warning blindly, or letting people think they\ncould keep the current behavior after Git 2.0).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"211195","messageId":"20130313040845.GA5057@sigill.intra.peff.net","threadId":"33110","inReplyTo":"vpqhakgiusr.fsf@grenoble-inp.fr","subject":"Re: [PATCH 1/2] require pathspec for \"git add -u/-A\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-13T04:08:45Z","receivedAt":"2013-03-13T04:08:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 12, 2013 at 02:58:44PM +0100, Matthieu Moy wrote:\n\n> > I guess we already rejected the idea of being able to shut off the\n> > warning and just get the new behavior, in favor of having people\n> > specify it manually each time?\n> \n> Somehow, but we may find a way to do so, as long as it temporary (i.e.\n> something that will have no effect after the transition period), and\n> that is is crystal clear that it's temporary.\n\nYeah, I think this is easy as long as it is \"enable the new behavior\nnow\" and not \"toggle between new and old behavior\". That is, a boolean\nrather than a selector, with a note that it will go away at the behavior\nswitch.\n\nThe only downside I see is that a user may switch it on now, saying\n\"Yes, I understand and prefer the new behavior\", but some script they\nrun might not expect it. We can warn against that in the documentation,\nbut that may or may not be enough.\n\nHere's a series which does that; if it's the direction we want to go, I\nthink we'd want to rebase Junio's \"now add -u is full-tree\" patch on\ntop.\n\n [1/2]: t2200: check that \"add -u\" limits itself to subdirectory\n [2/2]: add: respect add.updateroot config option\n\n-Peff\n"},{"id":"211196","messageId":"20130313041022.GA5378@sigill.intra.peff.net","threadId":"33110","inReplyTo":"20130313040845.GA5057@sigill.intra.peff.net","subject":"[PATCH 1/2] t2200: check that \"add -u\" limits itself to subdirectory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-13T04:10:22Z","receivedAt":"2013-03-13T04:10:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This behavior is due to change in the future, but let's test\nit anyway. That helps make sure we do not accidentally\nswitch the behavior too soon while we are working in the\narea, and it means that we can easily verify the change when\nwe do make it.\n\nSigned-off-by: Jeff King \n---\nWe didn't seem to be testing this transition at all. I think it's sane\nto do so now, and Junio's \"now it is 2.0, let's switch\" patch should\nupdate the test.\n\n t/t2200-add-update.sh | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh\nindex 4cdebda..fe4f548 100755\n--- a/t/t2200-add-update.sh\n+++ b/t/t2200-add-update.sh\n@@ -80,6 +80,17 @@ test_expect_success 'change gets noticed' '\n \n '\n \n+# Note that this is scheduled to change in Git 2.0, when\n+# \"git add -u\" will become full-tree by default.\n+test_expect_success 'update did not touch files at root' '\n+\tcat >expect <<-\\EOF &&\n+\tcheck\n+\ttop\n+\tEOF\n+\tgit diff-files --name-only >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success SYMLINKS 'replace a file with a symlink' '\n \n \trm foo &&\n-- \n1.8.2.rc2.7.gef06216\n"},{"id":"211197","messageId":"20130313041037.GB5378@sigill.intra.peff.net","threadId":"33110","inReplyTo":"20130313040845.GA5057@sigill.intra.peff.net","subject":"[PATCH 2/2] add: respect add.updateroot config option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-13T04:10:37Z","receivedAt":"2013-03-13T04:10:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The `git add -u` command operates on the current subdir of\nthe repository, but is transitioning to operate on the whole\nrepository (see commit 0fa2eb5 for details). During the\ntransition period, we issue a warning. For users who have\nread and accepted the warning, there is no way to jump\ndirectly to the future behavior and silence the warning. The\nconfig option added by this patch gives them such an option.\n\nSigned-off-by: Jeff King \n---\n Documentation/config.txt | 19 +++++++++++++++++++\n builtin/add.c | 10 ++++++++--\n t/t2200-add-update.sh | 14 ++++++++++++++\n 3 files changed, 41 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex bbba728..be0f6fc 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -657,6 +657,25 @@ add.ignoreErrors::\n \tconvention for configuration variables. Newer versions of Git\n \thonor `add.ignoreErrors` as well.\n \n+add.updateroot::\n+\tHistorically, when the `git add -u` and `git add -A` commands\n+\tare run from a subdirectory of the repository and are not given\n+\tany pathspec, they have operated only on the subdirectory from\n+\twhich they were called. In a future version of git, this behavior\n+\twill be switched to operate on the full repository instead. In\n+\tthe meantime, issuing `git add -u` (or `-A`) from a subdirectory\n+\tcontinues to work on the subdirectory, but will now issue a\n+\twarning. If you are ready to move to the new behavior now,\n+\taccepting that you may be breaking existing scripts which expect\n+\tthe old behavior, you can do so by setting `add.updateroot` to\n+\t`true`.\n++\n+Note that this is meant to let early adopters move to the new behavior\n+immediately; it is not a toggle switch that will work forever. After\n+git transitions to the new behavior, this option will become a no-op;\n+`git add -u` will always update the whole tree, and `git add -u .` will\n+remain the correct way to limit to the current directory.\n+\n alias.*::\n \tCommand aliases for the linkgit:git[1] command wrapper - e.g.\n \tafter defining \"alias.last = cat-file commit HEAD\", the invocation\ndiff --git a/builtin/add.c b/builtin/add.c\nindex ab1c9e8..95fe35e 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -22,6 +22,7 @@ static int take_worktree_changes;\n };\n static int patch_interactive, add_interactive, edit_interactive;\n static int take_worktree_changes;\n+static int update_root;\n \n struct update_callback_data {\n \tint flags;\n@@ -297,6 +298,10 @@ static int add_config(const char *var, const char *value, void *cb)\n \t\tignore_add_errors = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"add.updateroot\")) {\n+\t\tupdate_root = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \treturn git_default_config(var, value, cb);\n }\n \n@@ -390,12 +395,13 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\tshort_option_with_implicit_dot = \"-u\";\n \t}\n \tif (option_with_implicit_dot && !argc) {\n+\t\tstatic const char *root[2] = { \":/\", NULL };\n \t\tstatic const char *here[2] = { \".\", NULL };\n-\t\tif (prefix)\n+\t\tif (!update_root && prefix)\n \t\t\twarn_pathless_add(option_with_implicit_dot,\n \t\t\t\t\t short_option_with_implicit_dot);\n \t\targc = 1;\n-\t\targv = here;\n+\t\targv = update_root ? root : here;\n \t}\n \n \tadd_new_files = !take_worktree_changes && !refresh_only;\ndiff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh\nindex fe4f548..b9215d3 100755\n--- a/t/t2200-add-update.sh\n+++ b/t/t2200-add-update.sh\n@@ -91,6 +91,20 @@ test_expect_success 'update did not touch files at root' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'update with add.updateroot set' '\n+\t(\n+\t\tcd dir1 &&\n+\t\techo more >>sub2 &&\n+\t\tgit -c add.updateroot=true add -u\n+\t)\n+'\n+\n+test_expect_success 'all paths are updated' '\n+\t>expect &&\n+\tgit diff-files --name-status >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success SYMLINKS 'replace a file with a symlink' '\n \n \trm foo &&\n-- \n1.8.2.rc2.7.gef06216\n"},{"id":"211201","messageId":"vpq8v5rel5o.fsf@grenoble-inp.fr","threadId":"33110","inReplyTo":"20130313041022.GA5378@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] t2200: check that \"add -u\" limits itself to subdirectory","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-03-13T08:52:51Z","receivedAt":"2013-03-13T08:52:51Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jeff King writes:\n\n> This behavior is due to change in the future, but let's test\n> it anyway.\n\nThanks. This should be merged regardless of PATCH 2/2 I think.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"211203","messageId":"vpqr4jjd5wp.fsf@grenoble-inp.fr","threadId":"33110","inReplyTo":"20130313041037.GB5378@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] add: respect add.updateroot config option","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-03-13T09:07:34Z","receivedAt":"2013-03-13T09:07:34Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jeff King writes:\n\n> For users who have read and accepted the warning, there is no way to\n> jump directly to the future behavior and silence the warning.\n\nI think the idea makes sense. The transition period is necessary for\npeople who use different versions of Git (which includes anybody\nwritting and distributing scripts), but for poor mortals who only use a\nsingle version of Git, it's nice to be able to jump to the future\nbehavior once for all as soon as possible.\n\nYour patch doesn't advertise the option in the warning message, which I\nthink is good. You may mention it the commit message that this is a\ndeliberate choice.\n\n> +add.updateroot::\n\nDetail: option names are normally camelCased => updateRoot.\n\nI think the option name needs a bit more thinking. Without reading the\ndoc,\n\n[add]\n\tupdateRoot = false\n\nwould be a very tempting thing to try. Perhaps explicitly warning when\nadd.updateroot is set to false would avoid possible confusion.\n\nIn case you had missed it, here's a previous piece of discussion on the\nsubject:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/216818/focus=216846\n\nI liked David's suggestion of using future.* to mean \"start using\nbehavior from the future right now\".\n\n> +\twhich they were called. In a future version of git, this behavior\n\nCapital \"Git\".\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"211206","messageId":"20130313092754.GA12658@sigill.intra.peff.net","threadId":"33110","inReplyTo":"vpqr4jjd5wp.fsf@grenoble-inp.fr","subject":"Re: [PATCH 2/2] add: respect add.updateroot config option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-13T09:27:54Z","receivedAt":"2013-03-13T09:27:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 13, 2013 at 10:07:34AM +0100, Matthieu Moy wrote:\n\n> Jeff King writes:\n> \n> > For users who have read and accepted the warning, there is no way to\n> > jump directly to the future behavior and silence the warning.\n> \n> I think the idea makes sense. The transition period is necessary for\n> people who use different versions of Git (which includes anybody\n> writting and distributing scripts), but for poor mortals who only use a\n> single version of Git, it's nice to be able to jump to the future\n> behavior once for all as soon as possible.\n\nI think the biggest risk is from people who think they are safe to jump,\nand then find out that some script they depend on is not ready. Even if\nthey do not even realize they are relying on it. Part of the point of\nthe transition period is to get script authors to update their scripts,\nand to let the new versions trickle down to the users.\n\n> Your patch doesn't advertise the option in the warning message, which I\n> think is good. You may mention it the commit message that this is a\n> deliberate choice.\n\nYes, it was deliberate. I can add a note.\n\n> > +add.updateroot::\n> \n> Detail: option names are normally camelCased => updateRoot.\n\nGood point, thanks.\n\n> I think the option name needs a bit more thinking. Without reading the\n> doc,\n> \n> [add]\n> \tupdateRoot = false\n> \n> would be a very tempting thing to try. Perhaps explicitly warning when\n> add.updateroot is set to false would avoid possible confusion.\n\nYeah, that occurred to me, too, hence the note in the doc. Since it\nisn't advertised elsewhere, I had hoped that anybody who discovered it\nwould see the note. I suppose we can warn when we see add.updateRoot set\nto anything but true. That feels a bit hacky, as it's possible the user\ncould be overriding an earlier setting (although that is getting kind of\ncrazy).\n\n> I liked David's suggestion of using future.* to mean \"start using\n> behavior from the future right now\".\n\nI do like that idea, as it makes the meaning of the variable more clear.\n\nI dunno. I am not all that excited about this patch, due to all of the\ncaveats that we need to communicate to the user. The current warning has\nannoyed me a few times, but perhaps it is still too soon, and my fingers\nand brain just need retraining. I think a config option could help some\npeople, but maybe it will end up hurting more. I'm inclined at this\npoint to table the patch for now and see how I feel in a few weeks.\n\nI do think patch 1/2 makes sense regardless.\n\n-Peff\n"},{"id":"211220","messageId":"7vmwu747tf.fsf@alter.siamese.dyndns.org","threadId":"33110","inReplyTo":"20130313092754.GA12658@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] add: respect add.updateroot config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-13T15:51:08Z","receivedAt":"2013-03-13T15:51:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> Your patch doesn't advertise the option in the warning message, which I\n>> think is good. You may mention it the commit message that this is a\n>> deliberate choice.\n>\n> Yes, it was deliberate. I can add a note.\n>\n>> > +add.updateroot::\n>> \n>> Detail: option names are normally camelCased => updateRoot.\n>\n> Good point, thanks.\n>\n>> I think the option name needs a bit more thinking. Without reading the\n>> doc,\n>> \n>> [add]\n>> \tupdateRoot = false\n>> \n>> would be a very tempting thing to try. Perhaps explicitly warning when\n>> add.updateroot is set to false would avoid possible confusion.\n\nThis is essentially the same as Matthieu's add.use2dot0Behavior but\nwith that \"it is an error to explicitly set it to false\" twist, it\nalleviates one of the worries. It still has the same \"the user saw\nit mentioned on stackoverflow and sets it without understanding that\nthe behaviour is getting changed\" effect.\n\nAlso it won't give chance for scripts to get fixed, even though I\nsuspect that instances of \"add -u/-A\" without pathspec end users\nwrite in their custom scripts almost always would want to be\ntree-wide and it is a bug that they do not pass \":/\" to them.\n\nThe \"future.*\" (I'd rather call that \"migration.*\") approach with\nthe \"never set it to false\" twist is probably OK from the \"complaint\navoidance\" perspective. The user cannot later complain \"I tried to\nsquelch the advice but at the same time I ended up adding updated\ncontents outside my diretory\", without getting told \"That is the\ndocumented behaviour, isn't it?\" But I still think it is much less\nnice from \"avoid hurting the users\" perspective. If the way to\nsquelch the user learns were to say \"git add -u .\", where the user\nexplicitly says \"take the updated contents from this directory and\nbelow\", there is no room for confusion. We won't hear complaints\neither way, but with the \"future.*\" the reason why we won't hear\ncomplaints is because the users do not have excuse to complain.\nWith the \"require explicit .\", it is because the users won't get\nhurt in the first place.\n\n> I dunno. I am not all that excited about this patch, due to all of the\n> caveats that we need to communicate to the user. The current warning has\n> annoyed me a few times, but perhaps it is still too soon, and my fingers\n> and brain just need retraining. I think a config option could help some\n> people, but maybe it will end up hurting more. I'm inclined at this\n> point to table the patch for now and see how I feel in a few weeks.\n>\n> I do think patch 1/2 makes sense regardless.\n\nThese two concluding paragraphs match my current thinking.\n\nThanks.\n"},{"id":"211235","messageId":"7vli9r2o06.fsf@alter.siamese.dyndns.org","threadId":"33110","inReplyTo":"20130313041022.GA5378@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] t2200: check that \"add -u\" limits itself to subdirectory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-13T17:44:25Z","receivedAt":"2013-03-13T17:44:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> We didn't seem to be testing this transition at all. I think it's sane\n> to do so now, and Junio's \"now it is 2.0, let's switch\" patch should\n> update the test.\n\nYes, but I am not sure if this is testing the right thing.\n\n> +# Note that this is scheduled to change in Git 2.0, when\n> +# \"git add -u\" will become full-tree by default.\n> +test_expect_success 'update did not touch files at root' '\n> +\tcat >expect <<-\\EOF &&\n> +\tcheck\n> +\ttop\n> +\tEOF\n> +\tgit diff-files --name-only >actual &&\n> +\ttest_cmp expect actual\n> +'\n\nThe last \"git add -u\" we have beforet his block is this test piece:\n\n test_expect_success 'update from a subdirectory' '\n (\n cd dir1 &&\n echo more >sub2 &&\n git add -u sub2\n )\n '\n\nThat is not \"git add -u\" without pathspec, which is the only thing\nwe are transitioning at Git 2.0 boundary.\n"},{"id":"211278","messageId":"20130314064404.GA6103@sigill.intra.peff.net","threadId":"33110","inReplyTo":"7vli9r2o06.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] t2200: check that \"add -u\" limits itself to subdirectory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-14T06:44:04Z","receivedAt":"2013-03-14T06:44:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 13, 2013 at 10:44:25AM -0700, Junio C Hamano wrote:\n\n> > +# Note that this is scheduled to change in Git 2.0, when\n> > +# \"git add -u\" will become full-tree by default.\n> > +test_expect_success 'update did not touch files at root' '\n> > +\tcat >expect <<-\\EOF &&\n> > +\tcheck\n> > +\ttop\n> > +\tEOF\n> > +\tgit diff-files --name-only >actual &&\n> > +\ttest_cmp expect actual\n> > +'\n> \n> The last \"git add -u\" we have beforet his block is this test piece:\n> \n> test_expect_success 'update from a subdirectory' '\n> (\n> cd dir1 &&\n> echo more >sub2 &&\n> git add -u sub2\n> )\n> '\n> \n> That is not \"git add -u\" without pathspec, which is the only thing\n> we are transitioning at Git 2.0 boundary.\n\nOops, you're right. I just saw the \"cd\" and totally missed the pathspec.\nThe correct test should be:\n\n-- >8 --\nSubject: [PATCH] t2200: check that \"add -u\" limits itself to subdirectory\n\nThis behavior is due to change in the future, but let's test\nit anyway. That helps make sure we do not accidentally\nswitch the behavior too soon while we are working in the\narea, and it means that we can easily verify the change when\nwe do make it.\n\nSigned-off-by: Jeff King \n---\n t/t2200-add-update.sh | 16 ++++++++++++++++\n 1 file changed, 16 insertions(+)\n\ndiff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh\nindex 4cdebda..c317254 100755\n--- a/t/t2200-add-update.sh\n+++ b/t/t2200-add-update.sh\n@@ -80,6 +80,22 @@ test_expect_success 'change gets noticed' '\n \n '\n \n+# Note that this is scheduled to change in Git 2.0, when\n+# \"git add -u\" will become full-tree by default.\n+test_expect_success 'non-limited update in subdir leaves root alone' '\n+\t(\n+\t\tcd dir1 &&\n+\t\techo even more >>sub2 &&\n+\t\tgit add -u\n+\t) &&\n+\tcat >expect <<-\\EOF &&\n+\tcheck\n+\ttop\n+\tEOF\n+\tgit diff-files --name-only >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success SYMLINKS 'replace a file with a symlink' '\n \n \trm foo &&\n-- \n1.8.2.rc2.7.gef06216\n"},{"id":"211290","messageId":"vpqwqta2m0y.fsf@grenoble-inp.fr","threadId":"33110","inReplyTo":"7vmwu747tf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] add: respect add.updateroot config option","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-03-14T12:39:25Z","receivedAt":"2013-03-14T12:39:25Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> It still has the same \"the user saw\n> it mentioned on stackoverflow and sets it without understanding that\n> the behaviour is getting changed\" effect.\n\nI'm not so worried about this point, as the mechanism is temporary, and\nit will take time before answers on stackoverflow reach the mass. The\ninitial state is that the option is not discoverable without reading the\ndocumentation, and Jeff's documentation is very clear that this is\ntemporary. People reading the doc are usually not the kind of people\ngiving dumb answers on the web.\n\nAnd even if some people set the option without understanding the\nconsequences, it's not that terrible. They'll just get the transition\nperiod shortened.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"211627","messageId":"20130319034415.GI5062@elie.Belkin","threadId":"33110","inReplyTo":"20130313041037.GB5378@sigill.intra.peff.net","subject":"[PATCH 0/4] make pathless 'add [-u|-A]' warning less noisy","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-19T03:44:15Z","receivedAt":"2013-03-19T03:44:15Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJeff King wrote:\n\n> The\n> config option added by this patch gives them such an option.\n\nI suspect the need for this config option is a sign that the warning\nis too eager. After all, the whole idea of the change being safe is\nthat it shouldn't make a difference the way people usually use git,\nno?\n\nIn other words, how about the following patches? With them applied,\nhopefully no one would mind even if the warning becomes a fatal error.\n\nLooking forward to your thoughts,\n\nJonathan Nieder (4):\n add: make pathless 'add [-u|-A]' warning a file-global function\n add: make warn_pathless_add() a no-op after first call\n add -u: only show pathless 'add -u' warning when changes exist outside cwd\n add -A: only show pathless 'add -A' warning when changes exist outside cwd\n\n builtin/add.c | 142 ++++++++++++++++++++++++++++++++++++++++------------------\n 1 file changed, 99 insertions(+), 43 deletions(-)\n"},{"id":"211628","messageId":"20130319034545.GJ5062@elie.Belkin","threadId":"33110","inReplyTo":"20130319034415.GI5062@elie.Belkin","subject":"[PATCH 1/4] add: make pathless 'add [-u|-A]' warning a file-global function","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-19T03:45:46Z","receivedAt":"2013-03-19T03:45:46Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Currently warn_pathless_add() is only called directly by cmd_add(),\nbut that is about to change. Move its definition higher in the file\nand pass the \"--update\" or \"--all\" option name used in its message\nthrough globals instead of function arguments to make it easier to\ncall without passing values that will not change through the call\nchain.\n\nSigned-off-by: Jonathan Nieder \n---\n builtin/add.c | 69 +++++++++++++++++++++++++++++++----------------------------\n 1 file changed, 36 insertions(+), 33 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex ab1c9e8f..a4028eea 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -28,6 +28,41 @@ struct update_callback_data {\n \tint add_errors;\n };\n \n+static const char *option_with_implicit_dot;\n+static const char *short_option_with_implicit_dot;\n+\n+static void warn_pathless_add(void)\n+{\n+\tassert(option_with_implicit_dot && short_option_with_implicit_dot);\n+\n+\t/*\n+\t * To be consistent with \"git add -p\" and most Git\n+\t * commands, we should default to being tree-wide, but\n+\t * this is not the original behavior and can't be\n+\t * changed until users trained themselves not to type\n+\t * \"git add -u\" or \"git add -A\". For now, we warn and\n+\t * keep the old behavior. Later, the behavior can be changed\n+\t * to tree-wide, keeping the warning for a while, and\n+\t * eventually we can drop the warning.\n+\t */\n+\twarning(_(\"The behavior of 'git add %s (or %s)' with no path argument from a\\n\"\n+\t\t \"subdirectory of the tree will change in Git 2.0 and should not be used anymore.\\n\"\n+\t\t \"To add content for the whole tree, run:\\n\"\n+\t\t \"\\n\"\n+\t\t \" git add %s :/\\n\"\n+\t\t \" (or git add %s :/)\\n\"\n+\t\t \"\\n\"\n+\t\t \"To restrict the command to the current directory, run:\\n\"\n+\t\t \"\\n\"\n+\t\t \" git add %s .\\n\"\n+\t\t \" (or git add %s .)\\n\"\n+\t\t \"\\n\"\n+\t\t \"With the current Git version, the command is restricted to the current directory.\"),\n+\t\toption_with_implicit_dot, short_option_with_implicit_dot,\n+\t\toption_with_implicit_dot, short_option_with_implicit_dot,\n+\t\toption_with_implicit_dot, short_option_with_implicit_dot);\n+}\n+\n static int fix_unmerged_status(struct diff_filepair *p,\n \t\t\t struct update_callback_data *data)\n {\n@@ -321,35 +356,6 @@ static int add_files(struct dir_struct *dir, int flags)\n \treturn exit_status;\n }\n \n-static void warn_pathless_add(const char *option_name, const char *short_name) {\n-\t/*\n-\t * To be consistent with \"git add -p\" and most Git\n-\t * commands, we should default to being tree-wide, but\n-\t * this is not the original behavior and can't be\n-\t * changed until users trained themselves not to type\n-\t * \"git add -u\" or \"git add -A\". For now, we warn and\n-\t * keep the old behavior. Later, the behavior can be changed\n-\t * to tree-wide, keeping the warning for a while, and\n-\t * eventually we can drop the warning.\n-\t */\n-\twarning(_(\"The behavior of 'git add %s (or %s)' with no path argument from a\\n\"\n-\t\t \"subdirectory of the tree will change in Git 2.0 and should not be used anymore.\\n\"\n-\t\t \"To add content for the whole tree, run:\\n\"\n-\t\t \"\\n\"\n-\t\t \" git add %s :/\\n\"\n-\t\t \" (or git add %s :/)\\n\"\n-\t\t \"\\n\"\n-\t\t \"To restrict the command to the current directory, run:\\n\"\n-\t\t \"\\n\"\n-\t\t \" git add %s .\\n\"\n-\t\t \" (or git add %s .)\\n\"\n-\t\t \"\\n\"\n-\t\t \"With the current Git version, the command is restricted to the current directory.\"),\n-\t\toption_name, short_name,\n-\t\toption_name, short_name,\n-\t\toption_name, short_name);\n-}\n-\n int cmd_add(int argc, const char **argv, const char *prefix)\n {\n \tint exit_status = 0;\n@@ -360,8 +366,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tint add_new_files;\n \tint require_pathspec;\n \tchar *seen = NULL;\n-\tconst char *option_with_implicit_dot = NULL;\n-\tconst char *short_option_with_implicit_dot = NULL;\n \n \tgit_config(add_config, NULL);\n \n@@ -392,8 +396,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tif (option_with_implicit_dot && !argc) {\n \t\tstatic const char *here[2] = { \".\", NULL };\n \t\tif (prefix)\n-\t\t\twarn_pathless_add(option_with_implicit_dot,\n-\t\t\t\t\t short_option_with_implicit_dot);\n+\t\t\twarn_pathless_add();\n \t\targc = 1;\n \t\targv = here;\n \t}\n-- \n1.8.2.rc3\n"},{"id":"211629","messageId":"20130319034636.GK5062@elie.Belkin","threadId":"33110","inReplyTo":"20130319034415.GI5062@elie.Belkin","subject":"[PATCH 2/4] add: make warn_pathless_add() a no-op after first call","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-19T03:46:36Z","receivedAt":"2013-03-19T03:46:36Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Make warn_pathless_add() print its warning the first time it is called\nand do nothing if called again. This will make it easier to show the\nwarning on the fly when a relevant condition is detected without\nrisking showing it multiple times when multiple such conditions hold.\n\nSigned-off-by: Jonathan Nieder \n---\n builtin/add.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex a4028eea..a424e69d 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -33,8 +33,13 @@ static const char *short_option_with_implicit_dot;\n \n static void warn_pathless_add(void)\n {\n+\tstatic int shown;\n \tassert(option_with_implicit_dot && short_option_with_implicit_dot);\n \n+\tif (shown)\n+\t\treturn;\n+\tshown = 1;\n+\n \t/*\n \t * To be consistent with \"git add -p\" and most Git\n \t * commands, we should default to being tree-wide, but\n-- \n1.8.2.rc3\n"},{"id":"211630","messageId":"20130319034822.GL5062@elie.Belkin","threadId":"33110","inReplyTo":"20130319034415.GI5062@elie.Belkin","subject":"[PATCH 3/4] add -u: only show pathless 'add -u' warning when changes exist outside cwd","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-19T03:48:22Z","receivedAt":"2013-03-19T03:48:22Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"A common workflow in large projects is to chdir into a subdirectory of\ninterest and only do work there:\n\n\tcd src\n\tvi foo.c\n\tmake test\n\tgit add -u\n\tgit commit\n\nThe upcoming change to 'git add -u' behavior would not affect such a\nworkflow: when the only changes present are in the current directory,\n'git add -u' will add all changes, and whether that happens via an\nimplicit \".\" or implicit \":/\" parameter is an unimportant\nimplementation detail.\n\nThe warning about use of 'git add -u' with no pathspec is annoying\nbecause it serves no purpose in this case. So suppress the warning\nunless there are changes outside the cwd that are not being added.\n\nSigned-off-by: Jonathan Nieder \n---\n builtin/add.c | 51 ++++++++++++++++++++++++++++++++++++++++++++-------\n 1 file changed, 44 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex a424e69d..f05ec1c1 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -89,6 +89,22 @@ static int fix_unmerged_status(struct diff_filepair *p,\n \t\treturn DIFF_STATUS_MODIFIED;\n }\n \n+static void warn_if_outside_pathspec(struct diff_queue_struct *q,\n+\t\t\t\t struct diff_options *opt, void *cbdata)\n+{\n+\tint i;\n+\tconst char **pathspec = cbdata;\n+\n+\tfor (i = 0; i < q->nr; i++) {\n+\t\tconst char *path = q->queue[i]->one->path;\n+\n+\t\tif (!match_pathspec(pathspec, path, strlen(path), 0, NULL)) {\n+\t\t\twarn_pathless_add();\n+\t\t\treturn;\n+\t\t}\n+\t}\n+}\n+\n static void update_callback(struct diff_queue_struct *q,\n \t\t\t struct diff_options *opt, void *cbdata)\n {\n@@ -121,20 +137,26 @@ static void update_callback(struct diff_queue_struct *q,\n \t}\n }\n \n-int add_files_to_cache(const char *prefix, const char **pathspec, int flags)\n+static void diff_files_with_callback(const char *prefix, const char **pathspec,\n+\t\t\t\t diff_format_fn_t callback, void *data)\n {\n-\tstruct update_callback_data data;\n \tstruct rev_info rev;\n \tinit_revisions(&rev, prefix);\n \tsetup_revisions(0, NULL, &rev, NULL);\n \tinit_pathspec(&rev.prune_data, pathspec);\n \trev.diffopt.output_format = DIFF_FORMAT_CALLBACK;\n-\trev.diffopt.format_callback = update_callback;\n-\tdata.flags = flags;\n-\tdata.add_errors = 0;\n-\trev.diffopt.format_callback_data = &data;\n+\trev.diffopt.format_callback = callback;\n+\trev.diffopt.format_callback_data = data;\n \trev.max_count = 0; /* do not compare unmerged paths with stage #2 */\n \trun_diff_files(&rev, DIFF_RACY_IS_MODIFIED);\n+}\n+\n+int add_files_to_cache(const char *prefix, const char **pathspec, int flags)\n+{\n+\tstruct update_callback_data data;\n+\tdata.flags = flags;\n+\tdata.add_errors = 0;\n+\tdiff_files_with_callback(prefix, pathspec, update_callback, &data);\n \treturn !!data.add_errors;\n }\n \n@@ -371,6 +393,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tint add_new_files;\n \tint require_pathspec;\n \tchar *seen = NULL;\n+\tint implicit_dot = 0;\n \n \tgit_config(add_config, NULL);\n \n@@ -400,10 +423,11 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t}\n \tif (option_with_implicit_dot && !argc) {\n \t\tstatic const char *here[2] = { \".\", NULL };\n-\t\tif (prefix)\n+\t\tif (prefix && addremove)\n \t\t\twarn_pathless_add();\n \t\targc = 1;\n \t\targv = here;\n+\t\timplicit_dot = 1;\n \t}\n \n \tadd_new_files = !take_worktree_changes && !refresh_only;\n@@ -450,6 +474,19 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\tgoto finish;\n \t}\n \n+\t/*\n+\t * Check if \"git add -A\" or \"git add -u\" was run from a\n+\t * subdirectory with a modified file outside that directory,\n+\t * and warn if so.\n+\t *\n+\t * \"git add -u\" will behave like \"git add -u :/\" instead of\n+\t * \"git add -u .\" in the future. This warning prepares for\n+\t * that change.\n+\t */\n+\tif (implicit_dot)\n+\t\tdiff_files_with_callback(prefix, NULL,\n+\t\t\t\twarn_if_outside_pathspec, pathspec);\n+\n \tif (pathspec) {\n \t\tint i;\n \t\tstruct path_exclude_check check;\n-- \n1.8.2.rc3\n"},{"id":"211631","messageId":"20130319034910.GM5062@elie.Belkin","threadId":"33110","inReplyTo":"20130319034415.GI5062@elie.Belkin","subject":"[PATCH 4/4] add -A: only show pathless 'add -A' warning when changes exist outside cwd","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-19T03:49:11Z","receivedAt":"2013-03-19T03:49:11Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"In the spirit of the recent similar change for 'git add -u', avoid\npestering users that restrict their attention to a subdirectory and\nwill not be affected by the coming change in the behavior of pathless\n'git add -A'.\n\nSigned-off-by: Jonathan Nieder \n---\n builtin/add.c | 21 ++++++++++++++++-----\n 1 file changed, 16 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex f05ec1c1..56ac4519 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -160,7 +160,9 @@ int add_files_to_cache(const char *prefix, const char **pathspec, int flags)\n \treturn !!data.add_errors;\n }\n \n-static char *prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)\n+#define WARN_IMPLICIT_DOT (1u << 0)\n+static char *prune_directory(struct dir_struct *dir, const char **pathspec,\n+\t\t\t int prefix, unsigned flag)\n {\n \tchar *seen;\n \tint i, specs;\n@@ -177,6 +179,16 @@ static char *prune_directory(struct dir_struct *dir, const char **pathspec, int\n \t\tif (match_pathspec(pathspec, entry->name, entry->len,\n \t\t\t\t prefix, seen))\n \t\t\t*dst++ = entry;\n+\t\telse if (flag & WARN_IMPLICIT_DOT)\n+\t\t\t/*\n+\t\t\t * \"git add -A\" was run from a subdirectory with a\n+\t\t\t * new file outside that directory.\n+\t\t\t *\n+\t\t\t * \"git add -A\" will behave like \"git add -A :/\"\n+\t\t\t * instead of \"git add -A .\" in the future.\n+\t\t\t * Warn about the coming behavior change.\n+\t\t\t */\n+\t\t\twarn_pathless_add();\n \t}\n \tdir->nr = dst - dir->entries;\n \tadd_pathspec_matches_against_index(pathspec, seen, specs);\n@@ -423,8 +435,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t}\n \tif (option_with_implicit_dot && !argc) {\n \t\tstatic const char *here[2] = { \".\", NULL };\n-\t\tif (prefix && addremove)\n-\t\t\twarn_pathless_add();\n \t\targc = 1;\n \t\targv = here;\n \t\timplicit_dot = 1;\n@@ -464,9 +474,10 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\t}\n \n \t\t/* This picks up the paths that are not tracked */\n-\t\tbaselen = fill_directory(&dir, pathspec);\n+\t\tbaselen = fill_directory(&dir, implicit_dot ? NULL : pathspec);\n \t\tif (pathspec)\n-\t\t\tseen = prune_directory(&dir, pathspec, baselen);\n+\t\t\tseen = prune_directory(&dir, pathspec, baselen,\n+\t\t\t\t\timplicit_dot ? WARN_IMPLICIT_DOT : 0);\n \t}\n \n \tif (refresh_only) {\n-- \n1.8.2.rc3\n"},{"id":"211633","messageId":"20130319042514.GA4807@sigill.intra.peff.net","threadId":"33110","inReplyTo":"20130319034415.GI5062@elie.Belkin","subject":"Re: [PATCH 0/4] make pathless 'add [-u|-A]' warning less noisy","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-19T04:25:14Z","receivedAt":"2013-03-19T04:25:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 18, 2013 at 08:44:15PM -0700, Jonathan Nieder wrote:\n\n> > The\n> > config option added by this patch gives them such an option.\n> \n> I suspect the need for this config option is a sign that the warning\n> is too eager. After all, the whole idea of the change being safe is\n> that it shouldn't make a difference the way people usually use git,\n> no?\n> \n> In other words, how about the following patches? With them applied,\n> hopefully no one would mind even if the warning becomes a fatal error.\n\nClever. I think it would help in my case. I sometimes follow the\nworkflow you describe in patch 3 (i.e., just working in a subdir), and\nsometimes do something more like:\n\n $ vi foo.c\n $ cd t\n $ vi tXXXX-foo.sh\n $ ./tXXXX-foo.sh\n $ git add -u\n\nWith your patches, we would continue to warn about the second case, but\nI think that is a good thing; git is not doing what I want. But by\nreducing the false positives from the first case, I would start to\nactually pay attention to the warning more.\n\n> Jonathan Nieder (4):\n> add: make pathless 'add [-u|-A]' warning a file-global function\n> add: make warn_pathless_add() a no-op after first call\n> add -u: only show pathless 'add -u' warning when changes exist outside cwd\n> add -A: only show pathless 'add -A' warning when changes exist outside cwd\n\nI don't see anything obviously wrong with the patches themselves. I\nwonder if we would want to change the warning to be more explicit that\nyes, there really were files that were impacted by this. And possibly\neven list them.\n\nI suspect I would not even mind that becoming the final behavior. I.e.,\ngoing to:\n\n $ cd subdir && git add -u\n warning: Using 'git add -u' without a pathspec operates only on the\n current subdirectory. Updates from the following files were NOT\n staged:\n\n file1\n file2\n other-subdir/file3\n\nnow, and then eventually converting the warning into a fatal error (and\ndemanding that the user use \":/\" or \".\" as appropriate).\n\nBut in the long run, I guess defaulting to \":/\" will be more convenient,\nso there is no point in complaining about the ambiguity forever. And in\nthat case, since the warning is just a placeholder, I don't know that\nit's worth much effort to make it fancier.\n\n-Peff\n"},{"id":"211634","messageId":"7vli9kkoci.fsf@alter.siamese.dyndns.org","threadId":"33110","inReplyTo":"20130319034822.GL5062@elie.Belkin","subject":"Re: [PATCH 3/4] add -u: only show pathless 'add -u' warning when changes exist outside cwd","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-19T04:25:33Z","receivedAt":"2013-03-19T04:25:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> A common workflow in large projects is to chdir into a subdirectory of\n> interest and only do work there:\n>\n> \tcd src\n> \tvi foo.c\n> \tmake test\n> \tgit add -u\n> \tgit commit\n>\n> The upcoming change to 'git add -u' behavior would not affect such a\n> workflow: when the only changes present are in the current directory,\n> 'git add -u' will add all changes, and whether that happens via an\n> implicit \".\" or implicit \":/\" parameter is an unimportant\n> implementation detail.\n>\n> The warning about use of 'git add -u' with no pathspec is annoying\n> because it serves no purpose in this case. So suppress the warning\n> unless there are changes outside the cwd that are not being added.\n\nThat is a logical extension of the reason why we do not emit\nwarnings when run at the top level. A user who has known about and\nis very much accustomed to the \"current directory only\" behaviour\nmay run \"git add -u/-A\" always from the top in the current project\nshe happens to be working on until Git 2.0 happens, and will not get\nany warnings. We are already robbing the chance to learn about and\nprepare for the upcoming change from her. And this patch makes it\neven more so. It does not make things fundamentally worse, but it\nmakes it more likely to hurt such a user.\n\nThe implemenation appears to run an extra diff_files() in addition\nto the one we already have to run in order to implement the \"add -u\"\nto collect modified/deleted paths. Is that the best we can do? \n\nI am wondering if we can special case the no-pathspec case to have\nadd_files_to_cache() call underlying run_diff_files() without any\npathspec, inspect the paths that are modified and/or deleted in the\nupdate_callback, add ones that are under the $prefix while noticing\nthe ones outside as warning worthy events.\n"},{"id":"211636","messageId":"20130319052805.GO5062@elie.Belkin","threadId":"33110","inReplyTo":"7vli9kkoci.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/4] add -u: only show pathless 'add -u' warning when changes exist outside cwd","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-19T05:28:06Z","receivedAt":"2013-03-19T05:28:06Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> The implemenation appears to run an extra diff_files() in addition\n> to the one we already have to run in order to implement the \"add -u\"\n> to collect modified/deleted paths. Is that the best we can do? \n>\n> I am wondering if we can special case the no-pathspec case to have\n> add_files_to_cache() call underlying run_diff_files() without any\n> pathspec, inspect the paths that are modified and/or deleted in the\n> update_callback, add ones that are under the $prefix while noticing\n> the ones outside as warning worthy events.\n\nYes, that can work, for example like this (replacing the patch you're\nreplying to).\n\n-- >8 --\nSubject: add -u: only show pathless 'add -u' warning when changes exist outside cwd\n\nA common workflow in large projects is to chdir into a subdirectory of\ninterest and only do work there:\n\n\tcd src\n\tvi foo.c\n\tmake test\n\tgit add -u\n\tgit commit\n\nThe upcoming change to 'git add -u' behavior would not affect such a\nworkflow: when the only changes present are in the current directory,\n'git add -u' will add all changes, and whether that happens via an\nimplicit \".\" or implicit \":/\" parameter is an unimportant\nimplementation detail.\n\nThe warning about use of 'git add -u' with no pathspec is annoying\nbecause it serves no purpose in this case. So suppress the warning\nunless there are changes outside the cwd that are not being added.\n\nSigned-off-by: Jonathan Nieder \n---\n builtin/add.c | 41 +++++++++++++++++++++++++++++++++++++----\n 1 file changed, 37 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex a424e69d..e3ed5d93 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -26,6 +26,7 @@ static int take_worktree_changes;\n struct update_callback_data {\n \tint flags;\n \tint add_errors;\n+\tconst char **implicit_dot;\n };\n \n static const char *option_with_implicit_dot;\n@@ -94,10 +95,26 @@ static void update_callback(struct diff_queue_struct *q,\n {\n \tint i;\n \tstruct update_callback_data *data = cbdata;\n+\tconst char **implicit_dot = data->implicit_dot;\n \n \tfor (i = 0; i < q->nr; i++) {\n \t\tstruct diff_filepair *p = q->queue[i];\n \t\tconst char *path = p->one->path;\n+\n+\t\t/*\n+\t\t * Check if \"git add -A\" or \"git add -u\" was run from a\n+\t\t * subdirectory with a modified file outside that directory,\n+\t\t * and warn if so.\n+\t\t *\n+\t\t * \"git add -u\" will behave like \"git add -u :/\" instead of\n+\t\t * \"git add -u .\" in the future. This warning prepares for\n+\t\t * that change.\n+\t\t */\n+\t\tif (implicit_dot &&\n+\t\t !match_pathspec(implicit_dot, path, strlen(path), 0, NULL)) {\n+\t\t\twarn_pathless_add();\n+\t\t\tcontinue;\n+\t\t}\n \t\tswitch (fix_unmerged_status(p, data)) {\n \t\tdefault:\n \t\t\tdie(_(\"unexpected diff status %c\"), p->status);\n@@ -121,17 +138,30 @@ static void update_callback(struct diff_queue_struct *q,\n \t}\n }\n \n+#define ADD_CACHE_IMPLICIT_DOT 32\n int add_files_to_cache(const char *prefix, const char **pathspec, int flags)\n {\n \tstruct update_callback_data data;\n \tstruct rev_info rev;\n+\n+\tdata.flags = flags & ~ADD_CACHE_IMPLICIT_DOT;\n+\tdata.add_errors = 0;\n+\tdata.implicit_dot = NULL;\n+\tif (flags & ADD_CACHE_IMPLICIT_DOT) {\n+\t\t/*\n+\t\t * Check for modified files throughout the worktree so\n+\t\t * update_callback has a chance to warn about changes\n+\t\t * outside the cwd.\n+\t\t */\n+\t\tdata.implicit_dot = pathspec;\n+\t\tpathspec = NULL;\n+\t}\n+\n \tinit_revisions(&rev, prefix);\n \tsetup_revisions(0, NULL, &rev, NULL);\n \tinit_pathspec(&rev.prune_data, pathspec);\n \trev.diffopt.output_format = DIFF_FORMAT_CALLBACK;\n \trev.diffopt.format_callback = update_callback;\n-\tdata.flags = flags;\n-\tdata.add_errors = 0;\n \trev.diffopt.format_callback_data = &data;\n \trev.max_count = 0; /* do not compare unmerged paths with stage #2 */\n \trun_diff_files(&rev, DIFF_RACY_IS_MODIFIED);\n@@ -371,6 +401,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tint add_new_files;\n \tint require_pathspec;\n \tchar *seen = NULL;\n+\tint implicit_dot = 0;\n \n \tgit_config(add_config, NULL);\n \n@@ -400,10 +431,11 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t}\n \tif (option_with_implicit_dot && !argc) {\n \t\tstatic const char *here[2] = { \".\", NULL };\n-\t\tif (prefix)\n+\t\tif (prefix && addremove)\n \t\t\twarn_pathless_add();\n \t\targc = 1;\n \t\targv = here;\n+\t\timplicit_dot = 1;\n \t}\n \n \tadd_new_files = !take_worktree_changes && !refresh_only;\n@@ -416,7 +448,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\t (intent_to_add ? ADD_CACHE_INTENT : 0) |\n \t\t (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |\n \t\t (!(addremove || take_worktree_changes)\n-\t\t ? ADD_CACHE_IGNORE_REMOVAL : 0));\n+\t\t ? ADD_CACHE_IGNORE_REMOVAL : 0)) |\n+\t\t (implicit_dot ? ADD_CACHE_IMPLICIT_DOT : 0);\n \n \tif (require_pathspec && argc == 0) {\n \t\tfprintf(stderr, _(\"Nothing specified, nothing added.\\n\"));\n-- \n1.8.2.rc3\n"},{"id":"211637","messageId":"20130319053402.GP5062@elie.Belkin","threadId":"33110","inReplyTo":"7vli9kkoci.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/4] add -u: only show pathless 'add -u' warning when changes exist outside cwd","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-19T05:34:02Z","receivedAt":"2013-03-19T05:34:02Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> The implemenation appears to run an extra diff_files() in addition\n> to the one we already have to run in order to implement the \"add -u\"\n> to collect modified/deleted paths. Is that the best we can do? \n\nAt least the following should be squashed in to avoid wasted effort in\nthe easy case (cwd at top of worktree). Thanks for catching it.\n\ndiff --git i/builtin/add.c w/builtin/add.c\nindex f05ec1c1..8e4cdfae 100644\n--- i/builtin/add.c\n+++ w/builtin/add.c\n@@ -483,7 +483,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t * \"git add -u .\" in the future. This warning prepares for\n \t * that change.\n \t */\n-\tif (implicit_dot)\n+\tif (prefix && implicit_dot)\n \t\tdiff_files_with_callback(prefix, NULL,\n \t\t\t\twarn_if_outside_pathspec, pathspec);\n \n"},{"id":"211638","messageId":"CACsJy8AjqwGVRRiQGMG0AN5qqtvxLk8FHKfUzCZB-7BykrHfjg@mail.gmail.com","threadId":"33110","inReplyTo":"7vli9kkoci.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/4] add -u: only show pathless 'add -u' warning when changes exist outside cwd","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-03-19T05:37:59Z","receivedAt":"2013-03-19T05:37:59Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Mar 19, 2013 at 11:25 AM, Junio C Hamano wrote:\n> I am wondering if we can special case the no-pathspec case to have\n> add_files_to_cache() call underlying run_diff_files() without any\n> pathspec, inspect the paths that are modified and/or deleted in the\n> update_callback, add ones that are under the $prefix while noticing\n> the ones outside as warning worthy events.\n\nMy concern is run full-tree diff can be expensive on large repos (one\nof the reasons the user may choose to work from within a\nsubdirectory). We can exit as soon as we find a difference outside\n$prefix. But in case we find nothing, we need to diff the whole\nworktree.\n-- \nDuy\n"},{"id":"211639","messageId":"20130319054410.GQ5062@elie.Belkin","threadId":"33110","inReplyTo":"CACsJy8AjqwGVRRiQGMG0AN5qqtvxLk8FHKfUzCZB-7BykrHfjg@mail.gmail.com","subject":"Re: [PATCH 3/4] add -u: only show pathless 'add -u' warning when changes exist outside cwd","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-19T05:44:10Z","receivedAt":"2013-03-19T05:44:10Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Duy Nguyen wrote:\n\n> My concern is run full-tree diff can be expensive on large repos (one\n> of the reasons the user may choose to work from within a\n> subdirectory). We can exit as soon as we find a difference outside\n> $prefix. But in case we find nothing, we need to diff the whole\n> worktree.\n\nYes. This is an argument for the single-diff approach that Junio\nsuggested, since at least it's not significantly slower than the\nfuture default behavior (\"git add -u :/\").\n\nSysadmins and others helping to train users will need to make sure\npeople working on large repos understand that they can use \"git add -u .\"\nto avoid a speed penalty.\n\nThanks for looking it over,\nJonathan\n"},{"id":"211640","messageId":"vpq1ubb3o5g.fsf@grenoble-inp.fr","threadId":"33110","inReplyTo":"20130319034822.GL5062@elie.Belkin","subject":"Re: [PATCH 3/4] add -u: only show pathless 'add -u' warning when changes exist outside cwd","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-03-19T06:21:47Z","receivedAt":"2013-03-19T06:21:47Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jonathan Nieder writes:\n\n> The warning about use of 'git add -u' with no pathspec is annoying\n> because it serves no purpose in this case. So suppress the warning\n> unless there are changes outside the cwd that are not being added.\n\nNo time to review the code now. I thought about implementing something\nlike that, but did not do it because I didn't want the change in the\ncode to be too big. At some point, we'll have to remove the warning and\nit's easier with my version than with yours. But the \"damage\" to the\ncode do not seem too big, so that's probably OK and will actually reduce\nthe pain for some users.\n\nDiscussions showed that many people were already using \"git add -u\"\nwithout knowing whether it was full tree or not, so for these people the\nchange is somehow harmless anyway ;-).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"211674","messageId":"7va9pzl9nh.fsf@alter.siamese.dyndns.org","threadId":"33110","inReplyTo":"20130319052805.GO5062@elie.Belkin","subject":"Re: [PATCH 3/4] add -u: only show pathless 'add -u' warning when changes exist outside cwd","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-19T14:57:38Z","receivedAt":"2013-03-19T14:57:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Yes, that can work, for example like this (replacing the patch you're\n> replying to).\n\nI think that would be a better approach if we were to do this. I\nstill have the same reservation that \"this is fundamentally not\nworse but still hurts the users more\".\n\n> +\t\t/*\n> +\t\t * Check if \"git add -A\" or \"git add -u\" was run from a\n> +\t\t * subdirectory with a modified file outside that directory,\n> +\t\t * and warn if so.\n> +\t\t *\n> +\t\t * \"git add -u\" will behave like \"git add -u :/\" instead of\n> +\t\t * \"git add -u .\" in the future. This warning prepares for\n> +\t\t * that change.\n> +\t\t */\n> +\t\tif (implicit_dot &&\n> +\t\t !match_pathspec(implicit_dot, path, strlen(path), 0, NULL)) {\n\nThis one really should *not* use match_pathspec(), I think.\n\nIt is a special case where we were asked to limit to our directory\nbut decided to grab everything instead and filtering the outcome\noutselves. We should have a \"path to the starting directory\" aka\n\"prefix\" in implicit_dot and check if path is covered by the prefix\ninstead.\n\n> +\t\t\twarn_pathless_add();\n> +\t\t\tcontinue;\n> +\t\t}\n> \t\tswitch (fix_unmerged_status(p, data)) {\n> \t\tdefault:\n> \t\t\tdie(_(\"unexpected diff status %c\"), p->status);\n"},{"id":"211675","messageId":"7v620nl99g.fsf@alter.siamese.dyndns.org","threadId":"33110","inReplyTo":"vpq1ubb3o5g.fsf@grenoble-inp.fr","subject":"Re: [PATCH 3/4] add -u: only show pathless 'add -u' warning when changes exist outside cwd","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-19T15:06:03Z","receivedAt":"2013-03-19T15:06:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> No time to review the code now. I thought about implementing something\n> like that, but did not do it because I didn't want the change in the\n> code to be too big. At some point, we'll have to remove the warning and\n> it's easier with my version than with yours. But the \"damage\" to the\n> code do not seem too big, so that's probably OK and will actually reduce\n> the pain for some users.\n\nGetting these warnings is a *good* thing.\n\nYou may happen to have no changed path outside the current directory\nwith this particular invocation of \"git add -u\", or you may do, or\nyou may not *even* remember if you touched the paths outside.\n\nTraining your fingers to type \"git add -u .\" without having to even\nthink, is primarily to help the last case.\n\nSquelching of the warning at the top-level is much less problematic\nas it is much harder to forget if you are at the top level of the\nworking tree than forget if you touched paths outside the current\ndirectory. \"I know I am at the top, so I type 'git add -u' without\ndot---why do you punish me with the warning?\" is a much more valid\ncomplaint.\n"},{"id":"211702","messageId":"20130319190624.GB3655@google.com","threadId":"33110","inReplyTo":"7v620nl99g.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/4] add -u: only show pathless 'add -u' warning when changes exist outside cwd","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-19T19:06:24Z","receivedAt":"2013-03-19T19:06:24Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Matthieu Moy writes:\n\n>> No time to review the code now. I thought about implementing something\n>> like that, but did not do it because I didn't want the change in the\n>> code to be too big. At some point, we'll have to remove the warning and\n>> it's easier with my version than with yours. But the \"damage\" to the\n>> code do not seem too big, so that's probably OK and will actually reduce\n>> the pain for some users.\n>\n> Getting these warnings is a *good* thing.\n>\n> You may happen to have no changed path outside the current directory\n> with this particular invocation of \"git add -u\", or you may do, or\n> you may not *even* remember if you touched the paths outside.\n>\n> Training your fingers to type \"git add -u .\" without having to even\n> think, is primarily to help the last case.\n\nThe problem is that these warnings are triggering way too often. It\nis like the story of the boy who cried \"wolf\": instead of training\npeople to type \"git add -u .\", we are training them to ignore\nwarnings.\n\nI personally often find myself in the following situation:\n\n\t$ cd repowithdeepsubdirs/third_party/git\n\t$ ... hack hack hack ...\n\t$ git add -u\n\nThe result is a pile of warning text that I cannot convince myself not\nto ignore because I already *knew* that the only changes present were\nunder the cwd. The old and new \"git add -u\" behaviors always have the\nsame effect in this case, so the warning is not relevant to me. So I\nfind myself being trained to ignore the warning.\n\nPresumably habitual Java hackers that do their work in a\ncom/long/package/name subdirectory of the toplevel would find this\neven more annoying.\n\nOne important exception is that if \"git add -u :/\" is slow, users need\nto learn to run \"git add -u .\" to speed the operation up. But I think\nthat is intuitive already. Running a full-tree diff which slows down\nthe code that decides whether to print a warning is a good way to\ntrain people regarding how long to expect a plain \"git add -u\" to\ntake.\n\nHoping that clarifies,\nJonathan\n"},{"id":"211704","messageId":"7vd2uvi33w.fsf@alter.siamese.dyndns.org","threadId":"33110","inReplyTo":"20130319190624.GB3655@google.com","subject":"Re: [PATCH 3/4] add -u: only show pathless 'add -u' warning when changes exist outside cwd","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-19T19:47:15Z","receivedAt":"2013-03-19T19:47:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> ... So I\n> find myself being trained to ignore the warning.\n> ... Running a full-tree diff which slows down\n> the code that decides whether to print a warning is a good way to\n> train people regarding how long to expect a plain \"git add -u\" to\n> take.\n\nOk, fair enough.\n\nIncidentally, I am rebuilding the 'next' by kicking many of the\ntopics back to 'pu' (essentially, only the ones marked as \"Safe\" in\nthe latest issue of \"What's cooking\" are kept in 'next'), so perhaps\nwe can rebuild the jc/add-2.0-u-A-sans-pathspec topic with your\nseries at the bottom, or something? I do not think I have time to\nget around to it myself until later in the evening, though.\n"},{"id":"211707","messageId":"20130319203447.GD3655@google.com","threadId":"33110","inReplyTo":"7vd2uvi33w.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/4] add -u: only show pathless 'add -u' warning when changes exist outside cwd","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-19T20:34:47Z","receivedAt":"2013-03-19T20:34:47Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> Incidentally, I am rebuilding the 'next' by kicking many of the\n> topics back to 'pu' (essentially, only the ones marked as \"Safe\" in\n> the latest issue of \"What's cooking\" are kept in 'next'), so perhaps\n> we can rebuild the jc/add-2.0-u-A-sans-pathspec topic with your\n> series at the bottom, or something? I do not think I have time to\n> get around to it myself until later in the evening, though.\n\nSounds good. I'll reroll with the use-prefix-not-pathspec tweak and\nincorporate the current patches from jc/add-2.0-u-A-sans-pathspec.\n"},{"id":"211713","messageId":"20130319224400.GA19014@google.com","threadId":"33110","inReplyTo":"1362786889-28688-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 0/6] make pathless 'add [-u|-A]' warning less noisy","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-19T22:44:00Z","receivedAt":"2013-03-19T22:44:00Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"As promised, here is a rerolled version of the \"make pathless 'add\n[-u|-A]' warning less noisy\", incorporating patches from\njc/add-2.0-u-A-sans-pathspec. Thanks for your help so far.\n\nJust like jc/add-2.0-u-A-sans-pathspec, the only transition in this\nseries is the \"pull the bandaid\" kind. That is, there are two steps:\n\n 0. the current state, where the warning is a little too noisy\n 1. the current state but with the warning only firing in cases\n where the user will be affected by the change to default\n 'git add -u' behavior\n 2. no more warning, 'git add -u' defaulting to 'git add -u :/'\n\nPatch 1 is the same as in jc/add-2.0-u-A-sans-pathspec, included for\nreference.\n\nPatches 2-5 correspond to the original patches 1-4. Any changes are\ndescribed after the '---' in each patch.\n\nPatch 6 is just the patch from the tip of jc/add-2.0-u-A-sans-pathspec,\nrebased. It is meant to be applied in the 2.0 cycle.\n\nJeff King (1):\n t2200: check that \"add -u\" limits itself to subdirectory\n\nJonathan Nieder (4):\n add: make pathless 'add [-u|-A]' warning a file-global function\n add: make warn_pathless_add() a no-op after first call\n add -u: only show pathless 'add -u' warning when changes exist outside\n cwd\n add -A: only show pathless 'add -A' warning when changes exist outside\n cwd\n\nJunio C Hamano (1):\n git add: -u/-A now affects the entire working tree\n\n Documentation/git-add.txt | 16 +++++++-------\n builtin/add.c | 56 ++++++++---------------------------------------\n t/t2200-add-update.sh | 11 ++++++++++\n 3 files changed, 28 insertions(+), 55 deletions(-)\n"},{"id":"211714","messageId":"20130319224451.GB19014@google.com","threadId":"33110","inReplyTo":"20130319224400.GA19014@google.com","subject":"[PATCH 1/6] t2200: check that \"add -u\" limits itself to subdirectory","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-19T22:44:51Z","receivedAt":"2013-03-19T22:44:51Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Jeff King \nDate: Thu, 14 Mar 2013 02:44:04 -0400\n\nThis behavior is due to change in the future, but let's test\nit anyway. That helps make sure we do not accidentally\nswitch the behavior too soon while we are working in the\narea, and it means that we can easily verify the change when\nwe do make it.\n\nSigned-off-by: Jeff King \nSigned-off-by: Junio C Hamano \nSigned-off-by: Jonathan Nieder \n---\nUnchanged, only included for reference.\n\n t/t2200-add-update.sh | 16 ++++++++++++++++\n 1 file changed, 16 insertions(+)\n\ndiff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh\nindex 4cdebda..c317254 100755\n--- a/t/t2200-add-update.sh\n+++ b/t/t2200-add-update.sh\n@@ -80,6 +80,22 @@ test_expect_success 'change gets noticed' '\n \n '\n \n+# Note that this is scheduled to change in Git 2.0, when\n+# \"git add -u\" will become full-tree by default.\n+test_expect_success 'non-limited update in subdir leaves root alone' '\n+\t(\n+\t\tcd dir1 &&\n+\t\techo even more >>sub2 &&\n+\t\tgit add -u\n+\t) &&\n+\tcat >expect <<-\\EOF &&\n+\tcheck\n+\ttop\n+\tEOF\n+\tgit diff-files --name-only >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success SYMLINKS 'replace a file with a symlink' '\n \n \trm foo &&\n-- \n1.8.2.rc3\n"},{"id":"211715","messageId":"20130319224526.GC19014@google.com","threadId":"33110","inReplyTo":"20130319224400.GA19014@google.com","subject":"[PATCH 2/6] add: make pathless 'add [-u|-A]' warning a file-global function","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-19T22:45:26Z","receivedAt":"2013-03-19T22:45:26Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Currently warn_pathless_add() is only called directly by cmd_add(),\nbut that is about to change. Move its definition higher in the file\nand pass the \"--update\" or \"--all\" option name used in its message\nthrough globals instead of function arguments to make it easier to\ncall without passing values that will not change through the call\nchain.\n\nSigned-off-by: Jonathan Nieder \n---\nUnchanged.\n\n builtin/add.c | 69 +++++++++++++++++++++++++++++++----------------------------\n 1 file changed, 36 insertions(+), 33 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex ab1c9e8..a4028ee 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -28,6 +28,41 @@ struct update_callback_data {\n \tint add_errors;\n };\n \n+static const char *option_with_implicit_dot;\n+static const char *short_option_with_implicit_dot;\n+\n+static void warn_pathless_add(void)\n+{\n+\tassert(option_with_implicit_dot && short_option_with_implicit_dot);\n+\n+\t/*\n+\t * To be consistent with \"git add -p\" and most Git\n+\t * commands, we should default to being tree-wide, but\n+\t * this is not the original behavior and can't be\n+\t * changed until users trained themselves not to type\n+\t * \"git add -u\" or \"git add -A\". For now, we warn and\n+\t * keep the old behavior. Later, the behavior can be changed\n+\t * to tree-wide, keeping the warning for a while, and\n+\t * eventually we can drop the warning.\n+\t */\n+\twarning(_(\"The behavior of 'git add %s (or %s)' with no path argument from a\\n\"\n+\t\t \"subdirectory of the tree will change in Git 2.0 and should not be used anymore.\\n\"\n+\t\t \"To add content for the whole tree, run:\\n\"\n+\t\t \"\\n\"\n+\t\t \" git add %s :/\\n\"\n+\t\t \" (or git add %s :/)\\n\"\n+\t\t \"\\n\"\n+\t\t \"To restrict the command to the current directory, run:\\n\"\n+\t\t \"\\n\"\n+\t\t \" git add %s .\\n\"\n+\t\t \" (or git add %s .)\\n\"\n+\t\t \"\\n\"\n+\t\t \"With the current Git version, the command is restricted to the current directory.\"),\n+\t\toption_with_implicit_dot, short_option_with_implicit_dot,\n+\t\toption_with_implicit_dot, short_option_with_implicit_dot,\n+\t\toption_with_implicit_dot, short_option_with_implicit_dot);\n+}\n+\n static int fix_unmerged_status(struct diff_filepair *p,\n \t\t\t struct update_callback_data *data)\n {\n@@ -321,35 +356,6 @@ static int add_files(struct dir_struct *dir, int flags)\n \treturn exit_status;\n }\n \n-static void warn_pathless_add(const char *option_name, const char *short_name) {\n-\t/*\n-\t * To be consistent with \"git add -p\" and most Git\n-\t * commands, we should default to being tree-wide, but\n-\t * this is not the original behavior and can't be\n-\t * changed until users trained themselves not to type\n-\t * \"git add -u\" or \"git add -A\". For now, we warn and\n-\t * keep the old behavior. Later, the behavior can be changed\n-\t * to tree-wide, keeping the warning for a while, and\n-\t * eventually we can drop the warning.\n-\t */\n-\twarning(_(\"The behavior of 'git add %s (or %s)' with no path argument from a\\n\"\n-\t\t \"subdirectory of the tree will change in Git 2.0 and should not be used anymore.\\n\"\n-\t\t \"To add content for the whole tree, run:\\n\"\n-\t\t \"\\n\"\n-\t\t \" git add %s :/\\n\"\n-\t\t \" (or git add %s :/)\\n\"\n-\t\t \"\\n\"\n-\t\t \"To restrict the command to the current directory, run:\\n\"\n-\t\t \"\\n\"\n-\t\t \" git add %s .\\n\"\n-\t\t \" (or git add %s .)\\n\"\n-\t\t \"\\n\"\n-\t\t \"With the current Git version, the command is restricted to the current directory.\"),\n-\t\toption_name, short_name,\n-\t\toption_name, short_name,\n-\t\toption_name, short_name);\n-}\n-\n int cmd_add(int argc, const char **argv, const char *prefix)\n {\n \tint exit_status = 0;\n@@ -360,8 +366,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tint add_new_files;\n \tint require_pathspec;\n \tchar *seen = NULL;\n-\tconst char *option_with_implicit_dot = NULL;\n-\tconst char *short_option_with_implicit_dot = NULL;\n \n \tgit_config(add_config, NULL);\n \n@@ -392,8 +396,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tif (option_with_implicit_dot && !argc) {\n \t\tstatic const char *here[2] = { \".\", NULL };\n \t\tif (prefix)\n-\t\t\twarn_pathless_add(option_with_implicit_dot,\n-\t\t\t\t\t short_option_with_implicit_dot);\n+\t\t\twarn_pathless_add();\n \t\targc = 1;\n \t\targv = here;\n \t}\n-- \n1.8.2.rc3\n"},{"id":"211716","messageId":"20130319224552.GD19014@google.com","threadId":"33110","inReplyTo":"20130319224400.GA19014@google.com","subject":"[PATCH 3/6] add: make warn_pathless_add() a no-op after first call","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-19T22:45:52Z","receivedAt":"2013-03-19T22:45:52Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Make warn_pathless_add() print its warning the first time it is called\nand do nothing if called again. This will make it easier to show the\nwarning on the fly when a relevant condition is detected without\nrisking showing it multiple times when multiple such conditions hold.\n\nSigned-off-by: Jonathan Nieder \n---\nAs before.\n\n builtin/add.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex a4028ee..a424e69 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -33,8 +33,13 @@ static const char *short_option_with_implicit_dot;\n \n static void warn_pathless_add(void)\n {\n+\tstatic int shown;\n \tassert(option_with_implicit_dot && short_option_with_implicit_dot);\n \n+\tif (shown)\n+\t\treturn;\n+\tshown = 1;\n+\n \t/*\n \t * To be consistent with \"git add -p\" and most Git\n \t * commands, we should default to being tree-wide, but\n-- \n1.8.2.rc3\n"},{"id":"211717","messageId":"20130319225050.GE19014@google.com","threadId":"33110","inReplyTo":"20130319224400.GA19014@google.com","subject":"[PATCH 4/6] add -u: only show pathless 'add -u' warning when changes exist outside cwd","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-19T22:50:50Z","receivedAt":"2013-03-19T22:50:50Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"A common workflow in large projects is to chdir into a subdirectory of\ninterest and only do work there:\n\n\tcd src\n\tvi foo.c\n\tmake test\n\tgit add -u\n\tgit commit\n\nThe upcoming change to 'git add -u' behavior would not affect such a\nworkflow: when the only changes present are in the current directory,\n'git add -u' will add all changes, and whether that happens via an\nimplicit \".\" or implicit \":/\" parameter is an unimportant\nimplementation detail.\n\nThe warning about use of 'git add -u' with no pathspec is annoying\nbecause it seemingly serves no purpose in this case. So suppress the\nwarning unless there are changes outside the cwd that are not being\nadded.\n\nA previous version of this patch ran two I/O-intensive diff-files\npasses: one to find changes outside the cwd, and another to find\nchanges to add to the index within the cwd. This version runs one\nfull-tree diff and decides for each change whether to add it or warn\nand suppress it in update_callback. As a result, even on very large\nrepositories \"git add -u\" will not be significantly slower than the\nfuture default behavior (\"git add -u :/\"), and the slowdown relative\nto \"git add -u .\" should be a useful clue to users of such\nrepositories to get into the habit of explicitly passing '.'.\n\nSigned-off-by: Jonathan Nieder \nAcked-by: Jeff King \nImproved-by: Junio C Hamano \n---\nThis is the interesting one.\n\nChanges since v1:\n * run only one diff-files pass, as explained in the log message\n * use strncmp_icase, not match_pathspec, to check if paths are\n under cwd\n * expand log message with performance implications\n * summarized Peff's review with an Ack. I hope that's ok.\n\nThanks for your help getting this into good shape.\n\n builtin/add.c | 43 +++++++++++++++++++++++++++++++++++++++----\n 1 file changed, 39 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex a424e69..4d8d441 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -26,6 +26,8 @@ static int take_worktree_changes;\n struct update_callback_data {\n \tint flags;\n \tint add_errors;\n+\tconst char *implicit_dot;\n+\tsize_t implicit_dot_len;\n };\n \n static const char *option_with_implicit_dot;\n@@ -94,10 +96,27 @@ static void update_callback(struct diff_queue_struct *q,\n {\n \tint i;\n \tstruct update_callback_data *data = cbdata;\n+\tconst char *implicit_dot = data->implicit_dot;\n+\tsize_t implicit_dot_len = data->implicit_dot_len;\n \n \tfor (i = 0; i < q->nr; i++) {\n \t\tstruct diff_filepair *p = q->queue[i];\n \t\tconst char *path = p->one->path;\n+\n+\t\t/*\n+\t\t * Check if \"git add -A\" or \"git add -u\" was run from a\n+\t\t * subdirectory with a modified file outside that directory,\n+\t\t * and warn if so.\n+\t\t *\n+\t\t * \"git add -u\" will behave like \"git add -u :/\" instead of\n+\t\t * \"git add -u .\" in the future. This warning prepares for\n+\t\t * that change.\n+\t\t */\n+\t\tif (implicit_dot &&\n+\t\t strncmp_icase(path, implicit_dot, implicit_dot_len)) {\n+\t\t\twarn_pathless_add();\n+\t\t\tcontinue;\n+\t\t}\n \t\tswitch (fix_unmerged_status(p, data)) {\n \t\tdefault:\n \t\t\tdie(_(\"unexpected diff status %c\"), p->status);\n@@ -121,17 +140,30 @@ static void update_callback(struct diff_queue_struct *q,\n \t}\n }\n \n+#define ADD_CACHE_IMPLICIT_DOT 32\n int add_files_to_cache(const char *prefix, const char **pathspec, int flags)\n {\n \tstruct update_callback_data data;\n \tstruct rev_info rev;\n+\n+\tmemset(&data, 0, sizeof(data));\n+\tdata.flags = flags & ~ADD_CACHE_IMPLICIT_DOT;\n+\tif ((flags & ADD_CACHE_IMPLICIT_DOT) && prefix) {\n+\t\t/*\n+\t\t * Check for modified files throughout the worktree so\n+\t\t * update_callback has a chance to warn about changes\n+\t\t * outside the cwd.\n+\t\t */\n+\t\tdata.implicit_dot = prefix;\n+\t\tdata.implicit_dot_len = strlen(prefix);\n+\t\tpathspec = NULL;\n+\t}\n+\n \tinit_revisions(&rev, prefix);\n \tsetup_revisions(0, NULL, &rev, NULL);\n \tinit_pathspec(&rev.prune_data, pathspec);\n \trev.diffopt.output_format = DIFF_FORMAT_CALLBACK;\n \trev.diffopt.format_callback = update_callback;\n-\tdata.flags = flags;\n-\tdata.add_errors = 0;\n \trev.diffopt.format_callback_data = &data;\n \trev.max_count = 0; /* do not compare unmerged paths with stage #2 */\n \trun_diff_files(&rev, DIFF_RACY_IS_MODIFIED);\n@@ -371,6 +403,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tint add_new_files;\n \tint require_pathspec;\n \tchar *seen = NULL;\n+\tint implicit_dot = 0;\n \n \tgit_config(add_config, NULL);\n \n@@ -400,10 +433,11 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t}\n \tif (option_with_implicit_dot && !argc) {\n \t\tstatic const char *here[2] = { \".\", NULL };\n-\t\tif (prefix)\n+\t\tif (prefix && addremove)\n \t\t\twarn_pathless_add();\n \t\targc = 1;\n \t\targv = here;\n+\t\timplicit_dot = 1;\n \t}\n \n \tadd_new_files = !take_worktree_changes && !refresh_only;\n@@ -416,7 +450,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\t (intent_to_add ? ADD_CACHE_INTENT : 0) |\n \t\t (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |\n \t\t (!(addremove || take_worktree_changes)\n-\t\t ? ADD_CACHE_IGNORE_REMOVAL : 0));\n+\t\t ? ADD_CACHE_IGNORE_REMOVAL : 0)) |\n+\t\t (implicit_dot ? ADD_CACHE_IMPLICIT_DOT : 0);\n \n \tif (require_pathspec && argc == 0) {\n \t\tfprintf(stderr, _(\"Nothing specified, nothing added.\\n\"));\n-- \n1.8.2.rc3\n"},{"id":"211718","messageId":"20130319225134.GF19014@google.com","threadId":"33110","inReplyTo":"20130319224400.GA19014@google.com","subject":"[PATCH 5/6] add -A: only show pathless 'add -A' warning when changes exist outside cwd","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-19T22:51:34Z","receivedAt":"2013-03-19T22:51:34Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"In the spirit of the recent similar change for 'git add -u', avoid\npestering users that restrict their attention to a subdirectory and\nwill not be affected by the coming change in the behavior of pathless\n'git add -A'.\n\nSigned-off-by: Jonathan Nieder \n---\nAs before.\n\n builtin/add.c | 21 ++++++++++++++++-----\n 1 file changed, 16 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 4d8d441..2493493 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -170,7 +170,9 @@ int add_files_to_cache(const char *prefix, const char **pathspec, int flags)\n \treturn !!data.add_errors;\n }\n \n-static char *prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)\n+#define WARN_IMPLICIT_DOT (1u << 0)\n+static char *prune_directory(struct dir_struct *dir, const char **pathspec,\n+\t\t\t int prefix, unsigned flag)\n {\n \tchar *seen;\n \tint i, specs;\n@@ -187,6 +189,16 @@ static char *prune_directory(struct dir_struct *dir, const char **pathspec, int\n \t\tif (match_pathspec(pathspec, entry->name, entry->len,\n \t\t\t\t prefix, seen))\n \t\t\t*dst++ = entry;\n+\t\telse if (flag & WARN_IMPLICIT_DOT)\n+\t\t\t/*\n+\t\t\t * \"git add -A\" was run from a subdirectory with a\n+\t\t\t * new file outside that directory.\n+\t\t\t *\n+\t\t\t * \"git add -A\" will behave like \"git add -A :/\"\n+\t\t\t * instead of \"git add -A .\" in the future.\n+\t\t\t * Warn about the coming behavior change.\n+\t\t\t */\n+\t\t\twarn_pathless_add();\n \t}\n \tdir->nr = dst - dir->entries;\n \tadd_pathspec_matches_against_index(pathspec, seen, specs);\n@@ -433,8 +445,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t}\n \tif (option_with_implicit_dot && !argc) {\n \t\tstatic const char *here[2] = { \".\", NULL };\n-\t\tif (prefix && addremove)\n-\t\t\twarn_pathless_add();\n \t\targc = 1;\n \t\targv = here;\n \t\timplicit_dot = 1;\n@@ -475,9 +485,10 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\t}\n \n \t\t/* This picks up the paths that are not tracked */\n-\t\tbaselen = fill_directory(&dir, pathspec);\n+\t\tbaselen = fill_directory(&dir, implicit_dot ? NULL : pathspec);\n \t\tif (pathspec)\n-\t\t\tseen = prune_directory(&dir, pathspec, baselen);\n+\t\t\tseen = prune_directory(&dir, pathspec, baselen,\n+\t\t\t\t\timplicit_dot ? WARN_IMPLICIT_DOT : 0);\n \t}\n \n \tif (refresh_only) {\n-- \n1.8.2.rc3\n"},{"id":"211719","messageId":"20130319225317.GG19014@google.com","threadId":"33110","inReplyTo":"20130319224400.GA19014@google.com","subject":"[PATCH 6/6] git add: -u/-A now affects the entire working tree","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-19T22:53:17Z","receivedAt":"2013-03-19T22:53:17Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Junio C Hamano \n\nAs promised in 0fa2eb530fb7 (add: warn when -u or -A is used without\npathspec, 2013-01-28), in Git 2.0, \"git add -u/-A\" that is run\nwithout pathspec in a subdirectory updates all updated paths in the\nentire working tree, not just the current directory and its\nsubdirectories.\n\nSigned-off-by: Junio C Hamano \nSigned-off-by: Jonathan Nieder \n---\nThanks for reading.\n\n Documentation/git-add.txt | 16 +++----\n builtin/add.c | 110 ++++------------------------------------------\n t/t2200-add-update.sh | 9 +---\n 3 files changed, 19 insertions(+), 116 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex b0944e5..02b99cb 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -104,10 +104,10 @@ apply to the index. See EDITING PATCHES below.\n \t. This removes as well as modifies index entries to\n \tmatch the working tree, but adds no new files.\n +\n-If no is given, the current version of Git defaults to\n-\".\"; in other words, update all tracked files in the current directory\n-and its subdirectories. This default will change in a future version\n-of Git, hence the form without should not be used.\n+If no is given when `-u` option is used, all\n+tracked files in the entire working tree are updated (old versions\n+of Git used to limit the update to the current directory and its\n+subdirectories).\n \n -A::\n --all::\n@@ -116,10 +116,10 @@ of Git, hence the form without should not be used.\n \tentry.\tThis adds, modifies, and removes index entries to\n \tmatch the working tree.\n +\n-If no is given, the current version of Git defaults to\n-\".\"; in other words, update all files in the current directory\n-and its subdirectories. This default will change in a future version\n-of Git, hence the form without should not be used.\n+If no is given when `-A` option is used, all\n+files in the entire working tree are updated (old versions\n+of Git used to limit the update to the current directory and its\n+subdirectories).\n \n -N::\n --intent-to-add::\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 2493493..5c0a869 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -26,50 +26,8 @@ static int take_worktree_changes;\n struct update_callback_data {\n \tint flags;\n \tint add_errors;\n-\tconst char *implicit_dot;\n-\tsize_t implicit_dot_len;\n };\n \n-static const char *option_with_implicit_dot;\n-static const char *short_option_with_implicit_dot;\n-\n-static void warn_pathless_add(void)\n-{\n-\tstatic int shown;\n-\tassert(option_with_implicit_dot && short_option_with_implicit_dot);\n-\n-\tif (shown)\n-\t\treturn;\n-\tshown = 1;\n-\n-\t/*\n-\t * To be consistent with \"git add -p\" and most Git\n-\t * commands, we should default to being tree-wide, but\n-\t * this is not the original behavior and can't be\n-\t * changed until users trained themselves not to type\n-\t * \"git add -u\" or \"git add -A\". For now, we warn and\n-\t * keep the old behavior. Later, the behavior can be changed\n-\t * to tree-wide, keeping the warning for a while, and\n-\t * eventually we can drop the warning.\n-\t */\n-\twarning(_(\"The behavior of 'git add %s (or %s)' with no path argument from a\\n\"\n-\t\t \"subdirectory of the tree will change in Git 2.0 and should not be used anymore.\\n\"\n-\t\t \"To add content for the whole tree, run:\\n\"\n-\t\t \"\\n\"\n-\t\t \" git add %s :/\\n\"\n-\t\t \" (or git add %s :/)\\n\"\n-\t\t \"\\n\"\n-\t\t \"To restrict the command to the current directory, run:\\n\"\n-\t\t \"\\n\"\n-\t\t \" git add %s .\\n\"\n-\t\t \" (or git add %s .)\\n\"\n-\t\t \"\\n\"\n-\t\t \"With the current Git version, the command is restricted to the current directory.\"),\n-\t\toption_with_implicit_dot, short_option_with_implicit_dot,\n-\t\toption_with_implicit_dot, short_option_with_implicit_dot,\n-\t\toption_with_implicit_dot, short_option_with_implicit_dot);\n-}\n-\n static int fix_unmerged_status(struct diff_filepair *p,\n \t\t\t struct update_callback_data *data)\n {\n@@ -96,27 +54,11 @@ static void update_callback(struct diff_queue_struct *q,\n {\n \tint i;\n \tstruct update_callback_data *data = cbdata;\n-\tconst char *implicit_dot = data->implicit_dot;\n-\tsize_t implicit_dot_len = data->implicit_dot_len;\n \n \tfor (i = 0; i < q->nr; i++) {\n \t\tstruct diff_filepair *p = q->queue[i];\n \t\tconst char *path = p->one->path;\n \n-\t\t/*\n-\t\t * Check if \"git add -A\" or \"git add -u\" was run from a\n-\t\t * subdirectory with a modified file outside that directory,\n-\t\t * and warn if so.\n-\t\t *\n-\t\t * \"git add -u\" will behave like \"git add -u :/\" instead of\n-\t\t * \"git add -u .\" in the future. This warning prepares for\n-\t\t * that change.\n-\t\t */\n-\t\tif (implicit_dot &&\n-\t\t strncmp_icase(path, implicit_dot, implicit_dot_len)) {\n-\t\t\twarn_pathless_add();\n-\t\t\tcontinue;\n-\t\t}\n \t\tswitch (fix_unmerged_status(p, data)) {\n \t\tdefault:\n \t\t\tdie(_(\"unexpected diff status %c\"), p->status);\n@@ -140,24 +82,13 @@ static void update_callback(struct diff_queue_struct *q,\n \t}\n }\n \n-#define ADD_CACHE_IMPLICIT_DOT 32\n int add_files_to_cache(const char *prefix, const char **pathspec, int flags)\n {\n \tstruct update_callback_data data;\n \tstruct rev_info rev;\n \n \tmemset(&data, 0, sizeof(data));\n-\tdata.flags = flags & ~ADD_CACHE_IMPLICIT_DOT;\n-\tif ((flags & ADD_CACHE_IMPLICIT_DOT) && prefix) {\n-\t\t/*\n-\t\t * Check for modified files throughout the worktree so\n-\t\t * update_callback has a chance to warn about changes\n-\t\t * outside the cwd.\n-\t\t */\n-\t\tdata.implicit_dot = prefix;\n-\t\tdata.implicit_dot_len = strlen(prefix);\n-\t\tpathspec = NULL;\n-\t}\n+\tdata.flags = flags;\n \n \tinit_revisions(&rev, prefix);\n \tsetup_revisions(0, NULL, &rev, NULL);\n@@ -170,9 +101,7 @@ int add_files_to_cache(const char *prefix, const char **pathspec, int flags)\n \treturn !!data.add_errors;\n }\n \n-#define WARN_IMPLICIT_DOT (1u << 0)\n-static char *prune_directory(struct dir_struct *dir, const char **pathspec,\n-\t\t\t int prefix, unsigned flag)\n+static char *prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)\n {\n \tchar *seen;\n \tint i, specs;\n@@ -189,16 +118,6 @@ static char *prune_directory(struct dir_struct *dir, const char **pathspec,\n \t\tif (match_pathspec(pathspec, entry->name, entry->len,\n \t\t\t\t prefix, seen))\n \t\t\t*dst++ = entry;\n-\t\telse if (flag & WARN_IMPLICIT_DOT)\n-\t\t\t/*\n-\t\t\t * \"git add -A\" was run from a subdirectory with a\n-\t\t\t * new file outside that directory.\n-\t\t\t *\n-\t\t\t * \"git add -A\" will behave like \"git add -A :/\"\n-\t\t\t * instead of \"git add -A .\" in the future.\n-\t\t\t * Warn about the coming behavior change.\n-\t\t\t */\n-\t\t\twarn_pathless_add();\n \t}\n \tdir->nr = dst - dir->entries;\n \tadd_pathspec_matches_against_index(pathspec, seen, specs);\n@@ -415,7 +334,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tint add_new_files;\n \tint require_pathspec;\n \tchar *seen = NULL;\n-\tint implicit_dot = 0;\n \n \tgit_config(add_config, NULL);\n \n@@ -435,19 +353,11 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"-A and -u are mutually incompatible\"));\n \tif (!show_only && ignore_missing)\n \t\tdie(_(\"Option --ignore-missing can only be used together with --dry-run\"));\n-\tif (addremove) {\n-\t\toption_with_implicit_dot = \"--all\";\n-\t\tshort_option_with_implicit_dot = \"-A\";\n-\t}\n-\tif (take_worktree_changes) {\n-\t\toption_with_implicit_dot = \"--update\";\n-\t\tshort_option_with_implicit_dot = \"-u\";\n-\t}\n-\tif (option_with_implicit_dot && !argc) {\n-\t\tstatic const char *here[2] = { \".\", NULL };\n+\n+\tif ((addremove || take_worktree_changes) && !argc) {\n+\t\tstatic const char *whole[2] = { \":/\", NULL };\n \t\targc = 1;\n-\t\targv = here;\n-\t\timplicit_dot = 1;\n+\t\targv = whole;\n \t}\n \n \tadd_new_files = !take_worktree_changes && !refresh_only;\n@@ -460,8 +370,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\t (intent_to_add ? ADD_CACHE_INTENT : 0) |\n \t\t (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |\n \t\t (!(addremove || take_worktree_changes)\n-\t\t ? ADD_CACHE_IGNORE_REMOVAL : 0)) |\n-\t\t (implicit_dot ? ADD_CACHE_IMPLICIT_DOT : 0);\n+\t\t ? ADD_CACHE_IGNORE_REMOVAL : 0));\n \n \tif (require_pathspec && argc == 0) {\n \t\tfprintf(stderr, _(\"Nothing specified, nothing added.\\n\"));\n@@ -485,10 +394,9 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\t}\n \n \t\t/* This picks up the paths that are not tracked */\n-\t\tbaselen = fill_directory(&dir, implicit_dot ? NULL : pathspec);\n+\t\tbaselen = fill_directory(&dir, pathspec);\n \t\tif (pathspec)\n-\t\t\tseen = prune_directory(&dir, pathspec, baselen,\n-\t\t\t\t\timplicit_dot ? WARN_IMPLICIT_DOT : 0);\n+\t\t\tseen = prune_directory(&dir, pathspec, baselen);\n \t}\n \n \tif (refresh_only) {\ndiff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh\nindex c317254..4281e18 100755\n--- a/t/t2200-add-update.sh\n+++ b/t/t2200-add-update.sh\n@@ -80,18 +80,13 @@ test_expect_success 'change gets noticed' '\n \n '\n \n-# Note that this is scheduled to change in Git 2.0, when\n-# \"git add -u\" will become full-tree by default.\n-test_expect_success 'non-limited update in subdir leaves root alone' '\n+test_expect_success 'non-qualified update in subdir updates from the root' '\n \t(\n \t\tcd dir1 &&\n \t\techo even more >>sub2 &&\n \t\tgit add -u\n \t) &&\n-\tcat >expect <<-\\EOF &&\n-\tcheck\n-\ttop\n-\tEOF\n+\t: >expect &&\n \tgit diff-files --name-only >actual &&\n \ttest_cmp expect actual\n '\n-- \n1.8.2.rc3\n"},{"id":"211723","messageId":"20130320050648.GB1839@sigill.intra.peff.net","threadId":"33110","inReplyTo":"20130319225050.GE19014@google.com","subject":"Re: [PATCH 4/6] add -u: only show pathless 'add -u' warning when changes exist outside cwd","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-20T05:06:49Z","receivedAt":"2013-03-20T05:06:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 19, 2013 at 03:50:50PM -0700, Jonathan Nieder wrote:\n\n> This is the interesting one.\n> [...]\n> * summarized Peff's review with an Ack. I hope that's ok.\n\nYeah, OK with me. I certainly agree with the intent, and I think your\nreasoning on the performance change is valid.\n\nI don't see anything wrong in the patch itself, except for one minor\nnit:\n\n> @@ -121,17 +140,30 @@ static void update_callback(struct diff_queue_struct *q,\n> \t}\n> }\n> \n> +#define ADD_CACHE_IMPLICIT_DOT 32\n> int add_files_to_cache(const char *prefix, const char **pathspec, int flags)\n> {\n> \tstruct update_callback_data data;\n> \tstruct rev_info rev;\n\nShould this be defined in cache.h with the other flags? I realize it's\nmostly private to builtin/add.c, but without even a comment in cache.h\nsaying \"/* 32 is reserved */\", we run the risk of another commit\nadding a new flag with the same number.\n\n-Peff\n"},{"id":"211759","messageId":"7v620mgl9y.fsf@alter.siamese.dyndns.org","threadId":"33110","inReplyTo":"20130319225050.GE19014@google.com","subject":"Re: [PATCH 4/6] add -u: only show pathless 'add -u' warning when changes exist outside cwd","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-20T15:10:01Z","receivedAt":"2013-03-20T15:10:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> A common workflow in large projects is to chdir into a subdirectory of\n> interest and only do work there:\n>\n> \tcd src\n> \tvi foo.c\n> \tmake test\n> \tgit add -u\n> \tgit commit\n>\n> The upcoming change to 'git add -u' behavior would not affect such a\n> workflow: when the only changes present are in the current directory,\n> 'git add -u' will add all changes, and whether that happens via an\n> implicit \".\" or implicit \":/\" parameter is an unimportant\n> implementation detail.\n>\n> The warning about use of 'git add -u' with no pathspec is annoying\n> because it seemingly serves no purpose in this case. So suppress the\n> warning unless there are changes outside the cwd that are not being\n> added.\n>\n> A previous version of this patch ran two I/O-intensive diff-files\n> passes: one to find changes outside the cwd, and another to find\n> changes to add to the index within the cwd. This version runs one\n> full-tree diff and decides for each change whether to add it or warn\n> and suppress it in update_callback. As a result, even on very large\n> repositories \"git add -u\" will not be significantly slower than the\n> future default behavior (\"git add -u :/\"), and the slowdown relative\n> to \"git add -u .\" should be a useful clue to users of such\n> repositories to get into the habit of explicitly passing '.'.\n\nI'll queue this as-is for now, but the point Peff raised on\nIMPLICIT_DOT needs to be addressed.\n\nThis is a tangent, but I would also suggest at least considering to\nmeasure how big the working tree is relative to the area covered by\nthe implicit dot [*1*]. If you are running \"add -u\" in a project\nwith 30k paths but are working with only a few dozen files, you may\nwant more encouragement to use an explicit \".\" than the command\nmysteriously and silently getting slower at Git version boundary.\n\nIOW, an advice message issued only when (1) you are indeed working\nin a narrow directory, (2) you did not touch anything outside, and\n(3) you did not give an explicit \".\". We would want to keep the\nadvice in place even after Git 2.0 switches the default to \":/\". In\nfact, it would become even more valuable after the transition. And\nmake it protected under advice.addUAuseexplicitdot or something.\n\nBy the way, I am not a big fan of the approach taken in [3/6]. It\nmay be a lot cleaner to separate the \"do we need to warn?\" logic\nthat flips a global (or a field in a callback, if we make some of\nthese codepaths callable from other places) and the code to issue\nthe warning, no?\n\n\n[Footnote]\n\n*1* The former you can get an approximation from active_nr, the\nlatter you can count after first finding where prefix would insert\nto the active_cache with index_name_pos(). I suspect that you also\ncould add a new read-only API to cache-tree to see if it already\nknows how many index entries a given subtree for the prefix covers.\n"},{"id":"211760","messageId":"7v1ubagkbe.fsf@alter.siamese.dyndns.org","threadId":"33110","inReplyTo":"20130319225134.GF19014@google.com","subject":"Re: [PATCH 5/6] add -A: only show pathless 'add -A' warning when changes exist outside cwd","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-20T15:30:45Z","receivedAt":"2013-03-20T15:30:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> In the spirit of the recent similar change for 'git add -u', avoid\n> pestering users that restrict their attention to a subdirectory and\n> will not be affected by the coming change in the behavior of pathless\n> 'git add -A'.\n>\n> Signed-off-by: Jonathan Nieder \n> ---\n> As before.\n\nHave you considered implementing this by adding a separate check\nimmediately after fill_directory() to inspect the paths in dir with\nthe same strncmp_icase() against the prefix, without touching\nprune_directory() at all? I suspect that would be much cleaner and\neasier to change in the version boundary.\n\nSame comment about measuring the size of the working tree and the\narea the user is working on applies to this. After Git 2.0, we\nwould still want to advise users who say \"git add -A\" without\npathspecs to see if the user would have been better off with an\nexplicit \".\", so unless advice.addUAuseexplicitdot is set to false,\nwe would still want to inspect the result from fill_directory (and\nin that case we won't be calling into prune_directory).\n\n> builtin/add.c | 21 ++++++++++++++++-----\n> 1 file changed, 16 insertions(+), 5 deletions(-)\n>\n> diff --git a/builtin/add.c b/builtin/add.c\n> index 4d8d441..2493493 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -170,7 +170,9 @@ int add_files_to_cache(const char *prefix, const char **pathspec, int flags)\n> \treturn !!data.add_errors;\n> }\n> \n> -static char *prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)\n> +#define WARN_IMPLICIT_DOT (1u << 0)\n> +static char *prune_directory(struct dir_struct *dir, const char **pathspec,\n> +\t\t\t int prefix, unsigned flag)\n> {\n> \tchar *seen;\n> \tint i, specs;\n> @@ -187,6 +189,16 @@ static char *prune_directory(struct dir_struct *dir, const char **pathspec, int\n> \t\tif (match_pathspec(pathspec, entry->name, entry->len,\n> \t\t\t\t prefix, seen))\n> \t\t\t*dst++ = entry;\n> +\t\telse if (flag & WARN_IMPLICIT_DOT)\n> +\t\t\t/*\n> +\t\t\t * \"git add -A\" was run from a subdirectory with a\n> +\t\t\t * new file outside that directory.\n> +\t\t\t *\n> +\t\t\t * \"git add -A\" will behave like \"git add -A :/\"\n> +\t\t\t * instead of \"git add -A .\" in the future.\n> +\t\t\t * Warn about the coming behavior change.\n> +\t\t\t */\n> +\t\t\twarn_pathless_add();\n> \t}\n> \tdir->nr = dst - dir->entries;\n> \tadd_pathspec_matches_against_index(pathspec, seen, specs);\n> @@ -433,8 +445,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n> \t}\n> \tif (option_with_implicit_dot && !argc) {\n> \t\tstatic const char *here[2] = { \".\", NULL };\n> -\t\tif (prefix && addremove)\n> -\t\t\twarn_pathless_add();\n> \t\targc = 1;\n> \t\targv = here;\n> \t\timplicit_dot = 1;\n> @@ -475,9 +485,10 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n> \t\t}\n> \n> \t\t/* This picks up the paths that are not tracked */\n> -\t\tbaselen = fill_directory(&dir, pathspec);\n> +\t\tbaselen = fill_directory(&dir, implicit_dot ? NULL : pathspec);\n> \t\tif (pathspec)\n> -\t\t\tseen = prune_directory(&dir, pathspec, baselen);\n> +\t\t\tseen = prune_directory(&dir, pathspec, baselen,\n> +\t\t\t\t\timplicit_dot ? WARN_IMPLICIT_DOT : 0);\n> \t}\n> \n> \tif (refresh_only) {\n"},{"id":"212883","messageId":"vpqehetgfh0.fsf@grenoble-inp.fr","threadId":"33110","inReplyTo":"7v7gldc44p.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] add: add a newline at the end of pathless 'add [-u|-A]' warning","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-04-02T14:43:55Z","receivedAt":"2013-04-02T14:43:55Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Sorry for the late reply,\n\nJunio C Hamano writes:\n\n> Matthieu Moy writes:\n>\n>> When the commands give an actual output (e.g. when ran with -v), the\n>> output is visually mixed with the warning. The newline makes the actual\n>> output more visible.\n>>\n>> Signed-off-by: Matthieu Moy \n>> ---\n>\n> It would have been easier to immediately understand what is going on\n> if you said \"blank line\" instead of \"newline\" ;-)\n\nIndeed.\n\n> An obvious issues is what if user does not run with \"-v\" or if \"-v\"\n> produces no results. We will be left with an extra blank line at\n> the end.\n\nRight, but displaying the blank line only when there's an actual output\ndoes not seem easy, and I'd rather avoid too much damage in the code for\na warning which is only temporary.\n\n> I suspect that the true reason why the warning does not stand out\n> and other output looks mixed in it may be because we only prefix the\n> first line with the \"warning: \" label. In the longer term, I have a\n> feeling that we should be showing something like this instead:\n>\n> $ cd t && echo >>t0000*.sh && git add -u -v\n> warning: The behavior of 'git add --update (or -u)' with no path ar...\n> warning: subdirectory of the tree will change in Git 2.0 and should...\n> warning: To add content for the whole tree, run:\n\nI personnally do not like this kind of output, the \"warning:\" on the 2nd\nand 3rd lines break the flow reading the message. But that's probably a\nmatter of taste.\n\n> using a logic similar to what strbuf_add_commented_lines() and\n> strbuf_add_lines() use.\n\nThis would mean changing the warning() function, which would change all\nwarnings.\n\nI'm fine with either dropping my patch or applying it as-is (with\ns/newline/blank line/ in the commit message); a bit reluctant to\nchanging the output of warning(...), but that's an option if other\npeople like it.\n\n>> builtin/add.c | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/builtin/add.c b/builtin/add.c\n>> index ab1c9e8..620bf00 100644\n>> --- a/builtin/add.c\n>> +++ b/builtin/add.c\n>> @@ -344,7 +344,7 @@ static void warn_pathless_add(const char *option_name, const char *short_name) {\n>> \t\t \" git add %s .\\n\"\n>> \t\t \" (or git add %s .)\\n\"\n>> \t\t \"\\n\"\n>> -\t\t \"With the current Git version, the command is restricted to the current directory.\"),\n>> +\t\t \"With the current Git version, the command is restricted to the current directory.\\n\"),\n>> \t\toption_name, short_name,\n>> \t\toption_name, short_name,\n>> \t\toption_name, short_name);\n>\n>\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"212899","messageId":"7vppycgahu.fsf@alter.siamese.dyndns.org","threadId":"33110","inReplyTo":"vpqehetgfh0.fsf@grenoble-inp.fr","subject":"Re: [PATCH 2/2] add: add a newline at the end of pathless 'add [-u|-A]' warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-02T16:31:25Z","receivedAt":"2013-04-02T16:31:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> I'm fine with either dropping my patch or applying it as-is (with\n> s/newline/blank line/ in the commit message).\n\nOK; let's insert it immediately after e24afab09137 (add: make\npathless 'add [-u|-A]' warning a file-global function, 2013-03-19),\nlike the attached.\n\nI'd prefer to spell this\n\n\tdie(\"...\\n\"\n\t \"\");\n\nover\n\n\tdie(\"...\\n\");\n\nas the latter _looks_ as if the author didn't know die() gives us\nline termination. The former hopefully is more explicit that we do\nwant an empty line at the end.\n\ncommit a8ed21a59219e8fe81b76ed0961641555f25cdad\nAuthor: Matthieu Moy \nDate: Mon Mar 11 09:01:33 2013 +0100\n\n add: add a blank line at the end of pathless 'add [-u|-A]' warning\n \n When the commands give an actual output (e.g. when ran with -v), the\n output is visually mixed with the warning.\n \n An additional blank line makes the actual output more visible.\n \n Signed-off-by: Matthieu Moy \n Signed-off-by: Junio C Hamano \n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex a4028ee..db02233 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -57,7 +57,9 @@ static void warn_pathless_add(void)\n \t\t \" git add %s .\\n\"\n \t\t \" (or git add %s .)\\n\"\n \t\t \"\\n\"\n-\t\t \"With the current Git version, the command is restricted to the current directory.\"),\n+\t\t \"With the current Git version, the command is restricted to \"\n+\t\t \"the current directory.\\n\",\n+\t\t \"\"),\n \t\toption_with_implicit_dot, short_option_with_implicit_dot,\n \t\toption_with_implicit_dot, short_option_with_implicit_dot,\n \t\toption_with_implicit_dot, short_option_with_implicit_dot);\n"},{"id":"212907","messageId":"vpqvc84euq5.fsf@grenoble-inp.fr","threadId":"33110","inReplyTo":"7vppycgahu.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] add: add a newline at the end of pathless 'add [-u|-A]' warning","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-04-02T16:57:22Z","receivedAt":"2013-04-02T16:57:22Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> diff --git a/builtin/add.c b/builtin/add.c\n> index a4028ee..db02233 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -57,7 +57,9 @@ static void warn_pathless_add(void)\n> \t\t \" git add %s .\\n\"\n> \t\t \" (or git add %s .)\\n\"\n> \t\t \"\\n\"\n> -\t\t \"With the current Git version, the command is restricted to the current directory.\"),\n> +\t\t \"With the current Git version, the command is restricted to \"\n> +\t\t \"the current directory.\\n\",\n> +\t\t \"\"),\n\nLooks good. Thanks,\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}