[ANNOUNCE] Git 1.7.10-rc3

[ANNOUNCE] Git 1.7.10-rc3

[Junio C Hamano]

A release candidate Git 1.7.10-rc3 is now available for testing at
the usual places.

New translations and gitk updates are included in this update. Otherwise
things are calm.

*B* *U* *T*

This one has the updates to the "git merge" that was discussed a few weeks
ago (cf. http://git-blame.blogspot.com/2012/02/anticipating-git-1710.html).
You can try this version to make sure you have enough time updating your
scripts (and debug your updates to your scripts if needed) before the release
happens.

The release tarballs are found at:

    http://code.google.com/p/git-core/downloads/list

and their SHA-1 checksums are:

9d940b9616dc37b06d76d500d59e2bf8c7c4ef8f  git-1.7.10.rc3.tar.gz
810d7cdca63c9fa73fdbbfe1e6ab17bfa41b2c0b  git-htmldocs-1.7.10.rc3.tar.gz
18962a2078d28d5955a04e79d838e3264d646539  git-manpages-1.7.10.rc3.tar.gz

Also the following public repositories all have a copy of the v1.7.10.rc3
tag and the master branch that the tag points at:

  url = git://repo.or.cz/alt-git.git
  url = https://code.google.com/p/git-core/
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git

--------------------------------

Git v1.7.10 Release Notes (draft)
=========================

Compatibility Notes
-------------------

 * From this release on, the "git merge" command in an interactive
   session will start an editor when it automatically resolves the
   merge for the user to explain the resulting commit, just like the
   "git commit" command does when it wasn't given a commit message.

   If you have a script that runs "git merge" and keeps its standard
   input and output attached to the user's terminal, and if you do not
   want the user to explain the resulting merge commits, you can
   export GIT_MERGE_AUTOEDIT environment variable set to "no", like
   this:

	#!/bin/sh
	GIT_MERGE_AUTOEDIT=no
	export GIT_MERGE_AUTOEDIT

   to disable this behaviour (if you want your users to explain their
   merge commits, you do not have to do anything).  Alternatively, you
   can give the "--no-edit" option to individual invocations of the
   "git merge" command if you know everybody who uses your script has
   Git v1.7.8 or newer.

 * The "--binary/-b" options to "git am" have been a no-op for quite a
   while and were deprecated in mid 2008 (v1.6.0).  When you give these
   options to "git am", it will now warn and ask you not to use them.

 * When you do not tell which branches and tags to push to the "git push"
   command in any way, the command used "matching refs" rule to update
   remote branches and tags with branches and tags with the same name you
   locally have.  In future versions of Git, this will change to use the
   "upstream" rule to update the branch at the remote you would "pull"
   from into your current branch with your local current branch.  The
   release after 1.7.10 will start issuing a warning about this change,
   to encourage you to tell the command what to push out, e.g. by setting
   push.default configuration.


Updates since v1.7.9
--------------------

UI, Workflows & Features

 * various "gitk" updates.
   - show the path to the top level directory in the window title
   - update preference edit dialog
   - display file list correctly when directories are given on command line
   - make "git-describe" output in the log message into a clickable link
   - avoid matching the UNIX timestamp part when searching all fields
   - give preference to symbolic font names like sans & monospace
   - allow comparing two commits using a mark
   - "gitk" honors log.showroot configuration.

 * Teams for localizing the messages from the Porcelain layer of
   commands are starting to form, thanks to Jiang Xin who volunteered
   to be the localization coordinator.  Translated messages for
   simplified Chinese and Swedish are available.

 * The configuration mechanism learned an "include" facility; an
   assignment to the include.path pseudo-variable causes the named
   file to be included in-place when Git looks up configuration
   variables.

 * A content filter (clean/smudge) used to be just a way to make the
   recorded contents "more useful", and allowed to fail; a filter can
   now optionally be marked as "required".

 * Options whose names begin with "--no-" (e.g. the "--no-verify"
   option of the "git commit" command) can be negated by omitting
   "no-" from its name, e.g. "git commit --verify".

 * "git am" learned to pass "-b" option to underlying "git mailinfo", so
   that a bracketed string other than "PATCH" at the beginning can be kept.

 * "git clone" learned "--single-branch" option to limit cloning to a
   single branch (surprise!); tags that do not point into the history
   of the branch are not fetched.

 * "git clone" learned to detach the HEAD in the resulting repository
   when the user specifies a tag with "--branch" (e.g., "--branch=v1.0").
   Clone also learned to print the usual "detached HEAD" advice in such
   a case, similar to "git checkout v1.0".

 * When showing a patch while ignoring whitespace changes, the context
   lines are taken from the postimage, in order to make it easier to
   view the output.

 * "git diff --stat" learned to adjust the width of the output on
   wider terminals, and give more columns to pathnames as needed.

 * "diff-highlight" filter (in contrib/) was updated to produce more
   aesthetically pleasing output.

 * "fsck" learned "--no-dangling" option to omit dangling object
   information.

 * "git log -G" and "git log -S" learned to pay attention to the "-i"
   option.  With "-i", "log -G" ignores the case when finding patch
   hunks that introduce or remove a string that matches the given
   pattern.  Similarly with "-i", "log -S" ignores the case when
   finding the commit the given block of text appears or disappears
   from the file.

 * "git merge" in an interactive session learned to spawn the editor
   by default to let the user edit the auto-generated merge message,
   to encourage people to explain their merges better. Legacy scripts
   can export GIT_MERGE_AUTOEDIT=no to retain the historical behavior.
   Both "git merge" and "git pull" can be given --no-edit from the
   command line to accept the auto-generated merge message.

 * The advice message given when the user didn't give enough clue on
   what to merge to "git pull" and "git merge" has been updated to
   be more concise and easier to understand.

 * "git push" learned the "--prune" option, similar to "git fetch".

 * The whole directory that houses a top-level superproject managed by
   "git submodule" can be moved to another place.

 * "git symbolic-ref" learned the "--short" option to abbreviate the
   refname it shows unambiguously.

 * "git tag --list" can be given "--points-at <object>" to limit its
   output to those that point at the given object.

 * "gitweb" allows intermediate entries in the directory hierarchy
   that leads to a project to be clicked, which in turn shows the
   list of projects inside that directory.

 * "gitweb" learned to read various pieces of information for the
   repositories lazily, instead of reading everything that could be
   needed (including the ones that are not necessary for a specific
   task).

 * Project search in "gitweb" shows the substring that matched in the
   project name and description highlighted.

 * A new script "diffall" is added to contrib/; it drives an
   external tool to perform a directory diff of two Git revisions
   in one go, unlike "difftool" that compares one file at a time.

Foreign Interface

 * Improved handling of views, labels and branches in "git-p4" (in contrib).

 * "git-p4" (in contrib) suffered from unnecessary merge conflicts when
   p4 expanded the embedded $RCS$-like keywords; it can be now told to
   unexpand them.

 * Some "git-svn" updates.

 * "vcs-svn"/"svn-fe" learned to read dumps with svn-deltas and
   support incremental imports.

 * "git difftool/mergetool" learned to drive DeltaWalker.

Performance

 * Unnecessary calls to parse_object() "git upload-pack" makes in
   response to "git fetch", have been eliminated, to help performance
   in repositories with excessive number of refs.

Internal Implementation (please report possible regressions)

 * Recursive call chains in "git index-pack" to deal with long delta
   chains have been flattened, to reduce the stack footprint.

 * Use of add_extra_ref() API is now gone, to make it possible to
   cleanly restructure the overall refs API.

 * The command line parser of "git pack-objects" now uses parse-options
   API.

 * The test suite supports the new "test_pause" helper function.

 * Parallel to the test suite, there is a beginning of performance
   benchmarking framework.

 * t/Makefile is adjusted to prevent newer versions of GNU make from
   running tests in seemingly random order.

 * The code to check if a path points at a file beyond a symbolic link
   has been restructured to be thread-safe.

 * When pruning directories that has become empty during "git prune"
   and "git prune-packed", call closedir() that iterates over a
   directory before rmdir() it.

Also contains minor documentation updates and code clean-ups.


Fixes since v1.7.9
------------------

Unless otherwise noted, all the fixes since v1.7.9 in the maintenance
releases are contained in this release (see release notes to them for
details).

 * Build with NO_PERL_MAKEMAKER was broken and Git::I18N did not work
   with versions of Perl older than 5.8.3.
   (merge 5eb660e ab/perl-i18n later to maint).

 * "git tag -s" honored "gpg.program" configuration variable since
   1.7.9, but "git tag -v" and "git verify-tag" didn't.
   (merge a2c2506 az/verify-tag-use-gpg-config later to maint).

 * "configure" script learned to take "--with-sane-tool-path" from the
   command line to record SANE_TOOL_PATH (used to avoid broken platform
   tools in /usr/bin) in config.mak.autogen.  This may be useful for
   people on Solaris who have saner tools outside /usr/xpg[46]/bin.

 * zsh port of bash completion script needed another workaround.

Re: [ANNOUNCE] Git 1.7.10-rc3

[Jeff King]

On Wed, Mar 28, 2012 at 12:47:12PM -0700, Junio C Hamano wrote:

>  * When you do not tell which branches and tags to push to the "git push"
>    command in any way, the command used "matching refs" rule to update
>    remote branches and tags with branches and tags with the same name you
>    locally have.  In future versions of Git, this will change to use the
>    "upstream" rule to update the branch at the remote you would "pull"
>    from into your current branch with your local current branch.  The
>    release after 1.7.10 will start issuing a warning about this change,
>    to encourage you to tell the command what to push out, e.g. by setting
>    push.default configuration.

Did we decide that "upstream" will be the new rule in future versions? I
still have some misgivings about that (versus "current"), but I thought
the only decision we were settling now was whether to change at all.

-Peff

Re: [ANNOUNCE] Git 1.7.10-rc3

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Wed, Mar 28, 2012 at 12:47:12PM -0700, Junio C Hamano wrote:
>
>>  * When you do not tell which branches and tags to push to the "git push"
>>    command in any way, the command used "matching refs" rule to update
>>    remote branches and tags with branches and tags with the same name you
>>    locally have.  In future versions of Git, this will change to use the
>>    "upstream" rule to update the branch at the remote you would "pull"
>>    from into your current branch with your local current branch.  The
>>    release after 1.7.10 will start issuing a warning about this change,
>>    to encourage you to tell the command what to push out, e.g. by setting
>>    push.default configuration.
>
> Did we decide that "upstream" will be the new rule in future versions? I
> still have some misgivings about that (versus "current"), but I thought
> the only decision we were settling now was whether to change at all.

I counted the AOL me-too on "upstream" vs "current" ;-)

Seriously speaking, I think we have enough time to make sure that
"upstream" errors out with an appropriate advice when:

 - The user says "git push" (no remote, no refspec) on a branch without
   any tracking set; or

 - The user says "git push $remote" (either remote nick or url, no
   refspec) when there is no "remote.$remote.push" and the current branch
   does not have tracking set to that remote (includes the cases where it
   does not have any tracking set, and where it has tracking set to
   different remote).

Once that happens, there no longer is a reason "current" is more suitable
for beginners, I would think.

The "easy to understand for beginners" explanation for "upstream" becomes:

  Nothing is pushed until you explicitly say what is pushed where, and you
  can say that by either:

   - setting remote.$remote.push;
   - setting branch.$current.merge; or
   - saying it on the command line.

Compared to this, the "easy to understand for beginners" explanation for
"current" which is "The current is pushed to the other end with the same
name", may still be far easier to understand, but much less useful.

Re: [ANNOUNCE] Git 1.7.10-rc3

[Jeff King]

On Thu, Mar 29, 2012 at 02:22:31PM -0700, Junio C Hamano wrote:

> > Did we decide that "upstream" will be the new rule in future versions? I
> > still have some misgivings about that (versus "current"), but I thought
> > the only decision we were settling now was whether to change at all.
> 
> I counted the AOL me-too on "upstream" vs "current" ;-)

I did, too, but as you are so fond of reminding us, this is not a
democracy. :)

> Seriously speaking, I think we have enough time to make sure that
> "upstream" errors out with an appropriate advice when:
> 
>  - The user says "git push" (no remote, no refspec) on a branch without
>    any tracking set; or
> 
>  - The user says "git push $remote" (either remote nick or url, no
>    refspec) when there is no "remote.$remote.push" and the current branch
>    does not have tracking set to that remote (includes the cases where it
>    does not have any tracking set, and where it has tracking set to
>    different remote).

Right. This was one of my two concerns: many upstream corner cases do not
currently make sense, and before we switch to it as a default, those
bugs need to be dealt with. And I think that refusing to push in those
cases is the right thing.

But I would withhold a decision on "upstream" versus "current" until
those bugs are ironed out, because what people think of as "upstream"
(today's current behavior)  may not be exactly what it ends up as.
In particular, the common beginner workflow of:

  $ git clone ...
  $ git checkout -b topic
  $ hack hack hack
  $ git push

would error out (whereas with "current", it would do something
reasonably sane and predictable). The "upstream" push default relies on
the upstream config being set up in a sane way, but in my experience,
that does not always happen in every workflow.

> The "easy to understand for beginners" explanation for "upstream" becomes:
> 
>   Nothing is pushed until you explicitly say what is pushed where, and you
>   can say that by either:
> 
>    - setting remote.$remote.push;
>    - setting branch.$current.merge; or
>    - saying it on the command line.

Or "git has set up branch.$current.merge for you already". The second of
my two concerns is that this:

  $ git clone ...
  $ git checkout -b topic origin/master
  $ hack hack hack
  $ git push

will try to implicitly fast-forward merge your commits onto master. Some
people have said they really like that behavior, but I think it can be a
bit surprising for beginners.

Anyway, I didn't exactly want to re-open the upstream versus current
debate at this point (and actually, I think a hybrid "do nothing unless
upstream and current would agree on the behavior, and give copious
advice" approach might be the best thing). I just wanted to make sure
things were still open for consideration, and was concerned that we are
creating false expectations by putting "upstream" into the release
notes.

-Peff

Re: [ANNOUNCE] Git 1.7.10-rc3

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> But I would withhold a decision on "upstream" versus "current" until
> those bugs are ironed out, because what people think of as "upstream"
> (today's current behavior) may not be exactly what it ends up as.
> ...
> Anyway, I didn't exactly want to re-open the upstream versus current
> debate at this point ...

Actually I did want to ;-) An announcement "We would be switching but we
don't know what to" does not make sense.

push.default: current vs upstream

[Jeff King]

On Thu, Mar 29, 2012 at 06:54:51PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > But I would withhold a decision on "upstream" versus "current" until
> > those bugs are ironed out, because what people think of as "upstream"
> > (today's current behavior) may not be exactly what it ends up as.
> > ...
> > Anyway, I didn't exactly want to re-open the upstream versus current
> > debate at this point ...
> 
> Actually I did want to ;-) An announcement "We would be switching but we
> don't know what to" does not make sense.

OK. Then I think we shouldn't switch to upstream, and I'm ready to
debate it. :) I already posted my arguments earlier in the thread[1].
What do you think?

I think we can deal with my first issue (some workflows will cause "git
push" to error out without doing anything) with targeted advice for each
situation.  But I still worry about the "implied merge" concern I
raised, and I think the only way to fix that is to have a new mode that
is almost but not quite "upstream" (like the upstream-current hybrid I
mentioned).

Has somebody volunteered to make the necessary fixes to "push.default =
upstream" in the first place? At the very least we need the fixes you
mentioned in your mail[2] before it can become the default. So maybe
doing those is a good first step (of course we are in release freeze,
and it would be nice to settle this before v1.7.10 ships, so maybe there
is not time).

-Peff

[1] http://article.gmane.org/gmane.comp.version-control.git/194299

[2] http://article.gmane.org/gmane.comp.version-control.git/194295

Re: push.default: current vs upstream

[Junio C Hamano]

[dragging Matthieu, an advocate for "upstream", into this]

Jeff King <peff@peff.net> writes:

> OK. Then I think we shouldn't switch to upstream, and I'm ready to
> debate it. :) I already posted my arguments earlier in the thread[1].
> What do you think?

Honestly speaking, I do not care too deeply either way.  The difference
between "mixed" and "current/upstream" is a very deep-rooted one and has
much more to do with how "batchy" person you are, than if you are pushing
to a shared or your own repository.

The underlying assumption for the original push semantics of "mixed" (or
use of remote.$there.push configuration) is that the way you work is to
prepare all your branches to be pushed $there ready by the time when you
push $there.  Concentrate perfecting your local work, perhaps even being
oblivious to what others do in the meantime, and then push out everything
in one go.  The "oblivious to what others do" part is obviously harder to
arrange if you are pushing into a shared repository, but the connection
between the "shared repository workflow" and "push everything in one go"
is not fundamental.

On the other hand, both "current/upstream" are for people whose work habit
is to complete _only_ the current branch; when you push it out $there, it
does not matter if the other branches that will eventually be pushed out
to the same repository are ready.

And this "only the doneness of the current branch matter" is fundamentally
different from "push everything in one go", and I am already happy to see
that future Git is moving in this direction, which also matches the way
vast majority of people seem to work.  So in that sense, I do not care
which one we picked between "current" and "upstream".  Obviously the
former is much simpler to explain and understand, as people do not have to
learn upstream tracking before doing their first "push".

> I think we can deal with my first issue (some workflows will cause "git
> push" to error out without doing anything) with targeted advice for each
> situation.

Yes.  I think that is up to the people who favored "upstream" over
"current" to share their anecdotes to polish such advice messages.

> But I still worry about the "implied merge" concern I
> raised, and I think the only way to fix that is to have a new mode that
> is almost but not quite "upstream" (like the upstream-current hybrid I
> mentioned).

... which is this.

> my two concerns is that this:
>
>   $ git clone ...
>   $ git checkout -b topic origin/master
>   $ hack hack hack
>   $ git push
>
> will try to implicitly fast-forward merge your commits onto master.

And the reason why it is surprising to the beginners is?  Because "topic"
and "master" (of "origin/master") are not the same name?

If you strip default features out of fear that it may be surprising to the
beginners, you would end up telling them to always explicitly say what
they want to push and to where, so we would need to draw a line somewhere.
I tend to think that this is on the "understandable" side of the line
(after all, I said "Let's start a topic to be merged to origin/master"
when I started the topic, and I've been rebasing the topic up to date from
time to time), but obviously you don't think so.

And I think your aversion to the "implicit fast-forward" will lead you to
teach beginners to do this instead:

   $ git clone ...
   $ git checkout -b topic origin/master
   $ hack hack hack
   $ git checkout master
   $ git merge topic
   $ eyeball, test, think
   $ git push

which arguably is a more disciplined way, but I do not know if we can
expect that people can be trained to be _that_ well disciplined.

> Has somebody volunteered to make the necessary fixes to "push.default =
> upstream" in the first place? At the very least we need the fixes you
> mentioned in your mail[2] before it can become the default. So maybe
> doing those is a good first step (of course we are in release freeze,
> and it would be nice to settle this before v1.7.10 ships, so maybe there
> is not time).

If we were to take "upstream", as long as we commit to make it ready by
the time we switch, I do not see it as much of an issue.  Of course, the
"will change to 'upstream'" message may need to be rephrased to say that
the idiotic usages that would trigger bugs in the current "upstream"
implementation I listed in the message you are referring to (which were
actually taken from your earlier message, IIRC) are not supported and
currently give undefined results, but will be fixed by the time we
actually do the switch.

> [1] http://article.gmane.org/gmane.comp.version-control.git/194299
>
> [2] http://article.gmane.org/gmane.comp.version-control.git/194295

Re: push.default: current vs upstream

[Jeff King]

On Fri, Mar 30, 2012 at 01:25:03PM -0700, Junio C Hamano wrote:

> And this "only the doneness of the current branch matter" is fundamentally
> different from "push everything in one go", and I am already happy to see
> that future Git is moving in this direction, which also matches the way
> vast majority of people seem to work.  So in that sense, I do not care
> which one we picked between "current" and "upstream".  Obviously the
> former is much simpler to explain and understand, as people do not have to
> learn upstream tracking before doing their first "push".

Right. I also think either is a huge improvement for new users over
"matching". But since we are going through the pain of changing the
default, I think it's worth nit-picking between the options to come up
with the best default.

> > I think we can deal with my first issue (some workflows will cause "git
> > push" to error out without doing anything) with targeted advice for each
> > situation.
> 
> Yes.  I think that is up to the people who favored "upstream" over
> "current" to share their anecdotes to polish such advice messages.

I guess part of me is just cynical. We are announcing "the default will
change to upstream" under the assumption that upstream will get polished
to everyone's liking. But until that polishing is actually done, I am
pessimistic. :)

> > my two concerns is that this:
> >
> >   $ git clone ...
> >   $ git checkout -b topic origin/master
> >   $ hack hack hack
> >   $ git push
> >
> > will try to implicitly fast-forward merge your commits onto master.
> 
> And the reason why it is surprising to the beginners is?  Because "topic"
> and "master" (of "origin/master") are not the same name?

Sort of. It is more because "upstream" is an overloaded concept. Perhaps
you created the branch from origin/master because you wanted to say
"this is where my topic is based, and when I 'rebase -i' later, I want
it to be considered the baseline". Or perhaps you meant to say "I am
going to work on origin's master branch, but I would prefer to call it
'topic' here".

In the latter case, pushing back to origin/master makes sense. They are
forks of the same branch to you, and pushing back is how
you will share your changes to master. But in the former case, you may
or may not consider them the same branch, and you may be pushing simply
to share your work-in-progress of the topic. Putting that work onto
"master" would be confusing in that case.

Note that "current" has the same assumption in reverse. If you create a
local "master" branch (whether or not it is based on a remote
"origin/master"), you may or may not mean them to be the same branch.

So we have to decide when two things are forks of "the same branch", and
when it is merely "X is based on Y", or "X happens to have the same name
as Y". And I think the "name is the same" semantics are way more
obvious.

Do you recall discussions a few years back about git's branching model
versus that of mercurial and other systems? One of the confusing things
for people new to git was the idea that git fundamentally doesn't care
about "what is a branch". They got confused that "master" in the local
repository really had no connection to "master" on the remote repository
(whereas in hg, I think there is some magic in the DAG that connects
them). But that leads me to think that people really do consider "same
name is the same branch", which means "current" is going to be a lot
less likely to confuse people (for that matter, look at the current
matching semantics, which use name mapping; people get confused that we
are pushing all matching branches, but I don't remember anyone ever
complaining that they expect "foo" to go to "bar").

> I tend to think that this is on the "understandable" side of the line
> (after all, I said "Let's start a topic to be merged to origin/master"
> when I started the topic, and I've been rebasing the topic up to date from
> time to time), but obviously you don't think so.

Is that what you said? Or did you say "I am starting a new topic that
will be based on origin/master?" I feel like the concept of "upstream"
is very loosely specified, and can mean many things. And even if you do
eventually expect it to be merged into master, it does not mean you
expect it to do so by default during "git push". You might also simply
want to push the current state of your topic.

> And I think your aversion to the "implicit fast-forward" will lead you to
> teach beginners to do this instead:
> 
>    $ git clone ...
>    $ git checkout -b topic origin/master
>    $ hack hack hack
>    $ git checkout master
>    $ git merge topic
>    $ eyeball, test, think
>    $ git push
> 
> which arguably is a more disciplined way, but I do not know if we can
> expect that people can be trained to be _that_ well disciplined.

Sure, I think that is a better workflow. But I don't expect everyone to
follow it, and I don't think "upstream" is wrong to behave the other
way. You could also teach them what "upstream" means, and have them set
push.default to it.

Ultimately, it is not about whether one workflow is better than the
other. It is about having a default that stops the user and says "hey, I
don't know what workflow you're using. So you need to tell me before I
can continue."

-Peff

Re: push.default: current vs upstream

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Fri, Mar 30, 2012 at 01:25:03PM -0700, Junio C Hamano wrote:
>
>> And the reason why it is surprising to the beginners is?  Because "topic"
>> and "master" (of "origin/master") are not the same name?
>
> Sort of. It is more because "upstream" is an overloaded concept. Perhaps
> you created the branch from origin/master because you wanted to say
> "this is where my topic is based, and when I 'rebase -i' later, I want
> it to be considered the baseline". Or perhaps you meant to say "I am
> going to work on origin's master branch, but I would prefer to call it
> 'topic' here".

In either case, you seem to be assuming (and it is a correct assumption,
even though we may not use such a workflow) that the resulting branch, if
long lived, will be rebasing on top of origin/master.  And the reason why
you do that is because...

Because you would eventually want to get it integrated into origin's
master.  Otherwise you can stay apart from origin/master and keep your
foundation solidly anchored to where you started.

So in that sense, in both cases, pushing back to origin/master is likely
to be what the user expected in the first place.

Like it or not, many parts of Git that is "upstream"-aware have been over
time taught to make it easier for the eventual result easier to be pushed
back to the "upstream" at the remote, I think.  "log --left-right @{u}..."
will show what you and others have done, "rebase" knows to use @{u} when
figuring out on which commit to rebuild your current branch, etc.

> So we have to decide when two things are forks of "the same branch", and
> when it is merely "X is based on Y", or "X happens to have the same name
> as Y". And I think the "name is the same" semantics are way more
> obvious.

I obviously agree with that.

>> I tend to think that this is on the "understandable" side of the line
>> (after all, I said "Let's start a topic to be merged to origin/master"
>> when I started the topic, and I've been rebasing the topic up to date from
>> time to time), but obviously you don't think so.
>
> Is that what you said? Or did you say "I am starting a new topic that
> will be based on origin/master?"

See above ;-)  Having said that...

> I feel like the concept of "upstream"
> is very loosely specified, and can mean many things.

...I tend to agree with it.  But I am not sure if that leads to "we should
default to 'current' because 'upstream' is too messy and blurry".  At
least, not yet.

> Ultimately, it is not about whether one workflow is better than the
> other. It is about having a default that stops the user and says "hey, I
> don't know what workflow you're using. So you need to tell me before I
> can continue."

OK.  Obviously we would not want to default to 'push.default = nothing',
and 'current' is far simpler to explain.

But then I am afraid that you may be inviting teachers to blindly teach
beginners to first set push.default to upstream, just like they do today
where the default is matching, as most of them do know that upstream works
fairly well with the way how _they_ work, without having an understanding
these gotchas in upstream you are (validly) raising as possible issues
here.  So in the end, we would have to clarify whatever 'upstream' does
anyway, no?

Re: push.default: current vs upstream

[Jeff King]

On Fri, Mar 30, 2012 at 02:28:31PM -0700, Junio C Hamano wrote:

> In either case, you seem to be assuming (and it is a correct assumption,
> even though we may not use such a workflow) that the resulting branch, if
> long lived, will be rebasing on top of origin/master.  And the reason why
> you do that is because...
> 
> Because you would eventually want to get it integrated into origin's
> master.  Otherwise you can stay apart from origin/master and keep your
> foundation solidly anchored to where you started.
> 
> So in that sense, in both cases, pushing back to origin/master is likely
> to be what the user expected in the first place.

OK, I can agree with that (although I can come up with some corner
cases, like a long-running branch that gets updates from master but is
never intended to be merged back in, I think we can safely say those are
advanced issues and not likely to be a problem for new git users).

So let's assume that you eventually plan for "topic" to go back into
"master", and focus on a more concrete issue: _when_ to merge. You fork
a topic branch from origin/master and make some commits. You then run
"git push". Did you mean:

  1. I am ready for this work to go back to origin/master.

  2. I am ready to publish my topic branch for others to review.

I think it's ambiguous. And getting it wrong is potentially hard to
retract (because you've published commits to what is probably supposed
to be a stable, non-rewinding branch).

> > I feel like the concept of "upstream"
> > is very loosely specified, and can mean many things.
> 
> ...I tend to agree with it.  But I am not sure if that leads to "we should
> default to 'current' because 'upstream' is too messy and blurry".  At
> least, not yet.

I tend to think "upstream" is hopelessly blurry, and must remain so
because there are too many similar concepts. That is, to fix it, you
would need to split it into several sub-concepts.

For example, I generally base all of my git.git topics on origin/master
(where "origin" is your repo). But when I push them, they go to my
publishing point. So there are two things I am interested in asking
about:

  1. Where is my work compared to master? This is useful for enumerating
     which commits are part of my topic, and for rebasing on top of
     master.

  2. What do my local branches have compared to their published
     versions?  This is useful for knowing that I have work to be
     published, or for realizing that I published work from another
     machine that does not exist on the current machine.

Those are two very different notions of upstream, and I want to use them
with different commands. I set @{u} to origin/master to handle (1). And
I do (2) by matching names. But I would never want git-push to look at
@{u}, because it is a totally different concept. Pushing is always about
(2) for me.

Yes, this is a more complex workflow than many beginning git users will
have. But I think it is at the heart of the upstream confusion: just
because you are based on some branch, and just because you ultimately
want to merge with it, does not mean that it is a good push destination.
_Sometimes_ it is, and that is why the "upstream" push.default exists.

> But then I am afraid that you may be inviting teachers to blindly teach
> beginners to first set push.default to upstream, just like they do today
> where the default is matching, as most of them do know that upstream works
> fairly well with the way how _they_ work, without having an understanding
> these gotchas in upstream you are (validly) raising as possible issues
> here.  So in the end, we would have to clarify whatever 'upstream' does
> anyway, no?

Yes, and we should make upstream better, no matter what the default is.
But I hoped that it would not be "blindly teach" but rather "teach what
upstream is". Perhaps that is naive. But I feel like at least we will
have done the best we can by giving the user an opportunity to read the
documentation or have somebody instruct them before setting "upstream",
and not simply shipping it out of the box.

-Peff

Re: push.default: current vs upstream

[Junio C Hamano]

In the meantime, we can do this.

 Documentation/RelNotes/1.7.10.txt |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/Documentation/RelNotes/1.7.10.txt b/Documentation/RelNotes/1.7.10.txt
index d326ff8..6dcaf45 100644
--- a/Documentation/RelNotes/1.7.10.txt
+++ b/Documentation/RelNotes/1.7.10.txt
@@ -32,12 +32,9 @@ Compatibility Notes
  * When you do not tell which branches and tags to push to the "git push"
    command in any way, the command used "matching refs" rule to update
    remote branches and tags with branches and tags with the same name you
-   locally have.  In future versions of Git, this will change to use the
-   "upstream" rule to update the branch at the remote you would "pull"
-   from into your current branch with your local current branch.  The
-   release after 1.7.10 will start issuing a warning about this change,
-   to encourage you to tell the command what to push out, e.g. by setting
-   push.default configuration.
+   locally have.  In future versions of Git, this will change to and push
+   out only your current branch according to either "upstream" or "current"
+   rule (we haven't yet decided which).
 
 
 Updates since v1.7.9

Re: push.default: current vs upstream

[Jeff King]

On Fri, Mar 30, 2012 at 03:15:34PM -0700, Junio C Hamano wrote:

> In the meantime, we can do this.

I'm OK with this. We could perhaps even invite people to take part in
the discussion. I'm not sure if that will generate anything more than a
set of "me too" votes, though.

> -   locally have.  In future versions of Git, this will change to use the
> -   "upstream" rule to update the branch at the remote you would "pull"
> -   from into your current branch with your local current branch.  The
> -   release after 1.7.10 will start issuing a warning about this change,
> -   to encourage you to tell the command what to push out, e.g. by setting
> -   push.default configuration.
> +   locally have.  In future versions of Git, this will change to and push
> +   out only your current branch according to either "upstream" or "current"
> +   rule (we haven't yet decided which).

s/to and/to/

-Peff

Re: push.default: current vs upstream

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Has somebody volunteered to make the necessary fixes to "push.default =
> upstream" in the first place? At the very least we need the fixes you
> mentioned in your mail[2] before it can become the default.
> ...
> [2] http://article.gmane.org/gmane.comp.version-control.git/194295

There were two issues I raised in that message.  It turns out that we
already have the code for the first one.  The second one should look
something like this:

-- >8 --
Subject: push: detect nonsense "upstream" check more carefully

The user can say "git push" without specifying any refspec.  When using
"upstream" semantics via the push.default configuration, the user wants to
update the "upstream" branch, which is the branch at a remote repository
the current branch is set to integrate with, with this command.

There are cases that "git push" do not make sense when push.default is set
to "upstream":

 - The current branch does not have branch.$name.remote configured.  By
   definition, "git push" that does not name where to push to will not
   know where to push to.  The user may explicitly say "git push $there",
   but again, by definition, no branch at repository $there is set to
   integrate with the current branch in this case and we wouldn't know
   which remote branch to update.

 - The current branch does have branch.$name.remote configured, but it
   does not specify branch.$name.merge that names what branch at the
   remote this branch integrates with. "git push" knows where to push in
   this case (or the user may explicitly say "git push $remote" to tell us
   where to push), but we do not know which remote branch to update.

 - The current branch does have both branch.$name.remote and
   branch.$name.merge configured, but the user said "git push $there",
   where $there does not match what "branch.$name.remote" is configured
   to.  By definition, no branch at repository $there is set to integrate
   with the current branch in this case and we wouldn't know which remote
   branch to update.

The first two cases were already checked correctly, but the third case was
not checked and we ended up updating the branch named branch.$name.merge
at repository $there, which was totally bogus.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/push.c |   32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index d315475..3e18cd3 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -65,6 +65,16 @@ static void set_refspecs(const char **refs, int nr)
 	}
 }
 
+static int push_url_of_remote(struct remote *remote, const char ***url_p)
+{
+	if (remote->pushurl_nr) {
+		*url_p = remote->pushurl;
+		return remote->pushurl_nr;
+	}
+	*url_p = remote->url;
+	return remote->url_nr;
+}
+
 static void setup_push_upstream(struct remote *remote)
 {
 	struct strbuf refspec = STRBUF_INIT;
@@ -76,7 +86,7 @@ static void setup_push_upstream(struct remote *remote)
 		    "\n"
 		    "    git push %s HEAD:<name-of-remote-branch>\n"),
 		    remote->name);
-	if (!branch->merge_nr || !branch->merge)
+	if (!branch->merge_nr || !branch->merge || !branch->remote_name)
 		die(_("The current branch %s has no upstream branch.\n"
 		    "To push the current branch and set the remote as upstream, use\n"
 		    "\n"
@@ -87,6 +97,18 @@ static void setup_push_upstream(struct remote *remote)
 	if (branch->merge_nr != 1)
 		die(_("The current branch %s has multiple upstream branches, "
 		    "refusing to push."), branch->name);
+	if (strcmp(branch->remote_name, remote->name)) {
+		struct remote *branch_dest = remote_get(branch->remote_name);
+		const char **branch_dest_url, **dest_url;
+
+		if (!push_url_of_remote(remote, &dest_url) ||
+		    !push_url_of_remote(branch_dest, &branch_dest_url) ||
+		    strcmp(dest_url[0], branch_dest_url[0]))
+			die(_("You are pushing to remote '%s', which is not the "
+			      "upstream of your\ncurrent branch '%s'.\n"),
+			    remote->name, branch->name);
+	}
+
 	strbuf_addf(&refspec, "%s:%s", branch->name, branch->merge[0]->src);
 	add_refspec(refspec.buf);
 }
@@ -196,13 +218,7 @@ static int do_push(const char *repo, int flags)
 			setup_default_push_refspecs(remote);
 	}
 	errs = 0;
