[PATCH v2] New config push.default to decide default behavior for push

[PATCH v2] New config push.default to decide default behavior for push

[Finn Arne Gangstad]

This is intended to replace fg/push-default, I'll repost some of the missing
parts as other patches.

--8<--
New config push.default to decide default behavior for push

Currently git push will push all matching branches to the current
remote.  For some workflows this default is suboptimal, for new users
it is often surprising, and some have found the default behaviour too
easy to trig by accident with unwanted consequences.

Introduce a new configuration variable "push.default" that decides what
action git push should take if no refspecs are given or implied by the
command line arguments or the current remote configuration. If this
variable is unconfigured, display a prominent warning when default
behavior is trigged.

Possible values are:
'nothing'  : Push nothing
'matching' : Current default behaviour, push all branches that already exist
   in the current remote
'tracking' : Push the current branch to whatever it is tracking
'current'  : Push the current branch to a branch of the same name

Signed-off-by: Finn Arne Gangstad <finnag@pvv.org>
---
 Documentation/RelNotes-1.6.3.txt |    7 +++
 Documentation/config.txt         |   19 +++++++++
 builtin-push.c                   |   80 +++++++++++++++++++++++++++++++++++--
 cache.h                          |    9 ++++
 config.c                         |   25 ++++++++++++
 environment.c                    |    1 +
 6 files changed, 136 insertions(+), 5 deletions(-)

diff --git a/Documentation/RelNotes-1.6.3.txt b/Documentation/RelNotes-1.6.3.txt
index ee1fddb..3eb6bf0 100644
--- a/Documentation/RelNotes-1.6.3.txt
+++ b/Documentation/RelNotes-1.6.3.txt
@@ -22,6 +22,13 @@ branch pointed at by its HEAD, gets a large warning.  You can choose what
 should happen upon such a push by setting the configuration variable
 receive.denyDeleteCurrent in the receiving repository.
 
+In a future release, the default of "git push" without further
+arguments may be changed. Currently, it will push all matching
+refspecs to the current remote.  A configuration variable push.default
+has been introduced to select the default behaviour.  To ease the
+transition, a big warning is issued if this is not configured and a
+git push without arguments is attempted.
+
 
 Updates since v1.6.2
 --------------------
diff --git a/Documentation/config.txt b/Documentation/config.txt
index f5152c5..6fdf829 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1160,6 +1160,25 @@ pull.octopus::
 pull.twohead::
 	The default merge strategy to use when pulling a single branch.
 
+push.default::
+	Defines the action git push should take if no refspec is given
+	on the command line, no refspec is configured in the remote, and
+	no refspec is implied by any of the options given on the command
+	line.
+
+	The term `current remote` means the remote configured for the current
+	branch, or `origin` if no remote is configured. `origin` is also used
+	if you are not on any branch.
+
+	Possible values are:
++
+* `nothing` do not push anything.
+* `matching` push all matching branches to the current remote.
+  All branches having the same name in both ends are considered to be
+  matching. This is the current default value.
+* `tracking` push the current branch to whatever it is tracking.
+* `current` push the current branch to a branch of the same name.
+
 receive.fsckObjects::
 	If it is set to true, git-receive-pack will check all received
 	objects. It will abort in the case of a malformed object or a
diff --git a/builtin-push.c b/builtin-push.c
index 122fdcf..fa5eabb 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -48,6 +48,75 @@ static void set_refspecs(const char **refs, int nr)
 	}
 }
 
