git push origin error (1.6.3 new default functionality)

git push origin error (1.6.3 new default functionality)

[Caleb Cushing]

in the past git push origin would just push all matching branches to
the remote, and it worked. the new error says that's still the
default. The new functionality is nice, but is it really the git way
to yell at you if you haven't explicitly set the default? I think the
default should remain the default, and it should continue to work
without yelling at you for not explicitly setting it. if you want to
change it that's fine.

-- 
Caleb Cushing

http://xenoterracide.blogspot.com

Re: git push origin error (1.6.3 new default functionality)

[Michael J Gruber]

Caleb Cushing venit, vidit, dixit 12.05.2009 03:26:
> in the past git push origin would just push all matching branches to
> the remote, and it worked. the new error says that's still the
> default. The new functionality is nice, but is it really the git way
> to yell at you if you haven't explicitly set the default? I think the
> default should remain the default, and it should continue to work
> without yelling at you for not explicitly setting it. if you want to
> change it that's fine.
> 
\begin{rambling}
It is a fall-out from the new user friendliness initiative. Watch out
for parentheses:
it's ( (new user) friendliness ), not ( new (user friendliness) ).

The principle is: if a user is about to do something which is documented
but might not have been intended we throw a half-screen full of text at
them. The idea is that it is virtually impossible to grasp at a glance
from that much text what happened, so that the user is forced to read
the whole text. edugit, so to say.
\end{rambling}

Seriously, we had that discussion when the feature (change of default
behaviour) and warning were introduced, so it's too late for a change.
But it's never too late to do

git config --global push.default matching

and be done with it.

The weird thing is that the default is "matching" already. But it makes
a difference whether you have set the variable to its default value
explicitly or not. No man page says so (neither git-push nor
git-config), and I can't think of other variables with such a behaviour.

Michael

Re: git push origin error (1.6.3 new default functionality)

[Caleb Cushing]

On Tue, May 12, 2009 at 7:11 AM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> Seriously, we had that discussion when the feature (change of default
> behaviour) and warning were introduced, so it's too late for a change.
> But it's never too late to do
>

It's open source. it's too  late to change 1.6.3 but the error message
could be easily remove in 1.6.3.1

I'll reiterate that I shouldn't have to explicitly set default
behavior to not see error messages.
-- 
Caleb Cushing

http://xenoterracide.blogspot.com

Re: git push origin error (1.6.3 new default functionality)

[Jeff King]

On Wed, May 13, 2009 at 01:26:26AM -0400, Caleb Cushing wrote:

> On Tue, May 12, 2009 at 7:11 AM, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
> > Seriously, we had that discussion when the feature (change of default
> > behaviour) and warning were introduced, so it's too late for a change.
> > But it's never too late to do
> >
> 
> It's open source. it's too  late to change 1.6.3 but the error message
> could be easily remove in 1.6.3.1
> 
> I'll reiterate that I shouldn't have to explicitly set default
> behavior to not see error messages.

Are you proposing not to change the default behavior? If you are, then
you should at least address the concerns raised in all of the existing
threads.

Or are you proposing to still change the default behavior, but drop the
warning whose aim is to inform people about the impending change? In
that case, I think you should address the concerns that arose from
previous changes in default behavior (and which this warning is meant to
address), and propose an alternate plan for making the transition more
smooth.

-Peff

Re: git push origin error (1.6.3 new default functionality)

[Johannes Sixt]

Context: This is about the warning that a plain "git push" produces if no
push refspec are configured. The default behavior is to push matching
branches.

Jeff King schrieb:
> Or are you proposing to still change the default behavior, but drop the
> warning whose aim is to inform people about the impending change? In
> that case, I think you should address the concerns that arose from
> previous changes in default behavior (and which this warning is meant to
> address), and propose an alternate plan for making the transition more
> smooth.

Unfortunately, the case with this warning is not that "simple" because it
is not about a planned change of the default behavior, but about a default
behavior that may be unexpected for newbies (see the release notes of
1.6.3). I *can* understand that Caleb is upset by the warning, since he's
comfortable with the (current and future) default behavior. But I don't
know what to do in cases like these.

-- Hannes

Re: git push origin error (1.6.3 new default functionality)

[Jeff King]

On Wed, May 13, 2009 at 10:44:33AM +0200, Johannes Sixt wrote:

> Unfortunately, the case with this warning is not that "simple" because it
> is not about a planned change of the default behavior, but about a default
> behavior that may be unexpected for newbies (see the release notes of
> 1.6.3). I *can* understand that Caleb is upset by the warning, since he's
> comfortable with the (current and future) default behavior. But I don't
> know what to do in cases like these.

I thought this was in preparation for an eventual change, but I might be
wrong (1.6.3 introduced several such warnings).

Regardless, my point was: the warning was introduced for a purpose
(either to point out potentially confusing behavior, or to warn the user
about an upcoming change in default behavior). Showing up now and saying
"I don't like this warning" without addressing any of the points in the
original discussion or making any sort of proposal to try to accomplish
the same goals is just counterproductive.

-Peff

Re: git push origin error (1.6.3 new default functionality)

[Michael J Gruber]

Jeff King venit, vidit, dixit 13.05.2009 11:03:
> On Wed, May 13, 2009 at 10:44:33AM +0200, Johannes Sixt wrote:
> 
>> Unfortunately, the case with this warning is not that "simple" because it
>> is not about a planned change of the default behavior, but about a default
>> behavior that may be unexpected for newbies (see the release notes of
>> 1.6.3). I *can* understand that Caleb is upset by the warning, since he's
>> comfortable with the (current and future) default behavior. But I don't
>> know what to do in cases like these.
> 
> I thought this was in preparation for an eventual change, but I might be
> wrong (1.6.3 introduced several such warnings).
> 
> Regardless, my point was: the warning was introduced for a purpose
> (either to point out potentially confusing behavior, or to warn the user
> about an upcoming change in default behavior). Showing up now and saying
> "I don't like this warning" without addressing any of the points in the
> original discussion or making any sort of proposal to try to accomplish
> the same goals is just counterproductive.

I don't want to stir this up to much again - as I said, set config and
be done.

My main issue is the fact that we have a config variable (push.default)
which causes a different behaviour depending on whether it is unset or
set to its default (!) value. That is a completely new UI approach. We
may follow through with this for a "beginners' mode" for git, where
commands with possibly unintended side effects issue warnings, as
opposed to an "advanced mode" (activated by 1 config variable) which
shuts these off. Right now this new behaviour is isolated and feels strange.

Michael

Re: git push origin error (1.6.3 new default functionality)

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> Seriously, we had that discussion when the feature (change of default
> behaviour) and warning were introduced, so it's too late for a change.
> But it's never too late to do
>
> git config --global push.default matching
>
> and be done with it.

Thanks for saying this concisely, and saving me from repeating this.

> The weird thing is that the default is "matching" already. But it makes
> a difference whether you have set the variable to its default value
> explicitly or not. No man page says so (neither git-push nor
> git-config), and I can't think of other variables with such a behaviour.

It is not weird at all.  Although I am still not convinced (and I suspect
some old timers are not either), the default could change in the future
and by setting it to 'matching', you will protect yourself from such a
change.  Consider it a vaccination ;-)

Re: git push origin error (1.6.3 new default functionality)

[Caleb Cushing]

On Wed, May 13, 2009 at 2:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Thanks for saying this concisely, and saving me from repeating this.

I just don't think one should have to explicitly set something to shut
warnings up. defaults are there for a reason. next thing you know it's
going to ask me if I'd like to continue, and then it will ask me to
press n for next.

Why even have them? if people want to know what's changed they should
read release notes.

maybe a better solution would be to have a setting. set all warnings to off...
-- 
Caleb Cushing

http://xenoterracide.blogspot.com

Re: git push origin error (1.6.3 new default functionality)

[Junio C Hamano]

Caleb Cushing <xenoterracide@gmail.com> writes:

> On Wed, May 13, 2009 at 2:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Thanks for saying this concisely, and saving me from repeating this.
>
> I just don't think one should have to explicitly set something to shut
> warnings up. defaults are there for a reason. next thing you know it's
> going to ask me if I'd like to continue, and then it will ask me to
> press n for next.
>
> Why even have them?

Why do you waste other people's time after repeatedly told this was
discussed to death and everything is recoded in the list archive?

Re: git push origin error (1.6.3 new default functionality)

[Junio C Hamano]

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

> Caleb Cushing <xenoterracide@gmail.com> writes:
>
>> On Wed, May 13, 2009 at 2:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Thanks for saying this concisely, and saving me from repeating this.
>>
>> I just don't think one should have to explicitly set something to shut
>> warnings up. defaults are there for a reason. next thing you know it's
>> going to ask me if I'd like to continue, and then it will ask me to
>> press n for next.
>>
>> Why even have them?
>
> Why do you waste other people's time after repeatedly told this was
> discussed to death and everything is recoded in the list archive?