-	if (remote->pushurl_nr) {
-		url = remote->pushurl;
-		url_nr = remote->pushurl_nr;
-	} else {
-		url = remote->url;
-		url_nr = remote->url_nr;
-	}
+	url_nr = push_url_of_remote(remote, &url);
 	if (url_nr) {
 		for (i = 0; i < url_nr; i++) {
 			struct transport *transport =

Re: push.default: current vs upstream

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Fri, Mar 30, 2012 at 03:15:34PM -0700, Junio C Hamano wrote:
>
>> In the meantime, we can do this.
>
> I'm OK with this. We could perhaps even invite people to take part in
> the discussion. I'm not sure if that will generate anything more than a
> set of "me too" votes, though.

Yeah, but at that point, people who pushed against "matching" will help
us, instead of letting us hang dry.

I really hated that 1.6.0 transition, where noisy vocal ones disappeared
once they got what they wanted without defending the choice they forced on
us.

 Documentation/RelNotes/1.7.10.txt |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/RelNotes/1.7.10.txt b/Documentation/RelNotes/1.7.10.txt
index d326ff8..5d72446 100644
--- a/Documentation/RelNotes/1.7.10.txt
+++ b/Documentation/RelNotes/1.7.10.txt
@@ -32,12 +32,12 @@ Compatibility Notes
  * When you do not tell which branches and tags to push to the "git push"
    command in any way, the command used "matching refs" rule to update
    remote branches and tags with branches and tags with the same name you
-   locally have.  In future versions of Git, this will change to use the
-   "upstream" rule to update the branch at the remote you would "pull"
-   from into your current branch with your local current branch.  The
-   release after 1.7.10 will start issuing a warning about this change,
-   to encourage you to tell the command what to push out, e.g. by setting
-   push.default configuration.
+   locally have.  In future versions of Git, this will change to push out
+   only your current branch according to either the "upstream" or the
+   "current" rule.  Although "upstream" may be more powerful once the
+   user understands Git better, the semantics "current" gives is
+   simpler and easier to understand for beginners and may be a safer
+   and better default option, but we haven't decided yet.
 
 
 Updates since v1.7.9

Re: push.default: current vs upstream

[Nathan Gray]

On Fri, Mar 30, 2012 at 2:01 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Mar 30, 2012 at 01:25:03PM -0700, Junio C Hamano wrote:
>> > my two concerns is that this:
>> >
>> >   $ git clone ...
>> >   $ git checkout -b topic origin/master
>> >   $ hack hack hack
>> >   $ git push
>> >
>> > will try to implicitly fast-forward merge your commits onto master.
>>
>> And the reason why it is surprising to the beginners is?  Because "topic"
>> and "master" (of "origin/master") are not the same name?
>
> Sort of. It is more because "upstream" is an overloaded concept. Perhaps
> you created the branch from origin/master because you wanted to say
> "this is where my topic is based, and when I 'rebase -i' later, I want
> it to be considered the baseline". Or perhaps you meant to say "I am
> going to work on origin's master branch, but I would prefer to call it
> 'topic' here".
>
> In the latter case, pushing back to origin/master makes sense. They are
> forks of the same branch to you, and pushing back is how
> you will share your changes to master. But in the former case, you may
> or may not consider them the same branch, and you may be pushing simply
> to share your work-in-progress of the topic. Putting that work onto
> "master" would be confusing in that case.
>
> Note that "current" has the same assumption in reverse. If you create a
> local "master" branch (whether or not it is based on a remote
> "origin/master"), you may or may not mean them to be the same branch.
>
> So we have to decide when two things are forks of "the same branch", and
> when it is merely "X is based on Y", or "X happens to have the same name
> as Y". And I think the "name is the same" semantics are way more
> obvious.

I'd like to offer my strong agreement as somebody who has just led his
team through a transition from svn to git.  Branch names are really
important to people.  If they've decided to branch "origin/master" as
something that's not called "master", there's a really good chance
that they mean it to be a new branch.  I think it's also really
important to consider the consequences of getting it wrong under
either scenario.

If a user does some work on his new "features/frobnitz" branch and
does a "git push" only to find that his work has been committed to the
company's master branch he will be confused, frustrated, and publicly
embarrassed.  He then has to apologize and figure out how to revert
the changes.  I really don't see any scenario where that user ends up
saying "oh yeah, I guess git was right and I was wrong."

Compare that outcome to somebody who expects upstream behavior and
gets current.  That person pushes "features/frobnitz" expecting to see
his changes appear on "origin/master", only to find that they didn't.
Instead there's a new branch "origin/features/frobnitz".  Maybe it's
not what he expected but it's easy enough to understand.  He hasn't
inconvenienced anybody else, so there's no need for apology.  He just
takes a look at the docs and figures out how to accomplish what he
intended and deletes the branch he accidentally created.

It's this public vs. private embarrassment factor that motivated me to
recommend "current" as our company policy and most strongly convinces
me that "current" is the right default.

Cheers,
-Nathan

-- 
http://n8gray.org

Re: push.default: current vs upstream

[Seth Robertson]

In message <CA+7g9JxK5DHj3vbdGgF2dEJxvn=_ZfjAv7Y+AL_P-aO1FVB6-w@mail.gmail.com>, Nathan Gray writes:

    If a user does some work on his new "features/frobnitz" branch and
    does a "git push" only to find that his work has been committed to the
    company's master branch he will be confused, frustrated, and publicly
    embarrassed.  He then has to apologize and figure out how to revert
    the changes.  I really don't see any scenario where that user ends up
    saying "oh yeah, I guess git was right and I was wrong."

When working with a single remote, I tend to agree with you (though
since I also think receive.denyDeletes should be on by default for
shared repos the public humiliation of creating a branch when you did
not mean to might still exist but of course it will be less damaging
to others) .  However, tracking really comes into its own when working
with multiple remotes.  Is creating a stumbling block between naïve
use and more sophisticated use really necessary?

However, the current message for this use case could seem to be
tweaked to take care of this:

$ git branch BB origin/B
Branch BB set up to track remote branch B from origin.

Add "If you push your changes will go there."
And "See git branch --upstream to modify both settings"

This provides the power of tracking with smaller possibility of
the type of embarrassment you envision.

					-Seth Robertson

Re: push.default: current vs upstream

[Junio C Hamano]

Seth Robertson <in-gitvger@baka.org> writes:

> However, the current message for this use case could seem to be
> tweaked to take care of this:
>
> $ git branch BB origin/B
> Branch BB set up to track remote branch B from origin.
>
> Add "If you push your changes will go there."

But you need to limit that only when push.default is set to "upstream",
no?  Otherwise the message will be confusing the user.

Re: push.default: current vs upstream

[Nathan Gray]

On Sat, Mar 31, 2012 at 4:48 PM, Seth Robertson <in-gitvger@baka.org> wrote:
>
> In message <CA+7g9JxK5DHj3vbdGgF2dEJxvn=_ZfjAv7Y+AL_P-aO1FVB6-w@mail.gmail.com>, Nathan Gray writes:
>
>    If a user does some work on his new "features/frobnitz" branch and
>    does a "git push" only to find that his work has been committed to the
>    company's master branch he will be confused, frustrated, and publicly
>    embarrassed.  He then has to apologize and figure out how to revert
>    the changes.  I really don't see any scenario where that user ends up
>    saying "oh yeah, I guess git was right and I was wrong."
>
> When working with a single remote, I tend to agree with you (though
> since I also think receive.denyDeletes should be on by default for
> shared repos the public humiliation of creating a branch when you did
> not mean to might still exist but of course it will be less damaging
> to others) .  However, tracking really comes into its own when working
> with multiple remotes.  Is creating a stumbling block between naïve
> use and more sophisticated use really necessary?

Where's the stumbling block?  Nobody's talking about taking away the
upstream option, so sophisticated users who prefer upstream behavior
can configure their repos to get it.  The default should be tailored
for naïve users, not power users.

> However, the current message for this use case could seem to be
> tweaked to take care of this:
>
> $ git branch BB origin/B
> Branch BB set up to track remote branch B from origin.
>
> Add "If you push your changes will go there."
> And "See git branch --upstream to modify both settings"
>
> This provides the power of tracking with smaller possibility of
> the type of embarrassment you envision.

I like the idea of telling users where their pushes will go, but I
don't think this will work as well as you might expect.  You're
relying on a new user to read every part of the message, understand
the terminology, and internalize the meaning well enough to remember
it in the distant future when he's ready to push.  Considering the
onslaught of new concepts to absorb when a user switches from
centralized to distributed SCM that's a pretty tall order.

I think it's backwards to put a heavier cognitive load on newbies for
the sake of saving power users from running git config.  After all,
power users *love* running git config.  ;^)

Cheers,
-Nathan

-- 
http://n8gray.org

Re: push.default: current vs upstream

[Matthieu Moy]

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

> Obviously the
> former is much simpler to explain and understand, as people do not have to
> learn upstream tracking before doing their first "push".

Again, this is simple only for people who never run "git pull" without
argument.

For the others, they already have to learn about the "upstream"
semantics. And making argumentless "git pull" and "git push" purposely
asymetric to make it simple for the user sounds like an oxymoron to me.

The discussion seems to focuse on 'let's make "git push" easy to
explain', but I think the right thing to do is to make _Git_ easy to
explain. With "push.default = current", we'll have a hard time
explaining how "git pull" works.

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

Re: push.default: current vs upstream

[Junio C Hamano]

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

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Obviously the
>> former is much simpler to explain and understand, as people do not have to
>> learn upstream tracking before doing their first "push".
>
> Again, this is simple only for people who never run "git pull" without
> argument.

If you are running "git pull" with what to pull and integrate, you know
system much better than those who only use the canned "git pull without
argument" settings, so customizing push.default should be easier for you
than the real beginners whom we try to avoid confusing with the built-in
default, no?

Before saying "again", perhaps we should read and think about what the
other side said.  I think [*1*] raises a good point.

> For the others, they already have to learn about the "upstream"
> semantics.

But the others have graduated from CVS/SVN mentality.  Also, "upstream"
needs to be carefully chosen; I suspect that it is not as trivial for the
beginners to wrap their mind around it as you seem to be implying.

After a "git clone", you may want to work on whatever you are doing on
your topic branch, and then share it with your collaborator and polish it
to be suitable for the master, you may want to do this:

    git checkout -b topic ; work work work
    git push origin topic

But if your work on topic is ready to be published for everybody, you may 
just do this:

    git checkout -b topic ; work work work
    git push origin master

The upstream of topic for the former case would be origin/topic while for
the latter case it would be origin/master.

I and you know that.  But is the rest of the user experience set up to
easily arrange this automatically?  With branch.autosetupmerge, I suspect
that the above can be generalized to:

	git push origin master:topic ;# create the shared starting point
	git checkout -b topic origin/topic ;# and fork it

        repeat the three steps below 0 or more times
          work work work
          git push ;# goes to @{u} that is origin/topic
          git pull ;# takes work by collaborators from @{u}

	git push origin HEAD:master ;# topic is fully cooked, ready for master

and everything *should* go smoothly when @{u} is set up correctly.  At
least, that was the plan for @{u} mechanism.

The last step that pushes "git push origin HEAD:master" *might* be simpler
to explain if it were done this way:

	git checkout master
        git pull ;# syncs to the origin/master
        git merge topic
        git push

Also see Peff's 194414 in the same thread.

[References]

*1* http://thread.gmane.org/gmane.comp.version-control.git/194175/focus=194470

Re: push.default: current vs upstream

[Matthieu Moy]

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

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Obviously the
>>> former is much simpler to explain and understand, as people do not have to
>>> learn upstream tracking before doing their first "push".
>>
>> Again, this is simple only for people who never run "git pull" without
>> argument.
>
> If you are running "git pull" with what to pull and integrate, you know
> system much better than those who only use the canned "git pull without
> argument" settings, so customizing push.default should be easier for you
> than the real beginners whom we try to avoid confusing with the built-in
> default, no?

I don't understand what you mean, sorry.

I'll rephrase my point in case it wasn't clear. My claim is that a
beginner who want to run argumentless "git pull" has to understand where
his changes come from. So, he already has to understand what "upstream
branch" means to Git.

I agree that explaining "push.default=current" is easy alone, but once
you've taught "git pull", explaining that "git push" does something else
and why doesn't seem that easy to me. OTOH, once you understand "git
pull", explaining "push.default=upstream" should be as simple as "send
your changes where git pull would get them".

> Before saying "again", perhaps we should read and think about what the
> other side said.  I think [*1*] raises a good point.

> *1* http://thread.gmane.org/gmane.comp.version-control.git/194175/focus=194470

I think this message precisely supports my claim: we focus the
discussion on "git push", without thinking on the big picture "git pull"
AND "git push". The message you point to does not talk at all about "git
pull".

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

Re: push.default: current vs upstream

[Junio C Hamano]

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

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Before saying "again", perhaps we should read and think about what the
>> other side said.  I think [*1*] raises a good point.
>
>> *1* http://thread.gmane.org/gmane.comp.version-control.git/194175/focus=194470
>
> I think this message precisely supports my claim: we focus the
> discussion on "git push", without thinking on the big picture "git pull"
> AND "git push". The message you point to does not talk at all about "git
> pull".

I do not think so; that "name" argument is about this part from Peff's
message, to which it is a response:

>> > my two concerns is that this:
>> >
>> >   $ git clone ...
>> >   $ git checkout -b topic origin/master
>> >   $ hack hack hack
>> >   $ git push
>> >
>> > will try to implicitly fast-forward merge your commits onto master.
>> 
>> And the reason why it is surprising to the beginners is?  Because "topic"
>> and "master" (of "origin/master") are not the same name?
>
> Sort of. It is more because "upstream" is an overloaded concept. Perhaps
> you created the branch from origin/master because you wanted to say
> "this is where my topic is based, and when I 'rebase -i' later, I want
> it to be considered the baseline". Or perhaps you meant to say "I am
> going to work on origin's master branch, but I would prefer to call it
> 'topic' here".

If you re-read it, it should be clear that this is _also_ about "git pull";
"I am going to work on origin's master branch" is about pushing the result
back there.

In the former case, you may want to push it to 'topic' to work further
with your collaborators.  In the latter case, you would want to push it
back to 'master', even though you are calling it locally 'topic' for some
sick reason (read: because you can).

Re: push.default: current vs upstream

[Matthieu Moy]

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

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Before saying "again", perhaps we should read and think about what the
>>> other side said.  I think [*1*] raises a good point.
>>
>>> *1* http://thread.gmane.org/gmane.comp.version-control.git/194175/focus=194470
>>
>> I think this message precisely supports my claim: we focus the
>> discussion on "git push", without thinking on the big picture "git pull"
>> AND "git push". The message you point to does not talk at all about "git
>> pull".
>
> I do not think so; that "name" argument is about this part from Peff's
> message, to which it is a response:

What I read in the message is that branch names are important, and "same
name" usually have some sort of semantics for users. I agree with that.
But why doesn't the same applies to "git pull"? Why would it be natural
for "git pull" to pull from a branch other than the one with the same
name?

>>> > my two concerns is that this:
>>> >
>>> >   $ git clone ...
>>> >   $ git checkout -b topic origin/master
>>> >   $ hack hack hack
>>> >   $ git push
>>> >
>>> > will try to implicitly fast-forward merge your commits onto master.
>>> 
>>> And the reason why it is surprising to the beginners is?  Because "topic"
>>> and "master" (of "origin/master") are not the same name?
>>
>> Sort of. It is more because "upstream" is an overloaded concept. Perhaps
>> you created the branch from origin/master because you wanted to say
>> "this is where my topic is based, and when I 'rebase -i' later, I want
>> it to be considered the baseline". Or perhaps you meant to say "I am
>> going to work on origin's master branch, but I would prefer to call it
>> 'topic' here".
>
> If you re-read it, it should be clear that this is _also_ about "git pull";
> "I am going to work on origin's master branch" is about pushing the result
> back there.

That's still not clear. Your explanation shows me how "git push" is
involved, not "git pull".

> In the former case, you may want to push it to 'topic' to work further
> with your collaborators.  In the latter case, you would want to push it
> back to 'master', even though you are calling it locally 'topic' for some
> sick reason (read: because you can).

I still don't see pull involved here.

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

Re: push.default: current vs upstream

[Junio C Hamano]

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

> Junio C Hamano <gitster@pobox.com> writes:
> ...
>> In the former case, you may want to push it to 'topic' to work further
>> with your collaborators.  In the latter case, you would want to push it
>> back to 'master', even though you are calling it locally 'topic' for some
>> sick reason (read: because you can).
>
> I still don't see pull involved here.

Think again.  Hint: realize that these people are not working alone, and
will be keeping their topic in sync with collaborators, and then imagine
how they are doing so.

No more words from me on this subthread.

Re: push.default: current vs upstream

[Matthieu Moy]

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

> No more words from me on this subthread.

It's a pity. I still have no answer to my question:

| But why doesn't the same applies to "git pull"? Why would it be natural
| for "git pull" to pull from a branch other than the one with the same
| name?

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

Re: push.default: current vs upstream

[Junio C Hamano]

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

> Junio C Hamano <gitster@pobox.com> writes:
>
>> No more words from me on this subthread.
>
> It's a pity. I still have no answer to my question:

That's OK.

The above is not "I hate you enough that I won't talk to you", but "I
realize that I am not the best person to explain".  You'll hopefully get
responses from those who prefer to see 'current' over 'upstream' ;-).

Re: push.default: current vs upstream

[demerphq]

On 2 April 2012 22:40, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> No more words from me on this subthread.
>
> It's a pity. I still have no answer to my question:
>
> | But why doesn't the same applies to "git pull"? Why would it be natural
> | for "git pull" to pull from a branch other than the one with the same
> | name?

The choice of "fetch" and "pull" and "push" were in some respects
unfortunate. They sound like opposites, but they are not. The very
fact that you have two commands "pull" and "fetch" both of which are
more or less the semantic opposites of "push" leads to confusion.

And actually I find your use of "git pull" and "pull" in the
expression "pull from a branch other than one with the same name"
confusing. Barring misconfiguration pull operates on only one local
branch and it is usually the one with the same name. However push
operates on multiple local branches. So from the point of view of
"what local branches are involved in the operation" the current
default leads to inconsistency.

If you had replaced "pull" with "fetch" then your argument somewhat
makes sense, but IMO is overriden by the observation that "fetch" is
essentially non-destructive, it modifies only local copies of the
remotes state, and even if you consider that destructive it is
destructive of your own data , push on the other hand *is* potentially
destructive, and destructive of data on a remote system.

Lastly I have never really encountered any confusion with explaining
the default behaviour of git-fetch, nor actually git-pull, but I have
encountered lots of confusion of people using git-push.  They expect
git-push to be the opposite of git-pull not git-fetch.

Yves

-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

Re: push.default: current vs upstream

[Matthieu Moy]

demerphq <demerphq@gmail.com> writes:

> And actually I find your use of "git pull" and "pull" in the
> expression "pull from a branch other than one with the same name"
> confusing. Barring misconfiguration pull operates on only one local
> branch and it is usually the one with the same name. However push
> operates on multiple local branches.

It does with push.default = matching, but with either options we are
discussing here, argumentless "git push" would push only one branch.
The choice we have is whether to push to the branch with the same name,
or to the branch from which "git pull" would take the changes.

(I realize that in this discussion, "current" may be misleading. I mean
"push.default=current", not "the behavior we have currently")

> Lastly I have never really encountered any confusion with explaining
> the default behaviour of git-fetch, nor actually git-pull, but I have
> encountered lots of confusion of people using git-push.  They expect
> git-push to be the opposite of git-pull not git-fetch.

I do also expect "git pull" to be symmetrical to "git pull", and
"push.default=upstream" is the closest to symmetry.

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

Re: push.default: current vs upstream

[Jeff King]

On Fri, Mar 30, 2012 at 04:07:12PM -0700, Junio C Hamano wrote:

> There were two issues I raised in that message.  It turns out that we
> already have the code for the first one.  The second one should look
> something like this:
> 
> -- >8 --
> Subject: push: detect nonsense "upstream" check more carefully

Thanks, I think this is an improvement. I absolutely think the existing
behavior was a bug, but I do wonder if any users were depending on this
"feature".

Squashable tests are below.

> +	if (strcmp(branch->remote_name, remote->name)) {
> +		struct remote *branch_dest = remote_get(branch->remote_name);
> +		const char **branch_dest_url, **dest_url;
> +
> +		if (!push_url_of_remote(remote, &dest_url) ||
> +		    !push_url_of_remote(branch_dest, &branch_dest_url) ||
> +		    strcmp(dest_url[0], branch_dest_url[0]))
> +			die(_("You are pushing to remote '%s', which is not the "
> +			      "upstream of your\ncurrent branch '%s'.\n"),
> +			    remote->name, branch->name);
> +	}

Hmm. So this will actually detect "git push $URL" when $URL matches the
remote's configured URL. I feel like this distinction has come up
before, and we decided not to equate the two. But now I can't remember
where (maybe it when fetching via URL versus via remote?).

What should happen if there are multiple push URLs configured? Your code
will match iff it is the first one. I would think it should either
require all to match, or it should proceed if any of the URLs match.
I think the latter makes more sense, though personally I would simply
have compared the remote names.

-Peff

---
Here are the tests.

diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
new file mode 100755
index 0000000..c334c51
--- /dev/null
+++ b/t/t5528-push-default.sh
@@ -0,0 +1,54 @@
+#!/bin/sh
+
+test_description='check various push.default settings'
+. ./test-lib.sh
+
+test_expect_success 'setup bare remotes' '
+	git init --bare repo1 &&
+	git remote add parent1 repo1 &&
+	git init --bare repo2 &&
+	git remote add parent2 repo2 &&
+	test_commit one &&
+	git push parent1 HEAD &&
+	git push parent2 HEAD
+'
+
+test_expect_success '"upstream" pushes to configured upstream' '
+	git checkout master &&
+	test_config branch.master.remote parent1 &&
+	test_config branch.master.merge refs/heads/foo &&
+	test_config push.default upstream &&
+	test_commit two &&
+	git push &&
+	echo two >expect &&
+	git --git-dir=repo1 log -1 --format=%s foo >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '"upstream" does not push on unconfigured remote' '
+	git checkout master &&
+	test_unconfig branch.master.remote &&
+	test_config push.default upstream &&
+	test_commit three &&
+	test_must_fail git push
+'
+
+test_expect_success '"upstream" does not push on unconfigured branch' '
+	git checkout master &&
+	test_config branch.master.remote parent1 &&
+	test_unconfig branch.master.merge &&
+	test_config push.default upstream
+	test_commit four &&
+	test_must_fail git push
+'
+
+test_expect_success '"upstream" does not push when remotes do not match' '
+	git checkout master &&
+	test_config branch.master.remote parent1 &&
+	test_config branch.master.merge refs/heads/foo &&
+	test_config push.default upstream &&
+	test_commit five &&
+	test_must_fail git push parent2
+'
+
+test_done

Re: push.default: current vs upstream

[Jeff King]

On Tue, Apr 03, 2012 at 04:59:07PM -0400, Jeff King wrote:

> > +		if (!push_url_of_remote(remote, &dest_url) ||
> > +		    !push_url_of_remote(branch_dest, &branch_dest_url) ||
> > +		    strcmp(dest_url[0], branch_dest_url[0]))
> > +			die(_("You are pushing to remote '%s', which is not the "
> > +			      "upstream of your\ncurrent branch '%s'.\n"),
> > +			    remote->name, branch->name);
> > +	}
> 
> Hmm. So this will actually detect "git push $URL" when $URL matches the
> remote's configured URL. I feel like this distinction has come up
> before, and we decided not to equate the two. But now I can't remember
> where (maybe it when fetching via URL versus via remote?).
> 
> What should happen if there are multiple push URLs configured? Your code
> will match iff it is the first one. I would think it should either
> require all to match, or it should proceed if any of the URLs match.
> I think the latter makes more sense, though personally I would simply
> have compared the remote names.

If this is the behavior we want, here are some squashable tests (on top
of my other tests) to check the URL-matching, and to expose the
multiple-URL case.

---
diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
index c334c51..d809615 100755
--- a/t/t5528-push-default.sh
+++ b/t/t5528-push-default.sh
@@ -51,4 +51,29 @@ test_expect_success '"upstream" does not push when remotes do not match' '
 	test_must_fail git push parent2
 '
 
+test_expect_success '"upstream" remote-match checks URLs' '
+	git checkout master &&
+	test_config branch.master.remote parent1 &&
+	test_config branch.master.merge refs/heads/foo &&
+	test_config push.default upstream &&
+	test_commit six &&
+	git push repo1 &&
+	echo six >expect &&
+	git --git-dir=repo1 log -1 --format=%s foo >actual &&
+	test_cmp expect actual
+'
+
+test_expect_failure '"upstream" remote-match checks all URLs' '
+	git checkout master &&
+	git config --add remote.parent1.push repo2 &&
+	test_config branch.master.remote parent1 &&
+	test_config branch.master.merge refs/heads/foo &&
+	test_config push.default upstream &&
+	test_commit seven &&
+	git push repo2 &&
+	echo seven >expect &&
+	git --git-dir=repo2 log -1 --format=%s foo >actual &&
+	test_cmp expect actual
+'
+
 test_done

Re: push.default: current vs upstream

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> +	if (strcmp(branch->remote_name, remote->name)) {
>> +		struct remote *branch_dest = remote_get(branch->remote_name);
>> +		const char **branch_dest_url, **dest_url;
>> +
>> +		if (!push_url_of_remote(remote, &dest_url) ||
>> +		    !push_url_of_remote(branch_dest, &branch_dest_url) ||
>> +		    strcmp(dest_url[0], branch_dest_url[0]))
>> +			die(_("You are pushing to remote '%s', which is not the "
>> +			      "upstream of your\ncurrent branch '%s'.\n"),
>> +			    remote->name, branch->name);
>> +	}
>
> Hmm. So this will actually detect "git push $URL" when $URL matches the
> remote's configured URL. I feel like this distinction has come up
> before, and we decided not to equate the two. But now I can't remember
> where (maybe it when fetching via URL versus via remote?).
>
> What should happen if there are multiple push URLs configured?

This is me merely try to be extra nice without succeeding.

Perhaps it was an ill-thought-out part of the patch.  The reasoning was
that when you know that your 'origin' is at $URL, it might be irritating
if "git push $URL" did not do what "git push origin" did, but we can
always say 'origin' that is a remoteo nickname is different from $URL; a
remote nickname does not have to be _only_ substitute of the URL, but it
can do more for you.  That would give you more incentive to define remotes
that you interact with often, while keeping the bare-metal flexibility
when interacting with other remotes in a one-shot fashion.

I personally would be perfectly fine if

	$ git push $URL

that does not say what to push out how, regardless of push.default
settings, errors out.

The same can be said when a remote has more than one URL to be pushed to.

Personally I do not care too much about it, but this is one more reason
not to support "upstream" over "current" as the default setting.

Re: push.default: current vs upstream

[Junio C Hamano]

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

> Jeff King <peff@peff.net> writes:
> ...
>> Hmm. So this will actually detect "git push $URL" when $URL matches the
>> remote's configured URL. I feel like this distinction has come up
>> before, and we decided not to equate the two. But now I can't remember
>> where (maybe it when fetching via URL versus via remote?).
>
> This is me merely try to be extra nice without succeeding.

An obvious patch to remove the misguided "be nice and try to see if URLs
are the same" bit should look like this.

 builtin/push.c |   15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 3e18cd3..765b19c 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -97,17 +97,10 @@ static void setup_push_upstream(struct remote *remote)
 	if (branch->merge_nr != 1)
 		die(_("The current branch %s has multiple upstream branches, "
 		    "refusing to push."), branch->name);
-	if (strcmp(branch->remote_name, remote->name)) {
-		struct remote *branch_dest = remote_get(branch->remote_name);
-		const char **branch_dest_url, **dest_url;
-
-		if (!push_url_of_remote(remote, &dest_url) ||
-		    !push_url_of_remote(branch_dest, &branch_dest_url) ||
-		    strcmp(dest_url[0], branch_dest_url[0]))
-			die(_("You are pushing to remote '%s', which is not the "
-			      "upstream of your\ncurrent branch '%s'.\n"),
-			    remote->name, branch->name);
-	}
+	if (strcmp(branch->remote_name, remote->name))
+		die(_("You are pushing to remote '%s', which is not the "
+		      "upstream of your\ncurrent branch '%s'.\n"),
+		    remote->name, branch->name);
 
 	strbuf_addf(&refspec, "%s:%s", branch->name, branch->merge[0]->src);
 	add_refspec(refspec.buf);

Re: push.default: current vs upstream

[demerphq]

On 2 April 2012 23:16, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
> demerphq <demerphq@gmail.com> writes:
>
>> And actually I find your use of "git pull" and "pull" in the
>> expression "pull from a branch other than one with the same name"
>> confusing. Barring misconfiguration pull operates on only one local
>> branch and it is usually the one with the same name. However push
>> operates on multiple local branches.
>
> It does with push.default = matching, but with either options we are
> discussing here, argumentless "git push" would push only one branch.
> The choice we have is whether to push to the branch with the same name,
> or to the branch from which "git pull" would take the changes.

Ah, then I misunderstood your point. Probably because of the point you
make next.

> (I realize that in this discussion, "current" may be misleading. I mean
> "push.default=current", not "the behavior we have currently")
>
>> Lastly I have never really encountered any confusion with explaining
>> the default behaviour of git-fetch, nor actually git-pull, but I have
>> encountered lots of confusion of people using git-push.  They expect
>> git-push to be the opposite of git-pull not git-fetch.
>
> I do also expect "git pull" to be symmetrical to "git pull", and
> "push.default=upstream" is the closest to symmetry.

After clarifying what he meant off-list I'd just like to say that I
agree with Matthieu. Picking a default that makes git-push not be the
opposite of git-pull will end up surprising some people some of the
time. Enough that I suspect that this conversation will be coming up
again in the future.

cheers,
Yves


-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

Re: push.default: current vs upstream

[Jeff King]

On Tue, Apr 03, 2012 at 03:29:34PM -0700, Junio C Hamano wrote:

> > Hmm. So this will actually detect "git push $URL" when $URL matches the
> > remote's configured URL. I feel like this distinction has come up
> > before, and we decided not to equate the two. But now I can't remember
> > where (maybe it when fetching via URL versus via remote?).
> >
> > What should happen if there are multiple push URLs configured?
> 
> This is me merely try to be extra nice without succeeding.
> 
> Perhaps it was an ill-thought-out part of the patch.  The reasoning was
> that when you know that your 'origin' is at $URL, it might be irritating
> if "git push $URL" did not do what "git push origin" did, but we can
> always say 'origin' that is a remoteo nickname is different from $URL; a
> remote nickname does not have to be _only_ substitute of the URL, but it
> can do more for you.  That would give you more incentive to define remotes
> that you interact with often, while keeping the bare-metal flexibility
> when interacting with other remotes in a one-shot fashion.

Yeah, this better matches what we do with fetching, where "git fetch
origin" will respect remote.origin.fetch, but "git fetch $(git config
remote.origin.url)" will not. I do not care too much which way we go,
but I think that it makes sense to be consistent in the two cases.

> I personally would be perfectly fine if
> 
> 	$ git push $URL
> 
> that does not say what to push out how, regardless of push.default
> settings, errors out.
> 
> The same can be said when a remote has more than one URL to be pushed to.

I actually think "git push $URL" makes sense for 'current' and
'matching'. I don't think people tend to do a lot of one-off pushes, but
certainly I have done:

  $ git init
  $ hack hack hack
  $ commit commit commit
  $ ssh example.com git init --bare foo.git
  $ git push example.com:foo.git

(and sometimes even followed by "cd .. && rm -rf foo", if my next step
is to actually clone foo.git somewhere else).  Having to say "HEAD" or
":" is not the end of the world (and in fact 'matching' already errors
out in this case), but it's nice to do the right thing when it's obvious
(i.e., for 'current').

> Personally I do not care too much about it, but this is one more reason
> not to support "upstream" over "current" as the default setting.

Agreed.

-Peff

Re: push.default: current vs upstream

[Jeff King]

On Mon, Apr 02, 2012 at 09:40:22AM +0200, Matthieu Moy wrote:

> For the others, they already have to learn about the "upstream"
> semantics. And making argumentless "git pull" and "git push" purposely
> asymetric to make it simple for the user sounds like an oxymoron to me.

We can make the operations technically symmetric in terms of the actual
sources and destinations from which commits are moved, but they are not
necessarily symmetric in the user's workflow.

Let's imagine I have a branch "topic" that has an upstream of
"origin/master". You are arguing that "git pull" moves commits from
"origin/master" onto "topic", and therefore "git push" should move
commits from "topic" onto "origin/master". That is symmetric at a low
level.

But what does it mean to me as a user? Those operations may not be
symmetric in my workflow if the branches have special meaning. For a
project using a topic-branch workflow, it is not big deal to move
commits from master onto a topic branch. But it _is_ a big deal to move
commits from a topic branch onto master, because that has social
implications within the project (e.g., saying "this topic is ready for
prime-time").

So yeah, the low-level symmetry provides one nice way of explaining
those commands when the symmetry is helpful to your workflow. But I'm
concerned about the cases where what the user wants _isn't_ symmetric.
When they say "git push" because they want to publish their topic
branch, and it does an embarrassing and difficult-to-revert thing to the
public master branch. Telling them "ah, but you should have seen that
pull and push are symmetric! It all makes sense!" is going to be small
consolation.

Fundamentally I am less concerned about explainibility and more about
safety when somebody has not even gotten to the point of having the
thing explained.

> The discussion seems to focuse on 'let's make "git push" easy to
> explain', but I think the right thing to do is to make _Git_ easy to
> explain. With "push.default = current", we'll have a hard time
> explaining how "git pull" works.

Do we have a hard time explaining how "git pull" works now?

-Peff

Re: push.default: current vs upstream

[Matthieu Moy]

Jeff King <peff@peff.net> writes:

> On Mon, Apr 02, 2012 at 09:40:22AM +0200, Matthieu Moy wrote:
>
>> For the others, they already have to learn about the "upstream"
>> semantics. And making argumentless "git pull" and "git push" purposely
>> asymetric to make it simple for the user sounds like an oxymoron to me.
>
> We can make the operations technically symmetric in terms of the actual
> sources and destinations from which commits are moved, but they are not
> necessarily symmetric in the user's workflow.

It seems rather natural to me to have "asymetric workflow, asymetric
commands" by default. So, if one wants to push to a place other than
upstream, say "git push public-repo branch", or set your upstream to
where you want to push (simple with "git push -u"), and say explicitely
"git pull repo branch".

I can hardly imagine someone knowing what "git pull" does, and
_surprised_ to see that "git push" sends commits to the same place. I
agree that sending commits to upstream may be a mistake, but I don't
think it can happen "by surprise".

There are also ways to shoot yourself in the foot with when setting
upstream to something other that where you usually push. For example,
run "git rebase -i" without argument, and it will offer you to rewrite
some published history. "git pull --rebase" also becomes a potentially
dangerous operation, while it's normally harmless with
'push.default=upstream'.

And I still have my concern with real beginners: what advice would you
give to a user whose "git push" is denied because of non-fast forward. I
raised this concern already:

  http://thread.gmane.org/gmane.comp.version-control.git/192547/focus=193196

and I essentially had the answer "telling the user to pull is wrong"
(with which I disagree), but no one managed to give another advice.

With real-real-newbies, this is my number 1 issue (they don't even do
branches, they just run push, git tells them to pull, and they come to
me saying "git is broken, we can't work"). With not-so-newbies, I have
less experience ;-).

