Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:57:50
Ramkumar Ramachandra [off-list ref] writes:
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.
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.
quoted hunk
* `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.
quoted hunk
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.