[PATCH] add: allow users to silence Git 2.0 warnings about "add -u"

[PATCH] add: allow users to silence Git 2.0 warnings about "add -u"

[David Aguilar]

When "git add -u" is invoked from a subdirectory it prints a
loud warning message about an upcoming Git 2.0 behavior change.
Some users do not care to be warned.  Accomodate them.

The "add.silence-pathless-warnings" configuration variable can
now be used to silence this warning.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
I found the warning a informative but also a little annoying.
I can imagine others might as well.

I would also like to change the warning message to mention what the Git 2.0
behavior will be (which it does not mention), but I realize that the string
has already been translated.  That can be a follow-on patch if this is seen as
a worthwhile change, but might not be worth the trouble since it's a problem
which will go away in 2.0.

 Documentation/config.txt |  7 +++++++
 builtin/add.c            |  8 +++++++-
 t/t2200-add-update.sh    | 11 +++++++++++
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3bb53da..b6ed859 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -648,6 +648,13 @@ core.abbrev::
 	for abbreviated object names to stay unique for sufficiently long
 	time.
 
+add.silence-pathless-warnings::
+	Tells 'git add' to silence warnings when 'git add -u' is used in
+	a subdirectory without specifying a path.  Git 2.0 updates the
+	whole tree.  Git 1.x updates the current directory only, and warns
+	about the upcoming change unless this variable is set to true.
+	False by default, and ignored by Git 2.0.
+
 add.ignore-errors::
 add.ignoreErrors::
 	Tells 'git add' to continue adding files when some files cannot be
diff --git a/builtin/add.c b/builtin/add.c
index 0dd014e..01b9cac 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -272,6 +272,7 @@ N_("The following paths are ignored by one of your .gitignore files:\n");
 
 static int verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0;
 static int ignore_add_errors, addremove, intent_to_add, ignore_missing = 0;
+static int silence_pathless_warnings;
 
 static struct option builtin_add_options[] = {
 	OPT__DRY_RUN(&show_only, N_("dry run")),
@@ -296,6 +297,8 @@ static int add_config(const char *var, const char *value, void *cb)
 	    !strcmp(var, "add.ignore-errors")) {
 		ignore_add_errors = git_config_bool(var, value);
 		return 0;
+	} else if (!strcmp(var, "add.silence-pathless-warnings")) {
+		silence_pathless_warnings = git_config_bool(var, value);
 	}
 	return git_default_config(var, value, cb);
 }
@@ -321,7 +324,8 @@ static int add_files(struct dir_struct *dir, int flags)
 	return exit_status;
 }
 