>> The discussion seems to focuse on 'let's make "git push" easy to
>> explain', but I think the right thing to do is to make _Git_ easy to
>> explain. With "push.default = current", we'll have a hard time
>> explaining how "git pull" works.
>
> Do we have a hard time explaining how "git pull" works now?

I don't think so, but Junio's argument is that explaining what push
would do with 'upstream' would be too complex, and that 'current' is
easier to explain. If 'git pull' is simple, then 'git -c
push.current=upstream push' is equally simple.

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

Re: push.default: current vs upstream

[Jeff King]

On Thu, Apr 05, 2012 at 06:46:51PM +0200, Matthieu Moy wrote:

> It seems rather natural to me to have "asymetric workflow, asymetric
> commands" by default. So, if one wants to push to a place other than
> upstream, say "git push public-repo branch", or set your upstream to
> where you want to push (simple with "git push -u"), and say explicitely
> "git pull repo branch".

That makes sense _if_ the user is thinking about pull and push as
symmetric commands. That may be immediately obvious for some people's
mental models. But I suspect it is not for others (it is not for mine,
though I obviously do not count as a beginner).

> I can hardly imagine someone knowing what "git pull" does, and
> _surprised_ to see that "git push" sends commits to the same place. I
> agree that sending commits to upstream may be a mistake, but I don't
> think it can happen "by surprise".

You are asking the new user to make a logical inference about the
relationship between push and pull. That inference may seem obvious to
you, and it may even be obvious to a large portion of new users. But
keep in mind that we are not debating whether "upstream" is a reasonable
thing for git to have, but rather whether it is a good default.  My
concern is that upstream as a default would have negligible benefit for
people who do make the inference, but be dangerous for the group who do
not. We don't know the size of the latter, but my feeling is that it is
non-trivial.

> There are also ways to shoot yourself in the foot with when setting
> upstream to something other that where you usually push. For example,
> run "git rebase -i" without argument, and it will offer you to rewrite
> some published history.

Yes, although that is often what you want in such a setup (e.g., you are
rebasing on top of the upstream branch, but publishing your work in
progress).  However, I do agree that it can potentially be dangerous.
Two helpful saving graces are:

  1. The first thing you see upon "git rebase -i" is a giant list of the
     commits from your upstream branch. It is usually quite obvious that
     you are rebasing more than you want in this case, and you can abort
     before doing anything.

  2. Even if you do rebase, you have made a _local_ error. You are not
     hurting anyone until you push, at which point you will get a
     non-fast-forward error, and you have a chance to fix things before
     disrupting other people.

> And I still have my concern with real beginners: what advice would you
> give to a user whose "git push" is denied because of non-fast forward. I
> raised this concern already:
> 
>   http://thread.gmane.org/gmane.comp.version-control.git/192547/focus=193196
> 
> and I essentially had the answer "telling the user to pull is wrong"
> (with which I disagree), but no one managed to give another advice.

It _is_ wrong unless the destination branch is also the configured
upstream. Which yes, it probably is if push.default is "upstream".
Unless you actually specified a push destination, in which case it may
not be. Or if you were pushing something besides HEAD.

If the push destination was $remote:$branch, it seems the only correct
thing is to suggest "git pull $remote $branch" in the general case, and
possibly simplify that to "git pull" if $remote:$branch is the
configured upstream. And if the source was HEAD, of course; otherwise
you would need to checkout.

So shouldn't the advice for a non-fast-forward push be:

   if $source_ref is currently checked out
           advise "git checkout $source_ref, and then..."
   fi
   if $dest_remote == branch.$source_ref.remote &&
      $dest_ref == branch.$source_ref.merge
           advise "git pull"
   else
           advise "git pull $dest_remote $dest_ref"
   fi

That handles only one ref, of course. If you get multiple non-ff
failures, I'm not sure what we should advise.

> >> The discussion seems to focuse on 'let's make "git push" easy to
> >> explain', but I think the right thing to do is to make _Git_ easy to
> >> explain. With "push.default = current", we'll have a hard time
> >> explaining how "git pull" works.
> >
> > Do we have a hard time explaining how "git pull" works now?
> 
> I don't think so, but Junio's argument is that explaining what push
> would do with 'upstream' would be too complex, and that 'current' is
> easier to explain. If 'git pull' is simple, then 'git -c
> push.current=upstream push' is equally simple.

You wrote above that we'll have a hard time explaining how "git pull"
works. But I don't think so; if it hasn't been a problem with
"matching", then why would it with "current"?

I agree that your symmetry explanation is reasonably simple for
explaining what "git push" will do for new users (though I also think
"current" is quite easy to explain). I'm less concerned with explaining
and more concerned about safe defaults.

-Peff

Re: push.default: current vs upstream

[Matthieu Moy]

Jeff King <peff@peff.net> writes:

> On Thu, Apr 05, 2012 at 06:46:51PM +0200, Matthieu Moy wrote:
>
>> It seems rather natural to me to have "asymetric workflow, asymetric
>> commands" by default. So, if one wants to push to a place other than
>> upstream, say "git push public-repo branch", or set your upstream to
>> where you want to push (simple with "git push -u"), and say explicitely
>> "git pull repo branch".
>
> That makes sense _if_ the user is thinking about pull and push as
> symmetric commands. That may be immediately obvious for some people's
> mental models. But I suspect it is not for others (it is not for mine,
> though I obviously do not count as a beginner).

I think exactly the opposite. Once you learnt about remote-tracking
branches, about "git fetch" Vs "git pull", you understand why "git pull"
and "git push" are not strictly speaking symmetrical operations.

But just from the wording, it should be obvious that the commands are
doing something somehow symmetrical.

> So shouldn't the advice for a non-fast-forward push be:
>
>    if $source_ref is currently checked out
>            advise "git checkout $source_ref, and then..."
>    fi
>    if $dest_remote == branch.$source_ref.remote &&
>       $dest_ref == branch.$source_ref.merge
>            advise "git pull"
>    else
>            advise "git pull $dest_remote $dest_ref"
>    fi
>
> That handles only one ref, of course. If you get multiple non-ff
> failures, I'm not sure what we should advise.

The topic ct/advise-push-default does essentially that indeed.

> You wrote above that we'll have a hard time explaining how "git pull"
> works. But I don't think so; if it hasn't been a problem with
> "matching", then why would it with "current"?

I mis-spoke. I think I meant something like "if the assumption that
explaining push -c push.current=upstream is hard, then we'll have a hard
time explaining git pull".

> I'm less concerned with explaining and more concerned about safe
> defaults.

Yes, my rant about simplicity was a reply to Junio, who was arguing
about simplicity:

Junio C Hamano writes:

| Obviously the former ["current"] is much simpler to explain and understand, as
| people do not have to learn upstream tracking before doing their first
| "push".

Your arguments about safety are valid (we don't agree on the relative
importance), but I think the argument of simplicity simply doesn't hold.

About safety, I don't think we can tell in general which bad push is the
most serious. push.default=current may create branches unexpectedly,
while push.default=upstream would ask you to "push --set-upstream" when
creating the remote branch. push.default=upstream may push to the master
when you wanted to create a remote topic branch. My feeling is that both
are equally bad, but maybe I'm wrong here.

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

Re: push.default: current vs upstream

[Jeff King]

On Fri, Apr 06, 2012 at 09:44:48AM +0200, Matthieu Moy wrote:

> I think exactly the opposite. Once you learnt about remote-tracking
> branches, about "git fetch" Vs "git pull", you understand why "git pull"
> and "git push" are not strictly speaking symmetrical operations.
> 
> But just from the wording, it should be obvious that the commands are
> doing something somehow symmetrical.

My mind may be warped from too much git and I may be misremembering my
early days, but I feel like I never considered "push" and "pull" to be
symmetric opposites. I dunno. We are both talking about some
hypothetical population of new users, and I'm not sure either of us has
hard data.

> > So shouldn't the advice for a non-fast-forward push be:
> >
> >    if $source_ref is currently checked out
> >            advise "git checkout $source_ref, and then..."
> >    fi
> >    if $dest_remote == branch.$source_ref.remote &&
> >       $dest_ref == branch.$source_ref.merge
> >            advise "git pull"
> >    else
> >            advise "git pull $dest_remote $dest_ref"
> >    fi
> >
> > That handles only one ref, of course. If you get multiple non-ff
> > failures, I'm not sure what we should advise.
> 
> The topic ct/advise-push-default does essentially that indeed.

I think it does the first half (recommend checkout if it was a non-HEAD). But
the "pull before push" message just says:

  Updates were rejected because the tip of your current branch is behind
  its remote counterpart. Merge the remote changes (e.g. 'git pull')
  before pushing again.

which is not exactly right. Saying "e.g." lets clueful people know that
the details of merging might be different, but I doubt that subtlety is
helpful for new users.

> > You wrote above that we'll have a hard time explaining how "git pull"
> > works. But I don't think so; if it hasn't been a problem with
> > "matching", then why would it with "current"?
> 
> I mis-spoke. I think I meant something like "if the assumption that
> explaining push -c push.current=upstream is hard, then we'll have a hard
> time explaining git pull".

That makes more sense to me.

> About safety, I don't think we can tell in general which bad push is the
> most serious. push.default=current may create branches unexpectedly,
> while push.default=upstream would ask you to "push --set-upstream" when
> creating the remote branch. push.default=upstream may push to the master
> when you wanted to create a remote topic branch. My feeling is that both
> are equally bad, but maybe I'm wrong here.

I would say without hesitation that fast-forwarding the upstream is more
likely to be disastrous than creating a new branch. However,
push.default=current can also fast-forward an existing branch (although
if you have a local "foo" and its upstream is _not_ the remote "foo", I
find it extremely unlikely that a remote "foo" also exists).

On the other hand, one thing we have not talked about is how one gets
into the "topic push fast-forwards master" situation. Which is running:

  $ git checkout -b topic origin/master

I'm not sure if that is something totally clueless people will run. And
maybe by the time people are intermediate enough git users to run that,
they will have figured out how upstream works. So maybe my concern is
overblown.  I consider the much more likely scenarios for a new user to
be:

  $ git clone ... && cd project
  $ hack hack hack
  $ git push

which will work with either "current" or "upstream", or:

  $ git clone ... && cd project
  $ git checkout foo ;# equivalent of "git checkout -b foo origin/foo"
  $ hack hack hack
  $ git push

which also works with both, or:

  $ git clone ... && cd project
  $ git checkout -b topic
  $ hack hack hack
  $ git push

which will error out for "upstream", and create a new branch "topic" on
the remote with "current".

-Peff

Re: push.default: current vs upstream

[Dmitry Potapov]

On Thu, Apr 5, 2012 at 8:46 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
>
> I can hardly imagine someone knowing what "git pull" does, and
> _surprised_ to see that "git push" sends commits to the same place.

It seems you assume that people use in a _centralized_ workflow.
In this case, 'upstream' does largely the right thing, so no one
will be surprised.

However, many of those who got used to a distributed workflow will find
that surprising, because when they created a new branch that meant it
to push as a _new_ branch. So other people could take a look, or they
may need the maintainer ACK to push anything to 'master'. Also they may
have a policy nothing should be fast-forwarded to master, but only to
be merged with a merge commit.

And then it is more natural for people to think in terms of names that
are immediately obvious to anyone, while 'upstream' behavior depends on
the state that is not immediately obvious. You may start a new topic
branch based on 'master' or some the latest release to have a more
stable base. And 'upstream' will work differently in this case. If you
know about 'tracking' then it may be obvious to you, but it is not so
obvious to those who only start to use git for short time...


>
> And I still have my concern with real beginners: what advice would you
> give to a user whose "git push" is denied because of non-fast forward. I
> raised this concern already:

Don't use a central workflow, because it sucks :)

Seriously, why do you care about beginners who use a centralized workflow
and not beginners who have to use with existing projects that use more or
less distributed workflow, where pushing to 'master' is more likely to be
the wrong thing to do than otherwise... And when push is denied, they may
ask someone whether they are doing something wrong. In case when master
is fast-forwarded silently, they are not likely to notice that they did
something wrong, and the fact that happens only sometimes (depending on
some "tracking" feature which they have not heard) is not very helpful.


Dmitry

Re: push.default: current vs upstream

[demerphq]

On 6 April 2012 13:38, Dmitry Potapov <dpotapov@gmail.com> wrote:
> Seriously, why do you care about beginners who use a centralized workflow
> and not beginners who have to use with existing projects that use more or
> less distributed workflow,

Because the former are unlikely to be self-selected users of git and
instead are likely to be forced to use git because their $work has
dictated it to be so. The self-selected users of git IMO would tend to
both have the motivation and the basic skills to learn whatever they
need and are unlikely to blame their mistakes on git. The ones forced
to use git are *very* likely to say "git is broken", or "git doesn't
work" and then start arguing that "cvs never had that problem". Do you
really want a bunch of users of your software thinking CVS was
superior?

I would say the right default is the one that keeps idiot users happy,
and has the least "out of the box surprises" for them. The smart ones
will figure things out anyway and configure their tools appropriately.

cheers
Yves




-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

Re: push.default: current vs upstream

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I would say without hesitation that fast-forwarding the upstream is more
> likely to be disastrous than creating a new branch. However,
> push.default=current can also fast-forward an existing branch (although
> if you have a local "foo" and its upstream is _not_ the remote "foo", I
> find it extremely unlikely that a remote "foo" also exists).
>
> On the other hand, one thing we have not talked about is how one gets
> into the "topic push fast-forwards master" situation. Which is running:
>
>   $ git checkout -b topic origin/master
>
> I'm not sure if that is something totally clueless people will run. And
> maybe by the time people are intermediate enough git users to run that,
> they will have figured out how upstream works. So maybe my concern is
> overblown....

I already said that I do not deeply care either way, and it appears that
only you two are the ones who deeply care *and* have things to say that
are worth listening to (we've seen many "me too wants upstream" without
deep thought a few weeks ago against the RFD).  Whichever way you two
agree to go eventually, could you come up with a (hopefully) small summary
of caveats that can be pasted in the future release notes that switches
the default away from the matching?

E.g. "The new default is now 'upstream', which does mostly what new people
do, but the user should not allow the default kick in in these situations:
A, B, C..."  It might end up saying "The new default is now 'current',..."
but the point is that either semantics would need to come with a warning.

Thanks.

Re: push.default: current vs upstream

[Jehan Bing]

On 2012-04-06 01:00, Jeff King wrote:
> On the other hand, one thing we have not talked about is how one gets
> into the "topic push fast-forwards master" situation. Which is running:
>
>   $ git checkout -b topic origin/master

That's what I was starting to think myself. This whole discussion about 
"current" is that it may not a good choice because "push" will not 
behave like "pull". But I believe the real problem is that _pull_ 
doesn't behave like _push_.

In other word, maybe "git checkout -b topic origin/master" shouldn't 
create a tracking branch in the first place (unless one uses "--track" 
or that some config setting has been explicitly set).
Or more precisely, I believe it should be create a tracking branch only 
if the local branch name is the same as the remote's branch.
(And the same for "git branch <local> origin/<remote>" of course)

How many newbies/clueless people will think that a branch "topic" tracks 
a branch "origin/master"? And how many of these same people will think 
that a branch "topic" doesn't track a branch "origin/topic"? For that 
matter, that's how I use git myself. And until it's easy to see what 
branch tracks what (i.e. until "git branch" shows the tracking 
branches), I don't want to think differently because it's just too risky.

So for the "me too" votes:
- +1 to change change "push.default" to current
- +1 to change "git checkout -b" to not create a tracking branch unless 
the local name matches the remote name .

Re: push.default: current vs upstream

[Dmitry Potapov]

On Fri, Apr 6, 2012 at 5:36 PM, demerphq <demerphq@gmail.com> wrote:
> On 6 April 2012 13:38, Dmitry Potapov <dpotapov@gmail.com> wrote:
>> Seriously, why do you care about beginners who use a centralized workflow
>> and not beginners who have to use with existing projects that use more or
>> less distributed workflow,
>
> Because the former are unlikely to be self-selected users of git and
> instead are likely to be forced to use git because their $work has
> dictated it to be so.

Any decision is made by people. On its own, $work does not dictate what
VCS or what workflow should be used. There are many ways for those who
are in charge to screw up things. And a centralized workflow is not very
scalable and many bad practices associated with it. While it is not easy
to to convert a CVS/SVN repository to git that alone does not bring most
of git advantages, because those advantages come from the workflow.

> The self-selected users of git IMO would tend to
> both have the motivation and the basic skills to learn whatever they
> need and are unlikely to blame their mistakes on git. The ones forced
> to use git are *very* likely to say "git is broken", or "git doesn't
> work" and then start arguing that "cvs never had that problem". Do you
> really want a bunch of users of your software thinking CVS was
> superior?

Git is a distributed version control system. There is another VCS whose
whole designed was dictated by being a better CVS. It's called SVN and
if someone is happy with it, why do not use it?

I think git default settings should respect the main goal of git design:
a good support of a distributed workflow. Certainly git can be used in
many other ways: some people use it with a centralized workflow, some
use it to back up their configuration files, etc.. But those usage
should not dictate the default settings for git.

Dmitry

Re: push.default: current vs upstream

[demerphq]

On 6 April 2012 20:03, Dmitry Potapov <dpotapov@gmail.com> wrote:
> On Fri, Apr 6, 2012 at 5:36 PM, demerphq <demerphq@gmail.com> wrote:
>> On 6 April 2012 13:38, Dmitry Potapov <dpotapov@gmail.com> wrote:
>>> Seriously, why do you care about beginners who use a centralized workflow
>>> and not beginners who have to use with existing projects that use more or
>>> less distributed workflow,
>>
>> Because the former are unlikely to be self-selected users of git and
>> instead are likely to be forced to use git because their $work has
>> dictated it to be so.
>
> Any decision is made by people. On its own, $work does not dictate what
> VCS or what workflow should be used. There are many ways for those who
> are in charge to screw up things. And a centralized workflow is not very
> scalable and many bad practices associated with it. While it is not easy
> to to convert a CVS/SVN repository to git that alone does not bring most
> of git advantages, because those advantages come from the workflow.

Pretty well every project that uses git has a "canonical upstream
repository". Including for instance this one. Which basically means at
some point there is a centralized master repo. It is either owned by
someone like Linus or Junio, or it is owned by a company. Companies
tend to like to know that their valuable data is properly backed up,
and etc. This basically means central repos are inevitable. And git
works just fine like that thank you very much.

>> The self-selected users of git IMO would tend to
>> both have the motivation and the basic skills to learn whatever they
>> need and are unlikely to blame their mistakes on git. The ones forced
>> to use git are *very* likely to say "git is broken", or "git doesn't
>> work" and then start arguing that "cvs never had that problem". Do you
>> really want a bunch of users of your software thinking CVS was
>> superior?
>
> Git is a distributed version control system. There is another VCS whose
> whole designed was dictated by being a better CVS. It's called SVN and
> if someone is happy with it, why do not use it?

Because it sucks.

> I think git default settings should respect the main goal of git design:
> a good support of a distributed workflow. Certainly git can be used in
> many other ways: some people use it with a centralized workflow, some
> use it to back up their configuration files, etc.. But those usage
> should not dictate the default settings for git.

I stick to my original point, it should be aimed at making dumb people
happy. The rest will sort themselves out.

Yves

-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

Re: push.default: current vs upstream

[Dmitry Potapov]

On Fri, Apr 6, 2012 at 10:48 PM, demerphq <demerphq@gmail.com> wrote:
> On 6 April 2012 20:03, Dmitry Potapov <dpotapov@gmail.com> wrote:
>> On Fri, Apr 6, 2012 at 5:36 PM, demerphq <demerphq@gmail.com> wrote:
>>> On 6 April 2012 13:38, Dmitry Potapov <dpotapov@gmail.com> wrote:
>>>> Seriously, why do you care about beginners who use a centralized workflow
>>>> and not beginners who have to use with existing projects that use more or
>>>> less distributed workflow,
>>>
>>> Because the former are unlikely to be self-selected users of git and
>>> instead are likely to be forced to use git because their $work has
>>> dictated it to be so.
>>
>> Any decision is made by people. On its own, $work does not dictate what
>> VCS or what workflow should be used. There are many ways for those who
>> are in charge to screw up things. And a centralized workflow is not very
>> scalable and many bad practices associated with it. While it is not easy
>> to to convert a CVS/SVN repository to git that alone does not bring most
>> of git advantages, because those advantages come from the workflow.
>
> Pretty well every project that uses git has a "canonical upstream
> repository". Including for instance this one. Which basically means at
> some point there is a centralized master repo. It is either owned by
> someone like Linus or Junio, or it is owned by a company. Companies
> tend to like to know that their valuable data is properly backed up,
> and etc. This basically means central repos are inevitable. And git
> works just fine like that thank you very much.

It seems you confuse a centralized workflow with existence of an official
(central) repository. It is not same...

Dmitry

Re: push.default: current vs upstream

[Michael Haggerty]

The very subject of this email is leading the discussion in circles.

Neither current nor upstream is safe for beginners because there are 
distinct workflows that require different behaviors in scenarios that 
git cannot distinguish.

On 04/06/2012 10:00 AM, Jeff King wrote:
> On Fri, Apr 06, 2012 at 09:44:48AM +0200, Matthieu Moy wrote:
> [...]
>> About safety, I don't think we can tell in general which bad push is the
>> most serious. push.default=current may create branches unexpectedly,
>> while push.default=upstream would ask you to "push --set-upstream" when
>> creating the remote branch. push.default=upstream may push to the master
>> when you wanted to create a remote topic branch. My feeling is that both
>> are equally bad, but maybe I'm wrong here.

Exactly.  Choosing between "current vs upstream" is a false dichotomy.

Let's create a new "push.default" option (call it "beginner" for the 
sake of discussion) that is intended for use *only* for people who 
haven't explicitly chosen another alternative.  Let the "beginner" 
option do the obvious thing when it is uncontroversial and undangerous, 
and let it output a beginner-level help message in any scenarios where 
the right thing to do is not obvious.  The help message should basically 
recommend that the user run "git config push.default VALUE" and explain 
the meaning of the possible VALUEs.

If "push.default" is not set, then have it default to "beginner" mode.

> On the other hand, one thing we have not talked about is how one gets
> into the "topic push fast-forwards master" situation. Which is running:
>
>    $ git checkout -b topic origin/master
>
> I'm not sure if that is something totally clueless people will run. And
> maybe by the time people are intermediate enough git users to run that,
> they will have figured out how upstream works. So maybe my concern is
> overblown.

When the "beginner" option is active, this case should trigger the 
informational help message because git cannot know for sure what the 
user wants.

 > I consider the much more likely scenarios for a new user to
> be:
>
>    $ git clone ...&&  cd project
>    $ hack hack hack
>    $ git push
>
> which will work with either "current" or "upstream", or:
>
>    $ git clone ...&&  cd project
>    $ git checkout foo ;# equivalent of "git checkout -b foo origin/foo"
>    $ hack hack hack
>    $ git push
>
> which also works with both

Since the first two scenarios are uncontroversial, the "beginner" option 
should do the expected thing.

 > or:
>
>    $ git clone ...&&  cd project
>    $ git checkout -b topic
>    $ hack hack hack
>    $ git push
>
> which will error out for "upstream", and create a new branch "topic" on
> the remote with "current".

The correct behavior in the above scenario depends on the workflow, so 
the "beginner" option should not do anything except supply help information.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: push.default: current vs upstream

[Jeff King]

On Sat, Apr 07, 2012 at 09:49:47AM +0200, Michael Haggerty wrote:

> Exactly.  Choosing between "current vs upstream" is a false dichotomy.
> 
> Let's create a new "push.default" option (call it "beginner" for the
> sake of discussion) that is intended for use *only* for people who
> haven't explicitly chosen another alternative.  Let the "beginner"
> option do the obvious thing when it is uncontroversial and
> undangerous, and let it output a beginner-level help message in any
> scenarios where the right thing to do is not obvious.  The help
> message should basically recommend that the user run "git config
> push.default VALUE" and explain the meaning of the possible VALUEs.
> 
> If "push.default" is not set, then have it default to "beginner" mode.

I would be fine with that (I've suggested it elsewhere in the thread,
though I think I stole the idea originally from you. Speaking of going
in circles. :) ).

-Peff

Re: push.default: current vs upstream

[Andrew Sayers]

On a slight aside, should we add @{downstream} to describe the opposite
of @{upstream}?  Seeing that around the place would give intermediate
users a clue about why pull and push aren't as related as they think,
and would be useful here and there in code (e.g. __git_ps1 could show a
better bash prompt with GIT_PS1_SHOWUPSTREAM).

	- Andrew

Re: push.default: current vs upstream

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Sat, Apr 07, 2012 at 09:49:47AM +0200, Michael Haggerty wrote:
> ...
>> If "push.default" is not set, then have it default to "beginner" mode.
>
> I would be fine with that (I've suggested it elsewhere in the thread,
> though I think I stole the idea originally from you. Speaking of going
> in circles. :) ).

This makes the first priority for 1.7.10 cycle to come up with a solid
implementation of the "beginner" mode, I guess.

Re: push.default: current vs upstream

[Felipe Contreras]

Hi,

I only now read this thread (or most of it).

First of all, personally I never do 'git push' because I'm never sure
of what it would do, and instead of trying to figure it out, I just
specify the branches/tags I want to push.

I've seen a lot of people vote for 'upstream', and while I think
that's an improvement, I saw very good arguments against it; namely;
the fact that the configured upstream might not be the right one, and
that it would be confusing to new users. Hell, I almost never know
what is the upstream branch of the branch I am currently on, so I
think 'current' makes more sense.

However, another good argument is that switching away from matching
would break the symmetry with 'git pull', while it has been argued
they are not really symmetric anyway, it would be nice if they were.
Which brings me to how I personally use 'git pull'; I never use it in
any shape or form, and the reason is similar to why I don't do 'git
push'; I don't know what it's going to do.

I think it's important to consider what people have been suggesting
new users, and one common suggestion has been to change the
'push.default' configuration right away, so it most definitely needs
to be changed, but another common suggestion is to avoid 'git pull'
completely.

Personally I think that if 'push.default' was 'current', I might use
'git push' without arguments more often, as I would know *exactly*
what's going to happen, and it's very often what I want. But also, it
would be great if 'git pull' without arguments did something
symmetrically similar; only pull a branch with the same name. In
addition to that, a lot of people end up doing merges by mistake,
because they use 'git pull' assuming it would do the same as in other
SCMs, so it would be great if 'git pull' would do --rebase by default,
and use the remote/current branch as the base for the rebase instead.
This is more or less what I always end up doing manually anyway (git
fetch origin; git rebase -i origin/master).

While the 'git pull' changes are a separate topic, I wanted to mention
them as a rationale as to why I think 'push.default' should be
'current'.

Cheers.

-- 
Felipe Contreras

[RFC/PATCH] Give better 'pull' advice when pushing non-ff updates to current branch

[Christopher Tiwald]

On Fri, Apr 06, 2012 at 03:15:20AM -0400, Jeff King wrote:
> So shouldn't the advice for a non-fast-forward push be:
> 
>    if $source_ref is currently checked out
>            advise "git checkout $source_ref, and then..."
>    fi
>    if $dest_remote == branch.$source_ref.remote &&
>       $dest_ref == branch.$source_ref.merge
>            advise "git pull"
>    else
>            advise "git pull $dest_remote $dest_ref"
>    fi
> 
> That handles only one ref, of course. If you get multiple non-ff
> failures, I'm not sure what we should advise.

Hmmm. Maybe something like this? Note to reviewers: This is necessarily
based on ct/advise-push-default.

Assuming this logic is sound and the patch is a reasonable change, I'm not
wedded to "pushNonFFCurrentUntracked" and "pushNonFFCurrentTracked". I'm
concerned both config options are a bit too long. Is there a better, more
concise way to specify those config options?

---- >8 ----
Suppose a user configured a local branch to track an upstream branch by
a different name or didn't set an upstream branch at all. In these
cases, issuing 'git pull' without specifying a remote repository or
refspec can be dangerous. In the first case, 'git pull --rebase' could
rewrite published history. In the second, 'git pull' without argument
will fail.

Modify 'git push's non-fast-forward advice to account for these cases.
Instruct users who push a non-fast-forward update to their current
branch to 'git pull <repository> <refspec>' when the branch is untracked
or tracks to a different repo or refspec then the one they specified.
Otherwise, instruct users to 'git pull'. Make both types of advice
configurable, so that users who disable one won't disable the other on
accident. Finally, offer users who configure a branch for octopus
merges, i.e. where 'branch->merge_nr > 1', the simple 'git pull' advice.

Signed-off-by: Christopher Tiwald <christiwald@gmail.com>
---
 Documentation/config.txt |    9 +++++++--
 advice.c                 |    6 ++++--
 advice.h                 |    3 ++-
 builtin/push.c           |   48 +++++++++++++++++++++++++++++++++++++++++-----
 4 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index fb386ab..fd72120 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -141,9 +141,14 @@ advice.*::
 		Set this variable to 'false' if you want to disable
 		'pushNonFFCurrent', 'pushNonFFDefault', and
 		'pushNonFFMatching' simultaneously.
-	pushNonFFCurrent::
+	pushNonFFCurrentUntracked::
 		Advice shown when linkgit:git-push[1] fails due to a
-		non-fast-forward update to the current branch.
+		non-fast-forward update to the current branch and that
+		branch doesn't match the tracked remote and refspec.
+	pushNonFFCurrentTracked::
+		Advice shown when linkgit:git-push[1] fails due to a
+		non-fast-forward update to the current branch and that
+		branch matches the tracked remote and refspec.
 	pushNonFFDefault::
 		Advice to set 'push.default' to 'upstream' or 'current'
 		when you ran linkgit:git-push[1] and pushed 'matching
diff --git a/advice.c b/advice.c
index a492eea..828a41b 100644
--- a/advice.c
+++ b/advice.c
@@ -1,7 +1,8 @@
 #include "cache.h"
 
 int advice_push_nonfastforward = 1;
-int advice_push_non_ff_current = 1;
+int advice_push_non_ff_current_untracked = 1;
+int advice_push_non_ff_current_tracked = 1;
 int advice_push_non_ff_default = 1;
 int advice_push_non_ff_matching = 1;
 int advice_status_hints = 1;
@@ -15,7 +16,8 @@ static struct {
 	int *preference;
 } advice_config[] = {
 	{ "pushnonfastforward", &advice_push_nonfastforward },
-	{ "pushnonffcurrent", &advice_push_non_ff_current },
+	{ "pushnonffcurrentuntracked", &advice_push_non_ff_current_untracked },
+	{ "pushnonffcurrenttracked", &advice_push_non_ff_current_tracked },
 	{ "pushnonffdefault", &advice_push_non_ff_default },
 	{ "pushnonffmatching", &advice_push_non_ff_matching },
 	{ "statushints", &advice_status_hints },
diff --git a/advice.h b/advice.h
index f3cdbbf..c18809f 100644
--- a/advice.h
+++ b/advice.h
@@ -4,7 +4,8 @@
 #include "git-compat-util.h"
 
 extern int advice_push_nonfastforward;
-extern int advice_push_non_ff_current;
+extern int advice_push_non_ff_current_untracked;
+extern int advice_push_non_ff_current_tracked;
 extern int advice_push_non_ff_default;
 extern int advice_push_non_ff_matching;
 extern int advice_status_hints;
diff --git a/builtin/push.c b/builtin/push.c
index 8a14e4b..0671d27 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -118,12 +118,18 @@ static void setup_default_push_refspecs(struct remote *remote)
 	}
 }
 
-static const char message_advice_pull_before_push[] =
+static const char message_advice_tracked_pull_before_push[] =
 	N_("Updates were rejected because the tip of your current branch is behind\n"
 	   "its remote counterpart. Merge the remote changes (e.g. 'git pull')\n"
 	   "before pushing again.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
+static const char message_advice_untracked_pull_before_push[] =
+	N_("Updates were rejected because the tip of your current branch is behind\n"
+	   "its remote counterpart. Merge the remote changes to your local branch\n"
+	   "(e.g. 'git pull <repository> <refspec>') before pushing again.\n"
+	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
+
 static const char message_advice_use_upstream[] =
 	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
 	   "counterpart. If you did not intend to push that branch, you may want to\n"
@@ -136,11 +142,20 @@ static const char message_advice_checkout_pull_push[] =
 	   "(e.g. 'git pull') before pushing again.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
-static void advise_pull_before_push(void)
+static void advise_tracked_pull_before_push(void)
+{
+	if (!advice_push_non_ff_current_tracked ||
+	    !advice_push_nonfastforward)
+		return;
+	advise(_(message_advice_tracked_pull_before_push));
+}
+
+static void advise_untracked_pull_before_push(void)
 {
-	if (!advice_push_non_ff_current || !advice_push_nonfastforward)
+	if (!advice_push_non_ff_current_untracked ||
+	    !advice_push_nonfastforward)
 		return;
-	advise(_(message_advice_pull_before_push));
+	advise(_(message_advice_untracked_pull_before_push));
 }
 
 static void advise_use_upstream(void)
@@ -161,6 +176,16 @@ static int push_with_options(struct transport *transport, int flags)
 {
 	int err;
 	int nonfastforward;
+	struct branch *branch;
+	struct strbuf buf = STRBUF_INIT;
+
+	branch = branch_get(NULL);
+
+	if (branch) {
+		strbuf_addstr(&buf, transport->remote->name);
+		strbuf_addstr(&buf, "/");
+		strbuf_addstr(&buf, branch->name);
+	}
 
 	transport_set_verbosity(transport, verbosity, progress);
 
@@ -185,7 +210,18 @@ static int push_with_options(struct transport *transport, int flags)
 	default:
 		break;
 	case NON_FF_HEAD:
-		advise_pull_before_push();
+		/* Branches configured for octopus merges should advise
+		 * just 'git pull' */
+		if (branch->remote_name &&
+		    branch->merge &&
+		    branch->merge_nr == 1 &&
+		    !strcmp(transport->remote->name, branch->remote_name) &&
+		    !strcmp(strbuf_detach(&buf, NULL),
+			    prettify_refname(branch->merge[0]->dst))) {
+			advise_tracked_pull_before_push();
+		}
+		else
+			advise_untracked_pull_before_push();
 		break;
 	case NON_FF_OTHER:
 		if (default_matching_used)
@@ -195,6 +231,8 @@ static int push_with_options(struct transport *transport, int flags)
 		break;
 	}
 
+	strbuf_release(&buf);
+
 	return 1;
 }
 
-- 
1.7.10.4.g2c970.dirty

Re: push.default: current vs upstream

[Matthieu Moy]

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

> Jeff King <peff@peff.net> writes:
>
>> On Sat, Apr 07, 2012 at 09:49:47AM +0200, Michael Haggerty wrote:
>> ...
>>> If "push.default" is not set, then have it default to "beginner" mode.
>>
>> I would be fine with that (I've suggested it elsewhere in the thread,
>> though I think I stole the idea originally from you. Speaking of going
>> in circles. :) ).
>
> This makes the first priority for 1.7.10 cycle to come up with a solid
> implementation of the "beginner" mode, I guess.

