Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows

[Junio C Hamano <gitster@pobox.com>]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Junio C Hamano wrote:
>>> Decouple `simple` from `upstream` completely, and change it to mean
>>> `current` with a safety feature: a `push` and `pull` should not be
>>> asymmetrical in the special case of central workflows.
>>
>> Double negation confused my parser.  'push' and 'pull' should be
>> kept symmetrical in central workflows?
>
> They're not the same thing.  It is very much intentional and intended:
> the safety net is not to "ensure that the push and pull are
> symmetrical" (i.e. among other things, error out if
> branch.$branch.merge is unset), but rather "ensure that the push and
> pull are never asymmetrical".

Hmmmm....

    not to "ensure that the push and pull are symmetrical"
    rather "ensure that the push and pull are never asymmetrical".

They still talk the same thing to me.  What am I missing?

Am I being clueless, or is there something else going on?

Your "among other things", after reading it three times,
unfortunately did not help clarify anything to me, so perhaps
somebody other than me (or you for that matter) who is more clueful
can help find a different way to explain the difference you are
trying to express to me.

Help, anybody?

>> Provided that we would want to keep the "Push the current one to the
>> same name but you have to have it set up as your integration source"
>> safety for central workflow (which I am starting to think we
>> should), we would want something like this on top of your entire
>> series, I think.  The behaviour change can be seen in the revert of
>> one test you made to the test that expects "simple" to fail due to
>> the safety.
>
> Now I'd like to question what you are labelling as "safety".  What is
> the consequence of erroring out when branch.$branch.merge is unset
> when pushing using `upstream`?

Nothing noteworthy.

I wasn't suggesting to change what `upstream` does at all.

If you do not have a `upstream` configured, we do not know what
branch you are integrating with, and the operation has failed in the
code even before we started adding triangular.

I do not see a reason to change that in the triangular world;
`upstream` is about the central workflow as you originally wrote in
the "config.txt" patch in this series.

The name of the branch the repository you fetch from and integrate
with does not have anything to do with the name you want to push
your derived work as to a different repository

I thought we already discussed and agreed on this point.

  http://thread.gmane.org/gmane.comp.version-control.git/227028/focus=227313

The conclusion is that using push.default=`upstream` is meaningless
when you are using a triangular workflow.  If you are using a
centralized workflow, and if a branch does not have branch.*.merge
configured, we do not know to which branch you are pushing it back,
so we error out.

What I am changing from the patch you posted with the "how about
this on top" patch back to the current behaviour is what 'simple'
does for centralized workflow.

> I didn't want to contaminate this series with an unrelated improvement
> to `upstream`

I think we share that, and it is not just `upstream`, but also
`simple` when it is applied to folks who employ a centralized
workflow.  The safety we need to keep is the one we have had since
day one of introducing 'simple' for them.

When you are doing a centralized workflow, 'simple' was defined to
be 'upstream' with added restriction that the branch at the remote
you integrate with must have the same name as the current branch you
are pushing, i.e.  in

    [branch "frotz"]
	merge = refs/heads/$branch

the value of $branch must be 'frotz'; otherwise 'simple' errors out.

  http://thread.gmane.org/gmane.comp.version-control.git/194175/focus=196199

Now, no existing series has casted in stone the best behaviour for
`simple` in a triangular workflow.  With this series (and also with
my fixup patch I sent last night), it is defined to act as `current`,
but it may need a bit more safety to help new users avoid pushing
branches they did not intend to (perhaps pushing out `current` only
when the branch with the same name already exists at the destination?
I dunno).