Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 19, 2013, 20:00 UTC
- Message-ID
- <7v38sdzx8o.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1371640304-26019-4-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
Show 13 quoted lines
> When remote.pushdefault or branch.<name>.pushremote is set (a triangular
> workflow feature), master@{u} != origin, and push.default is set to
> `upstream` or `simple`:
>
> $ git push
> fatal: You are pushing to remote 'origin', which is not the upstream of
> your current branch 'master', without telling me what to push
> to update which remote branch.
>
> Unfortunately, in the case of `upstream`, the very name indicates that
> it is only suitable for use in central workflows; let us not even
> attempt to give it a new meaning in triangular workflows, and error out
> as usual.Sensible.
Show 7 quoted lines
> However, the `simple` does not have this problem: it is poised to > be the default for Git 2.0, and we would definitely like it to do > something sensible in triangular workflows. > > 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?
> +* `simple` - a safer version of `current`; push the current branch to > + update a branch with the same name on the receiving end, with a > + safety feature: in central workflows, error out if > + branch.$branch.merge is set and not equal to $branch,
If branch.$branch.merge is _not_ set, what happens in the current code, and what should happen?
> + to make sure > + that a `push` and `push` are never asymmetrical. It will become the > + default in Git 2.0.
Ditto.
Show 15 quoted lines
> * `matching` - push all branches having the same name on both ends
> (essentially ignoring all newly created local branches).
> diff --git a/builtin/push.c b/builtin/push.c
> index 2d84d10..d8d27d9 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -120,6 +120,25 @@ static const char message_detached_head_die[] =
> "\n"
> " git push %s HEAD:<name-of-remote-branch>\n");
>
> +static void setup_push_simple(struct remote *remote)
> +{
> + struct branch *branch = branch_get(NULL);
> + if (!branch)
> + die(_(message_detached_head_die), remote->name);OK.
> + if (!branch->merge_nr || !branch->merge || !branch->remote_name) > + /* No upstream configured */ > + goto end;
Without any configuration the current branch is pushed out, which loosens the safety we implemented in the current 'safer upstream'.
I am not convinced this is a good change. I am not convinced this is a bad change, either, yet, but this loosening smells bad.
Show 12 quoted lines
> diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh > index 69ce6bf..e54dd02 100755 > --- a/t/t5528-push-default.sh > +++ b/t/t5528-push-default.sh > @@ -85,7 +85,7 @@ test_expect_success 'push from/to new branch with current creates remote 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_success simple master && > test_push_failure upstream > '
Likewise.