-static void warn_pathless_add(const char *option_name, const char *short_name) {
+static void warn_pathless_add(const char *option_name, const char *short_name)
+{
 	/*
 	 * To be consistent with "git add -p" and most Git
 	 * commands, we should default to being tree-wide, but
@@ -332,6 +336,8 @@ static void warn_pathless_add(const char *option_name, const char *short_name) {
 	 * turned into a die(...), and eventually we may
 	 * reallow the command with a new behavior.
 	 */
+	if (silence_pathless_warnings)
+		return;
 	warning(_("The behavior of 'git add %s (or %s)' with no path argument from a\n"
 		  "subdirectory of the tree will change in Git 2.0 and should not be used anymore.\n"
 		  "To add content for the whole tree, run:\n"
diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
index 4cdebda..779dbe7 100755
--- a/t/t2200-add-update.sh
+++ b/t/t2200-add-update.sh
@@ -171,4 +171,15 @@ test_expect_success '"add -u non-existent" should fail' '
 	! (git ls-files | grep "non-existent")
 '
 
+test_expect_success 'add.silence-pathless-warnings configuration variable' '
+	: >expect &&
+	test_config add.silence-pathless-warnings true &&
+	(
+		cd dir1 &&
+		echo more >>sub2 &&
+		git add -u
+	) >actual 2>&1 &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.8.2.rc0.22.gb3600c3.dirty

Re: [PATCH] add: allow users to silence Git 2.0 warnings about "add -u"

[Junio C Hamano]

David Aguilar <davvid@gmail.com> writes:

> When "git add -u" is invoked from a subdirectory it prints a
> loud warning message about an upcoming Git 2.0 behavior change.
> Some users do not care to be warned.  Accomodate them.

I do not think this is what we discussed to do.

It was very much deliberate to make the way to "squelch the warning"
not a "set once and *forget*", aka configuration variable, but a
simple-to-type extra command line argument i.e. "git add -u .", that
you will *always* type to train your fingers to explicitly say what
you mean, so that the default switch will not matter to existing
users.

Re: [PATCH] add: allow users to silence Git 2.0 warnings about "add -u"

[David Aguilar]

On Thu, Feb 21, 2013 at 10:23 PM, Junio C Hamano <gitster@pobox.com> wrote:
> David Aguilar <davvid@gmail.com> writes:
>
>> When "git add -u" is invoked from a subdirectory it prints a
>> loud warning message about an upcoming Git 2.0 behavior change.
>> Some users do not care to be warned.  Accomodate them.
>
> I do not think this is what we discussed to do.
>
> It was very much deliberate to make the way to "squelch the warning"
> not a "set once and *forget*", aka configuration variable, but a
> simple-to-type extra command line argument i.e. "git add -u .", that
> you will *always* type to train your fingers to explicitly say what
> you mean, so that the default switch will not matter to existing
> users.

In my use case:

- The user is always in a subdir; the behavior change
is immaterial to them.

- The user does not care about these details,
and is not harmed by using the short and sweet command.

Please enlighten me.
Are we really getting rid of it and replacing it with ":/"?
That syntax looks like a meh face.. just sayin'

I was actually surprised when "add -u" didn't do the whole tree
and am happy that 2.0 will make it do the right thing...

(and perhaps I am deluded, and am not aware of what 2.0 will
do when not given pathspecs.. is it really going to die()?
that's so mean! ;)

Sorry if I am missing most of the context.
I was reading this in builtin/add.c:

	/*
	 * To be consistent with "git add -p" and most Git
	 * commands, we should default to being tree-wide, but
	 * this is not the original behavior and can't be
	 * changed until users trained themselves not to type
	 * "git add -u" or "git add -A". For now, we warn and
	 * keep the old behavior. Later, this warning can be
	 * turned into a die(...), and eventually we may
	 * reallow the command with a new behavior.
	 */

...and I was being too optimistic about, "and eventually".

I misread that and thought it meant that (eventually) 2.0
would default to the full tree and fix the consistency.

I didn't think that meant 2.0 would die and "git add -u"
will not be a valid syntax anymore.

Why punish these users?  They are going to have :/ face.

Unlike push.default, whose warning can be silenced with configuration,
git 1.x does not have a way to silence this warning without retraining
existing users.

In other words I will have to answer an email about it one day, and I'm lazy ;-)


Another example...

$ git grep 'stuff' :/

would it be too much to teach it to do:

$ git grep -u 'stuff'

(some users are really simple...)

but in 2.0 that -u would be a no-op because "grep" will be full tree, no?

Would having that as an option and configuration be a way to allow 1.x
users to transition themselves to a 2.0 world?

I need to read the old discussions.
Can someone tell me the magic google search syntax they use to dig them up?


Would a better way be a method to make "git add -u" behave like 2.0 today?

I'm thinking of Python's "from __future__ import better_behavior" as a analog.

If full-tree is a better default then that should be the default.
Surely that's better than die(), no?

Apologies in advance as I have not read the discussions (yet).
-- 
David

Re: [PATCH] add: allow users to silence Git 2.0 warnings about "add -u"

[Matthieu Moy]

David Aguilar <davvid@gmail.com> writes:

> Please enlighten me.
> Are we really getting rid of it and replacing it with ":/"?
> That syntax looks like a meh face.. just sayin'

The current behavior is indeed replaced by "git add -u .", not ":/".

> Unlike push.default, whose warning can be silenced with configuration,
> git 1.x does not have a way to silence this warning without retraining
> existing users.

Yes, but push.default is really different: there is a config variable,
and we want the behavior to be configurable. In the case of "git add",
I don't think adding a configuration option would be the right thing.
That would mean typing "git add -u" on an account which isn't yours will
be unpredictable *forever*.

OTOH, "git add -u :/" and "git add -u ." will behave predictibly on any
version of Git that accepts them, past present or future (:/ was added
in 1.7.6, a year and a half ago).

> Another example...
>
> $ git grep 'stuff' :/
>
> would it be too much to teach it to do:
>
> $ git grep -u 'stuff'

"git grep" is out of the scope of this change. Yes, it is inconsistant
with the rest of Git, but doesn't seem to surprise users as much as "git
add -u" (for which the inconsistancy appears within the "add" command).

I don't understand what you mean by "git grep -u". "git add -u" is a
shortcut for "git add --update", and "git grep --update" wouldn't make
sense to me. Do you mean we should add a "--full-tree" to "git grep"?
That seems really overkill to me since we already have the :/ pathspec.

> but in 2.0 that -u would be a no-op because "grep" will be full tree, no?

No it won't.

> I need to read the old discussions.
> Can someone tell me the magic google search syntax they use to dig them up?

See the discussion here:

http://thread.gmane.org/gmane.comp.version-control.git/213988/focus=214106

(recursively, there's a pointer to an older discussion)

> Would a better way be a method to make "git add -u" behave like 2.0 today?

As I said, I think adding a configuration option that would remain after
2.0 would do more harm than good. But after thinking about it, I'm not
against an option like a boolean add.use2dot0Behavior that would:

* Right now, adopt the future behavior and kill the warning

* From 2.0, kill the warning without changing the bevavior

* When we stop warning, disapear.

This, or the add.silence-pathless-warnings (which BTW should be spelled
add.silencePathlessWarnings) would not harm as long as they are not
advertized in the warning. What we don't want is dumb users reading half
the message and apply the quickest receipe they find to kill the warning
without thinking about the consequences.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] add: allow users to silence Git 2.0 warnings about "add -u"

[Junio C Hamano]

David Aguilar <davvid@gmail.com> writes:

> Please enlighten me.

As you lack the knowledge of previous discussion, I think you will
be the best person to proofread the paragraph on this issue in the
"backward compatibilty notes" section of the draft release notes to
v1.8.2 to see if that is understandable to the end users and point
out what are missing or confusing.

Re: [PATCH] add: allow users to silence Git 2.0 warnings about "add -u"

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Yes, but push.default is really different: there is a config variable,
> and we want the behavior to be configurable. In the case of "git add",
> I don't think adding a configuration option would be the right thing.
> That would mean typing "git add -u" on an account which isn't yours will
> be unpredictable *forever*.

Exactly.

>> $ git grep 'stuff' :/
>>
>> would it be too much to teach it to do:
>>
>> $ git grep -u 'stuff'
>
> "git grep" is out of the scope of this change. Yes, it is inconsistant
> with the rest of Git, but doesn't seem to surprise users as much as "git
> add -u" (for which the inconsistancy appears within the "add" command).

It is consistent with "grep", and the reason "git grep" behaves that
way is because consistency with "grep" matters more. I do not think
it is going to change.

Another is "clean", which I do not personally care too deeply about;
it being a destructive operation that is only used interactively and
occasionally), the current behaviour to limit it to the cwd is much
more sensible than making it go and touch parts of the tree that is
unrelated to cwd.

> I don't understand what you mean by "git grep -u".

I think he meant to add "git grep --full-tree" or something, and I
do not think it is going to happen when we have ":/" magic pathspec.

> As I said, I think adding a configuration option that would remain after
> 2.0 would do more harm than good. But after thinking about it, I'm not
> against an option like a boolean add.use2dot0Behavior that would:
>
> * Right now, adopt the future behavior and kill the warning
>
> * From 2.0, kill the warning without changing the bevavior
>
> * When we stop warning, disapear.

It is marginally better than David's "set once without thinking
because I read it on stackoverflow without fully understanding the
ramifications, and forget about it only to suffer when Git 2.0
happens" configuration variable, but by not much.  You can easily
imagine

	Q. Help, Git 1.8.2 is giving me this warning. What to do?
	A. Set this configuration variable. period.

and many users setting it without realizing that they are making the
operation tree-wide at the same time.  We'd want to see this answer
instead:

	A. Say "git add -u ."; you want to add changed paths in the
 	   current directory.

Another problem with use2dot0 configuration is that it would invite
people to imagine that setting it to false will keep the old
behaviour forever, which is against what you set out to do with the
patch under discussion.

Re: [PATCH] add: allow users to silence Git 2.0 warnings about "add -u"

[David Aguilar]

On Fri, Feb 22, 2013 at 9:30 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Yes, but push.default is really different: there is a config variable,
>> and we want the behavior to be configurable. In the case of "git add",
>> I don't think adding a configuration option would be the right thing.
>> That would mean typing "git add -u" on an account which isn't yours will
>> be unpredictable *forever*.
>
> Exactly.

I completely agree.  We don't want that [1].

I'm actually a big enemy of configuration, don't get me wrong.
The real point of the patch I sent was to start a conversation
about the thing I actually care about:

After reading the draft release notes I now realize that
"git add -u" will not die() in Git 2.0.  It will operate on the
full tree, as described in the note.  Sweet.

I was originally concerned that "git add -u" was going to die()
and we would no longer be able to use it without pathspec.
My concerns were unfounded.

(If I am not understanding this correctly then it is a sign
 that the draft release notes can be made more clear)

> Another problem with use2dot0 configuration is that it would invite
> people to imagine that setting it to false will keep the old
> behaviour forever, which is against what you set out to do with the
> patch under discussion.

I agree with both sides.  There's the part of me that wants the 2.0
behavior now with a config switch for the same reasons as was
discussed earlier:

http://thread.gmane.org/gmane.comp.version-control.git/166223/focus=168238

In addition, mindful users would see one less warning,
which is the only weight I've added to that side of the discussion.

We would never want to go back to the old behavior when 2.0
roll around.  Jakub's "future.wholeTree" suggestion makes
sense in that context as the entire "future.*" namespace
could be designated as variables with these semantics.

One downside is that adding such a configuration is just more
temporary code to maintain and rip out when 2.0 rolls around.

OTOH a positive thing about adding configuration to get
the better behavior is that the code path materializes
sooner, and it will be better exercised before 2.0.  This
increases confidence that removing the false side of the
imaginary "future.fullTree" configuration will be harmless.

In the original-original thread Matthieu and I seemed to
agree that configuration to get the new behavior
(but not get the old behavior) would be nice. Peff went
even farther and suggested that having a way to keep
the old behavior would be good, and I agree that this is
the thing [1] to avoid since it makes the command forever
unpredictable.

"future.*" means the ambiguous/unpredictable behavior
does eventually go away.

It's a flag day, there's no way around that.
Script writers will be hurt, there is no escaping that.

I guess the real question is whether it's a flag day that
happens through availability of configuration, or by the
inevitability of 2.0.

I have one scenario where "future.fullTree" would be
helpful to script writers: it would allow them to
test their scripts with the new behavior and back it out
if their scripts break.  This gives them more time to
make the tiny change needed to be portable across
different git versions, which helps make the later
default change into much less of an event.

Having such a configuration would probably mean that
git should probably warn for all pathless "git add -u",
even from the root.  It will help usher users towards
the new behavior.  The current behavior makes the
most sense since we do not have a config variable.

The current behavior is certainly the simplest.

I don't know what we can do about the clueless user on
stackoverflow that follows the first suggestion to
set the future.fullTree variable.  My gut feeling is
that optimizing for them is a lost cause.

Providing a way for mindful users to ease themselves
into the new behavior does help them, and git is
certainly the tool of choice for the mindful user. ;-)

I hope I haven't misrepresented anybody's opinions.
If I'm the only one who thinks that "future.fullTree"
is a good idea then I have no problem with the current
behavior since the noisy warning will be gone in 2.0.

Does anyone else have any weight to add to either side?
-- 
David

Re: [PATCH] add: allow users to silence Git 2.0 warnings about "add -u"

[Junio C Hamano]

David Aguilar <davvid@gmail.com> writes:

> I was originally concerned that "git add -u" was going to die()
> and we would no longer be able to use it without pathspec.
> My concerns were unfounded.
>
> (If I am not understanding this correctly then it is a sign
>  that the draft release notes can be made more clear)

Yes, that is exactly why I asked you to suggest improvements to that
paragraph.

>> Another problem with use2dot0 configuration is that it would invite
>> people to imagine that setting it to false will keep the old
>> behaviour forever, which is against what you set out to do with the
>> patch under discussion.
>
> I agree with both sides.  There's the part of me that wants the 2.0
> behavior now with a config switch for the same reasons as was
> discussed earlier...

If that is really the case and you want the full-tree behaviour, you
would have been using "git add -u :/" already, and then you wouldn't
have seen the warning.

Why do we have this thread then?

The reason may well be "I've heard about the :/ magic pathspec, and
I thought I understood what it does at the intellectual level, but
it has not sunk in enough for me to use it regularly".

The warning, and "you can squelch with either :/ or ." to train the
fingers (not "set once and forget"), is exactly to solve that
problem now *and* *in* *the* *future* during the 2.0 transition
period.

You also said that it often is the case for you that you stay in a
narrow subtree without touching other parts of the tree. If that is
the case, you may *not* want 2.0 behaviour, which forces Git to run
around the whole tree, trying to find modified paths outside of your
corner that do not exist, wasting cycles.  You want "git add .", and
you are better off starting to train your fingers so that they type
that without thinking now.  I think the conclusion during the old
discussion was not "we want configuration", but "this is not per
user and configuration is a poor approach. Depending on what you are
working on right now, you would want 'only here' sometimes and
'whole tree' some other times".

> We would never want to go back to the old behavior when 2.0
> roll around.  Jakub's "future.wholeTree" suggestion makes
> sense in that context as the entire "future.*" namespace
> could be designated as variables with these semantics.

Not at all. Even you who visit this list often do not regularly use
the ":/" to affect the whole tree and see the warning. What do you
imagine other people, who do not even know about this list do and
say, at sites like stackoverflow where uninformeds guide other
uninformeds?

  Q. Help, Git 1.8.2 is giving me this warning. What to do?
  A. Set this configuration variable. No other explanation.

Renaming use2dot0 to future does not solve anything.

> OTOH a positive thing about adding configuration to get
> the better behavior is that the code path materializes
> sooner, and it will be better exercised before 2.0.

That is not an argument for adding temporary configuration, as it is
not the only or even the best way to do so.  It can be easily an
cleanly achieved by cooking in next until 2.0.

An ulterior motive for going that way is to encourage more people to
run 'next' ;-). Recently we are seeing bugs discovered only after topics
graduate to 'master', which is not a good sign X-<.