I guess so. I found the idea relevant the first time it came in the
discussion, and I'm starting to get really convinced that this is the
way to go. And if we're wrong, it will be much easier to migrate to
either "current" or "upstream" later.

No time to code this right now, but I may come up with a patch in a few
days. Any idea on the real name? I'd call it "safeUpstream" or
"safeCurrent", but that's probably by lack of a better name.

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

Re: push.default: current vs upstream

[Junio C Hamano]

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

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Jeff King <peff@peff.net> writes:
>>
>>> On Sat, Apr 07, 2012 at 09:49:47AM +0200, Michael Haggerty wrote:
>>> ...
>>>> If "push.default" is not set, then have it default to "beginner" mode.
>>>
>>> I would be fine with that (I've suggested it elsewhere in the thread,
>>> though I think I stole the idea originally from you. Speaking of going
>>> in circles. :) ).
>>
>> This makes the first priority for 1.7.10 cycle to come up with a solid
>> implementation of the "beginner" mode, I guess.
>
> I guess so. I found the idea relevant the first time it came in the
> discussion, and I'm starting to get really convinced that this is the
> way to go. And if we're wrong, it will be much easier to migrate to
> either "current" or "upstream" later.
>
> No time to code this right now, but I may come up with a patch in a few
> days. Any idea on the real name? I'd call it "safeUpstream" or
> "safeCurrent", but that's probably by lack of a better name.

I'd say "simple" ;-)

Re: push.default: current vs upstream

[Jeff King]

On Sat, Apr 07, 2012 at 09:40:26AM +0100, Andrew Sayers wrote:

> On a slight aside, should we add @{downstream} to describe the opposite
> of @{upstream}?  Seeing that around the place would give intermediate
> users a clue about why pull and push aren't as related as they think,
> and would be useful here and there in code (e.g. __git_ps1 could show a
> better bash prompt with GIT_PS1_SHOWUPSTREAM).

Maybe. I don't really see how it is useful, but maybe you want to flesh
our your proposal with some examples? I do not use __git_ps1, so I'm not
sure what you want to improve there.

-Peff

Re: push.default: current vs upstream

[Jeff King]

On Wed, Apr 11, 2012 at 06:17:28PM +0200, Matthieu Moy wrote:

> > This makes the first priority for 1.7.10 cycle to come up with a solid
> > implementation of the "beginner" mode, I guess.
> 
> I guess so. I found the idea relevant the first time it came in the
> discussion, and I'm starting to get really convinced that this is the
> way to go. And if we're wrong, it will be much easier to migrate to
> either "current" or "upstream" later.
> 
> No time to code this right now, but I may come up with a patch in a few
> days. Any idea on the real name? I'd call it "safeUpstream" or
> "safeCurrent", but that's probably by lack of a better name.

I think it is as simple as applying the usual upstream rules, then
checking the remote name against the local name. Like this:

diff --git a/builtin/push.c b/builtin/push.c
index b6c0fee..6077ecc 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -75,7 +75,7 @@ static int push_url_of_remote(struct remote *remote, const char ***url_p)
 	return remote->url_nr;
 }
 
-static void setup_push_upstream(struct remote *remote)
+static void setup_push_upstream(struct remote *remote, int simple)
 {
 	struct strbuf refspec = STRBUF_INIT;
 	struct branch *branch = branch_get(NULL);
@@ -103,6 +103,29 @@ static void setup_push_upstream(struct remote *remote)
 		      "to update which remote branch."),
 		    remote->name, branch->name);
 
+	if (simple && strcmp(branch->refname, branch->merge[0]->src)) {
+		/*
+		 * There's no point in using shorten_unambiguous_ref here,
+		 * as the ambiguity would be on the remote side, not what
+		 * we have locally. Plus, this is supposed to be the simple
+		 * mode. If the user is doing something crazy like setting
+		 * upstream to a non-branch, we should probably be showing
+		 * them the big ugly fully qualified ref.
+		 */
+		const char *short_up = skip_prefix(branch->merge[0]->src, "refs/heads/");
+		die(_("The upstream branch of your current branch does not match\n"
+		      "the name of your current branch.  To push to the upstream branch\n"
+		      "on the remote, use\n"
+		      "\n"
+		      "    git push %s HEAD:%s\n"
+		      "\n"
+		      "To push to the branch of the same name on the remote, use\n"
+		      "\n"
+		      "    git push %s %s\n"),
+		    remote->name, short_up ? short_up : branch->merge[0]->src,
+		    remote->name, branch->name);
+	}
+
 	strbuf_addf(&refspec, "%s:%s", branch->name, branch->merge[0]->src);
 	add_refspec(refspec.buf);
 }
@@ -115,8 +138,12 @@ static void setup_default_push_refspecs(struct remote *remote)
 		add_refspec(":");
 		break;
 
+	case PUSH_DEFAULT_SIMPLE:
+		setup_push_upstream(remote, 1);
+		break;
+
 	case PUSH_DEFAULT_UPSTREAM:
-		setup_push_upstream(remote);
+		setup_push_upstream(remote, 0);
 		break;
 
 	case PUSH_DEFAULT_CURRENT:
diff --git a/cache.h b/cache.h
index e12b15f..0f862dd 100644
--- a/cache.h
+++ b/cache.h
@@ -624,6 +624,7 @@ enum rebase_setup_type {
 enum push_default_type {
 	PUSH_DEFAULT_NOTHING = 0,
 	PUSH_DEFAULT_MATCHING,
+	PUSH_DEFAULT_SIMPLE,
 	PUSH_DEFAULT_UPSTREAM,
 	PUSH_DEFAULT_CURRENT
 };
diff --git a/config.c b/config.c
index ad03908..0ca0a6f 100644
--- a/config.c
+++ b/config.c
@@ -824,6 +824,8 @@ static int git_default_push_config(const char *var, const char *value)
 			push_default = PUSH_DEFAULT_NOTHING;
 		else if (!strcmp(value, "matching"))
 			push_default = PUSH_DEFAULT_MATCHING;
+		else if (!strcmp(value, "simple"))
+			push_default = PUSH_DEFAULT_SIMPLE;
 		else if (!strcmp(value, "upstream"))
 			push_default = PUSH_DEFAULT_UPSTREAM;
 		else if (!strcmp(value, "tracking")) /* deprecated */
diff --git a/environment.c b/environment.c
index c93b8f4..1defcd4 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_SIMPLE;
 #ifndef OBJECT_CREATION_MODE
 #define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
 #endif

Re: push.default: current vs upstream

[Matthieu Moy]

Jeff King <peff@peff.net> writes:

> I think it is as simple as applying the usual upstream rules, then
> checking the remote name against the local name. Like this:

This fails if no upstream is configured. I think applying "current"
instead of failing in this case makes perfect sense (i.e. "simple"
should fail only if there is an upstream configured, and it has a
different name).

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

Re: push.default: current vs upstream

[Jeff King]

On Thu, Apr 12, 2012 at 10:09:35AM +0200, Matthieu Moy wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I think it is as simple as applying the usual upstream rules, then
> > checking the remote name against the local name. Like this:
> 
> This fails if no upstream is configured. I think applying "current"
> instead of failing in this case makes perfect sense (i.e. "simple"
> should fail only if there is an upstream configured, and it has a
> different name).

Then the rule is not really "act only if upstream and current would do
the same thing".

On the one hand, I think what you are suggesting is reasonable in most
cases. On the other hand, what if the lack of upstream is because the
user failed to configure it properly? Then it could be surprising.

I don't have a strong opinion either way. Do you want to pick up my
patch and start tweaking?

-Peff

Re: push.default: current vs upstream

[Matthieu Moy]

Jeff King <peff@peff.net> writes:

> Then the rule is not really "act only if upstream and current would do
> the same thing".

Right. That would be closer to "fail with explicit error when where to
push is not clear enough".

> On the one hand, I think what you are suggesting is reasonable in most
> cases. On the other hand, what if the lack of upstream is because the
> user failed to configure it properly? Then it could be surprising.
>
> I don't have a strong opinion either way.

No strong opinion either, but I wanted to raise the point to make sure
we agree.

With your patch, "git push" fails with

  fatal: The current branch branch-name has no upstream branch.
  To push the current branch and set the remote as upstream, use
  
      git push --set-upstream origin branch-name

so it's not really bad: the suggestion guides the user to a situation
where the next "git push" will succeed unambiguously. As a side effect,
the next "git pull" will fetch from the same branch, which is probably
what the user wants if he hasn't explicitely configured an upstream
branch yet.

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

Re: push.default: current vs upstream

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Sat, Apr 07, 2012 at 09:40:26AM +0100, Andrew Sayers wrote:
>
>> On a slight aside, should we add @{downstream} to describe the opposite
>> of @{upstream}?  Seeing that around the place would give intermediate
>> users a clue about why pull and push aren't as related as they think,
>> and would be useful here and there in code (e.g. __git_ps1 could show a
>> better bash prompt with GIT_PS1_SHOWUPSTREAM).
>
> Maybe. I don't really see how it is useful, but maybe you want to flesh
> our your proposal with some examples? I do not use __git_ps1, so I'm not
> sure what you want to improve there.

I took "downstream" as an opposite of "upstream".

The "upstream" of your branch is often (but not always) a remote tracking
branch, and because it makes sense to only have zero or one (but not more)
branch.$name.merge, "$name@{upstream}" would mean something.  There is an
N-to-1 mapping from branches to their upstreams.

Given a remote tracking branch $name, (or if you use "upstream" to fork
your branch off of another of your branches, it could be a local branch),
there can be many branches that call it an "upstream", which means that
the notation "$name@{downstream}" cannot even map to a single object name
or a refname; it is a 1-to-N mapping.

So I thought this was a joke and not a serious proposal.

Re: push.default: current vs upstream

[Junio C Hamano]

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

> Jeff King <peff@peff.net> writes:
>
>> Then the rule is not really "act only if upstream and current would do
>> the same thing".
>
> Right. That would be closer to "fail with explicit error when where to
> push is not clear enough".

I think that is a good explanation.

>> On the one hand, I think what you are suggesting is reasonable in most
>> cases. On the other hand, what if the lack of upstream is because the
>> user failed to configure it properly? Then it could be surprising.
>>
>> I don't have a strong opinion either way.
>
> No strong opinion either, but I wanted to raise the point to make sure
> we agree.
>
> With your patch, "git push" fails with
>
>   fatal: The current branch branch-name has no upstream branch.
>   To push the current branch and set the remote as upstream, use
>   
>       git push --set-upstream origin branch-name
>
> so it's not really bad: the suggestion guides the user to a situation
> where the next "git push" will succeed unambiguously. As a side effect,
> the next "git pull" will fetch from the same branch, which is probably
> what the user wants if he hasn't explicitely configured an upstream
> branch yet.

Sounds sensible.

Re: push.default: current vs upstream

[Andrew Sayers]

On 12/04/12 08:11, Jeff King wrote:
> On Sat, Apr 07, 2012 at 09:40:26AM +0100, Andrew Sayers wrote:
> 
>> On a slight aside, should we add @{downstream} to describe the opposite
>> of @{upstream}?  Seeing that around the place would give intermediate
>> users a clue about why pull and push aren't as related as they think,
>> and would be useful here and there in code (e.g. __git_ps1 could show a
>> better bash prompt with GIT_PS1_SHOWUPSTREAM).
> 
> Maybe. I don't really see how it is useful, but maybe you want to flesh
> our your proposal with some examples? I do not use __git_ps1, so I'm not
> sure what you want to improve there.

This discussion has highlighted the fact that a lot of people (myself
included) had difficulty with the idea that push and pull could be
asymmetrical operations.  Speaking for myself,
push/pull/upstream/downstream/etc. has always been one of those things
in my peripheral vision that worked well enough that I never really
thought about it.  I think this is a fairly important bug in the
documentation, which I guess didn't come across when I suggested a
solution that was mostly about code.

I could be wrong, but looking over this discussion suggests to me a
pattern where people who actually thought about this stuff understood it
pretty quickly, whereas those of us who hadn't previously thought about
it had to unlearn the vague notion that "upstream" was somehow both the
place you push to and the place you pull from, despite being
incompatible with the meaning of the word.

I described this before as being "in my peripheral vision", which is as
different from being in central vision (like it is for you guys) as it
is different from being out of vision altogether (like it is for new
people).  Peripheral processing is a notoriously troubled route for
ideas, because people tend to pick up on cues and internalise them in
irrational ways without noticing.  A classic version control example is
how nobody ever said "commits are expensive and should be used
cautiously", but everyone just sort of came to that conclusion without
really thinking about it.  As soon as DVCSs forced us to actually focus
on the problem and think about it rationally, it became obvious that we
needed to unlearn the vague assumption and go with a model that made
more sense.

I'm pretty sure the cue I internalised to make me think "push" and
"pull" were symmetrical was that the word "upstream" gets thrown around
a lot in the documentation, whereas nobody ever uses the word
"downstream".  Having heard "upstream this" and "upstream that" every
time I opened a man page, the word "upstream" just naturally popped into
my head when I wondered where `git push` went.  Just to be clear - I'm
not arguing that anybody that consciously asked themselves the question
"where does `git push` go?" would rationally conclude the answer is
"upstream", I'm saying that those of us who never really thought about
it had a mental process bubbling away deep in some murky bit of our
brain which plugged the gap in our understanding with the only word it knew.

So if the problem is that the documentation cues the reader to think
about upstreams but not to think about downstreams, the solution is to
find excuses to talk more about downstreams.  As far as I'm concerned
@{upstream} means "the place that commits come from when I `git pull`",
so it makes perfect sense to me that @{downstream} would mean "the place
commits go to when I `git push`".  That would let us write documentation
like the following:

    Git has been optimised to make branching and merging very easy.
    Most git workflows involve using an upstream branch (that you pull
    other people's work from), your current branch (that you commit
    your changes to) and a downstream branch (that you push the
    combined work to).  To understand the this a bit better, try using
    these commands:

    git log # commits in your local repository
    git log @{upstream} # commits in your upstream branch
    git log @{downstream} # commits in your downstream branch
    git log HEAD..@{upstream} # commits you haven't pulled yet
    git log @{downstream}..HEAD # commits you haven't pushed yet

    A lot of workflows simplify the process by assuming that your
    upstream is also your downstream, and that both exist in some
    shared repository, but as far as git's concerned this is just one
    special case.

I realise the above is a bit simplistic and would need to be written
better, but hopefully it demonstrates how code support for "downstream"
terminology would let us write documentation that provides better cues
about how the whole process works.

I mentioned __git_ps1 before as an example of where @{downstream} would
be useful in code.  The idea is that you can set your prompt to include
e.g. "+3-2" if you have 3 commits to push and 2 commits to pull, which
probably tells you lies right now when your push target happens not to
be the same as your upstream.

	- Andrew

Re: push.default: current vs upstream

[Junio C Hamano]

Andrew Sayers <andrew-git@pileofstuff.org> writes:

> So if the problem is that the documentation cues the reader to think
> about upstreams but not to think about downstreams, the solution is to
> find excuses to talk more about downstreams.  As far as I'm concerned
> @{upstream} means "the place that commits come from when I `git pull`",
> so it makes perfect sense to me that @{downstream} would mean "the place
> commits go to when I `git push`".

In a separate message I completely misunderstood what you meant by
"downstream".

If you had something like this:

	[remote "origin"]
        	url = ...
        [remote "destination"]
                pushURL = ...

	[branch "topic"]
        	remote = origin
                merge = refs/heads/master
		pushRemote = destination # new
                push = refs/heads/topic # new

you could express that asymmetric layout in a natural way.  When you say
"git push" while on your "topic" branch, it will go to "destination"
remote to update their "topic" branch.

Interesting...

Re: push.default: current vs upstream

[Jeff King]

On Thu, Apr 12, 2012 at 02:33:58PM -0700, Junio C Hamano wrote:

> Andrew Sayers <andrew-git@pileofstuff.org> writes:
> 
> > So if the problem is that the documentation cues the reader to think
> > about upstreams but not to think about downstreams, the solution is to
> > find excuses to talk more about downstreams.  As far as I'm concerned
> > @{upstream} means "the place that commits come from when I `git pull`",
> > so it makes perfect sense to me that @{downstream} would mean "the place
> > commits go to when I `git push`".
> 
> In a separate message I completely misunderstood what you meant by
> "downstream".

Yeah, I also took it to mean that the "downstream" of your "upstream"
would be where you started (though as you mentioned, it is not 1-to-1,
so that would not work anyway).

But this:

> If you had something like this:
> 
> 	[remote "origin"]
>         	url = ...
>         [remote "destination"]
>                 pushURL = ...
> 
> 	[branch "topic"]
>         	remote = origin
>                 merge = refs/heads/master
> 		pushRemote = destination # new
>                 push = refs/heads/topic # new
> 
> you could express that asymmetric layout in a natural way.  When you say
> "git push" while on your "topic" branch, it will go to "destination"
> remote to update their "topic" branch.

is much more useful (and I already complained about the lack of
something like pushRemote recently). I just think it should not be
called "downstream", as it is not the reverse of upstream.

-Peff

Re: push.default: current vs upstream

[Philip Oakley]

From: "Jeff King" <peff@peff.net> Sent: Thursday, April 12, 2012 11:11 PM
> On Thu, Apr 12, 2012 at 02:33:58PM -0700, Junio C Hamano wrote:
>
>> Andrew Sayers <andrew-git@pileofstuff.org> writes:
>>
>> > So if the problem is that the documentation cues the reader to think
>> > about upstreams but not to think about downstreams, the solution is to
>> > find excuses to talk more about downstreams.  As far as I'm concerned
>> > @{upstream} means "the place that commits come from when I `git pull`",
>> > so it makes perfect sense to me that @{downstream} would mean "the
>> > place
>> > commits go to when I `git push`".

>> In a separate message I completely misunderstood what you meant by
>> "downstream".
>

It would be useful to have "upstream" and "downstream" clarified in the 
documentation, say the Workflows man pages or some suitable place. Upstream 
is used often but isn't well defined (no obvious link anyway) - it's more of 
concept than a place (hence Andrew's option), while downstream is hardly 
mentioned at all.

For the push.default option can I suggest a different approach focused on
the beginner? If both current and upstream are potentially problematic, then
surely the default initially should be unset, so that a beginner's push is
refused with a pleasant message, e.g.
"  git push.default is unset,
   please use 'git push <remote> <branch>', or see its man page;
  or set the push.default to match your work style."

This offers the beginner a cause, an immediate solution, and guidance for
the long term. I deliberately called it a work style rather than workflow.

One work style for push.default could be "beginner" which would reduce the
message to:
-  git push.default is unset, use 'git push <remote> <branch>'

This gives the beginner time to learn their workflow, and gives a friendly 
version of the command.

While the expert user's regression would be to select _their_
current/upstream work style.

This approach does take the opposite route to most of the default settings
as it is about stopping beginners making major mistakes, rather than helping
them become productive.

One issue I had with the 'git push' man page is that 'refspec's are a fairly 
advanced (at least intermediate) concept that would confuse the beginner who 
is still getting to grips with their branches and remotes, so using the 'git 
push <remote> <branch>' version [*1*] will be friendlier to those beginners.

Philip

> Yeah, I also took it to mean that the "downstream" of your "upstream"
> would be where you started (though as you mentioned, it is not 1-to-1,
> so that would not work anyway).
>
> But this:
>
>> If you had something like this:
>>
>> [remote "origin"]
>>         url = ...
>>         [remote "destination"]
>>                 pushURL = ...
>>
>> [branch "topic"]
>>         remote = origin
>>                 merge = refs/heads/master
>> pushRemote = destination # new
>>                 push = refs/heads/topic # new
>>
>> you could express that asymmetric layout in a natural way.  When you say
>> "git push" while on your "topic" branch, it will go to "destination"
>> remote to update their "topic" branch.
>
> is much more useful (and I already complained about the lack of
> something like pushRemote recently). I just think it should not be
> called "downstream", as it is not the reverse of upstream.
>
> -Peff
> --
[1] http://gitready.com/beginner/2009/01/21/pushing-and-pulling.html 

Re: push.default: current vs upstream

[Junio C Hamano]

"Philip Oakley" <philipoakley@iee.org> writes:

> From: "Jeff King" <peff@peff.net> Sent: Thursday, April 12, 2012 11:11 PM
>> On Thu, Apr 12, 2012 at 02:33:58PM -0700, Junio C Hamano wrote:
>>
>>> Andrew Sayers <andrew-git@pileofstuff.org> writes:
>>>
>>> > So if the problem is that the documentation cues the reader to think
>>> > about upstreams but not to think about downstreams, the solution is to
>>> > find excuses to talk more about downstreams.  As far as I'm concerned
>>> > @{upstream} means "the place that commits come from when I `git pull`",
>>> > so it makes perfect sense to me that @{downstream} would mean "the
>>> > place
>>> > commits go to when I `git push`".
>
>>> In a separate message I completely misunderstood what you meant by
>>> "downstream".
>
> It would be useful to have "upstream" and "downstream" clarified in
> the documentation, say the Workflows man pages or some suitable
> place.

Perhaps, but I am not convinced.  I am not convinced that it is a bad idea
either, so I'll think aloud for several paragraphs, and probably will not
reach a conclusion in this message. Just a food for thought...

I think the word "downstream" has rarely been used in the context of Git,
but because it is a natural opposite for the word "upstream", I've seen it
used when people talk about others who fork from them, e.g. Linus can call
his lieutenants his downstream.

The word "upstream" almost always refers to "the place the updates from
others come to me from".  Linus is the upstream for his lieutenants---the
lieutenants pull from Linus.  In the shared repository "everybody pulls
from there to get updates from others, and everybody pushes there to
propagate their own work to others" setting, the shared repository is the
upstream for the project participants---again, they pull from there.

Notice however that the word "upstream" is meaningless for the integrator
with the above definition of the word.  "The place the updates from others
come to Linus from" ought to be his "upstream", but the workflow does not
go like so. Linus does not have a fixed "upstream" he pulls from before
he starts his day. For that matter, the lieutenants are not supposed to
pull from Linus every day before they start their work, either, but when
they need to synchronize with the upstream, they pull from Linus, so in
that sense, the word "upstream" means something to them.

I suspect that using the word "downstream" to mean "the place the result
of my work is pushed to" will only add to the confusion.

In a distributed "kernel-like" workflow, the lieutenants obviously do not
push to Linus's repository. If we raise the level of discussion to talk
about the flows of data, ignoring the difference of mechanism used
(i.e. "push" vs "format-patch | send-email"), the work by lieutenants is
still fed back to their "upstream".  They "upstream" (verb) the result of
their work.

Perhaps it was a mistake to use the word "upstream" in a shared repository
workflow, which invites the confusing word "downstream". In that context,
there is not really an "up" vs "down" relationship.  "shared" vs "mine"
relationship is all that exists in that context.

Re: push.default: current vs upstream

[Andrew Sayers]

On 12/04/12 22:33, Junio C Hamano wrote:
> Andrew Sayers <andrew-git@pileofstuff.org> writes:
> 
>> So if the problem is that the documentation cues the reader to think
>> about upstreams but not to think about downstreams, the solution is to
>> find excuses to talk more about downstreams.  As far as I'm concerned
>> @{upstream} means "the place that commits come from when I `git pull`",
>> so it makes perfect sense to me that @{downstream} would mean "the place
>> commits go to when I `git push`".
> 
> In a separate message I completely misunderstood what you meant by
> "downstream".
> 
> If you had something like this:
> 
> 	[remote "origin"]
>         	url = ...
>         [remote "destination"]
>                 pushURL = ...
> 
> 	[branch "topic"]
>         	remote = origin
>                 merge = refs/heads/master
> 		pushRemote = destination # new
>                 push = refs/heads/topic # new
> 
> you could express that asymmetric layout in a natural way.  When you say
> "git push" while on your "topic" branch, it will go to "destination"
> remote to update their "topic" branch.
> 
> Interesting...
> 

If "upstream" and "downstream" are ambiguous, it makes sense to avoid
the words altogether - you can't reduce ambiguity by adding more
definitions.  I'm replying to the above instead of your most recent
message because it might provide a fresh angle on the problem.

We already have language to describe "upstream" unambiguously, and an
intuitive way of describing the opposite of it, so specifying revisions
with the same language would help tie some things together and make git
easier to learn.  The only problem  is that we currently use two words
instead of one.

We could talk about @{remote}/@{pushRemote}, which I'd expect to be
intuitive to a beginner despite the inaccuracy - we already talk about
"remote repositories" and "remote tracking branches", so @{remote} would
be easy to guess.  Alternatively, @{merge}/@{push} would a bit more
accurate but a bit less obvious to people that don't really understand
merging yet (or are figuring out merge vs. rebase).  Or we could make up
some similar-but-distinct terms like @{mergeSource} and @{pushSink} that
reinforce the language above without overlapping.

	- Andrew

Re: push.default: current vs upstream

[Junio C Hamano]

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

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Jeff King <peff@peff.net> writes:
>> ...
>>> I don't have a strong opinion either way.
>>
>> No strong opinion either, but I wanted to raise the point to make sure
>> we agree.
>>
>> With your patch, "git push" fails with
>>
>>   fatal: The current branch branch-name has no upstream branch.
>>   To push the current branch and set the remote as upstream, use
>>   
>>       git push --set-upstream origin branch-name
>>
>> so it's not really bad: the suggestion guides the user to a situation
>> where the next "git push" will succeed unambiguously. As a side effect,
>> the next "git pull" will fetch from the same branch, which is probably
>> what the user wants if he hasn't explicitely configured an upstream
>> branch yet.
>
> Sounds sensible.

So what happened to this discussion?  Does anybody want to roll the "simple"
default based on Peff's patch?

Re: push.default: current vs upstream

[Matthieu Moy]

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

> So what happened to this discussion?  

I've been busy, so did nothing, and my procrastination didn't pay ;-).

> Does anybody want to roll the "simple" default based on Peff's patch?

I was about to, actually. I'll go for Peff's semantics at least for now,
and just add tests.

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

Re: push.default: current vs upstream

[Junio C Hamano]

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

> Junio C Hamano <gitster@pobox.com> writes:
>
>> So what happened to this discussion?  
>
> I've been busy, so did nothing, and my procrastination didn't pay ;-).

Heh, it rarely pays to procrastinate when it is clear that you are the
party who cares the most deeply about the issue among the ones capable
of changing the status quo.

As long as it is not forgotten, that is fine, and being busy is good ;-)

>> Does anybody want to roll the "simple" default based on Peff's patch?
>
> I was about to, actually. I'll go for Peff's semantics at least for now,
> and just add tests.

Thanks.

[RFC/PATCH 0/3] push.default upcomming change

[Matthieu Moy]

So, here's my first re-roll of Peff's patch. I've included the warning
change in the same patch serie because topics are related and to avoid
conflicts.

Compared to Peff's version, I've added testcases, but the 'simple'
mode still lacks doc (I will do soon).

Clemens Buchacher (1):
  t5570: use explicit push refspec

Matthieu Moy (2):
  push: introduce new push.default mode "simple"
  push: start warning upcoming default change for push.default

 builtin/push.c        |   72 +++++++++++++++++++++++++++++++++++++++++++++++--
 cache.h               |    4 ++-
 config.c              |    4 ++-
 environment.c         |    2 +-
 t/t5516-fetch-push.sh |   33 +++++++++++++++++++++++
 t/t5570-git-daemon.sh |   30 ++++++++++-----------
 6 files changed, 124 insertions(+), 21 deletions(-)

-- 
1.7.10.140.g8c333

[PATCH 1/3] push: introduce new push.default mode "simple"

[Matthieu Moy]

When calling "git push" without argument, we want to allow Git to do
something simple to explain and safe. push.default=matching is unsafe
when use to push to shared repositories, and hard to explain to beginners
in some context. It is debatable whether 'upstream' or 'current' is the
safest or the easiest to explain, so introduce a new mode called 'simple'
that is the intersection of them: push the upstream branch, but only if
it has the same name remotely. If not, give an error that suggest the
right command to push explicitely to 'upstream' or 'current'.

A question is whether to allow pushing when no upstream is configured. An
argument in favor of allowing the push is that it makes the new mode work
in more cases. On the other hand, refusing to push when no upstream is
configured encourages the user to set the upstream, which will be
beneficial on the next pull. Lacking better argument, we chose to deny
the push, because it will be easier to change in the future is someone
shows us wrong.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 builtin/push.c        |   47 +++++++++++++++++++++++++++++++++++++++++++++--
 cache.h               |    1 +
 config.c              |    4 +++-
 t/t5516-fetch-push.sh |   33 +++++++++++++++++++++++++++++++++
 4 files changed, 82 insertions(+), 3 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index d315475..4602cd8 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -65,7 +65,17 @@ static void set_refspecs(const char **refs, int nr)
 	}
 }
 
-static void setup_push_upstream(struct remote *remote)
+static int push_url_of_remote(struct remote *remote, const char ***url_p)
+{
+	if (remote->pushurl_nr) {
+		*url_p = remote->pushurl;
+		return remote->pushurl_nr;
+	}
+	*url_p = remote->url;
+	return remote->url_nr;
+}
+
+static void setup_push_upstream(struct remote *remote, int simple)
 {
 	struct strbuf refspec = STRBUF_INIT;
 	struct branch *branch = branch_get(NULL);
@@ -87,6 +97,35 @@ static void setup_push_upstream(struct remote *remote)
 	if (branch->merge_nr != 1)
 		die(_("The current branch %s has multiple upstream branches, "
 		    "refusing to push."), branch->name);
+	if (strcmp(branch->remote_name, remote->name))
+		die(_("You are pushing to remote '%s', which is not the upstream of\n"
+		      "your current branch '%s', without telling me what to push\n"
+		      "to update which remote branch."),
+		    remote->name, branch->name);
+
+	if (simple && strcmp(branch->refname, branch->merge[0]->src)) {
+		/*
+		 * There's no point in using shorten_unambiguous_ref here,
+		 * as the ambiguity would be on the remote side, not what
+		 * we have locally. Plus, this is supposed to be the simple
+		 * mode. If the user is doing something crazy like setting
+		 * upstream to a non-branch, we should probably be showing
+		 * them the big ugly fully qualified ref.
+		 */
+		const char *short_up = skip_prefix(branch->merge[0]->src, "refs/heads/");
+		die(_("The upstream branch of your current branch does not match\n"
+		      "the name of your current branch.  To push to the upstream branch\n"
+		      "on the remote, use\n"
+		      "\n"
+		      "    git push %s HEAD:%s\n"
+		      "\n"
+		      "To push to the branch of the same name on the remote, use\n"
+		      "\n"
+		      "    git push %s %s\n"),
+		    remote->name, short_up ? short_up : branch->merge[0]->src,
+		    remote->name, branch->name);
+	}
+
 	strbuf_addf(&refspec, "%s:%s", branch->name, branch->merge[0]->src);
 	add_refspec(refspec.buf);
 }
@@ -99,8 +138,12 @@ static void setup_default_push_refspecs(struct remote *remote)
 		add_refspec(":");
 		break;
 
+	case PUSH_DEFAULT_SIMPLE:
+		setup_push_upstream(remote, 1);
+		break;
+
 	case PUSH_DEFAULT_UPSTREAM:
-		setup_push_upstream(remote);
+		setup_push_upstream(remote, 0);
 		break;
 
 	case PUSH_DEFAULT_CURRENT:
diff --git a/cache.h b/cache.h
index 806bf2b..f1c1bb8 100644
--- a/cache.h
+++ b/cache.h
@@ -624,6 +624,7 @@ enum rebase_setup_type {
 enum push_default_type {
 	PUSH_DEFAULT_NOTHING = 0,
 	PUSH_DEFAULT_MATCHING,
+	PUSH_DEFAULT_SIMPLE,
 	PUSH_DEFAULT_UPSTREAM,
 	PUSH_DEFAULT_CURRENT
 };
diff --git a/config.c b/config.c
index 68d3294..024bc74 100644
--- a/config.c
+++ b/config.c
@@ -829,6 +829,8 @@ static int git_default_push_config(const char *var, const char *value)
 			push_default = PUSH_DEFAULT_NOTHING;
 		else if (!strcmp(value, "matching"))
 			push_default = PUSH_DEFAULT_MATCHING;
+		else if (!strcmp(value, "simple"))
+			push_default = PUSH_DEFAULT_SIMPLE;
 		else if (!strcmp(value, "upstream"))
 			push_default = PUSH_DEFAULT_UPSTREAM;
 		else if (!strcmp(value, "tracking")) /* deprecated */
@@ -837,7 +839,7 @@ static int git_default_push_config(const char *var, const char *value)
 			push_default = PUSH_DEFAULT_CURRENT;
 		else {
 			error("Malformed value for %s: %s", var, value);
-			return error("Must be one of nothing, matching, "
+			return error("Must be one of simple, nothing, matching, "
 				     "tracking or current.");
 		}
 		return 0;
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index b5417cc..f4f9d06 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -497,6 +497,39 @@ test_expect_success 'push with config remote.*.push = HEAD' '
 	check_push_result $the_first_commit heads/local
 '
 
+test_expect_success 'push without argument (push.default)' '
+	mk_test heads/master &&
+
+	test_debug "echo no remote branch" &&
+	git checkout $the_first_commit -b new-branch &&
+	test_must_fail git -c push.default=simple push &&
+	test_must_fail git -c push.default=matching push &&
+	test_must_fail git -c push.default=upstream push &&
+
+	test_debug "echo existing branch, no upstream configured" &&
+	git config branch.new-branch.remote there &&
+	git -c push.default=current push &&
+	git -c push.default=simple push &&
+	git -c push.default=matching push &&
+	git config --unset branch.new-branch.remote &&
+	test_must_fail git -c push.default=simple push &&
+
+	test_debug "echo upstream configured" &&
+	git push --set-upstream testrepo new-branch &&
+	git -c push.default=simple push &&
+	check_push_result $the_first_commit heads/new-branch &&
+	git config branch.new-branch.merge other-branch &&
+	test_must_fail git -c push.default=simple push &&
+	git -c push.default=upstream push &&
+	check_push_result $the_first_commit heads/other-branch &&
+	git push --set-upstream there new-branch &&
+
+	test_debug "echo advance local commit" &&
+	git merge $the_commit &&
+	git -c push.default=simple push &&
+	check_push_result $the_commit heads/new-branch
+'
+
 # clean up the cruft left with the previous one
 git config --remove-section remote.there
 git config --remove-section branch.master
-- 
1.7.10.140.g8c333

[PATCH 2/3] t5570: use explicit push refspec

[Matthieu Moy]

From: Clemens Buchacher <drizzd@aon.at>

The default mode for push without arguments will change. Some warnings
are about to be enabled for such use, which causes some t5570 tests to
fail because they do not expect this output.

Fix this by passing an explicit refspec to git push. To that end, change
the calling conventions of test_remote_error in order to accomodate
extra command arguments.

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5570-git-daemon.sh |   30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh
index 7cbc999..a3a4e47 100755
--- a/t/t5570-git-daemon.sh
+++ b/t/t5570-git-daemon.sh
@@ -103,14 +103,12 @@ test_remote_error()
 		esac
 	done
 
-	if test $# -ne 3
-	then
-		error "invalid number of arguments"
-	fi
-
+	msg=$1
+	shift
 	cmd=$1
-	repo=$2
-	msg=$3
+	shift
+	repo=$1
+	shift || error "invalid number of arguments"
 
 	if test -x "$GIT_DAEMON_DOCUMENT_ROOT_PATH/$repo"
 	then
@@ -122,7 +120,7 @@ test_remote_error()
 		fi
 	fi
 
-	test_must_fail git "$cmd" "$GIT_DAEMON_URL/$repo" 2>output &&
+	test_must_fail git "$cmd" "$GIT_DAEMON_URL/$repo" "$@" 2>output &&
 	echo "fatal: remote error: $msg: /$repo" >expect &&
 	test_cmp expect output
 	ret=$?
@@ -131,18 +129,18 @@ test_remote_error()
 }
 
 msg="access denied or repository not exported"
-test_expect_success 'clone non-existent' "test_remote_error    clone nowhere.git '$msg'"
-test_expect_success 'push disabled'      "test_remote_error    push  repo.git    '$msg'"
-test_expect_success 'read access denied' "test_remote_error -x fetch repo.git    '$msg'"
-test_expect_success 'not exported'       "test_remote_error -n fetch repo.git    '$msg'"
+test_expect_success 'clone non-existent' "test_remote_error    '$msg' clone nowhere.git    "
+test_expect_success 'push disabled'      "test_remote_error    '$msg' push  repo.git master"
+test_expect_success 'read access denied' "test_remote_error -x '$msg' fetch repo.git       "
+test_expect_success 'not exported'       "test_remote_error -n '$msg' fetch repo.git       "
 
 stop_git_daemon
 start_git_daemon --informative-errors
 
-test_expect_success 'clone non-existent' "test_remote_error    clone nowhere.git 'no such repository'"
-test_expect_success 'push disabled'      "test_remote_error    push  repo.git    'service not enabled'"
-test_expect_success 'read access denied' "test_remote_error -x fetch repo.git    'no such repository'"
-test_expect_success 'not exported'       "test_remote_error -n fetch repo.git    'repository not exported'"
+test_expect_success 'clone non-existent' "test_remote_error    'no such repository'      clone nowhere.git    "
+test_expect_success 'push disabled'      "test_remote_error    'service not enabled'     push  repo.git master"
+test_expect_success 'read access denied' "test_remote_error -x 'no such repository'      fetch repo.git       "
+test_expect_success 'not exported'       "test_remote_error -n 'repository not exported' fetch repo.git       "
 
 stop_git_daemon
 test_done
-- 
1.7.10.140.g8c333

[PATCH 3/3] push: start warning upcoming default change for push.default

[Matthieu Moy]

In preparation for flipping the default to the "simple" mode from
the "matching" mode that is the historical default, start warning
users when they rely on unconfigured "git push" to default to the
"matching" mode.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/push.c |   25 +++++++++++++++++++++++++
 cache.h        |    3 ++-
 environment.c  |    2 +-
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 4602cd8..3bb8ce7 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -130,10 +130,35 @@ static void setup_push_upstream(struct remote *remote, int simple)
 	add_refspec(refspec.buf);
 }
 
+static char warn_unspecified_push_default_msg[] =
+N_("push.default is unset; its implicit value is changing in\n"
+   "Git 2.0 from 'matching' to 'simple'. To squelch this message\n"
+   "and maintain the current behavior after the default changes, use:\n"
+   "\n"
+   "  git config --global push.default matching\n"
+   "\n"
+   "To squelch this message and adopt the new behavior now, use:\n"
+   "\n"
+   "  git config --global push.default simple\n"
+   "\n"
+   "See 'git help config' and search for 'push.default' for further information.");
+
+static void warn_unspecified_push_default_configuration(void)
+{
+	static int warn_once;
+
+	if (warn_once++)
+		return;
+	warning("%s\n", _(warn_unspecified_push_default_msg));
+}
+
 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 f1c1bb8..851a673 100644
--- a/cache.h
+++ b/cache.h
@@ -626,7 +626,8 @@ enum push_default_type {
 	PUSH_DEFAULT_MATCHING,
 	PUSH_DEFAULT_SIMPLE,
 	PUSH_DEFAULT_UPSTREAM,
-	PUSH_DEFAULT_CURRENT
+	PUSH_DEFAULT_CURRENT,
+	PUSH_DEFAULT_UNSPECIFIED
 };
 
 extern enum branch_track git_branch_track;
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
-- 
1.7.10.140.g8c333

Re: [PATCH 1/3] push: introduce new push.default mode "simple"

[Jeff King]

On Fri, Apr 20, 2012 at 12:57:12AM +0200, Matthieu Moy wrote:

> diff --git a/builtin/push.c b/builtin/push.c
> index d315475..4602cd8 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -65,7 +65,17 @@ static void set_refspecs(const char **refs, int nr)
>  	}
>  }
>  
> -static void setup_push_upstream(struct remote *remote)
> +static int push_url_of_remote(struct remote *remote, const char ***url_p)
> +{
> +	if (remote->pushurl_nr) {
> +		*url_p = remote->pushurl;
> +		return remote->pushurl_nr;
> +	}
> +	*url_p = remote->url;
> +	return remote->url_nr;
> +}
> +

Eh, what's this? This wasn't part of my patch. It was part of Junio's
patch which mine was based on (and it provokes a "defined but not used"
warning when your patch is applied on top of master).

> @@ -87,6 +97,35 @@ static void setup_push_upstream(struct remote *remote)
>  	if (branch->merge_nr != 1)
>  		die(_("The current branch %s has multiple upstream branches, "
>  		    "refusing to push."), branch->name);
> +	if (strcmp(branch->remote_name, remote->name))
> +		die(_("You are pushing to remote '%s', which is not the upstream of\n"
> +		      "your current branch '%s', without telling me what to push\n"
> +		      "to update which remote branch."),
> +		    remote->name, branch->name);

And this was from Junio's patch, which is really a separate topic (that
"git push foo" should not respect an upstream branch name when the
upstream remote is not "foo").

So I think your rebase turned out a little funny. We probably want to
pull in Junio's patch from the tip of jc/push-upstream-sanity (135dade).
Though even that can be pared down a little. The push_url_of_remote
refactoring was part of an early iteration and does not have to be part
of the final version (though I think it is a fine refactoring on its
own).

> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
> index b5417cc..f4f9d06 100755
> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh

135dade creates a new t5528 for testing push.default settings, so tests
could go there.

-Peff

Re: [PATCH 1/3] push: introduce new push.default mode "simple"

[Matthieu Moy]

Jeff King <peff@peff.net> writes:

> Eh, what's this? This wasn't part of my patch. It was part of Junio's
> patch which mine was based on (and it provokes a "defined but not used"
> warning when your patch is applied on top of master).

Oops, it seems I did some weird conflict resolution applying and
rebasing your patch, plus I sent the serie late in the evening ;-).

> 135dade creates a new t5528 for testing push.default settings, so tests
> could go there.

Excellent. Next re-roll will be based on next to use it.

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

[PATCH 0/4 v2] push.default upcomming change

[Matthieu Moy]

OK, so this v2 is not supposed to be a draft anymore. It has
documentation (while I was there, I added PATCH 1/4 thas tries to
document better the existing modes), and I removed the hunks that came
here after a broken merge resolution.

This is based on next, it at least requires 135dade that creates
t5528.

Clemens Buchacher (1):
  t5570: use explicit push refspec

Matthieu Moy (3):
  Documentation: explain push.default option a bit more
  push: introduce new push.default mode "simple"
  push: start warning upcoming default change for push.default

 Documentation/config.txt |   20 +++++++++++++--
 builtin/push.c           |   55 ++++++++++++++++++++++++++++++++++++++--
 cache.h                  |    1 +
 config.c                 |    4 ++-
 t/t5528-push-default.sh  |   63 +++++++++++++++++++++++++++++++++++++++++++---
 t/t5570-git-daemon.sh    |   30 +++++++++++-----------
 6 files changed, 149 insertions(+), 24 deletions(-)

-- 
1.7.10.140.g8c333

[PATCH 1/4] Documentation: explain push.default option a bit more

[Matthieu Moy]

The previous documentation was explaining _what_ the options were doing,
but were of little help explaining _why_ a user should set his default to
either of the options.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/config.txt |   15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index fb386ab..368a770 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1682,10 +1682,21 @@ push.default::
 * `nothing` - do not push anything.
 * `matching` - push all matching branches.
   All branches having the same name in both ends are considered to be
-  matching. This is the default.
-* `upstream` - push the current branch to its upstream branch.
+  matching. This is well suited when pushing to a non-shared
+  repository, but may give surprising results when used on a
+  repository shared by multiple users, since locally stalled
+  branches will attempt a non-fast forward push if other users
+  updated the branch remotely. This is the default.
+* `upstream` - push the current branch to its upstream branch. See
+  "branch.<name>.merge" for how to configure the upstream branch. This
+  makes `git push` and `git pull` symmetrical in the sense that `push`
+  will update the same remote ref as the one which is merged by
+  `git pull`.
 * `tracking` - deprecated synonym for `upstream`.
 * `current` - push the current branch to a branch of the same name.
+  This option allows publishing a branch to a remote repository using
+  the same naming convention locally and remotely, in a more
+  conservative and safer way than `matching`.
 
 rebase.stat::
 	Whether to show a diffstat of what changed upstream since the last
-- 
1.7.10.140.g8c333

[PATCH 2/4] push: introduce new push.default mode "simple"

[Matthieu Moy]

When calling "git push" without argument, we want to allow Git to do
something simple to explain and safe. push.default=matching is unsafe
when use to push to shared repositories, and hard to explain to beginners
in some context. It is debatable whether 'upstream' or 'current' is the
safest or the easiest to explain, so introduce a new mode called 'simple'
that is the intersection of them: push the upstream branch, but only if
it has the same name remotely. If not, give an error that suggest the
right command to push explicitely to 'upstream' or 'current'.

A question is whether to allow pushing when no upstream is configured. An
argument in favor of allowing the push is that it makes the new mode work
in more cases. On the other hand, refusing to push when no upstream is
configured encourages the user to set the upstream, which will be
beneficial on the next pull. Lacking better argument, we chose to deny
the push, because it will be easier to change in the future is someone
shows us wrong.

Original-patch-by: Jeff King <peff@peff.net>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Except for the broken-ness, this adds the last line in the warning message:

"To chose either option permanently, read about push.default in git-config(1)"

 Documentation/config.txt |    3 +++
 builtin/push.c           |   32 +++++++++++++++++++++--
 cache.h                  |    1 +
 config.c                 |    4 ++-
 t/t5528-push-default.sh  |   63 +++++++++++++++++++++++++++++++++++++++++++---
 5 files changed, 97 insertions(+), 6 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 368a770..05d1472 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1697,6 +1697,9 @@ push.default::
   This option allows publishing a branch to a remote repository using
   the same naming convention locally and remotely, in a more
   conservative and safer way than `matching`.
+* `simple` - like `upstream`, but refuses to push if the upstream
+  branch's name is different from the local one. This is the safest
+  option and is well-suited for beginners.
 
 rebase.stat::
 	Whether to show a diffstat of what changed upstream since the last
diff --git a/builtin/push.c b/builtin/push.c
index 6936713..ba0d6a0 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -76,7 +76,7 @@ static int push_url_of_remote(struct remote *remote, const char ***url_p)
 	return remote->url_nr;
 }
 
-static void setup_push_upstream(struct remote *remote)
+static void setup_push_upstream(struct remote *remote, int simple)
 {
 	struct strbuf refspec = STRBUF_INIT;
 	struct branch *branch = branch_get(NULL);
@@ -103,6 +103,30 @@ static void setup_push_upstream(struct remote *remote)
 		      "your current branch '%s', without telling me what to push\n"
 		      "to update which remote branch."),
 		    remote->name, branch->name);
+	if (simple && strcmp(branch->refname, branch->merge[0]->src)) {
+		/*
+		 * There's no point in using shorten_unambiguous_ref here,
+		 * as the ambiguity would be on the remote side, not what
+		 * we have locally. Plus, this is supposed to be the simple
+		 * mode. If the user is doing something crazy like setting
+		 * upstream to a non-branch, we should probably be showing
+		 * them the big ugly fully qualified ref.
+		 */
+		const char *short_up = skip_prefix(branch->merge[0]->src, "refs/heads/");
+		die(_("The upstream branch of your current branch does not match\n"
+		      "the name of your current branch.  To push to the upstream branch\n"
+		      "on the remote, use\n"
+		      "\n"
+		      "    git push %s HEAD:%s\n"
+		      "\n"
+		      "To push to the branch of the same name on the remote, use\n"
+		      "\n"
+		      "    git push %s %s\n"
+		      "\n"
+		      "To chose either option permanently, read about push.default in git-config(1)\n"),
+		    remote->name, short_up ? short_up : branch->merge[0]->src,
+		    remote->name, branch->name);
+	}
 
 	strbuf_addf(&refspec, "%s:%s", branch->name, branch->merge[0]->src);
 	add_refspec(refspec.buf);
@@ -119,8 +143,12 @@ static void setup_default_push_refspecs(struct remote *remote)
 		add_refspec(":");
 		break;
 
+	case PUSH_DEFAULT_SIMPLE:
+		setup_push_upstream(remote, 1);
+		break;
+
 	case PUSH_DEFAULT_UPSTREAM:
-		setup_push_upstream(remote);
+		setup_push_upstream(remote, 0);
 		break;
 
 	case PUSH_DEFAULT_CURRENT:
diff --git a/cache.h b/cache.h
index d8f6f1e..5e419a1 100644
--- a/cache.h
+++ b/cache.h
@@ -580,6 +580,7 @@ enum rebase_setup_type {
 enum push_default_type {
 	PUSH_DEFAULT_NOTHING = 0,
 	PUSH_DEFAULT_MATCHING,
+	PUSH_DEFAULT_SIMPLE,
 	PUSH_DEFAULT_UPSTREAM,
 	PUSH_DEFAULT_CURRENT,
 	PUSH_DEFAULT_UNSPECIFIED
diff --git a/config.c b/config.c
index 68d3294..024bc74 100644
--- a/config.c
+++ b/config.c
@@ -829,6 +829,8 @@ static int git_default_push_config(const char *var, const char *value)
 			push_default = PUSH_DEFAULT_NOTHING;
 		else if (!strcmp(value, "matching"))
 			push_default = PUSH_DEFAULT_MATCHING;
+		else if (!strcmp(value, "simple"))
+			push_default = PUSH_DEFAULT_SIMPLE;
 		else if (!strcmp(value, "upstream"))
 			push_default = PUSH_DEFAULT_UPSTREAM;
 		else if (!strcmp(value, "tracking")) /* deprecated */
@@ -837,7 +839,7 @@ static int git_default_push_config(const char *var, const char *value)
 			push_default = PUSH_DEFAULT_CURRENT;
 		else {
 			error("Malformed value for %s: %s", var, value);
-			return error("Must be one of nothing, matching, "
+			return error("Must be one of simple, nothing, matching, "
 				     "tracking or current.");
 		}
 		return 0;
diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
index c334c51..949dbdf 100755
--- a/t/t5528-push-default.sh
+++ b/t/t5528-push-default.sh
@@ -13,6 +13,22 @@ test_expect_success 'setup bare remotes' '
 	git push parent2 HEAD
 '
 
+# $1 = local revision
+# $2 = remote repository
+# $3 = remote revision (tested to be equal to the local one)
+check_pushed_commit () {
+	git rev-parse "$1" > expect &&
+	git --git-dir="$2" rev-parse "$3" > actual &&
+	test_cmp expect actual
+}
+
+# $1 = push.default value
+# $2 = expected target branch for the push
+test_push_success () {
+	git -c push.default="$1" push &&
+	check_pushed_commit HEAD repo1 "$2"
+}
+
 test_expect_success '"upstream" pushes to configured upstream' '
 	git checkout master &&
 	test_config branch.master.remote parent1 &&
@@ -20,9 +36,7 @@ test_expect_success '"upstream" pushes to configured upstream' '
 	test_config push.default upstream &&
 	test_commit two &&
 	git push &&
-	echo two >expect &&
-	git --git-dir=repo1 log -1 --format=%s foo >actual &&
-	test_cmp expect actual
+	check_pushed_commit HEAD repo1 foo
 '
 
 test_expect_success '"upstream" does not push on unconfigured remote' '
@@ -51,4 +65,47 @@ test_expect_success '"upstream" does not push when remotes do not match' '
 	test_must_fail git push parent2
 '
 
+test_expect_success 'push from/to new branch with upstream, matching and simple' '
+	git checkout -b new-branch &&
+	test_must_fail git -c push.default=simple push &&
+	test_must_fail git -c push.default=matching push &&
+	test_must_fail git -c push.default=upstream push
+'
+
+test_expect_success 'push from/to new branch with current creates remote branch' '
+	test_config branch.new-branch.remote repo1 &&
+	git checkout new-branch &&
+	test_push_success current new-branch
+'
+
+test_expect_success 'push to existing branch, with no upstream configured' '
+	test_config branch.master.remote repo1 &&
+	git checkout master &&
+	test_must_fail git -c push.default=simple push &&
+	test_must_fail git -c push.default=upstream push
+'
+
+test_expect_success 'push to existing branch, upstream configured with same name' '
+	test_config branch.master.remote repo1 &&
+	test_config branch.master.merge refs/heads/master &&
+	git checkout master &&
+	test_commit six &&
+	test_push_success upstream master &&
+	test_commit seven &&
+	test_push_success simple master &&
+	check_pushed_commit HEAD repo1 master
+'
+
+test_expect_success 'push to existing branch, upstream configured with different name' '
+	test_config branch.master.remote repo1 &&
+	test_config branch.master.merge refs/heads/other-name &&
+	git checkout master &&
+	test_commit eight &&
+	test_push_success upstream other-name &&
+	test_commit nine &&
+	test_must_fail git -c push.default=simple push &&
+	test_push_success current master &&
+	test_must_fail check_pushed_commit HEAD repo1 other-name
+'
+
 test_done
-- 
1.7.10.140.g8c333

[PATCH 3/4] t5570: use explicit push refspec

[Matthieu Moy]

From: Clemens Buchacher <drizzd@aon.at>

The default mode for push without arguments will change. Some warnings
are about to be enabled for such use, which causes some t5570 tests to
fail because they do not expect this output.

Fix this by passing an explicit refspec to git push. To that end, change
the calling conventions of test_remote_error in order to accomodate
extra command arguments.

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5570-git-daemon.sh |   30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh
index 7cbc999..a3a4e47 100755
--- a/t/t5570-git-daemon.sh
+++ b/t/t5570-git-daemon.sh
@@ -103,14 +103,12 @@ test_remote_error()
 		esac
 	done
 
-	if test $# -ne 3
-	then
-		error "invalid number of arguments"
-	fi
-
+	msg=$1
+	shift
 	cmd=$1
-	repo=$2
-	msg=$3
+	shift
+	repo=$1
+	shift || error "invalid number of arguments"
 
 	if test -x "$GIT_DAEMON_DOCUMENT_ROOT_PATH/$repo"
 	then
@@ -122,7 +120,7 @@ test_remote_error()
 		fi
 	fi
 
-	test_must_fail git "$cmd" "$GIT_DAEMON_URL/$repo" 2>output &&
+	test_must_fail git "$cmd" "$GIT_DAEMON_URL/$repo" "$@" 2>output &&
 	echo "fatal: remote error: $msg: /$repo" >expect &&
 	test_cmp expect output
 	ret=$?
@@ -131,18 +129,18 @@ test_remote_error()
 }
 
 msg="access denied or repository not exported"
-test_expect_success 'clone non-existent' "test_remote_error    clone nowhere.git '$msg'"
-test_expect_success 'push disabled'      "test_remote_error    push  repo.git    '$msg'"
-test_expect_success 'read access denied' "test_remote_error -x fetch repo.git    '$msg'"
-test_expect_success 'not exported'       "test_remote_error -n fetch repo.git    '$msg'"
+test_expect_success 'clone non-existent' "test_remote_error    '$msg' clone nowhere.git    "
+test_expect_success 'push disabled'      "test_remote_error    '$msg' push  repo.git master"
+test_expect_success 'read access denied' "test_remote_error -x '$msg' fetch repo.git       "
+test_expect_success 'not exported'       "test_remote_error -n '$msg' fetch repo.git       "
 
 stop_git_daemon
 start_git_daemon --informative-errors
 
-test_expect_success 'clone non-existent' "test_remote_error    clone nowhere.git 'no such repository'"
-test_expect_success 'push disabled'      "test_remote_error    push  repo.git    'service not enabled'"
-test_expect_success 'read access denied' "test_remote_error -x fetch repo.git    'no such repository'"
-test_expect_success 'not exported'       "test_remote_error -n fetch repo.git    'repository not exported'"
+test_expect_success 'clone non-existent' "test_remote_error    'no such repository'      clone nowhere.git    "
+test_expect_success 'push disabled'      "test_remote_error    'service not enabled'     push  repo.git master"
+test_expect_success 'read access denied' "test_remote_error -x 'no such repository'      fetch repo.git       "
+test_expect_success 'not exported'       "test_remote_error -n 'repository not exported' fetch repo.git       "
 
 stop_git_daemon
 test_done
-- 
1.7.10.140.g8c333

[PATCH 4/4] push: start warning upcoming default change for push.default

[Matthieu Moy]

In preparation for flipping the default to the "simple" mode from
the "matching" mode that is the historical default, start warning
users when they rely on unconfigured "git push" to default to the
"matching" mode.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
I've documented the upcomming change in Documentation/config.txt in
addition to the warning.

 Documentation/config.txt |    6 ++++--
 builtin/push.c           |   23 +++++++++++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 05d1472..bda3f47 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1686,7 +1686,8 @@ push.default::
   repository, but may give surprising results when used on a
   repository shared by multiple users, since locally stalled
   branches will attempt a non-fast forward push if other users
-  updated the branch remotely. This is the default.
+  updated the branch remotely. This is currently the default, but Git
+  2.0 will change the default to `simple`.
 * `upstream` - push the current branch to its upstream branch. See
   "branch.<name>.merge" for how to configure the upstream branch. This
   makes `git push` and `git pull` symmetrical in the sense that `push`
@@ -1699,7 +1700,8 @@ push.default::
   conservative and safer way than `matching`.
 * `simple` - like `upstream`, but refuses to push if the upstream
   branch's name is different from the local one. This is the safest
-  option and is well-suited for beginners.
+  option and is well-suited for beginners. It will become the default
+  in Git 2.0.
 
 rebase.stat::
 	Whether to show a diffstat of what changed upstream since the last
diff --git a/builtin/push.c b/builtin/push.c
index ba0d6a0..23cedf0 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -132,12 +132,35 @@ static void setup_push_upstream(struct remote *remote, int simple)
 	add_refspec(refspec.buf);
 }
 
+static char warn_unspecified_push_default_msg[] =
+N_("push.default is unset; its implicit value is changing in\n"
+   "Git 2.0 from 'matching' to 'simple'. To squelch this message\n"
+   "and maintain the current behavior after the default changes, use:\n"
+   "\n"
+   "  git config --global push.default matching\n"
+   "\n"
+   "To squelch this message and adopt the new behavior now, use:\n"
+   "\n"
+   "  git config --global push.default simple\n"
+   "\n"
+   "See 'git help config' and search for 'push.default' for further information.");
+
+static void warn_unspecified_push_default_configuration(void)
+{
+	static int warn_once;
+
+	if (warn_once++)
+		return;
+	warning("%s\n", _(warn_unspecified_push_default_msg));
+}
+
 static void setup_default_push_refspecs(struct remote *remote)
 {
 	switch (push_default) {
 	default:
 	case PUSH_DEFAULT_UNSPECIFIED:
 		default_matching_used = 1;
+		warn_unspecified_push_default_configuration();
 		/* fallthru */
 	case PUSH_DEFAULT_MATCHING:
 		add_refspec(":");
-- 
1.7.10.140.g8c333

Re: [PATCH 1/4] Documentation: explain push.default option a bit more

[Jeff King]

On Fri, Apr 20, 2012 at 04:59:01PM +0200, Matthieu Moy wrote:

> The previous documentation was explaining _what_ the options were doing,
> but were of little help explaining _why_ a user should set his default to
> either of the options.

I think your explanations are a definite improvement.

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index fb386ab..368a770 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
>[...]
>  * `tracking` - deprecated synonym for `upstream`.

This is not directly related to your patch, but maybe it is worth
removing this (from the documentation) for the sake of simplicity. We
will still support the synonym (it has only been deprecated for a year),
but new users don't need to see it in the already-large list of options.

-Peff

Re: [PATCH 2/4] push: introduce new push.default mode "simple"

[Jeff King]

On Fri, Apr 20, 2012 at 04:59:02PM +0200, Matthieu Moy wrote:

> it has the same name remotely. If not, give an error that suggest the
> right command to push explicitely to 'upstream' or 'current'.

s/suggest/&s/

> beneficial on the next pull. Lacking better argument, we chose to deny
> the push, because it will be easier to change in the future is someone
> shows us wrong.

s/is/if/

> Original-patch-by: Jeff King <peff@peff.net>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
> Except for the broken-ness, this adds the last line in the warning message:
> 
> "To chose either option permanently, read about push.default in git-config(1)"

I don't think that makes sense if I have set "push.default" to "simple"
myself. IOW, shouldn't that get added later, when it eventually becomes
the default (and then, only when it was chosen because it is the
default, not because somebody explicitly said they wanted it)?

> @@ -837,7 +839,7 @@ static int git_default_push_config(const char *var, const char *value)
>  			push_default = PUSH_DEFAULT_CURRENT;
>  		else {
>  			error("Malformed value for %s: %s", var, value);
> -			return error("Must be one of nothing, matching, "
> +			return error("Must be one of simple, nothing, matching, "
>  				     "tracking or current.");

Not your fault, but should this be s/tracking/upstream/ in the context
line?

> diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
> index c334c51..949dbdf 100755
> --- a/t/t5528-push-default.sh
> +++ b/t/t5528-push-default.sh
> @@ -13,6 +13,22 @@ test_expect_success 'setup bare remotes' '
>  	git push parent2 HEAD
>  '
>  
> +# $1 = local revision
> +# $2 = remote repository
> +# $3 = remote revision (tested to be equal to the local one)
> +check_pushed_commit () {
> +	git rev-parse "$1" > expect &&
> +	git --git-dir="$2" rev-parse "$3" > actual &&
> +	test_cmp expect actual
> +}

This is an extremely minor nit, but a test failure is often easier to
read if you use "log -1 --format=%s" here, assuming that the commits are
given reasonable subjects (so you get "-two\n+one" or similar instead of
some commit ids which aren't meaningful).

Also, I notice you take a repo argument here, but then all of the
callers just pass "repo1" (and the test_push_success function hardcodes
repo1).

> +test_expect_success 'push to existing branch, upstream configured with different name' '
> +	test_config branch.master.remote repo1 &&
> +	test_config branch.master.merge refs/heads/other-name &&
> +	git checkout master &&
> +	test_commit eight &&
> +	test_push_success upstream other-name &&
> +	test_commit nine &&
> +	test_must_fail git -c push.default=simple push &&
> +	test_push_success current master &&
> +	test_must_fail check_pushed_commit HEAD repo1 other-name
> +'

In the final must_fail, wouldn't it be more robust to use an affirmative
check that it was not touched, instead of checking that we failed to
find it to be equal (which could fail for other, unrelated reasons)?
Like this:

  check_pushed_commit HEAD^ repo1 other-name

I also wonder if it would make sense to wrap the whole "failed push"
thing in a function like this:

  test_push_failure() {
          git --git-dir=repo1 log -1 --format=%s "$2" >expect &&
          test_must_fail git -c push.default="$1" &&
          git --git-dir=repo1 log -1 --format=%s "$2" >actual &&
          test_cmp expect actual
  }

and then the earlier failures could also get this extra double-check for
free (that push not only reported failure, but that it failed without
actually touching the remote end, not for some unrelated reason).

That's all minor stuff; the bulk of the patch looks good to me.

-Peff

Re: [PATCH 0/4 v2] push.default upcomming change

[Jeff King]

On Fri, Apr 20, 2012 at 04:59:00PM +0200, Matthieu Moy wrote:

> OK, so this v2 is not supposed to be a draft anymore. It has
> documentation (while I was there, I added PATCH 1/4 thas tries to
> document better the existing modes), and I removed the hunks that came
> here after a broken merge resolution.
> 
> This is based on next, it at least requires 135dade that creates
> t5528.

You are better off to just build it on 135dade in that case, as that is
what Junio will apply it on (never directly on top of next). It
generally isn't a problem, but there's no point reason not to do so.

> Clemens Buchacher (1):
>   t5570: use explicit push refspec
> 
> Matthieu Moy (3):
>   Documentation: explain push.default option a bit more
>   push: introduce new push.default mode "simple"
>   push: start warning upcoming default change for push.default

I commented separately on patches 1 and 2, but modulo those minor
comments, the series looks OK to me. The "Git 2.0" mention in patch 4/4
might need to be tweaked. :)

-Peff

Re: [PATCH 1/4] Documentation: explain push.default option a bit more

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Fri, Apr 20, 2012 at 04:59:01PM +0200, Matthieu Moy wrote:
>
>> The previous documentation was explaining _what_ the options were doing,
>> but were of little help explaining _why_ a user should set his default to
>> either of the options.
>
> I think your explanations are a definite improvement.

<aol>Me too</aol>

Except for this part.

>  * `current` - push the current branch to a branch of the same name.
> +  This option allows publishing a branch to a remote repository using
> +  the same naming convention locally and remotely, in a more
> +  conservative and safer way than `matching`.

I am not sure if "in a more conservative and safer way than `matching`"
is a good thing to say here.  It sounds as if you are saying "you push
only one, so even if it does what you did not intend to do, you inflict
damage to at most one branch" and without mentioning the downside of
"you push only one, so you need to checkout and push out all the
branches one by one that you care about".

The workflow 'current', 'upstream' and 'simple' are suitable for is
fundamentally different from what 'matching' aims to support.

The former three are for people who want: "I've completed this single
branch, the one I have had in my working tree and have been working on
for all this time. I'll push *that* single branch out. All the other
branches do not participate in this push".

The 'matching' is for people who want: "I've worked on the set of
branches I want to publish until they are _all_ ready.  And now they
are.  Publish all of them with a single connection, atomically, in one
go."

Your text that compares between 'current' and 'matching' does not make
it clear that point.  In a workflow for which 'current' is suitable,
'matching' is not even "a more aggressive and riskier" alternative.  The
text does not make the reader aware of that, and will invite "current is
a more cumbersome and tedious alternative if you want to push out all
than matching", which is a faulty conclusion coming from the same
confusion.

Perhaps make this part a separate paragraph so that it stands out a bit
more, like this,

    * `current` - push the current branch to a branch of the same name.
    +
    The `current` and `upstream` modes are for those who want to
    push out a single branch after finishing work, even when the other
    branches are not yet ready to be pushed out.

and update the description for `matching` to explain why it is suited
for what we claim that it is suited for, like so:

    * `matching` - push all branches having the same name in both ends.
      This is for those who prepare all the branches into a publishable
      shape and push them out atomically, and suitable when pushing to a
      non-shared repository. It is not appropriate to use when pushing into
      a repository shared by multiple users, since locally stalled branches
      will attempt a non-fast forward push if other users updated the branch
      remotely.
    +
    This is the default.

Re: [PATCH 2/4] push: introduce new push.default mode "simple"

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> Except for the broken-ness, this adds the last line in the warning message:

Hmm?  What brokenness?

> +		      "To chose either option permanently, read about push.default in git-config(1)\n"),

Nice ;-)

> diff --git a/cache.h b/cache.h
> index d8f6f1e..5e419a1 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -580,6 +580,7 @@ enum rebase_setup_type {
>  enum push_default_type {
>  	PUSH_DEFAULT_NOTHING = 0,
>  	PUSH_DEFAULT_MATCHING,
> +	PUSH_DEFAULT_SIMPLE,
>  	PUSH_DEFAULT_UPSTREAM,
>  	PUSH_DEFAULT_CURRENT,
>  	PUSH_DEFAULT_UNSPECIFIED

I think SIMPLE should come between CURRENT and UPSTREAM in the order of
logical progression, i.e. CURRENT < SIMPLE < UPSTREAM, because CURRENT
is appropriate in the simplest of workflow (you clone and get master and
devel branches, work on master and push it out to master, work on devel
and push it out to devel, without any need for @{upstream}), SIMPLE is a
bit more advanced (you can take advantage of @{upstream}), and CURRENT
would be the most advanced, in the "one branch at a time" camp.

> diff --git a/config.c b/config.c
> index 68d3294..024bc74 100644
> --- a/config.c
> +++ b/config.c
> @@ -837,7 +839,7 @@ static int git_default_push_config(const char *var, const char *value)
>  			push_default = PUSH_DEFAULT_CURRENT;
>  		else {
>  			error("Malformed value for %s: %s", var, value);
> -			return error("Must be one of nothing, matching, "
> +			return error("Must be one of simple, nothing, matching, "
>  				     "tracking or current.");

And this should match.  I think

	nothing, matching, current, simple or upstream.

would be more natural.

Re: [PATCH 1/4] Documentation: explain push.default option a bit more

[Michael Haggerty]

On 04/20/2012 11:28 PM, Junio C Hamano wrote:
> [...]
> Perhaps make this part a separate paragraph so that it stands out a bit
> more, like this,
>
>      * `current` - push the current branch to a branch of the same name.
>      +
>      The `current` and `upstream` modes are for those who want to
>      push out a single branch after finishing work, even when the other
>      branches are not yet ready to be pushed out.
>
> and update the description for `matching` to explain why it is suited
> for what we claim that it is suited for, like so:
>
>      * `matching` - push all branches having the same name in both ends.
>        This is for those who prepare all the branches into a publishable
>        shape and push them out atomically, and suitable when pushing to a
>        non-shared repository. It is not appropriate to use when pushing into
>        a repository shared by multiple users, since locally stalled branches
>        will attempt a non-fast forward push if other users updated the branch
>        remotely.
>      +
>      This is the default.

"Atomic" implies that either the whole push succeeds or the whole push 
fails, and that readers will never see part of the push.  Is full 
atomicity really guaranteed?  Does the guarantee depend on the 
repository being non-shared?  What if the repo has one writer but 
multiple readers?

I have trouble answering questions like these because I haven't figured 
out the big picture of git's locking model.  In the code, all I have 
seen so far is fine-grained locks that only block writers (though the 
fast-index GSoC proposal includes a plan to introduce a lock on the 
index that blocks readers).  Is git's locking model documented somewhere?

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: [PATCH 1/4] Documentation: explain push.default option a bit more

[Junio C Hamano]

Michael Haggerty <mhagger@alum.mit.edu> writes:

> "Atomic" implies that either the whole push succeeds or the whole push
> fails, and that readers will never see part of the push.

Oh, I didn't mean "atomic" in that strict sense.  After all this was a
description at the workflow level--what the human user perceives.

When pushing into the repository you control and nobody else messes
with, you may want update both 'master' and 'devel' with the same "git
push", and that is quite different from current/upstream/simple
model. That is all I meant.

At the mechanical level, we:

 - read all the refs we are going to update and remember their values;
 - send and store all the necessary objects; and
 - for each ref:
    - lock it;
    - read it;
    - is it different from what we read originally?
      - if so, do not update it and remember the fact that we saw a failure;
      - otherwise update it;
    - unlock it;

so it won't be the kind of "atomic" in the "if 'master' will fail to
update due to non-fast-forward, not just 'master' but also 'devel' is
not updated" sense.


Also, if you happen to observe 'devel' and 'master' when a push is in
progress, you may get lucky and see the new value of 'devel' and old
value of 'master'. In that sense, too, it is not "atomic", either.

In the workflow where 'matching' is appropriate, the former won't be an
issue. The latter might be, but it is not like you push objects for
devel, update devel, then push objects for master and the update master,
so the window of race is very small.

Re: [PATCH 1/4] Documentation: explain push.default option a bit more

[Michael Haggerty]

On 04/21/2012 06:08 AM, Junio C Hamano wrote:
> Michael Haggerty<mhagger@alum.mit.edu>  writes:
>> "Atomic" implies that either the whole push succeeds or the whole push
>> fails, and that readers will never see part of the push.
>
> Oh, I didn't mean "atomic" in that strict sense.  After all this was a
> description at the workflow level--what the human user perceives.

That's what I suspected.

Given that the word "atomic", for technical people, has a strict meaning 
that is not met here, and for non-technical people probably only means 
"nuclear", I suggest that the word be avoided in this explanation.  Perhaps

>      * `matching` - push all branches having the same name in both ends.
>        This is for those who prepare all the branches into a publishable
>        shape and push them out atomically, and suitable when pushing to a
>        non-shared repository. [...]

could be changed to

>      * `matching` - push all branches having the same name in both ends.
>        This allows those who prepare all the branches into a publishable
>        shape to push them out to a non-shared repository with a single
 >        command. [...]

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: [PATCH 1/4] Documentation: explain push.default option a bit more

[Junio C Hamano]

Michael Haggerty <mhagger@alum.mit.edu> writes:

> only means "nuclear", I suggest that the word be avoided in this
> explanation.  Perhaps
>
>>      * `matching` - push all branches having the same name in both ends.
>>        This is for those who prepare all the branches into a publishable
>>        shape and push them out atomically, and suitable when pushing to a
>>        non-shared repository. [...]
>
> could be changed to
>
>>      * `matching` - push all branches having the same name in both ends.
>>        This allows those who prepare all the branches into a publishable
>>        shape to push them out to a non-shared repository with a single
>>        command. [...]

Sounds good.

Re: [PATCH 0/4 v2] push.default upcomming change

[Matthieu Moy]

Jeff King <peff@peff.net> writes:

> The "Git 2.0" mention in patch 4/4 might need to be tweaked. :)

The "Git 2.0" comes from Junio. I don't mind changing that to "Git 1.9.0
or Git 2.0, whichever is released earlier".

Actually, I'm starting to think that the "warning" part should be
delayed a bit: we probably want to wait for the "simple" implementation
to be available widely before we start advising people to set it
explicitely (otherwise, people using different Git versions on different
machines, but sharing the same configuration will get complaints from
Git about invalid value).

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

Re: [PATCH 2/4] push: introduce new push.default mode "simple"

[Zbigniew Jędrzejewski-Szmek]

Some minor spelling fixes:

> When calling "git push" without argument, we want to allow Git to do
> something simple to explain and safe. push.default=matching is unsafe
> when use to push to shared repositories, and hard to explain to beginners
       used
> in some context. It is debatable whether 'upstream' or 'current' is the
       contexts
> safest or the easiest to explain, so introduce a new mode called 'simple'
> that is the intersection of them: push the upstream branch, but only if
                                    push to the
> it has the same name remotely. If not, give an error that suggest the
> right command to push explicitely to 'upstream' or 'current'.


On 04/20/2012 10:33 PM, Jeff King wrote:
> On Fri, Apr 20, 2012 at 04:59:02PM +0200, Matthieu Moy wrote:
> 
>> it has the same name remotely. If not, give an error that suggest the
>> right command to push explicitely to 'upstream' or 'current'.
> 
> s/suggest/&s/
> 
>> beneficial on the next pull. Lacking better argument, we chose to deny
>> the push, because it will be easier to change in the future is someone
>> shows us wrong.
> 
> s/is/if/
> 
>> Original-patch-by: Jeff King <peff@peff.net>
>> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>> ---
>> Except for the broken-ness, this adds the last line in the warning message:
>>
>> "To chose either option permanently, read about push.default in git-config(1)"
       choose

> I don't think that makes sense if I have set "push.default" to "simple"
> myself. IOW, shouldn't that get added later, when it eventually becomes
> the default (and then, only when it was chosen because it is the
> default, not because somebody explicitly said they wanted it)?

-
Zbyszek

Re: [PATCH 0/4 v2] push.default upcomming change

[Junio C Hamano]

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

> Actually, I'm starting to think that the "warning" part should be
> delayed a bit: we probably want to wait for the "simple" implementation
> to be available widely before we start advising people to set it
> explicitely (otherwise, people using different Git versions on different
> machines, but sharing the same configuration will get complaints from
> Git about invalid value).

Yes, that is a good thinking.

[PATCH 0/7 v3] push.default upcomming change

[Matthieu Moy]

I think I've applied all feedback I've received.

Additionnally, I've split the "start warning" patch in a documentation
patch, that is meant to be applied now, and the warning patch itself,
which is meant to be applied after the first released version
introducing 'simple': we don't want to advertize 'simple' too loudly
until the option starts being deployed.

Clemens Buchacher (1):
  t5570: use explicit push refspec

Matthieu Moy (6):
  Documentation: explain push.default option a bit more
  Undocument deprecated alias 'push.default=tracking'
  t5528-push-default.sh: add helper functions
  push: introduce new push.default mode "simple"
  push: document the future default change for push.default (matching
    -> simple)
  push: start warning upcoming default change for push.default

 Documentation/config.txt |   28 ++++++++++++---
 builtin/push.c           |   71 +++++++++++++++++++++++++++++++++---
 cache.h                  |    1 +
 config.c                 |    6 ++--
 t/t5528-push-default.sh  |   89 ++++++++++++++++++++++++++++++++++++++++++----
 t/t5570-git-daemon.sh    |   30 ++++++++--------
 6 files changed, 191 insertions(+), 34 deletions(-)

-- 
1.7.10.234.ge65dd.dirty

[PATCH 1/7] Documentation: explain push.default option a bit more

[Matthieu Moy]

The previous documentation was explaining _what_ the options were doing,
but were of little help explaining _why_ a user should set his default to
either of the options.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/config.txt |   22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index fb386ab..e38fab1 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1680,12 +1680,26 @@ push.default::
 	line. Possible values are:
 +
 * `nothing` - do not push anything.
-* `matching` - push all matching branches.
-  All branches having the same name in both ends are considered to be
-  matching. This is the default.
-* `upstream` - push the current branch to its upstream branch.
+* `matching` - push all branches having the same name in both ends.
+  This allows those who prepare all the branches into a publishable
+  shape to push them out to a non-shared repository with a single
+  command. This is well suited when pushing to a non-shared
+  repository, but may give surprising results when used on a
+  repository shared by multiple users, since locally stalled
+  branches will attempt a non-fast forward push if other users
+  updated the branch remotely. This is the default.
+* `upstream` - push the current branch to its upstream branch. See
+  "branch.<name>.merge" for how to configure the upstream branch. This
+  makes `git push` and `git pull` symmetrical in the sense that `push`
+  will update the same remote ref as the one which is merged by
+  `git pull`.
 * `tracking` - deprecated synonym for `upstream`.
 * `current` - push the current branch to a branch of the same name.
+  +
+  The `current` and `upstream` modes are for those who want to
+  push out a single branch after finishing work, even when the other
+  branches are not yet ready to be pushed out. They are safe when
+  pushing to a shared repository.
 
 rebase.stat::
 	Whether to show a diffstat of what changed upstream since the last
-- 
1.7.10.234.ge65dd.dirty

[PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Matthieu Moy]

It's been deprecated since 53c4031 (Johan Herland, Wed Feb 16 2011,
push.default: Rename 'tracking' to 'upstream'), so it's OK to remove it
from documentation (even though it's still supported) to make the
explanations more readable.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Feel free to squash into previous one if needed.

 Documentation/config.txt |    1 -
 1 file changed, 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index e38fab1..ddf6043 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1693,7 +1693,6 @@ push.default::
   makes `git push` and `git pull` symmetrical in the sense that `push`
   will update the same remote ref as the one which is merged by
   `git pull`.
-* `tracking` - deprecated synonym for `upstream`.
 * `current` - push the current branch to a branch of the same name.
   +
   The `current` and `upstream` modes are for those who want to
-- 
1.7.10.234.ge65dd.dirty

[PATCH 3/7] t5528-push-default.sh: add helper functions

[Matthieu Moy]

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 t/t5528-push-default.sh |   45 ++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 38 insertions(+), 7 deletions(-)

diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
index c334c51..da7d3d8 100755
--- a/t/t5528-push-default.sh
+++ b/t/t5528-push-default.sh
@@ -13,16 +13,47 @@ test_expect_success 'setup bare remotes' '
 	git push parent2 HEAD
 '
 
+# $1 = local revision
+# $2 = remote revision (tested to be equal to the local one)
+check_pushed_commit () {
+	git log -1 --format='%h %s' >expect &&
+	git --git-dir=repo1 log -1 --format='%h %s' "$2" >actual &&
+	test_cmp expect actual
+}
+
+# $1 = push.default value
+# $2 = expected target branch for the push
+test_push_success () {
+	git -c push.default="$1" push &&
+	check_pushed_commit HEAD "$2"
+}
+
+# $1 = push.default value
+# other arguments = target branches that should not be touched
+test_push_failure () {
+	push_default=$1 &&
+	shift &&
+	if test $# -gt 0
+	then
+		# branch may not exist
+		test_might_fail git --git-dir=repo1 \
+			log --no-walk --format='%h %s' "$@" >expect
+	fi &&
+	test_must_fail git -c push.default="$1" &&
+	if test $# -gt 0
+	then
+		test_might_fail git --git-dir=repo1 \
+			log -1 --format='%h %s' "$@" >actual
+	fi &&
+	test_cmp expect actual
+}
+
 test_expect_success '"upstream" pushes to configured upstream' '
 	git checkout master &&
 	test_config branch.master.remote parent1 &&
 	test_config branch.master.merge refs/heads/foo &&
-	test_config push.default upstream &&
 	test_commit two &&
-	git push &&
-	echo two >expect &&
-	git --git-dir=repo1 log -1 --format=%s foo >actual &&
-	test_cmp expect actual
+	test_push_success upstream foo
 '
 
 test_expect_success '"upstream" does not push on unconfigured remote' '
@@ -30,7 +61,7 @@ test_expect_success '"upstream" does not push on unconfigured remote' '
 	test_unconfig branch.master.remote &&
 	test_config push.default upstream &&
 	test_commit three &&
-	test_must_fail git push
+	test_push_failure upstream master
 '
 
 test_expect_success '"upstream" does not push on unconfigured branch' '
@@ -39,7 +70,7 @@ test_expect_success '"upstream" does not push on unconfigured branch' '
 	test_unconfig branch.master.merge &&
 	test_config push.default upstream
 	test_commit four &&
-	test_must_fail git push
+	test_push_failure upstream master
 '
 
 test_expect_success '"upstream" does not push when remotes do not match' '
-- 
1.7.10.234.ge65dd.dirty

[PATCH 4/7] push: introduce new push.default mode "simple"

[Matthieu Moy]

When calling "git push" without argument, we want to allow Git to do
something simple to explain and safe. push.default=matching is unsafe
when used to push to shared repositories, and hard to explain to
beginners in some contexts. It is debatable whether 'upstream' or
'current' is the safest or the easiest to explain, so introduce a new
mode called 'simple' that is the intersection of them: push to the
upstream branch, but only if it has the same name remotely. If not, give
an error that suggests the right command to push explicitely to
'upstream' or 'current'.

A question is whether to allow pushing when no upstream is configured. An
argument in favor of allowing the push is that it makes the new mode work
in more cases. On the other hand, refusing to push when no upstream is
configured encourages the user to set the upstream, which will be
beneficial on the next pull. Lacking better argument, we chose to deny
the push, because it will be easier to change in the future if someone
shows us wrong.

Original-patch-by: Jeff King <peff@peff.net>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/config.txt |    5 ++++-
 builtin/push.c           |   44 ++++++++++++++++++++++++++++++++++++++++++--
 cache.h                  |    1 +
 config.c                 |    6 ++++--
 t/t5528-push-default.sh  |   44 ++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 95 insertions(+), 5 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ddf6043..88d739a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1688,6 +1688,9 @@ push.default::
   repository shared by multiple users, since locally stalled
   branches will attempt a non-fast forward push if other users
   updated the branch remotely. This is the default.
+* `simple` - like `upstream`, but refuses to push if the upstream
+  branch's name is different from the local one. This is the safest
+  option and is well-suited for beginners.
 * `upstream` - push the current branch to its upstream branch. See
   "branch.<name>.merge" for how to configure the upstream branch. This
   makes `git push` and `git pull` symmetrical in the sense that `push`
@@ -1695,7 +1698,7 @@ push.default::
   `git pull`.
 * `current` - push the current branch to a branch of the same name.
   +
-  The `current` and `upstream` modes are for those who want to
+  The `simple`, `current` and `upstream` modes are for those who want to
   push out a single branch after finishing work, even when the other
   branches are not yet ready to be pushed out. They are safe when
   pushing to a shared repository.
diff --git a/builtin/push.c b/builtin/push.c
index 6936713..dae8306 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -76,7 +76,40 @@ static int push_url_of_remote(struct remote *remote, const char ***url_p)
 	return remote->url_nr;
 }
 
-static void setup_push_upstream(struct remote *remote)
+NORETURN die_push_simple(struct branch *branch, struct remote *remote) {
+	/*
+	 * There's no point in using shorten_unambiguous_ref here,
+	 * as the ambiguity would be on the remote side, not what
+	 * we have locally. Plus, this is supposed to be the simple
+	 * mode. If the user is doing something crazy like setting
+	 * upstream to a non-branch, we should probably be showing
+	 * them the big ugly fully qualified ref.
+	 */
+	const char *short_up = skip_prefix(branch->merge[0]->src, "refs/heads/");
+	/*
+	 * Don't show advice for people who explicitely set
+	 * push.default.
+	 */
+	const char *advice_maybe = "";
+	if (push_default == PUSH_DEFAULT_UNSPECIFIED)
+		advice_maybe = _("\n"
+				 "To choose either option permanently, "
+				 "see push.default in 'git help config'.");
+	die(_("The upstream branch of your current branch does not match\n"
+	      "the name of your current branch.  To push to the upstream branch\n"
+	      "on the remote, use\n"
+	      "\n"
+	      "    git push %s HEAD:%s\n"
+	      "\n"
+	      "To push to the branch of the same name on the remote, use\n"
+	      "\n"
+	      "    git push %s %s\n"
+	      "%s"),
+	    remote->name, short_up ? short_up : branch->merge[0]->src,
+	    remote->name, branch->name, advice_maybe);
+}
+
+static void setup_push_upstream(struct remote *remote, int simple)
 {
 	struct strbuf refspec = STRBUF_INIT;
 	struct branch *branch = branch_get(NULL);
@@ -103,6 +136,9 @@ static void setup_push_upstream(struct remote *remote)
 		      "your current branch '%s', without telling me what to push\n"
 		      "to update which remote branch."),
 		    remote->name, branch->name);
+	if (simple && strcmp(branch->refname, branch->merge[0]->src)) {
+		die_push_simple(branch, remote);
+	}
 
 	strbuf_addf(&refspec, "%s:%s", branch->name, branch->merge[0]->src);
 	add_refspec(refspec.buf);
@@ -119,8 +155,12 @@ static void setup_default_push_refspecs(struct remote *remote)
 		add_refspec(":");
 		break;
 
+	case PUSH_DEFAULT_SIMPLE:
+		setup_push_upstream(remote, 1);
+		break;
+
 	case PUSH_DEFAULT_UPSTREAM:
-		setup_push_upstream(remote);
+		setup_push_upstream(remote, 0);
 		break;
 
 	case PUSH_DEFAULT_CURRENT:
diff --git a/cache.h b/cache.h
index 5bf59ff..b60d490 100644
--- a/cache.h
+++ b/cache.h
@@ -624,6 +624,7 @@ enum rebase_setup_type {
 enum push_default_type {
 	PUSH_DEFAULT_NOTHING = 0,
 	PUSH_DEFAULT_MATCHING,
+	PUSH_DEFAULT_SIMPLE,
 	PUSH_DEFAULT_UPSTREAM,
 	PUSH_DEFAULT_CURRENT,
 	PUSH_DEFAULT_UNSPECIFIED
diff --git a/config.c b/config.c
index 68d3294..bfe0c79 100644
--- a/config.c
+++ b/config.c
@@ -829,6 +829,8 @@ static int git_default_push_config(const char *var, const char *value)
 			push_default = PUSH_DEFAULT_NOTHING;
 		else if (!strcmp(value, "matching"))
 			push_default = PUSH_DEFAULT_MATCHING;
+		else if (!strcmp(value, "simple"))
+			push_default = PUSH_DEFAULT_SIMPLE;
 		else if (!strcmp(value, "upstream"))
 			push_default = PUSH_DEFAULT_UPSTREAM;
 		else if (!strcmp(value, "tracking")) /* deprecated */
@@ -837,8 +839,8 @@ static int git_default_push_config(const char *var, const char *value)
 			push_default = PUSH_DEFAULT_CURRENT;
 		else {
 			error("Malformed value for %s: %s", var, value);
-			return error("Must be one of nothing, matching, "
-				     "tracking or current.");
+			return error("Must be one of nothing, matching, simple, "
+				     "upstream or current.");
 		}
 		return 0;
 	}
diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
index da7d3d8..43dec43 100755
--- a/t/t5528-push-default.sh
+++ b/t/t5528-push-default.sh
@@ -82,4 +82,48 @@ test_expect_success '"upstream" does not push when remotes do not match' '
 	test_must_fail git push parent2
 '
 
+test_expect_success 'push from/to new branch with upstream, matching and simple' '
+	git checkout -b new-branch &&
+	test_push_failure simple new-branch &&
+	test_push_failure matching new-branch &&
+	test_push_failure upstream new-branch
+'
+
+test_expect_success 'push from/to new branch with current creates remote branch' '
+	test_config branch.new-branch.remote repo1 &&
+	git checkout new-branch &&
+	test_push_success current new-branch
+'
+
+test_expect_success 'push to existing branch, with no upstream configured' '
+	test_config branch.master.remote repo1 &&
+	git checkout master &&
+	test_push_failure simple master &&
+	test_push_failure upstream master
+'
+
+test_expect_success 'push to existing branch, upstream configured with same name' '
+	test_config branch.master.remote repo1 &&
+	test_config branch.master.merge refs/heads/master &&
+	git checkout master &&
+	test_commit six &&
+	test_push_success upstream master &&
+	test_commit seven &&
+	test_push_success simple master
+'
+
+test_expect_success 'push to existing branch, upstream configured with different name' '
+	test_config branch.master.remote repo1 &&
+	test_config branch.master.merge refs/heads/other-name &&
+	git checkout master &&
+	test_commit eight &&
+	test_push_success upstream other-name &&
+	test_commit nine &&
+	test_push_failure simple new-branch &&
+	git --git-dir=repo1 log -1 --format="%h %s" "other-name" >expect-other-name &&
+	test_push_success current master &&
+	git --git-dir=repo1 log -1 --format="%h %s" "other-name" >actual-other-name &&
+	test_cmp expect-other-name actual-other-name
+'
+
 test_done
-- 
1.7.10.234.ge65dd.dirty

[PATCH 5/7] t5570: use explicit push refspec

[Matthieu Moy]

From: Clemens Buchacher <drizzd@aon.at>

The default mode for push without arguments will change. Some warnings
are about to be enabled for such use, which causes some t5570 tests to
fail because they do not expect this output.

Fix this by passing an explicit refspec to git push. To that end, change
the calling conventions of test_remote_error in order to accomodate
extra command arguments.

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 t/t5570-git-daemon.sh |   30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh
index 7cbc999..a3a4e47 100755
--- a/t/t5570-git-daemon.sh
+++ b/t/t5570-git-daemon.sh
@@ -103,14 +103,12 @@ test_remote_error()
 		esac
 	done
 
-	if test $# -ne 3
-	then
-		error "invalid number of arguments"
-	fi
-
+	msg=$1
+	shift
 	cmd=$1
-	repo=$2
-	msg=$3
+	shift
+	repo=$1
+	shift || error "invalid number of arguments"
 
 	if test -x "$GIT_DAEMON_DOCUMENT_ROOT_PATH/$repo"
 	then
@@ -122,7 +120,7 @@ test_remote_error()
 		fi
 	fi
 
-	test_must_fail git "$cmd" "$GIT_DAEMON_URL/$repo" 2>output &&
+	test_must_fail git "$cmd" "$GIT_DAEMON_URL/$repo" "$@" 2>output &&
 	echo "fatal: remote error: $msg: /$repo" >expect &&
 	test_cmp expect output
 	ret=$?
@@ -131,18 +129,18 @@ test_remote_error()
 }
 
 msg="access denied or repository not exported"
-test_expect_success 'clone non-existent' "test_remote_error    clone nowhere.git '$msg'"
-test_expect_success 'push disabled'      "test_remote_error    push  repo.git    '$msg'"
-test_expect_success 'read access denied' "test_remote_error -x fetch repo.git    '$msg'"
-test_expect_success 'not exported'       "test_remote_error -n fetch repo.git    '$msg'"
+test_expect_success 'clone non-existent' "test_remote_error    '$msg' clone nowhere.git    "
+test_expect_success 'push disabled'      "test_remote_error    '$msg' push  repo.git master"
+test_expect_success 'read access denied' "test_remote_error -x '$msg' fetch repo.git       "
+test_expect_success 'not exported'       "test_remote_error -n '$msg' fetch repo.git       "
 
 stop_git_daemon
 start_git_daemon --informative-errors
 
-test_expect_success 'clone non-existent' "test_remote_error    clone nowhere.git 'no such repository'"
-test_expect_success 'push disabled'      "test_remote_error    push  repo.git    'service not enabled'"
-test_expect_success 'read access denied' "test_remote_error -x fetch repo.git    'no such repository'"
-test_expect_success 'not exported'       "test_remote_error -n fetch repo.git    'repository not exported'"
+test_expect_success 'clone non-existent' "test_remote_error    'no such repository'      clone nowhere.git    "
+test_expect_success 'push disabled'      "test_remote_error    'service not enabled'     push  repo.git master"
+test_expect_success 'read access denied' "test_remote_error -x 'no such repository'      fetch repo.git       "
+test_expect_success 'not exported'       "test_remote_error -n 'repository not exported' fetch repo.git       "
 
 stop_git_daemon
 test_done
-- 
1.7.10.234.ge65dd.dirty

[PATCH 6/7] push: document the future default change for push.default (matching -> simple)

[Matthieu Moy]

It is too early to start warning loudly about the future default change
in favor of 'simple', since many users use different versions of Git, and
would be harmed if we advised them to explicitely set
'push.default=simple' when using old versions of Git.

Still, we want to document the upcomming change so that:

* Users who may be affected by the change get one more chance to know it
  in advance.

* We actually commit to changing the default, and avoid repeating past
  errors.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/config.txt |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 88d739a..696544e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1687,10 +1687,12 @@ push.default::
   repository, but may give surprising results when used on a
   repository shared by multiple users, since locally stalled
   branches will attempt a non-fast forward push if other users
-  updated the branch remotely. This is the default.
+  updated the branch remotely. This is currently the default, but Git
+  2.0 will change the default to `simple`.
 * `simple` - like `upstream`, but refuses to push if the upstream
   branch's name is different from the local one. This is the safest
-  option and is well-suited for beginners.
+  option and is well-suited for beginners. It will become the default
+  in Git 2.0.
 * `upstream` - push the current branch to its upstream branch. See
   "branch.<name>.merge" for how to configure the upstream branch. This
   makes `git push` and `git pull` symmetrical in the sense that `push`
-- 
1.7.10.234.ge65dd.dirty

[PATCH 7/7] push: start warning upcoming default change for push.default

[Matthieu Moy]

In preparation for flipping the default to the "simple" mode from
the "matching" mode that is the historical default, start warning
users when they rely on unconfigured "git push" to default to the
"matching" mode.

Also, advertise for 'simple' where 'current' and 'upstream' are advised.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/push.c |   27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index dae8306..7a845a8 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -144,12 +144,35 @@ static void setup_push_upstream(struct remote *remote, int simple)
 	add_refspec(refspec.buf);
 }
 
+static char warn_unspecified_push_default_msg[] =
+N_("push.default is unset; its implicit value is changing in\n"
+   "Git 2.0 from 'matching' to 'simple'. To squelch this message\n"
+   "and maintain the current behavior after the default changes, use:\n"
+   "\n"
+   "  git config --global push.default matching\n"
+   "\n"
+   "To squelch this message and adopt the new behavior now, use:\n"
+   "\n"
+   "  git config --global push.default simple\n"
+   "\n"
+   "See 'git help config' and search for 'push.default' for further information.");
+
+static void warn_unspecified_push_default_configuration(void)
+{
+	static int warn_once;
+
+	if (warn_once++)
+		return;
+	warning("%s\n", _(warn_unspecified_push_default_msg));
+}
+
 static void setup_default_push_refspecs(struct remote *remote)
 {
 	switch (push_default) {
 	default:
 	case PUSH_DEFAULT_UNSPECIFIED:
 		default_matching_used = 1;
+		warn_unspecified_push_default_configuration();
 		/* fallthru */
 	case PUSH_DEFAULT_MATCHING:
 		add_refspec(":");
@@ -183,8 +206,8 @@ static const char message_advice_pull_before_push[] =
 static const char message_advice_use_upstream[] =
 	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
 	   "counterpart. If you did not intend to push that branch, you may want to\n"
-	   "specify branches to push or set the 'push.default' configuration\n"
-	   "variable to 'current' or 'upstream' to push only the current branch.");
+	   "specify branches to push or set the 'push.default' configuration variable\n"
+	   "to 'simple', 'current' or 'upstream' to push only the current branch.");
 
 static const char message_advice_checkout_pull_push[] =
 	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
-- 
1.7.10.234.ge65dd.dirty

Re: [PATCH 2/4] push: introduce new push.default mode "simple"

[Matthieu Moy]

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

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> Except for the broken-ness, this adds the last line in the warning message:
>
> Hmm?  What brokenness?

The brokenness of my previous iteration.

>> +		      "To chose either option permanently, read about push.default in git-config(1)\n"),
>
> Nice ;-)

But as Jeff pointed out, this should be displayed only if the option
hasn't been set explicitely.

> I think SIMPLE should come between CURRENT and UPSTREAM in the order of
> logical progression, i.e. CURRENT < SIMPLE < UPSTREAM,

I disagree. There's actually a partial order with SIMPLE < CURRENT,
SIMPLE < UPSTREAM, and CURRENT not comparable with UPSTREAM. Any
successfull push with SIMPLE would have done exactly the same thing for
either CURRENT and UPSTREAM.

> SIMPLE is a bit more advanced (you can take advantage of
> @{upstream})

You don't really "take advantage of @{upstream}". You just get
suspicious pushes denied.

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

Re: [PATCH 4/7] push: introduce new push.default mode "simple"

[Michael Haggerty]

On 04/23/2012 10:38 AM, Matthieu Moy wrote:
> [...]
> A question is whether to allow pushing when no upstream is configured. An
> argument in favor of allowing the push is that it makes the new mode work
> in more cases. On the other hand, refusing to push when no upstream is
> configured encourages the user to set the upstream, which will be
> beneficial on the next pull. Lacking better argument, we chose to deny
> the push, because it will be easier to change in the future if someone
> shows us wrong.

I like your conservative approach to this decision.  I agree that a push 
that would create a new branch on the remote server should fail if no 
upstream is configured.

But what do people think about letting push succeed when no upstream is 
configured *provided that* there is already a branch on the remote 
server with the same name as the current branch?  I think this policy 
would cover the bulk of "safe" scenarios without adding 
dangerous/ambiguous ones.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: [PATCH 4/7] push: introduce new push.default mode "simple"

[Matthieu Moy]

Michael Haggerty <mhagger@alum.mit.edu> writes:

> I like your conservative approach to this decision.

Conservativeness is not the only argument. I originally thought we
should follow 'current' when no upstream is configured, but I changed my
mind noticing that the error message suggests "git push --set-upstream".

The question is, if the user has no upstream configured, whether we
should let him continue without configuring it, or whether it's better
to encourage him to set the upstream.

I can see senarios where people would want to push to current, and merge
from @{upstream} (e.g. push = publish, pull = get changes from another
developer). But I think most if not all senarios would benefit from
having the upstream configured (even if you never pull, the ability to
run "git log ^@{upstream}", or argumentless "git rebase -i" and the
hints in "git status" telling you how many unpushed revisions you have
are nice).

Now, I agree that "denying the push with an advice" is a bit too strong
when the goal is "encourraging to set upstream", so that's why I think
it's debatable.

> But what do people think about letting push succeed when no upstream
> is configured *provided that* there is already a branch on the remote
> server with the same name as the current branch?  I think this policy
> would cover the bulk of "safe" scenarios without adding
> dangerous/ambiguous ones.

No strong opinion. My arguments above argue in favor of rejecting this,
but I'd be fine with that.

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

Re: [PATCH 1/7] Documentation: explain push.default option a bit more

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> The previous documentation was explaining _what_ the options were doing,
> but were of little help explaining _why_ a user should set his default to
> either of the options.

Thanks.

>  * `nothing` - do not push anything.
> +* `matching` - push all branches having the same name in both ends.
> +  This allows those who prepare all the branches into a publishable
> +  shape to push them out to a non-shared repository with a single
> +  command. This is well suited when pushing to a non-shared
> +  repository, but may give surprising results when used on a
> +  repository shared by multiple users, since locally stalled
> +  branches will attempt a non-fast forward push if other users
> +  updated the branch remotely. This is the default.

The thought does not flow smoothly with the repetition of "non-shared"
here.  How about rephrasing it a bit, perhaps like this?

  * `matching` - push all branches having the same name in both ends.

    This is for those who prepare all the branches into a publishable
    shape and then push them out with a single command.  It is not
    appropriate for pushing into a
    repository shared by multiple users, since locally stalled
    branches will attempt a non-fast forward push if other users
    updated the branch.
    
    This is the default.

> +* `upstream` - push the current branch to its upstream branch. See
> +  "branch.<name>.merge" for how to configure the upstream branch. This
> +  makes `git push` and `git pull` symmetrical in the sense that `push`
> +  will update the same remote ref as the one which is merged by
> +  `git pull`.

Reordering the sentences may make it read better; first tell the reader
why and what enough so that they can decide if this is good for her and
then tell her how to configure it.

  * `upstream` - push the current branch to its upstream branch. 

    With this, `git push` will update the same remote ref as the one which
    is merged by `git pull`, making `push` and `pull` symmetrical.
    See "branch.<name>.merge" for how to configure the upstream branch. 

>  * `tracking` - deprecated synonym for `upstream`.
>  * `current` - push the current branch to a branch of the same name.
> +  +
> +  The `current` and `upstream` modes are for those who want to
> +  push out a single branch after finishing work, even when the other
> +  branches are not yet ready to be pushed out. They are safe when
> +  pushing to a shared repository.

Do we really want to say "safe" here?  I think it is misleading in
multiple ways.

 - If your current branch has a name differnt from its upstream, using
   `current` when you meant `upstream` may result in the embarrasing
   fast forward discussed elsewhere, which is hardly "safe".

 - If you always make everything on your end ready before pushing things
   out, `matching` may attempt to update remote branches other than the
   one that corresponds to your current branch, but that is exactly what
   you want to see---there is no danger here.  You can make it dangerous
   with --force, but that is a separate issue.

 - An attempt to push a stale branch in all cases will error out without
   causing damage to the remote repository.  If you do not keep your
   branches up-to-date and still use `matching`, you have more chance of
   seeing this when used against a shared repository. It is unclear if
   the use of word "safe" in the above description means this "you are
   behind" error "a risk to be avoided", but

    - if so, `current` or `upstream` will see the same error in a shared
      repository where the same branch is updated by multiple people, so
      "They are safe" is not quite correct; and

    - if not, then `matching` is not less safe than others (it is just
      as safe and unsafe as `current`).

There is a high correlation between use of shared repository and the
style of "working on one branch at a time, pushing the result as soon as
that single branch is OK", so I am perfectly fine with saying that these
single-branch modes are most likely what people want to use when working
with a shared repository, but I do not think `safety` has much to do
with the choice.

	

Re: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> It's been deprecated since 53c4031 (Johan Herland, Wed Feb 16 2011,
> push.default: Rename 'tracking' to 'upstream'), so it's OK to remove it
> from documentation (even though it's still supported) to make the
> explanations more readable.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
> Feel free to squash into previous one if needed.

Thanks, but let's keep it separate.

>  Documentation/config.txt |    1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index e38fab1..ddf6043 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1693,7 +1693,6 @@ push.default::
>    makes `git push` and `git pull` symmetrical in the sense that `push`
>    will update the same remote ref as the one which is merged by
>    `git pull`.
> -* `tracking` - deprecated synonym for `upstream`.
>  * `current` - push the current branch to a branch of the same name.
>    +
>    The `current` and `upstream` modes are for those who want to

Re: [PATCH 3/7] t5528-push-default.sh: add helper functions

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
>  t/t5528-push-default.sh |   45 ++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 38 insertions(+), 7 deletions(-)

The use of helpers makes the body of the test easier to follow.

> diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
> index c334c51..da7d3d8 100755
> --- a/t/t5528-push-default.sh
> +++ b/t/t5528-push-default.sh
> @@ -13,16 +13,47 @@ test_expect_success 'setup bare remotes' '
>  	git push parent2 HEAD
>  '
>  
> +# $1 = local revision
> +# $2 = remote revision (tested to be equal to the local one)
> +check_pushed_commit () {
> +	git log -1 --format='%h %s' >expect &&
> +	git --git-dir=repo1 log -1 --format='%h %s' "$2" >actual &&
> +	test_cmp expect actual
> +}

Hmph.  How is $1 used in the above to make it compare between local and
remote?  Does the first one need to have "$1" before " >expect"?

> +# $1 = push.default value
> +# $2 = expected target branch for the push
> +test_push_success () {
> +	git -c push.default="$1" push &&
> +	check_pushed_commit HEAD "$2"
> +}
> +
> +# $1 = push.default value
> +# other arguments = target branches that should not be touched
> +test_push_failure () {
> +	push_default=$1 &&
> +	shift &&
> +	if test $# -gt 0
> +	then
> +		# branch may not exist
> +		test_might_fail git --git-dir=repo1 \
> +			log --no-walk --format='%h %s' "$@" >expect
> +	fi &&
> +	test_must_fail git -c push.default="$1" &&

What subcommand does this run with one-shot override configuration?  Do
we need " push" before " &&"?

> +	if test $# -gt 0
> +	then
> +		test_might_fail git --git-dir=repo1 \
> +			log -1 --format='%h %s' "$@" >actual
> +	fi &&
> +	test_cmp expect actual
> +}

This allows us to look at not just the failure of the push operation,
but also inspects the resulting repository to make sure things that must
stay intact do, which is very nice.  The callers can even pass "--all"
to it, instead of enumerating the branches, which would be very useful
when testing a single-branch modes like `current` and `upstream`.

>  test_expect_success '"upstream" pushes to configured upstream' '
>  	git checkout master &&
>  	test_config branch.master.remote parent1 &&
>  	test_config branch.master.merge refs/heads/foo &&
> -	test_config push.default upstream &&
>  	test_commit two &&
> -	git push &&
> -	echo two >expect &&
> -	git --git-dir=repo1 log -1 --format=%s foo >actual &&
> -	test_cmp expect actual
> +	test_push_success upstream foo
>  '
>
>  test_expect_success '"upstream" does not push on unconfigured remote' '
> @@ -30,7 +61,7 @@ test_expect_success '"upstream" does not push on unconfigured remote' '
>  	test_unconfig branch.master.remote &&
>  	test_config push.default upstream &&
>  	test_commit three &&
> -	test_must_fail git push
> +	test_push_failure upstream master
>  '

... and we can use --all not master here, right?

>  test_expect_success '"upstream" does not push on unconfigured branch' '
> @@ -39,7 +70,7 @@ test_expect_success '"upstream" does not push on unconfigured branch' '
>  	test_unconfig branch.master.merge &&
>  	test_config push.default upstream
>  	test_commit four &&
> -	test_must_fail git push
> +	test_push_failure upstream master
>  '

Same here.

Thanks.

Re: [PATCH 4/7] push: introduce new push.default mode "simple"

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> +* `simple` - like `upstream`, but refuses to push if the upstream
> +  branch's name is different from the local one. This is the safest
> +  option and is well-suited for beginners.

Looks good.

> diff --git a/builtin/push.c b/builtin/push.c
> index 6936713..dae8306 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -76,7 +76,40 @@ static int push_url_of_remote(struct remote *remote, const char ***url_p)
>  	return remote->url_nr;
>  }
>  
> -static void setup_push_upstream(struct remote *remote)
> +NORETURN die_push_simple(struct branch *branch, struct remote *remote) {

Not static?

> +	/*
> +	 * There's no point in using shorten_unambiguous_ref here,
> +	 * as the ambiguity would be on the remote side, not what
> +	 * we have locally. Plus, this is supposed to be the simple
> +	 * mode. If the user is doing something crazy like setting
> +	 * upstream to a non-branch, we should probably be showing
> +	 * them the big ugly fully qualified ref.
> +	 */
> +	const char *short_up = skip_prefix(branch->merge[0]->src, "refs/heads/");

Unless you change behaviour depending on NULL-ness of this variable
later in this code (and I do not think you do---this is only for a
message string as far as I can see), I'd prefer to see that ?: you have
at the use site here instead, i.e.

	if (!short_up)
		short_up = branch->merge[0]->src;

perhaps with s/short_up/dest_branch/ or something.

> +	/*
> +	 * Don't show advice for people who explicitely set
> +	 * push.default.
> +	 */
> +	const char *advice_maybe = "";
> +	if (push_default == PUSH_DEFAULT_UNSPECIFIED)
> +		advice_maybe = _("\n"
> +				 "To choose either option permanently, "
> +				 "see push.default in 'git help config'.");

Nice.

> +	die(_("The upstream branch of your current branch does not match\n"
> +	      "the name of your current branch.  To push to the upstream branch\n"
> +	      "on the remote, use\n"
> +	      "\n"
> +	      "    git push %s HEAD:%s\n"
> +	      "\n"
> +	      "To push to the branch of the same name on the remote, use\n"
> +	      "\n"
> +	      "    git push %s %s\n"
> +	      "%s"),
> +	    remote->name, short_up ? short_up : branch->merge[0]->src,
> +	    remote->name, branch->name, advice_maybe);
> +}

