All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
@ 2011-02-07  2:27 Nguyễn Thái Ngọc Duy
  2011-02-07  5:58 ` Junio C Hamano
  2011-02-27 10:46 ` Junio C Hamano
  0 siblings, 2 replies; 25+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2011-02-07  2:27 UTC (permalink / raw)
  To: git, Junio C Hamano, Sebastian Pipping, SZEDER Gábor, Matthieu Moy
  Cc: Nguyễn Thái Ngọc Duy

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

This makes -u (and -A) inconsistent with some other options, namely -p.
It's been four years since the unintentional breakage and people are
probably used to "git add -u" updating only current directory. Perhaps
it's time to bring the original behavior back? Current behavior can
always be achieved with "git add -u ."

Migration plan:

I'm bad at this. Can we start with a patch that warns users to do "git
add -u ." when they do "git add -u"? Hopefully they would have their
fingers retraied by the time the behavior is changed in 1.8.0.

PS. What about -A?

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 2011/2/7 Sebastian Pipping <webmaster@hartwork.org>:
 >> git add -u was tree-wide when it was introduced in dfdac5d (git-add
 >> -u: match the index with working tree., 2007-04-20), but 2ed2c22
 >> (git-add -u paths... now works from subdirectory, 2007-08-16) broke it
 >> while fixing something related.
 >
 > So my memory didn't fool me.  Thanks for digging this out.
 >
 > Can we have tree-wide "git add -u" back, please?

 Yup yup I like it too (and wanted the original behavior sometimes, even
 though I didn't know it was original behavior).

 Pulling Junio in for -A. It seems closely related to -u. In fact I revert
 one line from 1e5f764 (builtin-add.c: optimize -A option and "git add ."
 - 2008-07-22) but not fully understand why it was changed.

 builtin/add.c         |    7 +++++--
 t/t2200-add-update.sh |   13 +++++++------
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index 12b964e..f1f8b5a 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -389,7 +389,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		die("-A and -u are mutually incompatible");
 	if (!show_only && ignore_missing)
 		die("Option --ignore-missing can only be used together with --dry-run");
-	if ((addremove || take_worktree_changes) && !argc) {
+	if (addremove && !argc) {
 		static const char *here[2] = { ".", NULL };
 		argc = 1;
 		argv = here;
@@ -412,7 +412,10 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		fprintf(stderr, "Maybe you wanted to say 'git add .'?\n");
 		return 0;
 	}
-	pathspec = validate_pathspec(argc, argv, prefix);
+	if (take_worktree_changes && !argc)
+		pathspec = NULL;
+	else
+		pathspec = validate_pathspec(argc, argv, prefix);
 
 	if (read_cache() < 0)
 		die("index file corrupt");
diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
index 0692427..2201242 100755
--- a/t/t2200-add-update.sh
+++ b/t/t2200-add-update.sh
@@ -69,15 +69,16 @@ test_expect_success 'cache tree has not been corrupted' '
 test_expect_success 'update from a subdirectory' '
 	(
 		cd dir1 &&
-		echo more >sub2 &&
+		echo more >>sub2 &&
 		git add -u sub2
-	)
-'
-
-test_expect_success 'change gets noticed' '
-
+	) &&
 	test "$(git diff-files --name-status dir1)" = ""
+'
 
+test_expect_success 'update without args from subdir' '
+	echo more >>top &&
+	( cd dir1 && git add -u ) &&
+	test "$(git diff-files --name-status top)" = ""
 '
 
 test_expect_success SYMLINKS 'replace a file with a symlink' '
-- 
1.7.3.4.878.g439c7

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-02-07  2:27 [PATCH 1.8.0] add: make "add -u" update full tree without pathspec Nguyễn Thái Ngọc Duy
@ 2011-02-07  5:58 ` Junio C Hamano
  2011-02-07  6:14   ` Nguyen Thai Ngoc Duy
  2011-02-07 12:09   ` Matthieu Moy
  2011-02-27 10:46 ` Junio C Hamano
  1 sibling, 2 replies; 25+ messages in thread
From: Junio C Hamano @ 2011-02-07  5:58 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git, Junio C Hamano, Sebastian Pipping, SZEDER Gábor,
	Matthieu, Moy <Matthieu.Moy

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

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

As long as the command takes pathspecs, it should never be tree-wide.
Making it tree-wide when there is no pathspec is even worse.

If "add -p" does not limit operation to the current directory, probably
that is the inconsistency bug to be fixed.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-02-07  5:58 ` Junio C Hamano
@ 2011-02-07  6:14   ` Nguyen Thai Ngoc Duy
  2011-02-07  6:26     ` Miles Bader
  2011-02-09 10:58     ` Joshua Juran
  2011-02-07 12:09   ` Matthieu Moy
  1 sibling, 2 replies; 25+ messages in thread
From: Nguyen Thai Ngoc Duy @ 2011-02-07  6:14 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Sebastian Pipping, SZEDER Gábor, Matthieu, Matthieu.Moy

On Mon, Feb 7, 2011 at 12:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> When -u was introduced in dfdac5d (git-add -u: match the index with
>> working tree., 2007-04-20), "add -u" (without pathspec) added
>> everything. Shortly after, 2ed2c22 (git-add -u paths... now works from
>> subdirectory, 2007-08-16) broke it while fixing something related.
>
> As long as the command takes pathspecs, it should never be tree-wide.
> Making it tree-wide when there is no pathspec is even worse.

git log -p and diff family all can take pathspecs. All default to
tree-wide without pathspecs. This is what I'm doing all the time:

git diff
# checking, ok, looks good
git add -u
# ack, need to come to root dir first
-- 
Duy

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-02-07  6:14   ` Nguyen Thai Ngoc Duy
@ 2011-02-07  6:26     ` Miles Bader
  2011-02-09 10:58     ` Joshua Juran
  1 sibling, 0 replies; 25+ messages in thread
From: Miles Bader @ 2011-02-07  6:26 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy
  Cc: Junio C Hamano, git, Sebastian Pipping, SZEDER Gábor,
	Matthieu, Matthieu.Moy

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> git log -p and diff family all can take pathspecs. All default to
> tree-wide without pathspecs. This is what I'm doing all the time:
>
> git diff
> # checking, ok, looks good
> git add -u
> # ack, need to come to root dir first

Not to mention "git status", which shows everything in the tree when
given no args, but will only show changes underneath the cwd if given
"." as an argument.

I agree that the "default to root of git dir when given no args"
behavior seems both more convenient -- it's annoying to specify the root
of the tree explicitly, but very easy to specify the cwd explicitly
(".") -- and more intuitive -- If I want "everything underneath cwd",
I'll naturually specify "." as an argument; if I give no args, I almost
certainly want the whole tree, no matter where I happen to be in it.

-Miles

-- 
Christian, n. One who follows the teachings of Christ so long as they are not
inconsistent with a life of sin.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-02-07  5:58 ` Junio C Hamano
  2011-02-07  6:14   ` Nguyen Thai Ngoc Duy
@ 2011-02-07 12:09   ` Matthieu Moy
  1 sibling, 0 replies; 25+ messages in thread
From: Matthieu Moy @ 2011-02-07 12:09 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nguyễn Thái Ngọc Duy, git, Sebastian Pipping,
	SZEDER Gábor

[ Your mailer did something strange with my name and email ]

Junio C Hamano <gitster@pobox.com> writes:

> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> When -u was introduced in dfdac5d (git-add -u: match the index with
>> working tree., 2007-04-20), "add -u" (without pathspec) added
>> everything. Shortly after, 2ed2c22 (git-add -u paths... now works from
>> subdirectory, 2007-08-16) broke it while fixing something related.
>
> As long as the command takes pathspecs, it should never be tree-wide.
> Making it tree-wide when there is no pathspec is even worse.

This is against the common use in Git. As I mentionned in the other
thread, pretty much any git command taking an optional pathspec is
still tree-wide when called without argument. "git add -u" is the
exception.

> If "add -p" does not limit operation to the current directory, probably
> that is the inconsistency bug to be fixed.

Add "git log", "git status", "git commit", "git show", "git diff", and
a few others I've missed to the list of "exceptions" ...

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-02-07  6:14   ` Nguyen Thai Ngoc Duy
  2011-02-07  6:26     ` Miles Bader
@ 2011-02-09 10:58     ` Joshua Juran
  1 sibling, 0 replies; 25+ messages in thread
From: Joshua Juran @ 2011-02-09 10:58 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy
  Cc: Junio C Hamano, git, Sebastian Pipping, SZEDER Gábor,
	Matthieu, Matthieu.Moy

On Feb 6, 2011, at 10:14 PM, Nguyen Thai Ngoc Duy wrote:

> On Mon, Feb 7, 2011 at 12:58 PM, Junio C Hamano <gitster@pobox.com>  
> wrote:
>> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>>
>>> When -u was introduced in dfdac5d (git-add -u: match the index with
>>> working tree., 2007-04-20), "add -u" (without pathspec) added
>>> everything. Shortly after, 2ed2c22 (git-add -u paths... now works  
>>> from
>>> subdirectory, 2007-08-16) broke it while fixing something related.
>>
>> As long as the command takes pathspecs, it should never be tree-wide.
>> Making it tree-wide when there is no pathspec is even worse.
>
> git log -p and diff family all can take pathspecs. All default to
> tree-wide without pathspecs. This is what I'm doing all the time:
>
> git diff
> # checking, ok, looks good
> git add -u
> # ack, need to come to root dir first

The only reason I'm not routinely bitten by this is that I'm generally  
always at the root of the tree.  But I certainly *think* about it as  
an operation that's tree-wide, not local.  If I meant to limit the  
scope, I'd specify a path.

Josh

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-02-07  2:27 [PATCH 1.8.0] add: make "add -u" update full tree without pathspec Nguyễn Thái Ngọc Duy
  2011-02-07  5:58 ` Junio C Hamano
@ 2011-02-27 10:46 ` Junio C Hamano
  2011-02-27 11:43   ` Matthieu Moy
                     ` (2 more replies)
  1 sibling, 3 replies; 25+ messages in thread
From: Junio C Hamano @ 2011-02-27 10:46 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git, Sebastian Pipping, SZEDER Gábor, Matthieu Moy, Jeff King

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Migration plan:
>
> I'm bad at this. Can we start with a patch that warns users to do "git
> add -u ." when they do "git add -u"? Hopefully they would have their
> fingers retraied by the time the behavior is changed in 1.8.0.

Perhaps in this order:

Step 1, as soon as possible:

 * Introduce "static int make_update_global;" file scope static to
   builtin/add.c and default to 0;

 * Introduce "add.make_update_global" configuration variable, and toggle
   the above variable when it is explicitly given; also record the fact
   that you actually saw this variable in the config parser regardless of
   the value that is given;

 * Document the configuration variable as a new feature, without
   indicating that that will be the new default in the future, but
   strongly recommending that existing scripts should be updated and new
   scripts should be written the variable in mind---namely, their use of
   "add -u" should use "." if they are relying on the current "limit to
   cwd" behaviour.

 * Ship this as 1.7.X and see how well the new feature is accepted in the
   wider user community.  If the feature is not widely used, there is no
   point in proceeding further.

Step 2, two cycles (i.e. 6-8 months) before 1.8.0, provided if the
previous step is proven positive:

 * When "git add -u" is run without any pathspec, and when the user does
   not have the configuration variable, warn loudly that the default
   behaviour will change in 1.8.0, and tell the users that the current
   behaviour is still available by setting the configuration variable.

Step 3, at 1.8.0 and probably one or two more cycles:

 * When "git add -u" is run without any pathspec, and when the user does
   not have the configuration variable, warn loudly that the default
   behaviour _has_ changed in 1.8.0, to help people who migrated from
   early 1.7.X before Step 2 directly to 1.8.X series.

Step 4, 3 cycles after 1.8.0:

 * Drop the warning.

> PS. What about -A?

I personally think it should change exactly the same way as "-u" for
consistency, but I had the impression that the list concensus was to keep
the current behaviour.

Has anybody been keeping tallies on the discussion around

  http://thread.gmane.org/gmane.comp.version-control.git/166135/focus=166457

We seem to have lost track.  All I recall was that I changed my mind and
migrated from the third camp (the command defaults to whichever somebody
thought would be more convenient, lacking consistency across command set)
to "it would be more convenient to default to full-tree when there is no
pathspec, as limiting to cwd with '.' is easier" camp myself, but the
discussion is not about my personal preference but about seeing if we have
an overwhelming majority consensus, so...

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-02-27 10:46 ` Junio C Hamano
@ 2011-02-27 11:43   ` Matthieu Moy
  2011-02-27 17:04     ` Nguyen Thai Ngoc Duy
  2011-02-27 13:35   ` Sverre Rabbelier
  2011-02-27 16:52   ` Nguyen Thai Ngoc Duy
  2 siblings, 1 reply; 25+ messages in thread
From: Matthieu Moy @ 2011-02-27 11:43 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nguyễn Thái Ngọc Duy, git, Sebastian Pipping,
	SZEDER Gábor, Jeff King

Junio C Hamano <gitster@pobox.com> writes:

> Step 2, two cycles (i.e. 6-8 months) before 1.8.0, provided if the
> previous step is proven positive:
>
>  * When "git add -u" is run without any pathspec, and when the user does
>    not have the configuration variable, warn loudly that the default
>    behaviour will change in 1.8.0,

I'd even say "error out", since the old behavior is still easily
available with "git add -u ." (which should be mentionned in the
warning/error message).

>> PS. What about -A?
>
> I personally think it should change exactly the same way as "-u" for
> consistency,

Same here. I care less since I use "add -A" less, but both commands
should really behave the same since they're very similar.

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-02-27 10:46 ` Junio C Hamano
  2011-02-27 11:43   ` Matthieu Moy
@ 2011-02-27 13:35   ` Sverre Rabbelier
  2011-02-27 17:01     ` Nguyen Thai Ngoc Duy
  2011-02-27 16:52   ` Nguyen Thai Ngoc Duy
  2 siblings, 1 reply; 25+ messages in thread
From: Sverre Rabbelier @ 2011-02-27 13:35 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nguyễn Thái Ngọc, git, Sebastian Pipping,
	SZEDER Gábor, Matthieu Moy, Jeff King

Heya,

On Sun, Feb 27, 2011 at 11:46, Junio C Hamano <gitster@pobox.com> wrote:
>  * Ship this as 1.7.X and see how well the new feature is accepted in the
>   wider user community.  If the feature is not widely used, there is no
>   point in proceeding further.

I'm confused, I thought we were doing this for consistency reasons?
That would benefit mostly new users, right? New users, which are
unlikely to notice this new feature, or report to this list that they
find it useful?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-02-27 10:46 ` Junio C Hamano
  2011-02-27 11:43   ` Matthieu Moy
  2011-02-27 13:35   ` Sverre Rabbelier
@ 2011-02-27 16:52   ` Nguyen Thai Ngoc Duy
  2011-02-27 19:39     ` Junio C Hamano
  2 siblings, 1 reply; 25+ messages in thread
From: Nguyen Thai Ngoc Duy @ 2011-02-27 16:52 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Sebastian Pipping, SZEDER Gábor, Matthieu Moy, Jeff King

On Sun, Feb 27, 2011 at 5:46 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Perhaps in this order:
>
> Step 1, as soon as possible:
>
>  * Introduce "add.make_update_global" configuration variable, and toggle
>   the above variable when it is explicitly given; also record the fact
>   that you actually saw this variable in the config parser regardless of
>   the value that is given;

Ermm.. compat.make_update_global, with the intent that the config will
be dropped in future (1.9.0 maybe)?

>  * Document the configuration variable as a new feature, without
>   indicating that that will be the new default in the future, but
>   strongly recommending that existing scripts should be updated and new
>   scripts should be written the variable in mind---namely, their use of
>   "add -u" should use "." if they are relying on the current "limit to
>   cwd" behaviour.

There's a problem. I use git on many machines. Some will have this
config enabled, some will not (yet). Perhaps a third option, which
will print something when "git add -u" is issued as a reminder? I
don't know. What I need is to notice old behavior of "git add -u"
before it's too late.

> Step 4, 3 cycles after 1.8.0:
>
>  * Drop the warning.

Step 5 (1.9.0 or later):

 * Drop compat.make_update_global.
-- 
Duy

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-02-27 13:35   ` Sverre Rabbelier
@ 2011-02-27 17:01     ` Nguyen Thai Ngoc Duy
  0 siblings, 0 replies; 25+ messages in thread
From: Nguyen Thai Ngoc Duy @ 2011-02-27 17:01 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Junio C Hamano, git, Sebastian Pipping, SZEDER Gábor,
	Matthieu Moy, Jeff King

On Sun, Feb 27, 2011 at 8:35 PM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> Heya,
>
> On Sun, Feb 27, 2011 at 11:46, Junio C Hamano <gitster@pobox.com> wrote:
>>  * Ship this as 1.7.X and see how well the new feature is accepted in the
>>   wider user community.  If the feature is not widely used, there is no
>>   point in proceeding further.
>
> I'm confused, I thought we were doing this for consistency reasons?
> That would benefit mostly new users, right? New users, which are
> unlikely to notice this new feature, or report to this list that they
> find it useful?

I think the purpose is to see if this benefits current user base
first, which is large enough. If it beats the user base hard, it's not
worth adding even for consistency. Unfortunately not all of us users
reads release notes. I don't know how we know it's widely used.
Putting something like this in docs?

"This is experimental. If you use it and like it, raise your voice in git@ver"
-- 
Duy

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-02-27 11:43   ` Matthieu Moy
@ 2011-02-27 17:04     ` Nguyen Thai Ngoc Duy
  0 siblings, 0 replies; 25+ messages in thread
From: Nguyen Thai Ngoc Duy @ 2011-02-27 17:04 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: Junio C Hamano, git, Sebastian Pipping, SZEDER Gábor, Jeff King

On Sun, Feb 27, 2011 at 6:43 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Step 2, two cycles (i.e. 6-8 months) before 1.8.0, provided if the
>> previous step is proven positive:
>>
>>  * When "git add -u" is run without any pathspec, and when the user does
>>    not have the configuration variable, warn loudly that the default
>>    behaviour will change in 1.8.0,
>
> I'd even say "error out", since the old behavior is still easily
> available with "git add -u ." (which should be mentionned in the
> warning/error message).

You wouldn't want scripts to break this way. Warnings can give script
writers some time to update (assume that scripts are installed system
wide, not in $HOME).
-- 
Duy

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-02-27 16:52   ` Nguyen Thai Ngoc Duy
@ 2011-02-27 19:39     ` Junio C Hamano
  2011-02-28  6:37       ` Nguyen Thai Ngoc Duy
  0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2011-02-27 19:39 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy
  Cc: git, Sebastian Pipping, SZEDER Gábor, Matthieu Moy, Jeff King

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> On Sun, Feb 27, 2011 at 5:46 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Perhaps in this order:
>>
>> Step 1, as soon as possible:
>>
>>  * Introduce "add.make_update_global" configuration variable, and toggle
>>   the above variable when it is explicitly given; also record the fact
>>   that you actually saw this variable in the config parser regardless of
>>   the value that is given;
>
> Ermm.. compat.make_update_global, with the intent that the config will
> be dropped in future (1.9.0 maybe)?

As you haven't yet proven that this "new feature" is even useful to help
new people nor existing users at this step, you cannot claim "we plan to
drop this in the future", hence naming it "compat.*" is a no-go.  During
this step, we can not even say "we plan to make this the default"; we
would confuse the users otherwise (it is fine to say "we might make this
the default some day").

Even if we indeed end up proceeding to step 2, I don't see a point in
planning to drop the support from the beginning.  We might end up doing
so, but we can decide when that becomes necessary, and that would be long
after the tree-wide default proves a reasonable one, and preferably after
seeing a new person or two raise "what's the point of making 'add -u'
restricted to cwd?  we have too many options and this can go" on the list.

Then we would start deprecating the config, giving a warning when people
who still rely on their "add.make_update_global = false" say "add -u"
without pathspec in a few cycles, and then finally drop it at a version
bump boundary.

> There's a problem. I use git on many machines. Some will have this
> config enabled, some will not (yet). Perhaps a third option, which
> will print something when "git add -u" is issued as a reminder?

Such a warning would not help you on a machine that does not even have git
with Step 1 change.

What you conceive as a problem is just a reminder that any incompatible
change you plan to add will have pain involved.  On two machines, one with
a new feature and the other without the new feature, you would have to
work differently _or_ you would train yourself to use both versions in a
compatible way (e.g. when you mean tree-wide, you would cdup, and when you
mean cwd, you would explicitly say ".", from the command line).  That is
not limited to this particular feature but any incompatible change, no?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-02-27 19:39     ` Junio C Hamano
@ 2011-02-28  6:37       ` Nguyen Thai Ngoc Duy
  2011-02-28  6:56         ` Junio C Hamano
  0 siblings, 1 reply; 25+ messages in thread
From: Nguyen Thai Ngoc Duy @ 2011-02-28  6:37 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Sebastian Pipping, SZEDER Gábor, Matthieu Moy, Jeff King

On Mon, Feb 28, 2011 at 2:39 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>> There's a problem. I use git on many machines. Some will have this
>> config enabled, some will not (yet). Perhaps a third option, which
>> will print something when "git add -u" is issued as a reminder?
>
> Such a warning would not help you on a machine that does not even have git
> with Step 1 change.
>
> What you conceive as a problem is just a reminder that any incompatible
> change you plan to add will have pain involved.  On two machines, one with
> a new feature and the other without the new feature, you would have to
> work differently _or_ you would train yourself to use both versions in a
> compatible way (e.g. when you mean tree-wide, you would cdup, and when you
> mean cwd, you would explicitly say ".", from the command line).  That is
> not limited to this particular feature but any incompatible change, no?

No. But I hoped it would help me somehow, or at least remind me to
update git on the machines I touch.
-- 
Duy

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-02-28  6:37       ` Nguyen Thai Ngoc Duy
@ 2011-02-28  6:56         ` Junio C Hamano
  2011-03-01 11:22           ` Nguyen Thai Ngoc Duy
  2011-03-01 14:53           ` Matthieu Moy
  0 siblings, 2 replies; 25+ messages in thread
From: Junio C Hamano @ 2011-02-28  6:56 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy
  Cc: git, Sebastian Pipping, SZEDER Gábor, Matthieu Moy, Jeff King

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> On Mon, Feb 28, 2011 at 2:39 AM, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>> What you conceive as a problem is just a reminder that any incompatible
>> change you plan to add will have pain involved....
>
> No. But I hoped it would help me somehow, or at least remind me to
> update git on the machines I touch.

Perhaps the migration plan is not helpful enough?  If that is the case we
would need to rethink it to be even less impactful.

I already do not like the possibility of potential double deprecation
myself, one to flip the default and then possibly another to drop the
support of the traditional behavour, but anything I've seen so far would
hurt the end users more than that plan.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-02-28  6:56         ` Junio C Hamano
@ 2011-03-01 11:22           ` Nguyen Thai Ngoc Duy
  2011-03-01 13:46             ` Junio C Hamano
  2011-03-01 14:53           ` Matthieu Moy
  1 sibling, 1 reply; 25+ messages in thread
From: Nguyen Thai Ngoc Duy @ 2011-03-01 11:22 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Sebastian Pipping, SZEDER Gábor, Matthieu Moy, Jeff King

On Mon, Feb 28, 2011 at 1:56 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Perhaps the migration plan is not helpful enough?  If that is the case we
> would need to rethink it to be even less impactful.

But how many people may be impacted this way? If it's few, probably
not worth the headaches. I want to believe the number is few, but I
think anybody who ssh to some machine may have the same problem
because they may not control what's in that machine.

I can only think of another way, ugly though: add a new command name,
say "nad", that goes with new behavior, "add" remains old school. This
way if I mistakenly type 'nad' on unsupported git, it refuses and I'm
safe. Whether the new name is temporary (until the behavior flip in
"add") or permanent is another ugly matter to think about.

Or accept that evolution is painful and go with current plan, which I'm OK too.
-- 
Duy

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-03-01 11:22           ` Nguyen Thai Ngoc Duy
@ 2011-03-01 13:46             ` Junio C Hamano
  2011-03-01 14:15               ` Nguyen Thai Ngoc Duy
  0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2011-03-01 13:46 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy
  Cc: git, Sebastian Pipping, SZEDER Gábor, Matthieu Moy, Jeff King

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> On Mon, Feb 28, 2011 at 1:56 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> But how many people may be impacted this way? ...
> Or accept that evolution is painful and go with current plan, which I'm OK too.

So even though you raised a concern on possible pains during the
migration, you think that the migration plan outlined would be the
least bad one?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-03-01 13:46             ` Junio C Hamano
@ 2011-03-01 14:15               ` Nguyen Thai Ngoc Duy
  0 siblings, 0 replies; 25+ messages in thread
From: Nguyen Thai Ngoc Duy @ 2011-03-01 14:15 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Sebastian Pipping, SZEDER Gábor, Matthieu Moy, Jeff King

On Tue, Mar 1, 2011 at 8:46 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> On Mon, Feb 28, 2011 at 1:56 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> But how many people may be impacted this way? ...
>> Or accept that evolution is painful and go with current plan, which I'm OK too.
>
> So even though you raised a concern on possible pains during the
> migration, you think that the migration plan outlined would be the
> least bad one?

I only see two ways. The 'new command' approach only delays the
transition until a later (user decided) point. There's no guarantee
that I (user) would make a good decision to switch. Plus the outlined
plan looks simpler. Yes, it seems better.
-- 
Duy

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-02-28  6:56         ` Junio C Hamano
  2011-03-01 11:22           ` Nguyen Thai Ngoc Duy
@ 2011-03-01 14:53           ` Matthieu Moy
  2011-03-01 18:40             ` Junio C Hamano
  1 sibling, 1 reply; 25+ messages in thread
From: Matthieu Moy @ 2011-03-01 14:53 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nguyen Thai Ngoc Duy, git, Sebastian Pipping, SZEDER Gábor,
	Jeff King

Junio C Hamano <gitster@pobox.com> writes:

> I already do not like the possibility of potential double deprecation
> myself, one to flip the default and then possibly another to drop the
> support of the traditional behavour, but anything I've seen so far would
> hurt the end users more than that plan.

I think promoting "git add -u ." more than a configuration option would
reduce the pain.

As a user, if I get used to typing "git add -u ." instead of "git add -u", 
I get the current behavior regardless of the version of Git, without a
warning. Later, when all the machines I word on support the tree-wide
"git add -u" (either 1.7.x + some configuration or 1.8.y), I'll use it
as a new feature.

So, a warning like

  warning: the behavior of "git add -u" without pathspec will change in
  Git 1.8.0. To keep the current behavior, use this instead:
  
      git add -u .
  
  + explanations about the config options as already discussed here

would be fine.

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-03-01 14:53           ` Matthieu Moy
@ 2011-03-01 18:40             ` Junio C Hamano
  2011-03-01 18:51               ` Matthieu Moy
  0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2011-03-01 18:40 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: Nguyen Thai Ngoc Duy, git, Sebastian Pipping, SZEDER Gábor,
	Jeff King

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

> I think promoting "git add -u ." more than a configuration option would
> reduce the pain.

Yeah, I tend to agree.

> As a user, if I get used to typing "git add -u ." instead of "git add -u", 
> I get the current behavior regardless of the version of Git, without a
> warning. Later, when all the machines I word on support the tree-wide
> "git add -u" (either 1.7.x + some configuration or 1.8.y), I'll use it
> as a new feature.

Once your users (you as a script writer) have an option to set the
configuration to participate in the tree-wide party early, you would need
to update your scripts immediately so that they don't break on them; so
the introduction of the configuration becomes a flag-day event.  Hmph...

> So, a warning like
>
>   warning: the behavior of "git add -u" without pathspec will change in
>   Git 1.8.0. To keep the current behavior, use this instead:
>   
>       git add -u .
>   
>   + explanations about the config options as already discussed here
>
> would be fine.

Yeah, I think you convinced me that an elaborate configuration wouldn't
help us at all.  We just keep warning in 1.7.x series when "add -u" didn't
see any pathspec, and flip the default at 1.8.0

Simpler and cleaner ;-)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-03-01 18:40             ` Junio C Hamano
@ 2011-03-01 18:51               ` Matthieu Moy
  2011-03-01 19:36                 ` Jakub Narebski
                                   ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Matthieu Moy @ 2011-03-01 18:51 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nguyen Thai Ngoc Duy, git, Sebastian Pipping, SZEDER Gábor,
	Jeff King

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> I think promoting "git add -u ." more than a configuration option would
>> reduce the pain.
>
> Yeah, I tend to agree.
>
>> As a user, if I get used to typing "git add -u ." instead of "git add -u", 
>> I get the current behavior regardless of the version of Git, without a
>> warning. Later, when all the machines I word on support the tree-wide
>> "git add -u" (either 1.7.x + some configuration or 1.8.y), I'll use it
>> as a new feature.
>
> Once your users (you as a script writer) have an option to set the
> configuration to participate in the tree-wide party early, you would need
> to update your scripts immediately so that they don't break on them;

But "update" should mean "replace git add -u with git add -u .", which
is the portable way to do the same.

> so the introduction of the configuration becomes a flag-day event.
> Hmph...

The introduction of the config variable is a non-event if you already
use the portable . notation.

>> So, a warning like
>>
>>   warning: the behavior of "git add -u" without pathspec will change in
>>   Git 1.8.0. To keep the current behavior, use this instead:
>>   
>>       git add -u .
>>   
>>   + explanations about the config options as already discussed here
>>
>> would be fine.
>
> Yeah, I think you convinced me that an elaborate configuration wouldn't
> help us at all.  We just keep warning in 1.7.x series when "add -u" didn't
> see any pathspec, and flip the default at 1.8.0
>
> Simpler and cleaner ;-)

I think is still makes sense to have a config variable, so that people
who want the new behavior can get it ASAP. Right after 1.8 is out, I'll
still have machines where I'm too lazy to install a brand new Git, and
I'll want to get the 1.8 goodness for free ;-).

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-03-01 18:51               ` Matthieu Moy
@ 2011-03-01 19:36                 ` Jakub Narebski
  2011-03-01 20:00                 ` Jeff King
  2011-03-01 20:12                 ` Junio C Hamano
  2 siblings, 0 replies; 25+ messages in thread
From: Jakub Narebski @ 2011-03-01 19:36 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: Junio C Hamano, Nguyen Thai Ngoc Duy, git, Sebastian Pipping,
	SZEDER Gábor, Jeff King

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

[...]
>>> So, a warning like
>>>
>>>   warning: the behavior of "git add -u" without pathspec will change in
>>>   Git 1.8.0. To keep the current behavior, use this instead:
>>>   
>>>       git add -u .
>>>   
>>>   + explanations about the config options as already discussed here
>>>
>>> would be fine.
>>
>> Yeah, I think you convinced me that an elaborate configuration wouldn't
>> help us at all.  We just keep warning in 1.7.x series when "add -u" didn't
>> see any pathspec, and flip the default at 1.8.0
>>
>> Simpler and cleaner ;-)
> 
> I think is still makes sense to have a config variable, so that people
> who want the new behavior can get it ASAP. Right after 1.8 is out, I'll
> still have machines where I'm too lazy to install a brand new Git, and
> I'll want to get the 1.8 goodness for free ;-).