+static void setup_push_tracking(struct remote *remote)
+{
+	int n;
+	struct branch *branch = branch_get(NULL);
+	if (!branch)
+		die("You are not currently on a branch.");
+	if (!branch->merge_nr)
+		die("The current branch %s is not tracking anything.",
+		    branch->name);
+	if (branch->remote != remote)
+		die("The current branch is tracking \"%s\", not \"%s\"!",
+		    branch->remote->name, remote->name);
+	for (n = 0; n < branch->merge_nr; n++) {
+		struct strbuf rs = STRBUF_INIT;
+		strbuf_addf(&rs, "%s:%s", branch->name, branch->merge[n]->src);
+		add_refspec(rs.buf);
+	}
+}
+
+static const char *warn_unconfigured_push_msg[] = {
+	"You did not specify any refspecs to push, and the current remote",
+	"has not configured any push refspecs. The default action in this",
+	"case has been to push all matching refspecs, that is, all branches",
+	"that exist both locally and remotely will be updated.",
+	"This default may change in the future.",
+	"",
+	"You can specify what action you want to take in this case, and",
+	"avoid seeing this message again, by configuring 'push.default' to:",
+	"  'nothing'  : Do not push anythig",
+	"  'matching' : Push all matching branches (the current default)",
+	"  'tracking' : Push the current branch to whatever it is tracking",
+	"  'current'  : Push the current branch",
+	""
+};
+
+static void warn_unconfigured_push()
+{
+	int i;
+	for (i = 0; i < ARRAY_SIZE(warn_unconfigured_push_msg); i++)
+		warning("%s", warn_unconfigured_push_msg[i]);
+}
+
+static void do_default_push(struct remote *remote)
+{
+	git_config(git_default_config, NULL);
+	switch (push_default) {
+	case PUSH_DEFAULT_UNSPECIFIED:
+		warn_unconfigured_push();
+		/* fallthrough */
+
+	case PUSH_DEFAULT_MATCHING:
+		add_refspec(":");
+		break;
+
+	case PUSH_DEFAULT_TRACKING:
+		setup_push_tracking(remote);
+		break;
+
+	case PUSH_DEFAULT_CURRENT:
+		add_refspec("HEAD");
+		break;
+
+	case PUSH_DEFAULT_NOTHING:
+		die("Nothing to push, and push is configured to push nothing "
+		    "by default.");
+		break;
+	}
+}
+
 static int do_push(const char *repo, int flags)
 {
 	int i, errs;
@@ -76,11 +145,12 @@ static int do_push(const char *repo, int flags)
 		return error("--all and --mirror are incompatible");
 	}
 