> @@ -103,6 +136,9 @@ static void setup_push_upstream(struct remote *remote)
>  		      "your current branch '%s', without telling me what to push\n"
>  		      "to update which remote branch."),
>  		    remote->name, branch->name);
> +	if (simple && strcmp(branch->refname, branch->merge[0]->src)) {
> +		die_push_simple(branch, remote);
> +	}

Lose unnecessary {} pair, perhaps?

> +	git --git-dir=repo1 log -1 --format="%h %s" "other-name" >expect-other-name &&
> +	test_push_success current master &&
> +	git --git-dir=repo1 log -1 --format="%h %s" "other-name" >actual-other-name &&
> +	test_cmp expect-other-name actual-other-name

Hrm.

There is nothing wrong in the above part, but it shows taht it would be
very nice if test_push_success helper also encapsulated the "make sure
others did not change" logic.

Thanks for a pleasant read.

Re: [PATCH 3/7] t5528-push-default.sh: add helper functions

[Matthieu Moy]

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

> Hmph.  How is $1 used in the above to make it compare between local and
> remote?  Does the first one need to have "$1" before " >expect"?

Yes, good catch.

>> +	test_must_fail git -c push.default="$1" &&
>
> What subcommand does this run with one-shot override configuration?  Do
> we need " push" before " &&"?

Right.

Plus the "$1" should have been "$push_default" since we just did a
shift.

*sigh* this was supposed not to be a draft :-(.

>>  test_expect_success '"upstream" does not push on unconfigured remote' '
>> @@ -30,7 +61,7 @@ test_expect_success '"upstream" does not push on unconfigured remote' '
>>  	test_unconfig branch.master.remote &&
>>  	test_config push.default upstream &&
>>  	test_commit three &&
>> -	test_must_fail git push
>> +	test_push_failure upstream master
>>  '
>
> ... and we can use --all not master here, right?

Actually, we can even use --all everywhere. And then, we don't even need
the second argument, and we can simplify greatly the function:

# $1 = push.default value
# check that push fails and does not modify any remote branch
test_push_failure () {
	git --git-dir=repo1 log --no-walk --format='%h %s' --all >expect &&
	test_must_fail git -c push.default="$1" push &&
	git --git-dir=repo1 log --no-walk --format='%h %s' --all >actual &&
	test_cmp expect actual
}

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

Re: [PATCH 4/7] push: introduce new push.default mode "simple"

[Matthieu Moy]

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

>> -static void setup_push_upstream(struct remote *remote)
>> +NORETURN die_push_simple(struct branch *branch, struct remote *remote) {
>
> Not static?

fixed.

> Unless you change behaviour depending on NULL-ness of this variable
> later in this code (and I do not think you do---this is only for a
> message string as far as I can see), I'd prefer to see that ?: you have
> at the use site here instead, i.e.
>
> 	if (!short_up)
> 		short_up = branch->merge[0]->src;

applied.

> perhaps with s/short_up/dest_branch/ or something.

Hmm, not really. It's a candidate destination branch, but we'll use the
variable only if the push fails because there is another candidate.

I did s/short_up/short_upstream/ to make it clearer.

>> +	if (simple && strcmp(branch->refname, branch->merge[0]->src)) {
>> +		die_push_simple(branch, remote);
>> +	}
>
> Lose unnecessary {} pair, perhaps?

Yes, removed.

>> +	git --git-dir=repo1 log -1 --format="%h %s" "other-name" >expect-other-name &&
>> +	test_push_success current master &&
>> +	git --git-dir=repo1 log -1 --format="%h %s" "other-name" >actual-other-name &&
>> +	test_cmp expect-other-name actual-other-name
>
> Hrm.
>
> There is nothing wrong in the above part, but it shows taht it would be
> very nice if test_push_success helper also encapsulated the "make sure
> others did not change" logic.

The issue is that one has to define "others", and it is different for
current and upstream so we'd have to add arguments to specify that to
test_push_success. I'd rather keep the helpers API simple, and
special-case when needed as we did here.

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

Re: [PATCH 3/7] t5528-push-default.sh: add helper functions

[Junio C Hamano]

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

>> ... and we can use --all not master here, right?
>
> Actually, we can even use --all everywhere. And then, we don't even need
> the second argument, and we can simplify greatly the function:

That did cross my mind but I suspected that the reason to have the
argument was because you would want to use the helper also to test
'matching' case where you want to make sure ones that the pusher does
not have are left alone.

Re: [PATCH 3/7] t5528-push-default.sh: add helper functions

[Matthieu Moy]

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

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>>> ... and we can use --all not master here, right?
>>
>> Actually, we can even use --all everywhere. And then, we don't even need
>> the second argument, and we can simplify greatly the function:
>
> That did cross my mind but I suspected that the reason to have the
> argument was because you would want to use the helper also to test
> 'matching' case where you want to make sure ones that the pusher does
> not have are left alone.

I did not add much for "matching" (that would be a separate topic, and
my Git time budget is getting short). But I think the simplicity of the
new function (both caller and callee side) is worth it, even if we later
add something more complex for the case of "matching".

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

Re: [PATCH 3/7] t5528-push-default.sh: add helper functions

[Junio C Hamano]

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

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>>> ... and we can use --all not master here, right?
>>
>> Actually, we can even use --all everywhere. And then, we don't even need
>> the second argument, and we can simplify greatly the function:
>
> That did cross my mind but I suspected that the reason to have the
> argument was because you would want to use the helper also to test
> 'matching' case where you want to make sure ones that the pusher does
> not have are left alone.

In any case, here is the summary of my comments in patch form (I am not
squashing them myself).

-- >8 --
Subject: [PATCH 1/3] fixup! push: introduce new push.default mode "simple"

compilation fix; this should be static and needs type.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/push.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/push.c b/builtin/push.c
index 7a845a8..913ac7a 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -76,7 +76,7 @@ static int push_url_of_remote(struct remote *remote, const char ***url_p)
 	return remote->url_nr;
 }
 
-NORETURN die_push_simple(struct branch *branch, struct remote *remote) {
+static NORETURN int die_push_simple(struct branch *branch, struct remote *remote) {
 	/*
 	 * There's no point in using shorten_unambiguous_ref here,
 	 * as the ambiguity would be on the remote side, not what
-- 
1.7.10.376.g4eb25

[PATCH 2/3] fixup! t5528-push-default.sh: add helper functions

[Junio C Hamano]

Please eyeball the change to test_push_failure(); I think it is correct
to use the same "log --no-walk" to prepare expect and actual, but I may
have missed something.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5528-push-default.sh |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
index 43dec43..4a7d7fe 100755
--- a/t/t5528-push-default.sh
+++ b/t/t5528-push-default.sh
@@ -16,7 +16,7 @@ test_expect_success 'setup bare remotes' '
 # $1 = local revision
 # $2 = remote revision (tested to be equal to the local one)
 check_pushed_commit () {
-	git log -1 --format='%h %s' >expect &&
+	git log -1 --format='%h %s' "$1" >expect &&
 	git --git-dir=repo1 log -1 --format='%h %s' "$2" >actual &&
 	test_cmp expect actual
 }
@@ -39,11 +39,11 @@ test_push_failure () {
 		test_might_fail git --git-dir=repo1 \
 			log --no-walk --format='%h %s' "$@" >expect
 	fi &&
-	test_must_fail git -c push.default="$1" &&
+	test_must_fail git -c push.default="$push_default" push &&
 	if test $# -gt 0
 	then
 		test_might_fail git --git-dir=repo1 \
-			log -1 --format='%h %s' "$@" >actual
+			log --no-walk --format='%h %s' "$@" >actual
 	fi &&
 	test_cmp expect actual
 }
@@ -61,7 +61,7 @@ test_expect_success '"upstream" does not push on unconfigured remote' '
 	test_unconfig branch.master.remote &&
 	test_config push.default upstream &&
 	test_commit three &&
-	test_push_failure upstream master
+	test_push_failure upstream --all
 '
 
 test_expect_success '"upstream" does not push on unconfigured branch' '
@@ -70,7 +70,7 @@ test_expect_success '"upstream" does not push on unconfigured branch' '
 	test_unconfig branch.master.merge &&
 	test_config push.default upstream
 	test_commit four &&
-	test_push_failure upstream master
+	test_push_failure upstream --all
 '
 
 test_expect_success '"upstream" does not push when remotes do not match' '
-- 
1.7.10.376.g4eb25

[PATCH 3/3] push: suggested updates to push configuration documentation

[Junio C Hamano]

This is only to show how the wording suggested in my review comment will
read on top of the endpoint of your series, primarily for reviewing the
counterproposal (I didn't split this apart to apply in steps).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt |   30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 696544e..f724fc6 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1681,29 +1681,29 @@ push.default::
 +
 * `nothing` - do not push anything.
 * `matching` - push all branches having the same name in both ends.
-  This allows those who prepare all the branches into a publishable
-  shape to push them out to a non-shared repository with a single
-  command. This is well suited when pushing to a non-shared
-  repository, but may give surprising results when used on a
-  repository shared by multiple users, since locally stalled
-  branches will attempt a non-fast forward push if other users
-  updated the branch remotely. This is currently the default, but Git
-  2.0 will change the default to `simple`.
+  This is for those who prepare all the branches into a publishable
+  shape and then push them out with a single command.  It is not
+  appropriate for pushing into a repository shared by multiple users,
+  since locally stalled branches will attempt a non-fast forward push
+  if other users updated the branch.
+  +
+  This is currently the default, but Git 2.0 will change the default
+  to `simple`.
 * `simple` - like `upstream`, but refuses to push if the upstream
   branch's name is different from the local one. This is the safest
   option and is well-suited for beginners. It will become the default
   in Git 2.0.
-* `upstream` - push the current branch to its upstream branch. See
-  "branch.<name>.merge" for how to configure the upstream branch. This
-  makes `git push` and `git pull` symmetrical in the sense that `push`
-  will update the same remote ref as the one which is merged by
-  `git pull`.
+* `upstream` - push the current branch to its upstream branch.
+  With this, `git push` will update the same remote ref as the one which
+  is merged by `git pull`, making `push` and `pull` symmetrical.
+  See "branch.<name>.merge" for how to configure the upstream branch.
 * `current` - push the current branch to a branch of the same name.
   +
   The `simple`, `current` and `upstream` modes are for those who want to
   push out a single branch after finishing work, even when the other
-  branches are not yet ready to be pushed out. They are safe when
-  pushing to a shared repository.
+  branches are not yet ready to be pushed out. If you are working with
+  other people to push into the same shared repository, you would want
+  to use one of these.
 
 rebase.stat::
 	Whether to show a diffstat of what changed upstream since the last
-- 
1.7.10.376.g4eb25

Re: [PATCH 3/7] t5528-push-default.sh: add helper functions

[Matthieu Moy]

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

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>>
>>>> ... and we can use --all not master here, right?
>>>
>>> Actually, we can even use --all everywhere. And then, we don't even need
>>> the second argument, and we can simplify greatly the function:
>>
>> That did cross my mind but I suspected that the reason to have the
>> argument was because you would want to use the helper also to test
>> 'matching' case where you want to make sure ones that the pusher does
>> not have are left alone.
>
> I did not add much for "matching" (that would be a separate topic, and
> my Git time budget is getting short). But I think the simplicity of the
> new function (both caller and callee side) is worth it, even if we later
> add something more complex for the case of "matching".

Actually, there's a much stronger argument: we're talking about
test_push_failure (not success), so whatever the mode is, no branch
should be updated.

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

Re: [PATCH 3/7] t5528-push-default.sh: add helper functions

[Junio C Hamano]

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

> Actually, there's a much stronger argument: we're talking about
> test_push_failure (not success), so whatever the mode is, no branch
> should be updated.

Not really.  When you have 'master' and 'side' (two branches), have
updated both but the remote side has only updated 'master', a push with
'matching' behaves like this:

$ git -c push.default=matching push
Counting objects: 8, done.
Delta compression using up to 12 threads.
Compressing objects: 100% (2/2), done.
Writing objects: 100% (6/6), 399 bytes, done.
Total 6 (delta 1), reused 0 (delta 0)
Unpacking objects: 100% (6/6), done.
To /var/tmp/j/src
   9e1359e..4d3f497  side -> side
 ! [rejected]        master -> master (non-fast-forward)
error: failed to push some refs to '/var/tmp/j/src'
hint: Updates were rejected because a pushed branch tip is behind its remote
hint: counterpart. Check out this branch and merge the remote changes
hint: (e.g. 'git pull') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.
$ echo $?
1

Notice the mention of "some refs" on the "error:" line.  'side' branch
successfully fast-forwards.

Re: [PATCH 1/7] Documentation: explain push.default option a bit more

[Philip Oakley]

From: "Matthieu Moy" <Matthieu.Moy@imag.fr> Sent: Monday, April 23, 2012 
9:37 AM
> The previous documentation was explaining _what_ the options were doing,
> but were of little help explaining _why_ a user should set his default to
> either of the options.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
> Documentation/config.txt |   22 ++++++++++++++++++----
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index fb386ab..e38fab1 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1680,12 +1680,26 @@ push.default::
>  line. Possible values are:
> +
> * `nothing` - do not push anything.
> -* `matching` - push all matching branches.
> -  All branches having the same name in both ends are considered to be
> -  matching. This is the default.
> -* `upstream` - push the current branch to its upstream branch.
> +* `matching` - push all branches having the same name in both ends.
> +  This allows those who prepare all the branches into a publishable
> +  shape to push them out to a non-shared repository with a single
> +  command. This is well suited when pushing to a non-shared
> +  repository, but may give surprising results when used on a
> +  repository shared by multiple users, since locally stalled
> +  branches will attempt a non-fast forward push if other users
> +  updated the branch remotely. This is the default.

Given the expected future change to 'simple' as the default, surely "This is 
currently the default." give the hint toward that change.

> +* `upstream` - push the current branch to its upstream branch. See
> +  "branch.<name>.merge" for how to configure the upstream branch. This
> +  makes `git push` and `git pull` symmetrical in the sense that `push`
> +  will update the same remote ref as the one which is merged by
> +  `git pull`.
> * `tracking` - deprecated synonym for `upstream`.
> * `current` - push the current branch to a branch of the same name.
> +  +
> +  The `current` and `upstream` modes are for those who want to
> +  push out a single branch after finishing work, even when the other
> +  branches are not yet ready to be pushed out. They are safe when
> +  pushing to a shared repository.
>
> rebase.stat::
>  Whether to show a diffstat of what changed upstream since the last
> -- 
> 1.7.10.234.ge65dd.dirty
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
> -----
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 2012.0.1913 / Virus Database: 2411/4953 - Release Date: 04/22/12
> 

Re: [PATCH 1/7] Documentation: explain push.default option a bit more

[Junio C Hamano]

"Philip Oakley" <philipoakley@iee.org> writes:

> From: "Matthieu Moy" <Matthieu.Moy@imag.fr> Sent: Monday, April 23,
> 2012 9:37 AM
>> The previous documentation was explaining _what_ the options were doing,
>> but were of little help explaining _why_ a user should set his default to
>> either of the options.
>>
>> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>> ...
>> +* `matching` - push all branches having the same name in both ends.
>> + ...
>> +  updated the branch remotely. This is the default.
>
> Given the expected future change to 'simple' as the default, surely
> "This is currently the default." give the hint toward that change.

Correct, and that is exactly why this patch does not say "currently".

As the proposed commit log message explains, this change is about
clarifying what these options are and unrelated to "future" default
change at all at this step.

Re: [PATCH 1/7] Documentation: explain push.default option a bit more

[Philip Oakley]

From: "Junio C Hamano" <gitster@pobox.com>  Sent: Monday, April 23, 2012 
8:11 PM
> "Philip Oakley" <philipoakley@iee.org> writes:
>
>> From: "Matthieu Moy" <Matthieu.Moy@imag.fr> Sent: Monday, April 23,
>> 2012 9:37 AM
>>> The previous documentation was explaining _what_ the options were doing,
>>> but were of little help explaining _why_ a user should set his default 
>>> to
>>> either of the options.
>>>
>>> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>>> ...
>>> +* `matching` - push all branches having the same name in both ends.
>>> + ...
>>> +  updated the branch remotely. This is the default.
>>
>> Given the expected future change to 'simple' as the default, surely
>> "This is currently the default." give the hint toward that change.
>
> Correct, and that is exactly why this patch does not say "currently".
>
> As the proposed commit log message explains, this change is about
> clarifying what these options are and unrelated to "future" default
> change at all at this step.
>

My mistake. Sorry for the noise / misunderstanding. I now see that [PATCH 
6/7] has the change. 

[PATCH 0/7 v4] push.default upcomming change

[Matthieu Moy]

This version should address all the comments from Junio, except that I
went for my simple one-argument test_push_failure (we don't need the
complex one for now, and I'm not sure the complex one will be good
enough when we want to test 'matching'---for example, we may want to
parse the output of 'git push').

Clemens Buchacher (1):
  t5570: use explicit push refspec

Matthieu Moy (6):
  Documentation: explain push.default option a bit more
  Undocument deprecated alias 'push.default=tracking'
  t5528-push-default.sh: add helper functions
  push: introduce new push.default mode "simple"
  push: document the future default change for push.default (matching
    -> simple)
  push: start warning upcoming default change for push.default

 Documentation/config.txt |   26 +++++++++++++---
 builtin/push.c           |   73 ++++++++++++++++++++++++++++++++++++++++---
 cache.h                  |    1 +
 config.c                 |    6 ++--
 t/t5528-push-default.sh  |   78 +++++++++++++++++++++++++++++++++++++++++-----
 t/t5570-git-daemon.sh    |   30 +++++++++---------
 6 files changed, 181 insertions(+), 33 deletions(-)

-- 
1.7.10.234.g365b0

[PATCH 1/7] Documentation: explain push.default option a bit more

[Matthieu Moy]

The previous documentation was explaining _what_ the options were doing,
but were of little help explaining _why_ a user should set his default to
either of the options.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Should match exactly Junio's proposal. I like the wording.

 Documentation/config.txt |   18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index fb386ab..5f14871 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1680,12 +1680,24 @@ push.default::
 	line. Possible values are:
 +
 * `nothing` - do not push anything.
-* `matching` - push all matching branches.
-  All branches having the same name in both ends are considered to be
-  matching. This is the default.
+* `matching` - push all branches having the same name in both ends.
+  This is for those who prepare all the branches into a publishable
+  shape and then push them out with a single command.  It is not
+  appropriate for pushing into a repository shared by multiple users,
+  since locally stalled branches will attempt a non-fast forward push
+  if other users updated the branch.  This is the default.
 * `upstream` - push the current branch to its upstream branch.
+  With this, `git push` will update the same remote ref as the one which
+  is merged by `git pull`, making `push` and `pull` symmetrical.
+  See "branch.<name>.merge" for how to configure the upstream branch.
 * `tracking` - deprecated synonym for `upstream`.
 * `current` - push the current branch to a branch of the same name.
+  +
+  The `current` and `upstream` modes are for those who want to
+  push out a single branch after finishing work, even when the other
+  branches are not yet ready to be pushed out. If you are working with
+  other people to push into the same shared repository, you would want
+  to use one of these.
 
 rebase.stat::
 	Whether to show a diffstat of what changed upstream since the last
-- 
1.7.10.234.g365b0

[PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Matthieu Moy]

It's been deprecated since 53c4031 (Johan Herland, Wed Feb 16 2011,
push.default: Rename 'tracking' to 'upstream'), so it's OK to remove it
from documentation (even though it's still supported) to make the
explanations more readable.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/config.txt |    1 -
 1 file changed, 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5f14871..9617c53 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1690,7 +1690,6 @@ push.default::
   With this, `git push` will update the same remote ref as the one which
   is merged by `git pull`, making `push` and `pull` symmetrical.
   See "branch.<name>.merge" for how to configure the upstream branch.
-* `tracking` - deprecated synonym for `upstream`.
 * `current` - push the current branch to a branch of the same name.
   +
   The `current` and `upstream` modes are for those who want to
-- 
1.7.10.234.g365b0

[PATCH 3/7] t5528-push-default.sh: add helper functions

[Matthieu Moy]

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 t/t5528-push-default.sh |   34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
index c334c51..99e5519 100755
--- a/t/t5528-push-default.sh
+++ b/t/t5528-push-default.sh
@@ -13,16 +13,36 @@ test_expect_success 'setup bare remotes' '
 	git push parent2 HEAD
 '
 
+# $1 = local revision
+# $2 = remote revision (tested to be equal to the local one)
+check_pushed_commit () {
+	git log -1 --format='%h %s' "$1" >expect &&
+	git --git-dir=repo1 log -1 --format='%h %s' "$2" >actual &&
+	test_cmp expect actual
+}
+
+# $1 = push.default value
+# $2 = expected target branch for the push
+test_push_success () {
+	git -c push.default="$1" push &&
+	check_pushed_commit HEAD "$2"
+}
+
+# $1 = push.default value
+# check that push fails and does not modify any remote branch
+test_push_failure () {
+	git --git-dir=repo1 log --no-walk --format='%h %s' --all >expect &&
+	test_must_fail git -c push.default="$1" push &&
+	git --git-dir=repo1 log --no-walk --format='%h %s' --all >actual &&
+	test_cmp expect actual
+}
+
 test_expect_success '"upstream" pushes to configured upstream' '
 	git checkout master &&
 	test_config branch.master.remote parent1 &&
 	test_config branch.master.merge refs/heads/foo &&
-	test_config push.default upstream &&
 	test_commit two &&
-	git push &&
-	echo two >expect &&
-	git --git-dir=repo1 log -1 --format=%s foo >actual &&
-	test_cmp expect actual
+	test_push_success upstream foo
 '
 
 test_expect_success '"upstream" does not push on unconfigured remote' '
@@ -30,7 +50,7 @@ test_expect_success '"upstream" does not push on unconfigured remote' '
 	test_unconfig branch.master.remote &&
 	test_config push.default upstream &&
 	test_commit three &&
-	test_must_fail git push
+	test_push_failure upstream
 '
 
 test_expect_success '"upstream" does not push on unconfigured branch' '
@@ -39,7 +59,7 @@ test_expect_success '"upstream" does not push on unconfigured branch' '
 	test_unconfig branch.master.merge &&
 	test_config push.default upstream
 	test_commit four &&
-	test_must_fail git push
+	test_push_failure upstream
 '
 
 test_expect_success '"upstream" does not push when remotes do not match' '
-- 
1.7.10.234.g365b0

[PATCH 4/7] push: introduce new push.default mode "simple"

[Matthieu Moy]

When calling "git push" without argument, we want to allow Git to do
something simple to explain and safe. push.default=matching is unsafe
when used to push to shared repositories, and hard to explain to
beginners in some contexts. It is debatable whether 'upstream' or
'current' is the safest or the easiest to explain, so introduce a new
mode called 'simple' that is the intersection of them: push to the
upstream branch, but only if it has the same name remotely. If not, give
an error that suggests the right command to push explicitely to
'upstream' or 'current'.

A question is whether to allow pushing when no upstream is configured. An
argument in favor of allowing the push is that it makes the new mode work
in more cases. On the other hand, refusing to push when no upstream is
configured encourages the user to set the upstream, which will be
beneficial on the next pull. Lacking better argument, we chose to deny
the push, because it will be easier to change in the future if someone
shows us wrong.

Original-patch-by: Jeff King <peff@peff.net>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/config.txt |    5 ++++-
 builtin/push.c           |   46 ++++++++++++++++++++++++++++++++++++++++++++--
 cache.h                  |    1 +
 config.c                 |    6 ++++--
 t/t5528-push-default.sh  |   44 ++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 97 insertions(+), 5 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 9617c53..7f5ad1c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1686,13 +1686,16 @@ push.default::
   appropriate for pushing into a repository shared by multiple users,
   since locally stalled branches will attempt a non-fast forward push
   if other users updated the branch.  This is the default.
+* `simple` - like `upstream`, but refuses to push if the upstream
+  branch's name is different from the local one. This is the safest
+  option and is well-suited for beginners.
 * `upstream` - push the current branch to its upstream branch.
   With this, `git push` will update the same remote ref as the one which
   is merged by `git pull`, making `push` and `pull` symmetrical.
   See "branch.<name>.merge" for how to configure the upstream branch.
 * `current` - push the current branch to a branch of the same name.
   +
-  The `current` and `upstream` modes are for those who want to
+  The `simple`, `current` and `upstream` modes are for those who want to
   push out a single branch after finishing work, even when the other
   branches are not yet ready to be pushed out. If you are working with
   other people to push into the same shared repository, you would want
diff --git a/builtin/push.c b/builtin/push.c
index 6936713..8e663db 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -76,7 +76,43 @@ static int push_url_of_remote(struct remote *remote, const char ***url_p)
 	return remote->url_nr;
 }
 
-static void setup_push_upstream(struct remote *remote)
+static NORETURN int die_push_simple(struct branch *branch, struct remote *remote) {
+	/*
+	 * There's no point in using shorten_unambiguous_ref here,
+	 * as the ambiguity would be on the remote side, not what
+	 * we have locally. Plus, this is supposed to be the simple
+	 * mode. If the user is doing something crazy like setting
+	 * upstream to a non-branch, we should probably be showing
+	 * them the big ugly fully qualified ref.
+	 */
+	const char *short_upstream =
+		skip_prefix(branch->merge[0]->src, "refs/heads/");
+	if (!short_upstream)
+		short_upstream = branch->merge[0]->src;
+	/*
+	 * Don't show advice for people who explicitely set
+	 * push.default.
+	 */
+	const char *advice_maybe = "";
+	if (push_default == PUSH_DEFAULT_UNSPECIFIED)
+		advice_maybe = _("\n"
+				 "To choose either option permanently, "
+				 "see push.default in 'git help config'.");
+	die(_("The upstream branch of your current branch does not match\n"
+	      "the name of your current branch.  To push to the upstream branch\n"
+	      "on the remote, use\n"
+	      "\n"
+	      "    git push %s HEAD:%s\n"
+	      "\n"
+	      "To push to the branch of the same name on the remote, use\n"
+	      "\n"
+	      "    git push %s %s\n"
+	      "%s"),
+	    remote->name, short_upstream,
+	    remote->name, branch->name, advice_maybe);
+}
+
+static void setup_push_upstream(struct remote *remote, int simple)
 {
 	struct strbuf refspec = STRBUF_INIT;
 	struct branch *branch = branch_get(NULL);
@@ -103,6 +139,8 @@ static void setup_push_upstream(struct remote *remote)
 		      "your current branch '%s', without telling me what to push\n"
 		      "to update which remote branch."),
 		    remote->name, branch->name);
+	if (simple && strcmp(branch->refname, branch->merge[0]->src))
+		die_push_simple(branch, remote);
 
 	strbuf_addf(&refspec, "%s:%s", branch->name, branch->merge[0]->src);
 	add_refspec(refspec.buf);
@@ -119,8 +157,12 @@ static void setup_default_push_refspecs(struct remote *remote)
 		add_refspec(":");
 		break;
 
+	case PUSH_DEFAULT_SIMPLE:
+		setup_push_upstream(remote, 1);
+		break;
+
 	case PUSH_DEFAULT_UPSTREAM:
-		setup_push_upstream(remote);
+		setup_push_upstream(remote, 0);
 		break;
 
 	case PUSH_DEFAULT_CURRENT:
diff --git a/cache.h b/cache.h
index 5bf59ff..b60d490 100644
--- a/cache.h
+++ b/cache.h
@@ -624,6 +624,7 @@ enum rebase_setup_type {
 enum push_default_type {
 	PUSH_DEFAULT_NOTHING = 0,
 	PUSH_DEFAULT_MATCHING,
+	PUSH_DEFAULT_SIMPLE,
 	PUSH_DEFAULT_UPSTREAM,
 	PUSH_DEFAULT_CURRENT,
 	PUSH_DEFAULT_UNSPECIFIED
diff --git a/config.c b/config.c
index 68d3294..bfe0c79 100644
--- a/config.c
+++ b/config.c
@@ -829,6 +829,8 @@ static int git_default_push_config(const char *var, const char *value)
 			push_default = PUSH_DEFAULT_NOTHING;
 		else if (!strcmp(value, "matching"))
 			push_default = PUSH_DEFAULT_MATCHING;
+		else if (!strcmp(value, "simple"))
+			push_default = PUSH_DEFAULT_SIMPLE;
 		else if (!strcmp(value, "upstream"))
 			push_default = PUSH_DEFAULT_UPSTREAM;
 		else if (!strcmp(value, "tracking")) /* deprecated */
@@ -837,8 +839,8 @@ static int git_default_push_config(const char *var, const char *value)
 			push_default = PUSH_DEFAULT_CURRENT;
 		else {
 			error("Malformed value for %s: %s", var, value);
-			return error("Must be one of nothing, matching, "
-				     "tracking or current.");
+			return error("Must be one of nothing, matching, simple, "
+				     "upstream or current.");
 		}
 		return 0;
 	}
diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
index 99e5519..4736da8 100755
--- a/t/t5528-push-default.sh
+++ b/t/t5528-push-default.sh
@@ -71,4 +71,48 @@ test_expect_success '"upstream" does not push when remotes do not match' '
 	test_must_fail git push parent2
 '
 
+test_expect_success 'push from/to new branch with upstream, matching and simple' '
+	git checkout -b new-branch &&
+	test_push_failure simple &&
+	test_push_failure matching &&
+	test_push_failure upstream
+'
+
+test_expect_success 'push from/to new branch with current creates remote branch' '
+	test_config branch.new-branch.remote repo1 &&
+	git checkout new-branch &&
+	test_push_success current new-branch
+'
+
+test_expect_success 'push to existing branch, with no upstream configured' '
+	test_config branch.master.remote repo1 &&
+	git checkout master &&
+	test_push_failure simple &&
+	test_push_failure upstream
+'
+
+test_expect_success 'push to existing branch, upstream configured with same name' '
+	test_config branch.master.remote repo1 &&
+	test_config branch.master.merge refs/heads/master &&
+	git checkout master &&
+	test_commit six &&
+	test_push_success upstream master &&
+	test_commit seven &&
+	test_push_success simple master
+'
+
+test_expect_success 'push to existing branch, upstream configured with different name' '
+	test_config branch.master.remote repo1 &&
+	test_config branch.master.merge refs/heads/other-name &&
+	git checkout master &&
+	test_commit eight &&
+	test_push_success upstream other-name &&
+	test_commit nine &&
+	test_push_failure simple &&
+	git --git-dir=repo1 log -1 --format="%h %s" "other-name" >expect-other-name &&
+	test_push_success current master &&
+	git --git-dir=repo1 log -1 --format="%h %s" "other-name" >actual-other-name &&
+	test_cmp expect-other-name actual-other-name
+'
+
 test_done
-- 
1.7.10.234.g365b0

[PATCH 5/7] t5570: use explicit push refspec

[Matthieu Moy]

From: Clemens Buchacher <drizzd@aon.at>

The default mode for push without arguments will change. Some warnings
are about to be enabled for such use, which causes some t5570 tests to
fail because they do not expect this output.

Fix this by passing an explicit refspec to git push. To that end, change
the calling conventions of test_remote_error in order to accomodate
extra command arguments.

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 t/t5570-git-daemon.sh |   30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh
index 7cbc999..a3a4e47 100755
--- a/t/t5570-git-daemon.sh
+++ b/t/t5570-git-daemon.sh
@@ -103,14 +103,12 @@ test_remote_error()
 		esac
 	done
 
-	if test $# -ne 3
-	then
-		error "invalid number of arguments"
-	fi
-
+	msg=$1
+	shift
 	cmd=$1
-	repo=$2
-	msg=$3
+	shift
+	repo=$1
+	shift || error "invalid number of arguments"
 
 	if test -x "$GIT_DAEMON_DOCUMENT_ROOT_PATH/$repo"
 	then
@@ -122,7 +120,7 @@ test_remote_error()
 		fi
 	fi
 
-	test_must_fail git "$cmd" "$GIT_DAEMON_URL/$repo" 2>output &&
+	test_must_fail git "$cmd" "$GIT_DAEMON_URL/$repo" "$@" 2>output &&
 	echo "fatal: remote error: $msg: /$repo" >expect &&
 	test_cmp expect output
 	ret=$?
@@ -131,18 +129,18 @@ test_remote_error()
 }
 
 msg="access denied or repository not exported"
-test_expect_success 'clone non-existent' "test_remote_error    clone nowhere.git '$msg'"
-test_expect_success 'push disabled'      "test_remote_error    push  repo.git    '$msg'"
-test_expect_success 'read access denied' "test_remote_error -x fetch repo.git    '$msg'"
-test_expect_success 'not exported'       "test_remote_error -n fetch repo.git    '$msg'"
+test_expect_success 'clone non-existent' "test_remote_error    '$msg' clone nowhere.git    "
+test_expect_success 'push disabled'      "test_remote_error    '$msg' push  repo.git master"
+test_expect_success 'read access denied' "test_remote_error -x '$msg' fetch repo.git       "
+test_expect_success 'not exported'       "test_remote_error -n '$msg' fetch repo.git       "
 
 stop_git_daemon
 start_git_daemon --informative-errors
 
-test_expect_success 'clone non-existent' "test_remote_error    clone nowhere.git 'no such repository'"
-test_expect_success 'push disabled'      "test_remote_error    push  repo.git    'service not enabled'"
-test_expect_success 'read access denied' "test_remote_error -x fetch repo.git    'no such repository'"
-test_expect_success 'not exported'       "test_remote_error -n fetch repo.git    'repository not exported'"
+test_expect_success 'clone non-existent' "test_remote_error    'no such repository'      clone nowhere.git    "
+test_expect_success 'push disabled'      "test_remote_error    'service not enabled'     push  repo.git master"
+test_expect_success 'read access denied' "test_remote_error -x 'no such repository'      fetch repo.git       "
+test_expect_success 'not exported'       "test_remote_error -n 'repository not exported' fetch repo.git       "
 
 stop_git_daemon
 test_done
-- 
1.7.10.234.g365b0

[PATCH 6/7] push: document the future default change for push.default (matching -> simple)

[Matthieu Moy]

It is too early to start warning loudly about the future default change
in favor of 'simple', since many users use different versions of Git, and
would be harmed if we advised them to explicitely set
'push.default=simple' when using old versions of Git.

Still, we want to document the upcomming change so that:

* Users who may be affected by the change get one more chance to know it
  in advance.

* We actually commit to changing the default, and avoid repeating past
  errors.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/config.txt |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 7f5ad1c..f724fc6 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1685,10 +1685,14 @@ push.default::
   shape and then push them out with a single command.  It is not
   appropriate for pushing into a repository shared by multiple users,
   since locally stalled branches will attempt a non-fast forward push
-  if other users updated the branch.  This is the default.
+  if other users updated the branch.
+  +
+  This is currently the default, but Git 2.0 will change the default
+  to `simple`.
 * `simple` - like `upstream`, but refuses to push if the upstream
   branch's name is different from the local one. This is the safest
-  option and is well-suited for beginners.
+  option and is well-suited for beginners. It will become the default
+  in Git 2.0.
 * `upstream` - push the current branch to its upstream branch.
   With this, `git push` will update the same remote ref as the one which
   is merged by `git pull`, making `push` and `pull` symmetrical.
-- 
1.7.10.234.g365b0

[PATCH 7/7] push: start warning upcoming default change for push.default

[Matthieu Moy]

In preparation for flipping the default to the "simple" mode from
the "matching" mode that is the historical default, start warning
users when they rely on unconfigured "git push" to default to the
"matching" mode.

Also, advertise for 'simple' where 'current' and 'upstream' are advised.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/push.c |   27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 8e663db..be4525a 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -146,12 +146,35 @@ static void setup_push_upstream(struct remote *remote, int simple)
 	add_refspec(refspec.buf);
 }
 
