Re: [RFC PATCH] push: start warning upcoming default change for push.default

["Carlos Martín Nieto" <cmn@elego.de>]

[-- Attachment #1: Type: text/plain, Size: 4687 bytes --]

On Fri, 2012-03-09 at 09:31 +0100, Matthieu Moy wrote:
> More inexperienced users will often push right after committing, and at
> that time they're still very much in the "working-on-one-branch" state of
> mind.  "upstream" would be a safer default mode of operation for 'git push'
> for them even when they have their personal publishing repository (also in
> a shared public repository settings, "matching" is rarely the right
> default mode).
> 
> In preparation for flipping the default to the "upstream" mode from the
> "matching" mode that is the upstream default, start warning users when they
> rely on unconfigured "git push" to default to the "matching" mode.
> 
> Original patch and commit message by: Junio C Hamano <gitster@pobox.com>
> 
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
> 
> This patch prepares a transition to 'upstream', unlike the previous
> version which was advertizing 'current'. In most case, this would be
> the same, but 'upstream' is probably more sensible in case it points
> to a branch other than 'current'. I don't care much either way.
> 

For people using git as VCS that happens to have local history rather
than taking full advantage of the distributed nature (or who aren't
aware of it or don't get it), 'matching' is bound to be confusing.
However, IMO 'current' makes more sense. Consider

    git clone ../foo; cd foo;
    git checkout -b topic origin/develop
    ed main.c
    git push

With upstream I've just updated origin's dev branch, even though I might
have meant to create a new 'topic' branch. Alternatively, I might have
run

    git checkout -b topic

in which case I certainly want 'current'. I don't see that workflow
where the upstream branch is named differently from the local one should
be that big a consideration when trying to determine a default to help
people unfamiliar as they'd be certainly more likely to consider branch
names to be binding. Maybe you have seen this differently in your
students, but that's the impression I get from #git.

They also seem to expect 'git pull' to be magic, but that's a different
story.

> I've kept the wording from the original patch, which commits to a
> future change. We may instead relax this commitment and just say "the
> default is likely to change in a future version", or so.
> 
>  builtin/push.c |   22 ++++++++++++++++++++++
>  cache.h        |    1 +
>  environment.c  |    2 +-
>  3 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/builtin/push.c b/builtin/push.c
> index d315475..03545c0 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -91,10 +91,32 @@ static void setup_push_upstream(struct remote *remote)
>  	add_refspec(refspec.buf);
>  }
>  
> +static void warn_unspecified_push_default_configuration(void)
> +{
> +	static int warn_once;
> +
> +	if (warn_once++)
> +		return;
> +	warning(_("push.default is unset; its implicit value is changing in 1.8.0 from\n"
> +		  "'matching' to 'upstream'. To squelch this message and maintain the current\n"
> +		  "behavior post-1.8.0, use:\n"
> +		  "\n"
> +		  "  git config --global push.default matching\n"
> +		  "\n"
> +		  "To squelch this message and adopt the 1.8.0 behavior now, use:\n"
> +		  "\n"
> +		  "  git config --global push.default upstream\n"
> +		  "\n"
> +		  "See 'git help config' and search for 'push.default' for further information.\n"));
> +}
> +
>  static void setup_default_push_refspecs(struct remote *remote)
>  {
>  	switch (push_default) {
>  	default:
> +	case PUSH_DEFAULT_UNSPECIFIED:
> +		warn_unspecified_push_default_configuration();
> +		/* fallthru */
>  	case PUSH_DEFAULT_MATCHING:
>  		add_refspec(":");
>  		break;
> diff --git a/cache.h b/cache.h
> index e12b15f..e5c3f26 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -622,6 +622,7 @@ enum rebase_setup_type {
>  };
>  
>  enum push_default_type {
> +	PUSH_DEFAULT_UNSPECIFIED = -1,
>  	PUSH_DEFAULT_NOTHING = 0,
>  	PUSH_DEFAULT_MATCHING,
>  	PUSH_DEFAULT_UPSTREAM,
> diff --git a/environment.c b/environment.c
> index c93b8f4..d7e6c65 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -52,7 +52,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_MATCHING;
> +enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
>  #ifndef OBJECT_CREATION_MODE
>  #define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
>  #endif



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]