-	if (!refspec
-		&& !(flags & TRANSPORT_PUSH_ALL)
-		&& remote->push_refspec_nr) {
-		refspec = remote->push_refspec;
-		refspec_nr = remote->push_refspec_nr;
+	if (!refspec && !(flags & TRANSPORT_PUSH_ALL)) {
+		if (remote->push_refspec_nr) {
+			refspec = remote->push_refspec;
+			refspec_nr = remote->push_refspec_nr;
+		} else if (!(flags & TRANSPORT_PUSH_MIRROR))
+			do_default_push(remote);
 	}
 	errs = 0;
 	for (i = 0; i < remote->url_nr; i++) {
diff --git a/cache.h b/cache.h
index 189151d..df4f117 100644
--- a/cache.h
+++ b/cache.h
@@ -541,8 +541,17 @@ enum rebase_setup_type {
 	AUTOREBASE_ALWAYS,
 };
 
+enum push_default_type {
+	PUSH_DEFAULT_UNSPECIFIED = -1,
+	PUSH_DEFAULT_NOTHING = 0,
+	PUSH_DEFAULT_MATCHING,
+	PUSH_DEFAULT_TRACKING,
+	PUSH_DEFAULT_CURRENT,
+};
+
 extern enum branch_track git_branch_track;
 extern enum rebase_setup_type autorebase;
+extern enum push_default_type push_default;
 
 #define GIT_REPO_VERSION 0
 extern int repository_format_version;
diff --git a/config.c b/config.c
index 0c8c76f..e36a2b0 100644
--- a/config.c
+++ b/config.c
@@ -565,6 +565,28 @@ static int git_default_branch_config(const char *var, const char *value)
 	return 0;
 }
 
+static int git_default_push_config(const char *var, const char *value)
+{
+	if (!strcmp(var, "push.default")) {
+		if (!value)
+			return config_error_nonbool(var);
+		else if (!strcmp(value, "nothing"))
+			push_default = PUSH_DEFAULT_NOTHING;
+		else if (!strcmp(value, "matching"))
+			push_default = PUSH_DEFAULT_MATCHING;
+		else if (!strcmp(value, "tracking"))
+			push_default = PUSH_DEFAULT_TRACKING;
+		else if (!strcmp(value, "current"))
+			push_default = PUSH_DEFAULT_CURRENT;
+		else
+			return error("Malformed value for %s", var);
+		return 0;
+	}
+
+	/* Add other config variables here and to Documentation/config.txt. */
+	return 0;
+}
+
 static int git_default_mailmap_config(const char *var, const char *value)
 {
 	if (!strcmp(var, "mailmap.file"))
@@ -588,6 +610,9 @@ int git_default_config(const char *var, const char *value, void *dummy)
 	if (!prefixcmp(var, "branch."))
 		return git_default_branch_config(var, value);
 
+	if (!prefixcmp(var, "push."))
+		return git_default_push_config(var, value);
+
 	if (!prefixcmp(var, "mailmap."))
 		return git_default_mailmap_config(var, value);
 
diff --git a/environment.c b/environment.c
index e278bce..4696885 100644
--- a/environment.c
+++ b/environment.c
@@ -42,6 +42,7 @@ enum safe_crlf safe_crlf = SAFE_CRLF_WARN;
 unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;
 enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;
 enum rebase_setup_type autorebase = AUTOREBASE_NEVER;
+enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
 
 /* Parallel index stat data preload? */
 int core_preload_index = 0;
-- 
1.6.2.107.ge47ee.dirty

Re: [PATCH v2] New config push.default to decide default behavior for push

[Junio C Hamano]

Finn Arne Gangstad <finnag@pvv.org> writes:

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index f5152c5..6fdf829 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1160,6 +1160,25 @@ pull.octopus::
>  pull.twohead::
>  	The default merge strategy to use when pulling a single branch.
>  
> +push.default::
> +	Defines the action git push should take if no refspec is given
> +	on the command line, no refspec is configured in the remote, and
> +	no refspec is implied by any of the options given on the command
> +	line.
> +
> +	The term `current remote` means the remote configured for the current
> +	branch, or `origin` if no remote is configured. `origin` is also used
> +	if you are not on any branch.
> +
> +	Possible values are:
> ++
> +* `nothing` do not push anything.
> +* `matching` push all matching branches to the current remote.
> +  All branches having the same name in both ends are considered to be
> +  matching. This is the current default value.
> +* `tracking` push the current branch to whatever it is tracking.
> +* `current` push the current branch to a branch of the same name.
> +

I thought I fixed asciidoc formatting around this part in the version I
queued in 'pu'; in any case, the second and subsequent paragraphs need
dedenting.

> +static void warn_unconfigured_push()
> +{

I think I also fixed this "old style declaration".

> +static void do_default_push(struct remote *remote)
> +{
> +	git_config(git_default_config, NULL);
> +	switch (push_default) {
> +	case PUSH_DEFAULT_UNSPECIFIED:
> +		warn_unconfigured_push();
> +		/* fallthrough */
> +
> +	case PUSH_DEFAULT_MATCHING:
> +		add_refspec(":");
> +		break;
> +
> +	case PUSH_DEFAULT_TRACKING:
> +		setup_push_tracking(remote);
> +		break;
> +
> +	case PUSH_DEFAULT_CURRENT:
> +		add_refspec("HEAD");
> +		break;
> +
> +	case PUSH_DEFAULT_NOTHING:
> +		die("Nothing to push, and push is configured to push nothing "
> +		    "by default.");
> +		break;
> +	}
> +}

This part looks good, provided if we were to go ahead and plan to change
the default in the future.

> +static void setup_push_tracking(struct remote *remote)
> +{
> +	int n;
> +	struct branch *branch = branch_get(NULL);
> +	if (!branch)
> +		die("You are not currently on a branch.");
> +	if (!branch->merge_nr)
> +		die("The current branch %s is not tracking anything.",
> +		    branch->name);
> +	if (branch->remote != remote)
> +		die("The current branch is tracking \"%s\", not \"%s\"!",
> +		    branch->remote->name, remote->name);
> +	for (n = 0; n < branch->merge_nr; n++) {
> +		struct strbuf rs = STRBUF_INIT;
> +		strbuf_addf(&rs, "%s:%s", branch->name, branch->merge[n]->src);
> +		add_refspec(rs.buf);
> +	}
> +}

If a branch is configured to merge from multiple places (e.g. testing
branch similar to the linux-next tree to integrate from multiple staging
trees), a sane default would be not to push it out to any of the branches
it pulls from---it is a consumer to the other branches, and it is meant to
be sent to somewhere else, not back to any of the originators.  Instead,
this code will push to all of them, which I would not see any sane use
case for.  It might make a bit sense if you refused unless merge_nr is
exactly one.

Also I am not sure if the check between the name of the remote makes much
practical sense.  Many people use two remotes to name the same one for
pushing over ssh and fetching over git.  Which name comes in which?  I
think with this logic you are trying to catch a mistake like:

    $ git push --tracking $there

when the current branch tracks a remote branch that did not come from the
remote repository $there but somewhere else, but if that is the motivation
behind it, does it help to forbid "push --tracking" to take any
destination repository to reduce such a confusion?

Re: [PATCH v2] New config push.default to decide default behavior for push

[Finn Arne Gangstad]

On Wed, Mar 11, 2009 at 05:48:13PM -0700, Junio C Hamano wrote:

> I thought I fixed asciidoc formatting around this part in the version I
> queued in 'pu'; in any case, the second and subsequent paragraphs need
> dedenting.

I am sorry, it wasn't immediately obvious to me that you had changed
the version i sent. Fixed up this (and other fixes) in my working
branch I think.

> > +static void setup_push_tracking(struct remote *remote)
> > +{
> > +	int n;
> > +	struct branch *branch = branch_get(NULL);
> > +	if (!branch)
> > +		die("You are not currently on a branch.");
> > +	if (!branch->merge_nr)
> > +		die("The current branch %s is not tracking anything.",
> > +		    branch->name);
> > +	if (branch->remote != remote)
> > +		die("The current branch is tracking \"%s\", not \"%s\"!",
> > +		    branch->remote->name, remote->name);
> > +	for (n = 0; n < branch->merge_nr; n++) {
> > +		struct strbuf rs = STRBUF_INIT;
> > +		strbuf_addf(&rs, "%s:%s", branch->name, branch->merge[n]->src);
> > +		add_refspec(rs.buf);
> > +	}
> > +}
> 
> If a branch is configured to merge from multiple places (e.g. testing
> branch similar to the linux-next tree to integrate from multiple staging
> trees), a sane default would be not to push it out to any of the branches
> it pulls from---it is a consumer to the other branches, and it is meant to
> be sent to somewhere else, not back to any of the originators.  Instead,
> this code will push to all of them, which I would not see any sane use
> case for.  It might make a bit sense if you refused unless merge_nr is
> exactly one.

Yes I agree, fixed this up.

> Also I am not sure if the check between the name of the remote makes much
> practical sense.  Many people use two remotes to name the same one for
> pushing over ssh and fetching over git.  Which name comes in which?  I
> think with this logic you are trying to catch a mistake like:
> 
>     $ git push --tracking $there

Yes that was the idea. I was not familiar with the "multiple remotes
to the same thing" common usage, but have no problems supporting that
instead.

Something like this amended into the last commit? I can amend it on top
of the last one and resend if that is better.

--8<--
git push tracking mode fixes

If a branch is tracking multiple branches, refuse to push it.
Some asciidoc format fixes.

Signed-off-by: Finn Arne Gangstad <finnag@pvv.org>
---
To be amended into the previous commit

 Documentation/config.txt |   16 ++++++++--------
 builtin-push.c           |   25 +++++++++++--------------
 2 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6fdf829..986becc 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1165,19 +1165,19 @@ push.default::
 	on the command line, no refspec is configured in the remote, and
 	no refspec is implied by any of the options given on the command
 	line.
-
-	The term `current remote` means the remote configured for the current
-	branch, or `origin` if no remote is configured. `origin` is also used
-	if you are not on any branch.
-
-	Possible values are:
++
+The term `current remote` means the remote configured for the current
+branch, or `origin` if no remote is configured. `origin` is also used
+if you are not on any branch. Possible values are:
 +
 * `nothing` do not push anything.
 * `matching` push all matching branches to the current remote.
   All branches having the same name in both ends are considered to be
   matching. This is the current default value.
-* `tracking` push the current branch to whatever it is tracking.
-* `current` push the current branch to a branch of the same name.
+* `tracking` push the current branch to the branch it is tracking.
+* `current` push the current branch to a branch of the same name on the
+  current remote.
+
 
 receive.fsckObjects::
 	If it is set to true, git-receive-pack will check all received
diff --git a/builtin-push.c b/builtin-push.c
index fa5eabb..51f4c4a 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -48,23 +48,20 @@ static void set_refspecs(const char **refs, int nr)
 	}
 }
 
-static void setup_push_tracking(struct remote *remote)
+static void setup_push_tracking(void)
 {
-	int n;
+	struct strbuf refspec = STRBUF_INIT;
 	struct branch *branch = branch_get(NULL);
 	if (!branch)
 		die("You are not currently on a branch.");
 	if (!branch->merge_nr)
 		die("The current branch %s is not tracking anything.",
 		    branch->name);
-	if (branch->remote != remote)
-		die("The current branch is tracking \"%s\", not \"%s\"!",
-		    branch->remote->name, remote->name);
-	for (n = 0; n < branch->merge_nr; n++) {
-		struct strbuf rs = STRBUF_INIT;
-		strbuf_addf(&rs, "%s:%s", branch->name, branch->merge[n]->src);
-		add_refspec(rs.buf);
-	}
+	if (branch->merge_nr != 1)
+		die("The current branch %s is tracking multiple branches, "
+		    "refusing to push.", branch->name);
+	strbuf_addf(&refspec, "%s:%s", branch->name, branch->merge[0]->src);
+	add_refspec(refspec.buf);
 }
 
 static const char *warn_unconfigured_push_msg[] = {
@@ -83,14 +80,14 @@ static const char *warn_unconfigured_push_msg[] = {
 	""
 };
 
-static void warn_unconfigured_push()
+static void warn_unconfigured_push(void)
 {
 	int i;
 	for (i = 0; i < ARRAY_SIZE(warn_unconfigured_push_msg); i++)
 		warning("%s", warn_unconfigured_push_msg[i]);
 }
 
-static void do_default_push(struct remote *remote)
+static void do_default_push(void)
 {
 	git_config(git_default_config, NULL);
 	switch (push_default) {
@@ -103,7 +100,7 @@ static void do_default_push(struct remote *remote)
 		break;
 
 	case PUSH_DEFAULT_TRACKING:
-		setup_push_tracking(remote);
+		setup_push_tracking();
 		break;
 
 	case PUSH_DEFAULT_CURRENT:
@@ -150,7 +147,7 @@ static int do_push(const char *repo, int flags)
 			refspec = remote->push_refspec;
 			refspec_nr = remote->push_refspec_nr;
 		} else if (!(flags & TRANSPORT_PUSH_MIRROR))
-			do_default_push(remote);
+			do_default_push();
 	}
 	errs = 0;
 	for (i = 0; i < remote->url_nr; i++) {
-- 
1.6.2.81.gc6c21.dirty

Re: [PATCH v2] New config push.default to decide default behavior for push

[Junio C Hamano]

Finn Arne Gangstad <finnag@pvv.org> writes:

> Something like this amended into the last commit? I can amend it on top
> of the last one and resend if that is better.

Thanks.

I looked at these two patches after squashing them into one, and I think
it makes sense as the final shape of a two patch series.

Although the purist in me tells me that addition of the "tracking" and the
"current" should be in a separate commit as they are purely new features,
I think it is Ok.  In that sense, "nothing" is a new feature anyway, and
"current" is something we already have, so the true addition here is only
the "tracking" one.

I also reworded the commit log message a bit, like this:

    Author: Finn Arne Gangstad <finnag@pvv.org>
    Date:   Wed Mar 11 23:01:45 2009 +0100

    New config push.default to decide default behavior for push
    
    When "git push" is not told what refspecs to push, it pushes all matching
    branches to the current remote.  For some workflows this default is not
    useful, and surprises new users.  Some have even found that this default
    behaviour too easy to trigger by accident with unwanted consequences.
    
    Introduce a new configuration variable "push.default" that decides what
    action git push should take if no refspecs are given or implied by the
    command line arguments or the current remote configuration. If this
    variable is unconfigured, display a prominent warning when default
    behavior is triggered.
    
    Possible values are:
    
      'nothing'  : Push nothing;
      'matching' : Current default behaviour, push all branches that already
                   exist in the current remote;
      'tracking' : Push the current branch to whatever it is tracking;
      'current'  : Push the current branch to a branch of the same name,
      	           i.e. HEAD.

    Signed-off-by: Finn Arne Gangstad <finnag@pvv.org>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

I however had to scratch my head for 20 seconds wondering where the push
happens when do_default_push() codepath is taken, and it turns out that
the function is there merely to set up the push refspecs for the default
case; the function is misnamed.  I'd further squash the following patch
in.

diff --git a/builtin-push.c b/builtin-push.c
index 51f4c4a..e4988da 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -76,8 +76,7 @@ static const char *warn_unconfigured_push_msg[] = {
 	"  'nothing'  : Do not push anythig",
 	"  'matching' : Push all matching branches (the current default)",
 	"  'tracking' : Push the current branch to whatever it is tracking",
-	"  'current'  : Push the current branch",
-	""
+	"  'current'  : Push the current branch"
 };
 
 static void warn_unconfigured_push(void)
@@ -87,7 +86,7 @@ static void warn_unconfigured_push(void)
 		warning("%s", warn_unconfigured_push_msg[i]);
 }
 
-static void do_default_push(void)
+static void setup_default_push_refspecs(void)
 {
 	git_config(git_default_config, NULL);
 	switch (push_default) {
@@ -147,7 +146,7 @@ static int do_push(const char *repo, int flags)
 			refspec = remote->push_refspec;
 			refspec_nr = remote->push_refspec_nr;
 		} else if (!(flags & TRANSPORT_PUSH_MIRROR))
-			do_default_push();
+			setup_default_push_refspecs();
 	}
 	errs = 0;
 	for (i = 0; i < remote->url_nr; i++) {

Re: [PATCH v2] New config push.default to decide default behavior for push

[Junio C Hamano]

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

> Finn Arne Gangstad <finnag@pvv.org> writes:
>
>> Something like this amended into the last commit? I can amend it on top
>> of the last one and resend if that is better.
>
> Thanks.
>
> I looked at these two patches after squashing them into one, and I think
> it makes sense as the final shape of a two patch series.

I seem to have forgotten to say what the two pieces in the series should
look like here.  What I meant was a split along these lines:

 (1) introduce the configuration variable, so that people who do not like
     the current default can get a different default, but do not warn;

 (2) start warning to notify the users of possible different settings and
     forcing them to choose now, before we switch the default.

As some people still seem to object to the change of default (and that
includes 20-30% of myself), we may end up deciding not to switch the
default after all, but even in that case, applying the first half would
benefit people who would want different behaviour.

If we were to decide not to switch the default, applying (2) would force
people who want the current default to set the configuration only to
squelch the message, which is an annoying inconvenience.

But since then I thought about this a bit more, and I am inclined to
change my mind.  Having (2) will allow people who wish there were a way to
have a different default to discover that there is already an easy way to
do so.  The tradeoff between "a slight inconvenience to traditionalist" vs
"helping people to discover the feature designed for them, without which
they would be badly burned" does not look too bad to me.

Re: [PATCH v2] New config push.default to decide default behavior for push

[Finn Arne Gangstad]

On Sun, Mar 15, 2009 at 09:55:23PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> As some people still seem to object to the change of default (and that
> includes 20-30% of myself), we may end up deciding not to switch the
> default after all, but even in that case, applying the first half would
> benefit people who would want different behaviour.

I think the suggested new default is a lot safer then the current
one. A default of "nothing" will print a nice message if you end up
pushing nothing, which you will fix in a heartbeat with a single git
config command.

If you erroneously push one or more branches however, cleanup might
end up being very complicated. Many pushable repos are set up with
disallowing non-fast-forward pushes, so it may require intervention by
someone else to clean up, and by then someone else have already
fetched the bad push.

- Finn Arne

Re: [PATCH v2] New config push.default to decide default behavior for push

[Junio C Hamano]

Finn Arne Gangstad <finnag@pvv.org> writes:

> On Sun, Mar 15, 2009 at 09:55:23PM -0700, Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>> 
>> As some people still seem to object to the change of default (and that
>> includes 20-30% of myself), we may end up deciding not to switch the
>> default after all, but even in that case, applying the first half would
>> benefit people who would want different behaviour.
>
> I think the suggested new default is a lot safer then the current
> one. A default of "nothing" will print a nice message if you end up
> pushing nothing, which you will fix in a heartbeat with a single git
> config command.
>
> If you erroneously push one or more branches however, cleanup might
> end up being very complicated. Many pushable repos are set up with
> disallowing non-fast-forward pushes, so it may require intervention by
> someone else to clean up, and by then someone else have already
> fetched the bad push.

I think traditionalists who do not like changing the default already know
these, though.  I would not object to the push.default as a _choice_.

In fact, sourceforge.jp (they added git support late last year, and I keep
a secondary public repository just like my alt-git.git at repo.or.cz) is
one of such places.  It seems to forbid non fast-forward pushes, and that
is why I have been pushing only maint and master there.  It does allow
deletion, and I could push deletion followed by creation in two stages,
i.e. "git push sfjp :pu && git push sfjp pu", but I did not bother.

In a later part of the message you are responding to (but did not quote),
I was agreeing with all of what you wrote here, and even more ;-) Notice
the "tradeoff does not look too bad to me" part.

Your new [1/2] gives the choice without advertisement, and _if_ you remove
or tone down "The default may change in the future" from [2/2], it becomes
purely an advertisement of the feature to help people from burning
themselves.  I do not see anything a sane traditionist would object to at
that point, and I thought we could even squash the two into one commit
(which was what I meant by "I am inclined to change my mind" in the
message you are responding to).