You know what is most frustrating for me with this whole thing?

As you might have guessed already, I am one of the oldest users of git, am
accustomed to the way "matching push" works, and I like it as a sensible
default behaviour for _my workflow_.  If I, Linus and you were the only
git users, there won't be these half-page-full of warning messages.

But there are others, and one of them was motivated enough to write a
patch series to introduce push.default that allows a setting that may be
more suitable than 'matching' in certain workflows, even though I may not
ever use that workflow in my projects myself.  This early vaccination
approach was the least evil solution proposed back then (which I think was
modelled after the already in-progress "deny git push from updating the
current branch" topic), and you were not around to know that I even toned
down the series not to make it too strongly suggest that the default will
change.

No, "you were not around" part is not what is frustrating.  What is
frustrating is that the original author who felt strongly enough against
'matching' default to write the patch is not defending the change in this
thread, and I have to spend time writing responses like this that I
otherwise could be using for something else to improve the project with.
And what is even more frustrating is that I cannot afford the time to
repeat the full discussion here (nor I have inclinations to), and if you
are the type who does not do his own homework, it would appear to you as
if I am all for changing the default and as if I am being unreasonable.

I do not mind appearing to be a bad guy to you or anybody per-se, but I
think people who got what they wanted earlier should come and defend the
reason why they got what they wanted.

I am nice enough not to threaten them by saying something like "since
nobody seems to be serious enough to defend this earlier change, let's
change our mind and get rid of that warning" ;-)

Oh, I already anticipate that I'll have the same frustration defending the
"deny git push from updating the current branch" that was settled eons
ago.  I am not looking forward to it.

Re: git push origin error (1.6.3 new default functionality)

[Jeff King]

On Wed, May 13, 2009 at 11:54:20AM +0200, Michael J Gruber wrote:

> > Regardless, my point was: the warning was introduced for a purpose
> > (either to point out potentially confusing behavior, or to warn the user
> > about an upcoming change in default behavior). Showing up now and saying
> > "I don't like this warning" without addressing any of the points in the
> > original discussion or making any sort of proposal to try to accomplish
> > the same goals is just counterproductive.
> 
> I don't want to stir this up to much again - as I said, set config and
> be done.

Junio already posted a thoughtful (if perhaps somewhat frustrated) reply
elsewhere, and I agree with most of what he said. I did want to make one
additional point, though, because I think what I said may have appeared
mean. And I was really trying to be nice.

My initial reaction was to say "shut up and set the config variable".
But I really don't like doing that, because I don't want somebody
thinking that all decisions are closed, and it's not possible to come to
the table with new points that may make people change their minds.

When the subject was discussed before, there were people who preferred
various behaviors. They each made arguments, and in the aftermath, Junio
made a decision (presumably based on arguments by list members, opinions
of other developers, and whatever he thought was best) about what to
apply.

If somebody wants to bring up a new argument, new data, or point out
some or changed circumstance that may affect the decision, then I am all
for them doing so. And that is what I was trying to coax out of Caleb.
But without that, I don't see any reason why others should waste their
time reconsidering the decision. Let's assume that the original decision
making process was at least roughly deterministic and would just arrive
at the same answer.

And I think simply posting a "I would have been on the side to prefer X"
opinion isn't really new data. It pushes the tally for that preference
up by one, but the margin of error on such tallies is already huge (I
think we have seen in the past that there is a silent majority who are
_not_ on the git list, and we need to try to address their interests as
well). So while something like a well-managed survey of what git users
would prefer is new data, I consider a single (or even several) "me too"
messages on the list to just be noise in the data.

> My main issue is the fact that we have a config variable (push.default)
> which causes a different behaviour depending on whether it is unset or
> set to its default (!) value. That is a completely new UI approach. We

Well, it depends on how you think of the default. The default could be
"matched-and-warn", and you are fixing it by setting it to "matched". :)

-Peff

Re: git push origin error (1.6.3 new default functionality)

[Michael J Gruber]

Jeff King venit, vidit, dixit 14.05.2009 08:31:
> On Wed, May 13, 2009 at 11:54:20AM +0200, Michael J Gruber wrote:

[snip snip snip]

>> My main issue is the fact that we have a config variable (push.default)
>> which causes a different behaviour depending on whether it is unset or
>> set to its default (!) value. That is a completely new UI approach. We
> 
> Well, it depends on how you think of the default. The default could be
> "matched-and-warn", and you are fixing it by setting it to "matched". :)
> 
> -Peff