So it would be the opposite of compat.*; for example `future.wholeTree`
config variable? ;-)

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-03-01 18:51               ` Matthieu Moy
  2011-03-01 19:36                 ` Jakub Narebski
@ 2011-03-01 20:00                 ` Jeff King
  2011-03-01 20:12                 ` Junio C Hamano
  2 siblings, 0 replies; 25+ messages in thread
From: Jeff King @ 2011-03-01 20:00 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: Junio C Hamano, Nguyen Thai Ngoc Duy, git, Sebastian Pipping,
	SZEDER Gábor

On Tue, Mar 01, 2011 at 07:51:38PM +0100, Matthieu Moy wrote:

> > Once your users (you as a script writer) have an option to set the
> > configuration to participate in the tree-wide party early, you would need
> > to update your scripts immediately so that they don't break on them;
> 
> But "update" should mean "replace git add -u with git add -u .", which
> is the portable way to do the same.
> 
> > so the introduction of the configuration becomes a flag-day event.
> > Hmph...
> 
> The introduction of the config variable is a non-event if you already
> use the portable . notation.

Right. Once this happens, you can never say "git add -u" again portably.
You can't rely on the old behavior, because it's changing. You can't
rely on the new behavior, because you might be using an old version. So
as a script you _must_ say "git add -u .", and if you want top-level
behavior and are not at the top-level, you must cd to the toplevel and
"add .". Which sounds onerous, but it is what scripts have to do
already with the current behavior.