+static char warn_unspecified_push_default_msg[] =
+N_("push.default is unset; its implicit value is changing in\n"
+   "Git 2.0 from 'matching' to 'simple'. To squelch this message\n"
+   "and maintain the current behavior after the default changes, use:\n"
+   "\n"
+   "  git config --global push.default matching\n"
+   "\n"
+   "To squelch this message and adopt the new behavior now, use:\n"
+   "\n"
+   "  git config --global push.default simple\n"
+   "\n"
+   "See 'git help config' and search for 'push.default' for further information.");
+
+static void warn_unspecified_push_default_configuration(void)
+{
+	static int warn_once;
+
+	if (warn_once++)
+		return;
+	warning("%s\n", _(warn_unspecified_push_default_msg));
+}
+
 static void setup_default_push_refspecs(struct remote *remote)
 {
 	switch (push_default) {
 	default:
 	case PUSH_DEFAULT_UNSPECIFIED:
 		default_matching_used = 1;
+		warn_unspecified_push_default_configuration();
 		/* fallthru */
 	case PUSH_DEFAULT_MATCHING:
 		add_refspec(":");
@@ -185,8 +208,8 @@ static const char message_advice_pull_before_push[] =
 static const char message_advice_use_upstream[] =
 	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
 	   "counterpart. If you did not intend to push that branch, you may want to\n"
-	   "specify branches to push or set the 'push.default' configuration\n"
-	   "variable to 'current' or 'upstream' to push only the current branch.");
+	   "specify branches to push or set the 'push.default' configuration variable\n"
+	   "to 'simple', 'current' or 'upstream' to push only the current branch.");
 
 static const char message_advice_checkout_pull_push[] =
 	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
-- 
1.7.10.234.g365b0

Re: [PATCH 0/7 v4] push.default upcomming change

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> This version should address all the comments from Junio, except that I
> went for my simple one-argument test_push_failure (we don't need the
> complex one for now, and I'm not sure the complex one will be good
> enough when we want to test 'matching'---for example, we may want to
> parse the output of 'git push').

Fine by me, and the result looks good.

I'll queue everything before the final one, with a single readability
update (attached below) on top, on mm/simple-push topic. And then the
final one will be queued on a separate mm/push/default-switch-warning
topic that is forked from the former.

Thanks for working on this.  With acks from others I'm hoping that we
can merge the "simple" to 'next' soonish.

-- >8 --
Subject: [PATCH] push.default doc: explain simple after upstream

As the "simple" mode is described in terms of what "upstream" does,
swap the order of these two entries so that the reader sees "upstream"
first and then reads "simple" with the knowledge of what "upstream"
does.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f724fc6..6a4c130 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1689,14 +1689,14 @@ push.default::
   +
   This is currently the default, but Git 2.0 will change the default
   to `simple`.
-* `simple` - like `upstream`, but refuses to push if the upstream
-  branch's name is different from the local one. This is the safest
-  option and is well-suited for beginners. It will become the default
-  in Git 2.0.
 * `upstream` - push the current branch to its upstream branch.
   With this, `git push` will update the same remote ref as the one which
   is merged by `git pull`, making `push` and `pull` symmetrical.
   See "branch.<name>.merge" for how to configure the upstream branch.
+* `simple` - like `upstream`, but refuses to push if the upstream
+  branch's name is different from the local one. This is the safest
+  option and is well-suited for beginners. It will become the default
+  in Git 2.0.
 * `current` - push the current branch to a branch of the same name.
   +
   The `simple`, `current` and `upstream` modes are for those who want to
-- 
1.7.10.433.g8de07

Re: [PATCH 4/7] push: introduce new push.default mode "simple"

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> diff --git a/builtin/push.c b/builtin/push.c
> index 6936713..8e663db 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -76,7 +76,43 @@ static int push_url_of_remote(struct remote *remote, const char ***url_p)
>  	return remote->url_nr;
>  }
>  
> -static void setup_push_upstream(struct remote *remote)
> +static NORETURN int die_push_simple(struct branch *branch, struct remote *remote) {
> +	/*
> +	 * There's no point in using shorten_unambiguous_ref here,
> +	 * as the ambiguity would be on the remote side, not what
> +	 * we have locally. Plus, this is supposed to be the simple
> +	 * mode. If the user is doing something crazy like setting
> +	 * upstream to a non-branch, we should probably be showing
> +	 * them the big ugly fully qualified ref.
> +	 */
> +	const char *short_upstream =
> +		skip_prefix(branch->merge[0]->src, "refs/heads/");
> +	if (!short_upstream)
> +		short_upstream = branch->merge[0]->src;
> +	/*
> +	 * Don't show advice for people who explicitely set
> +	 * push.default.
> +	 */
> +	const char *advice_maybe = "";

I've amended this to avoid decl-after-stmt.

I think the series is ready for 'next' but I'll wait for a few days or
until I see acks from trusted others, whichever comes sooner.

Thanks.

Re: [PATCH 4/7] push: introduce new push.default mode "simple"

[Matthieu Moy]

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

>> +	if (!short_upstream)
>> +		short_upstream = branch->merge[0]->src;
>> +	/*
>> +	 * Don't show advice for people who explicitely set
>> +	 * push.default.
>> +	 */
>> +	const char *advice_maybe = "";
>
> I've amended this to avoid decl-after-stmt.

Oops, right, I didn't notice the advice_maybe was a declaration.

Thanks for the fix.

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

[PATCH] t5541: warning message is given even with --quiet

[Junio C Hamano]

Explicitly set push.default to matching to avoid the warning message;
the test wants to verify there is no post-push report when --quiet is
given, and this message interferes with it.

This change is *iffy*, not in the sense that it breaks the test it
touches, but because it is unclear if the output should be given when
the user explicitly asked --quiet.

On one hand, --quiet is a request to be "quiet", but on the other hand,
this warning is meant to be shown in the face in large flashing red
letters no matter what the user says, so...

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5541-http-push.sh |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
index 5b170be..07fa199 100755
--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -64,7 +64,10 @@ test_expect_success 'no empty path components' '
 
 test_expect_success 'clone remote repository' '
 	rm -rf test_repo_clone &&
-	git clone $HTTPD_URL/smart/test_repo.git test_repo_clone
+	git clone $HTTPD_URL/smart/test_repo.git test_repo_clone &&
+	(
+		cd test_repo_clone && git config push.default matching
+	)
 '
 
 test_expect_success 'push to remote repository (standard)' '
-- 
1.7.10.475.g8b959

Re: [PATCH] t5541: warning message is given even with --quiet

[Matthieu Moy]

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

> On one hand, --quiet is a request to be "quiet", but on the other hand,
> this warning is meant to be shown in the face in large flashing red
> letters no matter what the user says, so...

I'd say it's OK to show the warning with --quiet. Users will normally
set the config option in ~/.gitconfig and forget about it, both in
interactive use and in scripts.

So,

> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Acked-by: Matthieu Moy <Matthieu.Moy@imag.fr>

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

Re: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Ævar Arnfjörð Bjarmason]

On Mon, Apr 23, 2012 at 10:37 AM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> It's been deprecated since 53c4031 (Johan Herland, Wed Feb 16 2011,
> push.default: Rename 'tracking' to 'upstream'), so it's OK to remove it
> from documentation (even though it's still supported) to make the
> explanations more readable.

I don't think this was a good move for the documentation. Now every
time I find an old repo with "push.default=tracking" I end up
wondering what it was a synonym for again, and other users who don't
know what it does will just assume it's an invalid value or something.

We can't treat existing config values we still support as any other
deprecated feature. They still exist in files we have no control over,
and in people's brains who are reading "man git-config" trying to
remember what it meant.

> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
> Feel free to squash into previous one if needed.
>
>  Documentation/config.txt |    1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index e38fab1..ddf6043 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1693,7 +1693,6 @@ push.default::
>    makes `git push` and `git pull` symmetrical in the sense that `push`
>    will update the same remote ref as the one which is merged by
>    `git pull`.
> -* `tracking` - deprecated synonym for `upstream`.
>  * `current` - push the current branch to a branch of the same name.
>    +
>    The `current` and `upstream` modes are for those who want to
> --
> 1.7.10.234.ge65dd.dirty
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Mon, Apr 23, 2012 at 10:37 AM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
>> It's been deprecated since 53c4031 (Johan Herland, Wed Feb 16 2011,
>> push.default: Rename 'tracking' to 'upstream'), so it's OK to remove it
>> from documentation (even though it's still supported) to make the
>> explanations more readable.
>
> I don't think this was a good move for the documentation. Now every
> time I find an old repo with "push.default=tracking" I end up
> wondering what it was a synonym for again, and other users who don't
> know what it does will just assume it's an invalid value or something.
>
> We can't treat existing config values we still support as any other
> deprecated feature. They still exist in files we have no control over,
> and in people's brains who are reading "man git-config" trying to
> remember what it meant.

Wow, that's a blast from the past.

I tend to agree that deprecating and removing are quite different,
but a simple "revert" of the change would not be good, either.  We
still would want to _discourage_ its use.

I think I can be persuaded to apply a patch that mentions 'tracking'
as a side note.

Thanks.

>
>> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>> ---
>> Feel free to squash into previous one if needed.
>>
>>  Documentation/config.txt |    1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index e38fab1..ddf6043 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -1693,7 +1693,6 @@ push.default::
>>    makes `git push` and `git pull` symmetrical in the sense that `push`
>>    will update the same remote ref as the one which is merged by
>>    `git pull`.
>> -* `tracking` - deprecated synonym for `upstream`.
>>  * `current` - push the current branch to a branch of the same name.
>>    +
>>    The `current` and `upstream` modes are for those who want to
>> --
>> 1.7.10.234.ge65dd.dirty
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Jonathan Nieder]

Hi,

Junio C Hamano wrote:

> Wow, that's a blast from the past.
>
> I tend to agree that deprecating and removing are quite different,
> but a simple "revert" of the change would not be good, either.  We
> still would want to _discourage_ its use.

Hm, I was about to try adding a line in that vein, like

 * `tracking` - deprecated synonym for `upstream`.
 
Imagine my surprise when I saw that that is what you just said
would be no good:

[...]
>>>    `git pull`.
>>> -* `tracking` - deprecated synonym for `upstream`.
>>>  * `current` - push the current branch to a branch of the same name.

I really do think that including `tracking` in the same list would be
valuable.  When I look over a friend's .gitconfig file to help track
down a problem she is running into, it is helpful if I can find the
meaning of each item in a straightforward way.

Is the problem that "deprecated" is not precise enough?  For example,
would it make sense to say "deprecated synonym for `upstream`.  Will
be dropped in git 2.1" or something like that?

My two cents,
Jonathan

Re: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Jonathan Nieder]

Jonathan Nieder wrote:

> Is the problem that "deprecated" is not precise enough?  For example,
> would it make sense to say "deprecated synonym for `upstream`.  Will
> be dropped in git 2.1" or something like that?

Also, if we plan to remove support soon, we should start warning when
this setting is encountered so people know to update their
configuration.

Jonathan

Re: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Junio C Hamano wrote:
>
>> Wow, that's a blast from the past.
>>
>> I tend to agree that deprecating and removing are quite different,
>> but a simple "revert" of the change would not be good, either.  We
>> still would want to _discourage_ its use.
>
> Hm, I was about to try adding a line in that vein, like
>
>  * `tracking` - deprecated synonym for `upstream`.
>  
> Imagine my surprise when I saw that that is what you just said
> would be no good:
>
> [...]
>>>>    `git pull`.
>>>> -* `tracking` - deprecated synonym for `upstream`.
>>>>  * `current` - push the current branch to a branch of the same name.
>
> I really do think that including `tracking` in the same list would be
> valuable.  When I look over a friend's .gitconfig file to help track
> down a problem she is running into, it is helpful if I can find the
> meaning of each item in a straightforward way.

While I agree we would need a way for you to easily find `tracking`
mentioned near that point, listing it as if it is a proper part of
the same list of possibilities is not the only way to do so.

The enumeration is used by two different audiences.  For those who
want to _learn_ what possibilities are available to them (i.e. they
are not going from `tracking` to what it means, but going in the
opposite direction), it should be unmistakingly clear that
`tracking` is not a part of the choices they should make.  I do not
think the following list created by a simple "revert" makes it clear.

    * `nothing` - do not push anything.
    * `matching` - push all branches having the same name in both ends.
    * `upstream` - push the current branch to ...
    * `simple` - like `upstream`, but refuses to ...
    * `tracking` - deprecated synonym for `upstream`.
    * `current` - push the current branch to a branch of the same name.

When scanning, most people will scan lines to see there are 6
choices without reading anything after '-' first, and then start
reading the item that sounds plausible for them without necessarily
reading the others.  That will imprint the word `tracking` in the
context of choosing how to push, especially when that is not what
they end up using.

That is why I tend to prefer how check-ref-format documentation
describes --print:

        --normalize::
                Normalize 'refname' by removing any leading slash (`/`)
                characters and collapsing runs of adjacent slashes between
                name components into a single slash.  Iff the normalized
                refname is valid then print it to standard output and exit
                with a status of 0.  (`--print` is a deprecated way to spell
                `--normalize`.)

When you are going from `tracking` to what it means, you have \C-s
(if you are viewing in Emacs) or '/' (if you are using less)
available.

Re: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Jonathan Nieder]

Junio C Hamano wrote:

> That is why I tend to prefer how check-ref-format documentation
> describes --print:
>
>         --normalize::
>                 Normalize 'refname' by removing any leading slash (`/`)
>                 characters and collapsing runs of adjacent slashes between
>                 name components into a single slash.  Iff the normalized
>                 refname is valid then print it to standard output and exit
>                 with a status of 0.  (`--print` is a deprecated way to spell
>                 `--normalize`.)

That works because, as you mention, the usual way to look up an option
in manpages is to search for "--print", including the two minus signs.

Unfortunately an analagous approach in gitconfig(5) would be seriously
broken, because searching for "tracking" (no minus signs) is going to
hit many false positives.  I do not think such a change would be an
improvement.

Meanwhile I believe the prominent words "deprecated synonym" already
make it completely obvious that when I write a new config file, I should
use the modern option, unless I am trying to write a config file that
also works with older versions of git.  In the latter case (which
unfortunately is not too uncommon), hiding the option is not going to
make my life easier.  What would allow me to make an informed choice
is mentioning what version of git *introduced* the new name of the
option:

	- `tracking` - deprecated old name for `upstream`, used by git
	  versions before 1.7.4.2.  Don't use this.

Also I do not think anyone claimed we are removing "tracking" from the
documentation in order to stop people from using it.  The rationale
when the patch was proposed is that it makes the documentation easier
to read.  I agree with that rationale, with the caveat Avar mentioned.
There is a simple fix: just simplify the behavior being explained as
well, by biting the bullet and dropping the "tracking" synonym after a
suitable period in which it produces a warning.

In the meantime, the documentation is valuable, and pretending that
"tracking" does not exist for everyone who does not confusedly reread
the docs a few times is just a way to lie to ourselves and make users'
lives more difficult.  Is that really the intent?

Jonathan

Re: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Jonathan Nieder wrote:
>
>> Is the problem that "deprecated" is not precise enough?  For example,
>> would it make sense to say "deprecated synonym for `upstream`.  Will
>> be dropped in git 2.1" or something like that?
>
> Also, if we plan to remove support soon, we should start warning when
> this setting is encountered so people know to update their
> configuration.

I do not think this even needs to be removed.

We deprecate something for one of two reasons. One is when it was a
bad idea to support it, and we would want to remove the support
eventually.  This needs a migration plan.

The other is when there are better ways available but we do not want
to break the old way.  We still do not want to encourage the old way
to new users.

The change from 'upstream' from 'tracking' is the latter.  The
wording will confuse new users when they want to learn what
'tracking' as a concept is, and it is better spelt 'upstream'.  But
the breakage is not serious enough to warrant forcing an old timer
who can explain these two concepts to newbies when needed to update
his configuration files he is not planning to show to newbies.

Re: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Junio C Hamano wrote:
>
>> That is why I tend to prefer how check-ref-format documentation
>> describes --print:
>>
>>         --normalize::
>>                 Normalize 'refname' by removing any leading slash (`/`)
>>                 characters and collapsing runs of adjacent slashes between
>>                 name components into a single slash.  Iff the normalized
>>                 refname is valid then print it to standard output and exit
>>                 with a status of 0.  (`--print` is a deprecated way to spell
>>                 `--normalize`.)
>
> That works because, as you mention, the usual way to look up an option
> in manpages is to search for "--print", including the two minus signs.
>
> Unfortunately an analagous approach in gitconfig(5) would be seriously
> broken, because searching for "tracking" (no minus signs) is going to
> hit many false positives.  I do not think such a change would be an
> improvement.

I thought your example was that you saw "pull.default = tracking"
and wondering what it is.  Why do you need global search for
"tracking", not just near pull.default is described, in the first
place?

Re: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Jonathan Nieder]

Junio C Hamano wrote:

>                                                      For those who
> want to _learn_ what possibilities are available to them (i.e. they
> are not going from `tracking` to what it means, but going in the
> opposite direction), it should be unmistakingly clear that
> `tracking` is not a part of the choices they should make.

Until pre-1.7.4 versions of git fall out of use, I don't agree that
the above is true. :(


>                                                            I do not
> think the following list created by a simple "revert" makes it clear.
>
>     * `nothing` - do not push anything.
>     * `matching` - push all branches having the same name in both ends.
>     * `upstream` - push the current branch to ...
>     * `simple` - like `upstream`, but refuses to ...
>     * `tracking` - deprecated synonym for `upstream`.
>     * `current` - push the current branch to a branch of the same name.

How about the following?

    * `nothing` - ...
    * `matching` - ...
    * `upstream` - ...
    * `simple` - ...
    * `current` - ...

  For compatibility with ancient config files, the following synonym
  is also supported.  Don't use it.

    * `tracking` - old name for `upstream`

Re: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Matthieu Moy]

Jonathan Nieder <jrnieder@gmail.com> writes:

> How about the following?
>
>     * `nothing` - ...
>     * `matching` - ...
>     * `upstream` - ...
>     * `simple` - ...
>     * `current` - ...
>
>   For compatibility with ancient config files, the following synonym
>   is also supported.  Don't use it.
>
>     * `tracking` - old name for `upstream`

Sounds good to me.

I'm the author of the removal patch, but the patch was just part of a
larger serie explaining push.default, the idea of cleaning up the
obsolete alias came in the discussion and I did it, but I'm fine with
reintroducing it in the doc (as long as it does not disturb new users
too much).

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

Re: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Jonathan Nieder]

Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:

>> That works because, as you mention, the usual way to look up an option
>> in manpages is to search for "--print", including the two minus signs.
>>
>> Unfortunately an analagous approach in gitconfig(5) would be seriously
>> broken, because searching for "tracking" (no minus signs) is going to
>> hit many false positives.  I do not think such a change would be an
>> improvement.
>
> I thought your example was that you saw "pull.default = tracking"
> and wondering what it is.  Why do you need global search for
> "tracking", not just near pull.default is described, in the first
> place?

Because the UI for local searches in web browsers and man pagers is
seriously lacking.  Or, because people have bad habits and do not
take apppropriate advantage of search in small subsections of a
document.  All I know is that I have seen myself and others doing
searches analagous to "--print" and not seen searches analagous to
"tracking".

Am I really the only one that doesn't see the "--print" change as
hiding an option and sees burying "tracking" in the text as
qualitatively different?

Jonathan

Re: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Junio C Hamano wrote:
>
>>                                                      For those who
>> want to _learn_ what possibilities are available to them (i.e. they
>> are not going from `tracking` to what it means, but going in the
>> opposite direction), it should be unmistakingly clear that
>> `tracking` is not a part of the choices they should make.
>
> Until pre-1.7.4 versions of git fall out of use, I don't agree that
> the above is true. :(

The documentation ships with the version that the above is true.  We
are not making an update to documentation that comes with ancient
versions.


>>                                                            I do not
>> think the following list created by a simple "revert" makes it clear.
>>
>>     * `nothing` - do not push anything.
>>     * `matching` - push all branches having the same name in both ends.
>>     * `upstream` - push the current branch to ...
>>     * `simple` - like `upstream`, but refuses to ...
>>     * `tracking` - deprecated synonym for `upstream`.
>>     * `current` - push the current branch to a branch of the same name.
>
> How about the following?
>
>     * `nothing` - ...
>     * `matching` - ...
>     * `upstream` - ...
>     * `simple` - ...
>     * `current` - ...
>
>   For compatibility with ancient config files, the following synonym
>   is also supported.  Don't use it.
>
>     * `tracking` - old name for `upstream`

Didn't I say I am fine to mention it "as a side note" in the
original message you started responding to?

Re: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Philip Oakley]

From: "Jonathan Nieder" <jrnieder@gmail.com>
Sent: Thursday, January 31, 2013 8:04 PM
> Junio C Hamano wrote:
>
>>                                                      For those who
>> want to _learn_ what possibilities are available to them (i.e. they
>> are not going from `tracking` to what it means, but going in the
>> opposite direction), it should be unmistakingly clear that
>> `tracking` is not a part of the choices they should make.
>
> Until pre-1.7.4 versions of git fall out of use, I don't agree that
> the above is true. :(
>
>
>>                                                            I do not
>> think the following list created by a simple "revert" makes it clear.
>>
>>     * `nothing` - do not push anything.
>>     * `matching` - push all branches having the same name in both 
>> ends.
>>     * `upstream` - push the current branch to ...
>>     * `simple` - like `upstream`, but refuses to ...
>>     * `tracking` - deprecated synonym for `upstream`.

>From a simple user perspective, I'd simply place it, in brackets and at 
the end of the list.

e.g.     [* `tracking` - deprecated synonym for `upstream`.]

The leading bracket makes it obvious that it's different, and that the 
description needs to be read, rather than folk simply re-using a half 
remembered option, gleaned from an old blog.

However formatting the leading bracket may not be as easy as the plain 
text version.

>>     * `current` - push the current branch to a branch of the same 
>> name.
>
> How about the following?
>
>    * `nothing` - ...
>    * `matching` - ...
>    * `upstream` - ...
>    * `simple` - ...
>    * `current` - ...
>
>  For compatibility with ancient config files, the following synonym
>  is also supported.  Don't use it.
>
>    * `tracking` - old name for `upstream`

? s/old/deprecated/  or  s/old/outdated/  for those who don't understand 
the jargon. 

Re: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Am I really the only one that doesn't see the "--print" change as
> hiding an option and sees burying "tracking" in the text as
> qualitatively different?

Sorry, but I do not understand the question.

We are hiding/burying the "--print" option to make it clear that it
is not a member with the same footing as others belonging to the
group of options to the command.  It is accepted, but there is no
reason for the user to choose it over --normalize.

We want to make sure that "tracking" does not appear as if it is a
member of the pull.default set with equal rights as others.  It is
accepted, but there is no reason for the user to choose it over
"upstream".

Re: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Junio C Hamano]

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

> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Am I really the only one that doesn't see the "--print" change as
>> hiding an option and sees burying "tracking" in the text as
>> qualitatively different?
>
> Sorry, but I do not understand the question.
>
> We are hiding/burying the "--print" option to make it clear that it
> is not a member with the same footing as others belonging to the
> group of options to the command.  It is accepted, but there is no
> reason for the user to choose it over --normalize.
>
> We want to make sure that "tracking" does not appear as if it is a
> member of the pull.default set with equal rights as others.  It is
> accepted, but there is no reason for the user to choose it over
> "upstream".

Now I re-read the proposed update, I think it is an improvement over
a straight revert in that it makes it clear that 'tracking' does not
belong, but I still think it is better to make it a sidenote to the
'upstream'.  It makes the connection between the two stronger.

Re: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Junio C Hamano]

How about doing it this way?  I do not think anything that is
deprecated even deserves a separate section and "do not use it!"
heading.

-- >8 --
When looking at a configuration file edited long time ago, a user
may find 'pull.default = tracking' and wonder what it means.
Instead of not mentioning it, add it to the description in a way
that makes it clear that users have no reason to add new uses of it
preferring over 'upstream'.

 Documentation/config.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index d7ec507..8441050 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1795,7 +1795,8 @@ push.default::
   +
   This is currently the default, but Git 2.0 will change the default
   to `simple`.
-* `upstream` - push the current branch to its upstream branch.
+* `upstream` - push the current branch to its upstream branch
+  (`tracking` is a deprecated synonym for this).
   With this, `git push` will update the same remote ref as the one which
   is merged by `git pull`, making `push` and `pull` symmetrical.
   See "branch.<name>.merge" for how to configure the upstream branch.

Re: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Jonathan Nieder]

Junio C Hamano wrote:

> How about doing it this way?
[...]
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1795,7 +1795,8 @@ push.default::
>    +
>    This is currently the default, but Git 2.0 will change the default
>    to `simple`.
> -* `upstream` - push the current branch to its upstream branch.
> +* `upstream` - push the current branch to its upstream branch
> +  (`tracking` is a deprecated synonym for this).

I have already explained that I believe this is a bad idea and why and
proposed an alternative.  I take it that either we are
miscommunicating or we fundamentally disagree about the role of
documentation. :(

Re: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Jonathan Nieder]

Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>> Junio C Hamano wrote:

>>>                                                      For those who
>>> want to _learn_ what possibilities are available to them (i.e. they
>>> are not going from `tracking` to what it means, but going in the
>>> opposite direction), it should be unmistakingly clear that
>>> `tracking` is not a part of the choices they should make.
>>
>> Until pre-1.7.4 versions of git fall out of use, I don't agree that
>> the above is true. :(
>
> The documentation ships with the version that the above is true.  We
> are not making an update to documentation that comes with ancient
> versions.

Part of the context that I should have mentioned but didn't is that it
is common to put $HOME on a shared filesystem.

[...]
>> How about the following?
>>
>>     * `nothing` - ...
>>     * `matching` - ...
>>     * `upstream` - ...
>>     * `simple` - ...
>>     * `current` - ...
>>
>>   For compatibility with ancient config files, the following synonym
>>   is also supported.  Don't use it.
>>
>>     * `tracking` - old name for `upstream`
>
> Didn't I say I am fine to mention it "as a side note" in the
> original message you started responding to?

Yes, I understood what you were proposing and I directly disagreed
with it and explained why.

The above is something like a compromise --- more precisely, it is an
attempt to do something better than a straight revert and to
understand whether it would address your objection.  Clearly it
doesn't.  I don't understand why.

Perhaps the "Don't use it" is over the top and that is your complaint?
It's true that if I were writing it without your objection in mind, I
wouldn't have included that sentence.  It was written on the
assumption that you want to discourage people from using the
"tracking" synonym --- I am not personally convinced that that is
worth discouraging at all, but it's fine with me if the consensus is
to do so.

Jonathan

Re: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking'

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> [...]
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -1795,7 +1795,8 @@ push.default::
>>    +
>>    This is currently the default, but Git 2.0 will change the default
>>    to `simple`.
>> -* `upstream` - push the current branch to its upstream branch.
>> +* `upstream` - push the current branch to its upstream branch
>> +  (`tracking` is a deprecated synonym for this).
>
> I have already explained that I believe this is a bad idea and why and
> proposed an alternative.  I take it that either we are
> miscommunicating or we fundamentally disagree about the role of
> documentation. :(

Whatever.

For tonight, I'll queue this version on 'pu' primarily because I do
not want to think about it anymore today and because I do not want
to see us forget that we have to fix this in some way, and this was
the only one that I can simply "git am" on this topic. It is not
because I want to say "this is the version we are going to use,
stfu!"  This topic does not even deserve such inter-developer
tension, IMHO.

    doc: mention tracking for pull.default
    
    When looking at a configuration file edited long time ago, a user
    may find 'pull.default = tracking' and wonder what it means, but
    earlier we stopped mentioning this value, even though the code still
    support it and more importantly, we have no intention to force old
    timers to update their configuration files.
    
    Instead of not mentioning it, add it to the description in a way
    that makes it clear that users have no reason to add new uses of it
    preferring over 'upstream', by not listing it as a separate item on
    the same footing as other values but as a deprecated synonym of the
    'upstream' in its description.
    
    Signed-off-by: Junio C Hamano <gitster@pobox.com>