So, then we have a config variable which you can set to its default
value only by /unsetting/ it :)

In fact, I think that approach could be valuable in general, making git
more fool-proof for beginners while remaining efficient for the
regulars. As a new concept, to be taken up by "deny non-ff pushes",
"deny delation pushes", "pull without refspec" etc., I would like that.
In order to be useful, beginners should be able to rely on it, i.e.:
unless certain config is set, the git-gun should never go off when
pointed at users' own feet. Might be a worthy target for 1.7 (pun
semi-intended).

Michael

Re: git push origin error (1.6.3 new default functionality)

[Finn Arne Gangstad]

On Wed, May 13, 2009 at 10:29:40PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Caleb Cushing <xenoterracide@gmail.com> writes:
> >
> >> On Wed, May 13, 2009 at 2:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
> >>> Thanks for saying this concisely, and saving me from repeating this.
> >>
> >> I just don't think one should have to explicitly set something to shut
> >> warnings up. defaults are there for a reason. next thing you know it's
> >> going to ask me if I'd like to continue, and then it will ask me to
> >> press n for next.
> >>
> >> Why even have them?
> >
> > Why do you waste other people's time after repeatedly told this was
> > discussed to death and everything is recoded in the list archive?
> 
> You know what is most frustrating for me with this whole thing?
> 
> As you might have guessed already, I am one of the oldest users of git, am
> accustomed to the way "matching push" works, and I like it as a sensible
> default behaviour for _my workflow_.  If I, Linus and you were the only
> git users, there won't be these half-page-full of warning messages.
> 
> But there are others, and one of them was motivated enough to write a
> patch series to introduce push.default that allows a setting that may be
> more suitable than 'matching' in certain workflows, even though I may not
> ever use that workflow in my projects myself.  This early vaccination
> approach was the least evil solution proposed back then (which I think was
> modelled after the already in-progress "deny git push from updating the
> current branch" topic), and you were not around to know that I even toned
> down the series not to make it too strongly suggest that the default will
> change.
> 
> No, "you were not around" part is not what is frustrating.  What is
> frustrating is that the original author who felt strongly enough against
> 'matching' default to write the patch is not defending the change in this
> thread, and I have to spend time writing responses like this that I
> otherwise could be using for something else to improve the project with.

I am sorry to be the primary source of your frustration.

Anyway, to summarize the old thread quickly from my point of view:

"git push" is a very dangerous operation if you are using multiple
remotes, and you are not the owner/maintaner of all of them. A very
common workflow where this is wrong is when using a shared pushable
repo.

Advanced users can configure push as they want, the default is most
important for new users.  From the discussions we had it seems that it
is not possible to make a default that works well for different
workflows, so I argue we should make the least dangerous default the
default.  I would like to change the default behaviour of "git push" to
do nothing at all, and to give a short message hinting at how to
configure it if you invoke git push without any arguments.

The current state of git as of 1.6.3.1, to push matching, but to
complain, is not a good terminal state I think.  I have also noticed
that printing long warnings (10+ lines) is not helpful for people,
they really need to be as short as possible or they will not be read
or understood. So, something like this as the next stage perhaps (this
is not complete, man-pages and some tests should be updated as well):

--8<--
diff --git a/builtin-push.c b/builtin-push.c
index 2eabcd3..80c81ea 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -65,18 +65,8 @@ static void setup_push_tracking(void)
 }
 
 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 is to push all matching refspecs, that is, all branches",
-	"that exist both locally and remotely will be updated.  This may",
-	"not necessarily be what you want to happen.",
-	"",
-	"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 anything",
-	"  'matching' : Push all matching branches (default)",
-	"  'tracking' : Push the current branch to whatever it is tracking",
-	"  'current'  : Push the current branch"
+	"Nothing to push, and push.default is not configured.",
+	"See git-push(1) and git-config(1) for details."
 };
 
 static void warn_unconfigured_push(void)
@@ -92,7 +82,8 @@ static void setup_default_push_refspecs(void)
 	switch (push_default) {
 	case PUSH_DEFAULT_UNSPECIFIED:
 		warn_unconfigured_push();
-		/* fallthrough */
+		exit(EXIT_FAILURE);
+		break;
 
 	case PUSH_DEFAULT_MATCHING:
 		add_refspec(":");
--8<--

- Finn Arne