> > Yeah, I think you convinced me that an elaborate configuration wouldn't
> > help us at all.  We just keep warning in 1.7.x series when "add -u" didn't
> > see any pathspec, and flip the default at 1.8.0
> >
> > Simpler and cleaner ;-)
> 
> I think is still makes sense to have a config variable, so that people
> who want the new behavior can get it ASAP. Right after 1.8 is out, I'll
> still have machines where I'm too lazy to install a brand new Git, and
> I'll want to get the 1.8 goodness for free ;-).

Agreed. I also think there should be a setting to keep the current
behavior. I don't want to use it, but given that configuration does not
introduce any existing portability issues, we can help people who really
liked the old behavior. We are inconveniencing them by changing the
default, but it seems doubly mean to leave them with no way of restoring
it short of typing extra characters on every invocation.

-Peff

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-03-01 18:51               ` Matthieu Moy
  2011-03-01 19:36                 ` Jakub Narebski
  2011-03-01 20:00                 ` Jeff King
@ 2011-03-01 20:12                 ` Junio C Hamano
  2011-03-01 20:25                   ` Matthieu Moy
  2 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2011-03-01 20:12 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: Nguyen Thai Ngoc Duy, git, Sebastian Pipping, SZEDER Gábor,
	Jeff King

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

> But "update" should mean "replace git add -u with git add -u .", which
> is the portable way to do the same.
>
>> so the introduction of the configuration becomes a flag-day event.
>> Hmph...
>
> The introduction of the config variable is a non-event if you already
> use the portable . notation.

I think you got this part wrong.  Until now, there was no "portability" to
worry about when using "git add -u".  "git add -u" used to be _guaranteed_
to be relative to the cwd, with or without ".".  Our update will _break_
that expectation and suddenly make "." "the portable notation".

In other words, it is far from a non-event.  We cannot say "'add -u .' is
the portable way to write it anyway, so you should have been writing that
from day one" as an excuse for breaking their scripts.

We still have to say "We are sorry, we will be breaking your scripts.
Please add an extra dot at the end".  I am Ok saying that, but we should
admit that we are knowingly breaking things.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
  2011-03-01 20:12                 ` Junio C Hamano
@ 2011-03-01 20:25                   ` Matthieu Moy
  0 siblings, 0 replies; 25+ messages in thread
From: Matthieu Moy @ 2011-03-01 20:25 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nguyen Thai Ngoc Duy, git, Sebastian Pipping, SZEDER Gábor,
	Jeff King

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> But "update" should mean "replace git add -u with git add -u .", which
>> is the portable way to do the same.
>>
>>> so the introduction of the configuration becomes a flag-day event.
>>> Hmph...
>>
>> The introduction of the config variable is a non-event if you already
>> use the portable . notation.
>
> I think you got this part wrong.  Until now, there was no "portability" to
> worry about when using "git add -u".  "git add -u" used to be _guaranteed_
> to be relative to the cwd, with or without ".".  Our update will _break_
> that expectation and suddenly make "." "the portable notation".
>
> In other words, it is far from a non-event.

There is a "if" in my sentence, and I do claim that if you verify the
condition behind the "if", it is a non-event.

I did not claim that the whole change was a non-event, just that the
introduction of a config variable was.

> We still have to say "We are sorry, we will be breaking your scripts.
> Please add an extra dot at the end".

Absolutely. My claim is just that adding the extra dot is the way to fix
script wrt the future change, or at least a better way than relying on a
config variable.

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2011-03-01 20:25 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-07  2:27 [PATCH 1.8.0] add: make "add -u" update full tree without pathspec Nguyễn Thái Ngọc Duy
2011-02-07  5:58 ` Junio C Hamano
2011-02-07  6:14   ` Nguyen Thai Ngoc Duy
2011-02-07  6:26     ` Miles Bader
2011-02-09 10:58     ` Joshua Juran
2011-02-07 12:09   ` Matthieu Moy
2011-02-27 10:46 ` Junio C Hamano
2011-02-27 11:43   ` Matthieu Moy
2011-02-27 17:04     ` Nguyen Thai Ngoc Duy
2011-02-27 13:35   ` Sverre Rabbelier
2011-02-27 17:01     ` Nguyen Thai Ngoc Duy
2011-02-27 16:52   ` Nguyen Thai Ngoc Duy
2011-02-27 19:39     ` Junio C Hamano
2011-02-28  6:37       ` Nguyen Thai Ngoc Duy
2011-02-28  6:56         ` Junio C Hamano
2011-03-01 11:22           ` Nguyen Thai Ngoc Duy
2011-03-01 13:46             ` Junio C Hamano
2011-03-01 14:15               ` Nguyen Thai Ngoc Duy
2011-03-01 14:53           ` Matthieu Moy
2011-03-01 18:40             ` Junio C Hamano
2011-03-01 18:51               ` Matthieu Moy
2011-03-01 19:36                 ` Jakub Narebski
2011-03-01 20:00                 ` Jeff King
2011-03-01 20:12                 ` Junio C Hamano
2011-03-01 20:25                   ` Matthieu Moy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.