{"thread":{"id":"55796","subject":"[PATCH v2 1/6] push: hedge code of default=simple","startedAt":"2021-05-29T07:11:37Z","lastAt":"2021-06-01T23:53:09Z","messageCount":59,"participants":["Felipe Contreras","Philip Oakley","Ævar Arnfjörð Bjarmason","Mathias Kunter","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":6},"messages":[{"id":"425851","messageId":"20210529071115.1908310-2-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH v2 1/6] push: hedge code of default=simple","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:11:10Z","receivedAt":"2021-05-29T07:11:37Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"`simple` is the most important mode so move the relevant code to its own\nfunction to make it easier to see what it's doing.\n\nReviewed-by: Elijah Newren \nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 13 +++++++++----\n 1 file changed, 9 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 194967ed79..7045e4ef0c 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -223,6 +223,14 @@ static void setup_push_current(struct remote *remote, struct branch *branch)\n \trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n }\n \n+static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n+{\n+\tif (triangular)\n+\t\tsetup_push_current(remote, branch);\n+\telse\n+\t\tsetup_push_upstream(remote, branch, triangular, 1);\n+}\n+\n static int is_workflow_triangular(struct remote *remote)\n {\n \tstruct remote *fetch_remote = remote_get(NULL);\n@@ -242,10 +250,7 @@ static void setup_default_push_refspecs(struct remote *remote)\n \n \tcase PUSH_DEFAULT_UNSPECIFIED:\n \tcase PUSH_DEFAULT_SIMPLE:\n-\t\tif (triangular)\n-\t\t\tsetup_push_current(remote, branch);\n-\t\telse\n-\t\t\tsetup_push_upstream(remote, branch, triangular, 1);\n+\t\tsetup_push_simple(remote, branch, triangular);\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_UPSTREAM:\n-- \n2.32.0.rc0\n\n"},{"id":"425852","messageId":"20210529071115.1908310-1-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":null,"subject":"[PATCH v2 0/6] Unconvolutize push.default=simple","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:11:09Z","receivedAt":"2021-05-29T07:11:37Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Tired of jumping through hoops trying to understand what the \"simple\"\nmode does, I decided to reorganize it up for good so it's crystal\nclear.\n\nThere are no functional changes.\n\nBasically the simple mode pushes the current branch with the same name\non the remote.\n\nExcept... when there's no upstream branch configured with the same name.\n\nNow the code and the documentation are clear.\n\nThis is basically the same as v1, except I removed a bunch of patches which are now part of a\ndifferent series. Also, I updated some commit messages with suggestions from Elijah Newren.\n\nThe result of this series is in short this function:\n\nstatic void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n{\n\tif (!branch)\n\t\tdie(_(message_detached_head_die), remote->name);\n\n\tif (!triangular) {\n\t\tif (!branch->merge_nr || !branch->merge || !branch->remote_name)\n\t\t\tdie(_(\"The current branch %s has no upstream branch.\\n\"\n\t\t\t \"To push the current branch and set the remote as upstream, use\\n\"\n\t\t\t \"\\n\"\n\t\t\t \" git push --set-upstream %s %s\\n\"),\n\t\t\t branch->name,\n\t\t\t remote->name,\n\t\t\t branch->name);\n\t\tif (branch->merge_nr != 1)\n\t\t\tdie(_(\"The current branch %s has multiple upstream branches, \"\n\t\t\t \"refusing to push.\"), branch->name);\n\n\t\t/* Additional safety */\n\t\tif (strcmp(branch->refname, branch->merge[0]->src))\n\t\t\tdie_push_simple(branch, remote);\n\t}\n\trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n}\n\nFelipe Contreras (6):\n push: hedge code of default=simple\n push: move code to setup_push_simple()\n push: reorganize setup_push_simple()\n push: simplify setup_push_simple()\n push: remove unused code in setup_push_upstream()\n doc: push: explain default=simple correctly\n\n Documentation/config/push.txt | 13 ++++++------\n builtin/push.c | 40 ++++++++++++++++++++++++-----------\n 2 files changed, 34 insertions(+), 19 deletions(-)\n\nRange-diff against v1:\n 1: 2856711eb3 ! 1: f1f42bda32 push: hedge code of default=simple\n @@ Commit message\n `simple` is the most important mode so move the relevant code to its own\n function to make it easier to see what it's doing.\n \n + Reviewed-by: Elijah Newren \n Signed-off-by: Felipe Contreras \n \n ## builtin/push.c ##\n 2: 33acb09e82 ! 2: bb6d810011 push: move code to setup_push_simple()\n @@ Commit message\n \n The code is copied exactly as-is; no functional changes.\n \n + Reviewed-by: Elijah Newren \n Signed-off-by: Felipe Contreras \n \n ## builtin/push.c ##\n 3: de1b621b7e ! 3: d66a442fba push: reorganize setup_push_simple()\n @@ Metadata\n ## Commit message ##\n push: reorganize setup_push_simple()\n \n - Simply move the code around.\n + Simply move the code around and remove dead code. In particular the\n + 'trivial' conditional is a no-op since that part of the code is the\n + !trivial leg of the conditional beforehand.\n \n No functional changes.\n \n + Suggestions-by: Elijah Newren \n Signed-off-by: Felipe Contreras \n \n ## builtin/push.c ##\n 4: 83efcad143 ! 4: eaae6a826a push: simplify setup_push_simple()\n @@ Metadata\n ## Commit message ##\n push: simplify setup_push_simple()\n \n - branch->refname can never be different from branch->merge[0]->src.\n + There's a safety check to make sure branch->refname is not different\n + from branch->merge[0]->src, otherwise we die().\n \n + Therefore we always push to branch->refname.\n +\n + Suggestions-by: Elijah Newren \n Signed-off-by: Felipe Contreras \n \n ## builtin/push.c ##\n 5: d7489e9545 = 5: 8d9ae5e552 push: remove unused code in setup_push_upstream()\n 6: e693341403 < -: ---------- push: merge current and simple\n 7: 830a57c867 < -: ---------- push: remove redundant check\n 8: d2dded5998 < -: ---------- push: fix Yoda condition\n 9: 7528738091 < -: ---------- push: remove trivial function\n10: 1ae0546df6 < -: ---------- push: flip !triangular for centralized\n11: 3acd42e385 ! 6: b35bdf14dc doc: push: explain default=simple correctly\n @@ Metadata\n ## Commit message ##\n doc: push: explain default=simple correctly\n \n - Now that the code has been unconvolutized and it's clear what it's\n + Now that the code has been simplified and it's clear what it's\n actually doing, update the documentation to reflect that.\n \n Namely; the simple mode only barfs when working on a centralized\n-- \n2.32.0.rc0\n\n"},{"id":"425853","messageId":"20210529071115.1908310-5-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH v2 4/6] push: simplify setup_push_simple()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:11:13Z","receivedAt":"2021-05-29T07:11:37Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"There's a safety check to make sure branch->refname is not different\nfrom branch->merge[0]->src, otherwise we die().\n\nTherefore we always push to branch->refname.\n\nSuggestions-by: Elijah Newren \nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 10 ++--------\n 1 file changed, 2 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 9c807ed707..73fe083682 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -225,14 +225,10 @@ static void setup_push_current(struct remote *remote, struct branch *branch)\n \n static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n {\n-\tconst char *dst;\n-\n \tif (!branch)\n \t\tdie(_(message_detached_head_die), remote->name);\n \n-\tif (triangular) {\n-\t\tdst = branch->refname;\n-\t} else {\n+\tif (!triangular) {\n \t\tif (!branch->merge_nr || !branch->merge || !branch->remote_name)\n \t\t\tdie(_(\"The current branch %s has no upstream branch.\\n\"\n \t\t\t \"To push the current branch and set the remote as upstream, use\\n\"\n@@ -248,10 +244,8 @@ static void setup_push_simple(struct remote *remote, struct branch *branch, int\n \t\t/* Additional safety */\n \t\tif (strcmp(branch->refname, branch->merge[0]->src))\n \t\t\tdie_push_simple(branch, remote);\n-\n-\t\tdst = branch->merge[0]->src;\n \t}\n-\trefspec_appendf(&rs, \"%s:%s\", branch->refname, dst);\n+\trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n }\n \n static int is_workflow_triangular(struct remote *remote)\n-- \n2.32.0.rc0\n\n"},{"id":"425854","messageId":"20210529071115.1908310-3-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH v2 2/6] push: move code to setup_push_simple()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:11:11Z","receivedAt":"2021-05-29T07:11:37Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"In order to avoid doing unnecessary things and simplify it in further\npatches.\n\nThe code is copied exactly as-is; no functional changes.\n\nReviewed-by: Elijah Newren \nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 36 ++++++++++++++++++++++++++++++++----\n 1 file changed, 32 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 7045e4ef0c..d173c39283 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -225,10 +225,38 @@ static void setup_push_current(struct remote *remote, struct branch *branch)\n \n static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n {\n-\tif (triangular)\n-\t\tsetup_push_current(remote, branch);\n-\telse\n-\t\tsetup_push_upstream(remote, branch, triangular, 1);\n+\tif (triangular) {\n+\t\tif (!branch)\n+\t\t\tdie(_(message_detached_head_die), remote->name);\n+\t\trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n+\t} else {\n+\t\tif (!branch)\n+\t\t\tdie(_(message_detached_head_die), remote->name);\n+\t\tif (!branch->merge_nr || !branch->merge || !branch->remote_name)\n+\t\t\tdie(_(\"The current branch %s has no upstream branch.\\n\"\n+\t\t\t \"To push the current branch and set the remote as upstream, use\\n\"\n+\t\t\t \"\\n\"\n+\t\t\t \" git push --set-upstream %s %s\\n\"),\n+\t\t\t branch->name,\n+\t\t\t remote->name,\n+\t\t\t branch->name);\n+\t\tif (branch->merge_nr != 1)\n+\t\t\tdie(_(\"The current branch %s has multiple upstream branches, \"\n+\t\t\t \"refusing to push.\"), branch->name);\n+\t\tif (triangular)\n+\t\t\tdie(_(\"You are pushing to remote '%s', which is not the upstream of\\n\"\n+\t\t\t \"your current branch '%s', without telling me what to push\\n\"\n+\t\t\t \"to update which remote branch.\"),\n+\t\t\t remote->name, branch->name);\n+\n+\t\tif (1) {\n+\t\t\t/* Additional safety */\n+\t\t\tif (strcmp(branch->refname, branch->merge[0]->src))\n+\t\t\t\tdie_push_simple(branch, remote);\n+\t\t}\n+\n+\t\trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->merge[0]->src);\n+\t}\n }\n \n static int is_workflow_triangular(struct remote *remote)\n-- \n2.32.0.rc0\n\n"},{"id":"425855","messageId":"20210529071115.1908310-4-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH v2 3/6] push: reorganize setup_push_simple()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:11:12Z","receivedAt":"2021-05-29T07:11:37Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Simply move the code around and remove dead code. In particular the\n'trivial' conditional is a no-op since that part of the code is the\n!trivial leg of the conditional beforehand.\n\nNo functional changes.\n\nSuggestions-by: Elijah Newren \nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 29 ++++++++++++-----------------\n 1 file changed, 12 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex d173c39283..9c807ed707 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -225,13 +225,14 @@ static void setup_push_current(struct remote *remote, struct branch *branch)\n \n static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n {\n+\tconst char *dst;\n+\n+\tif (!branch)\n+\t\tdie(_(message_detached_head_die), remote->name);\n+\n \tif (triangular) {\n-\t\tif (!branch)\n-\t\t\tdie(_(message_detached_head_die), remote->name);\n-\t\trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n+\t\tdst = branch->refname;\n \t} else {\n-\t\tif (!branch)\n-\t\t\tdie(_(message_detached_head_die), remote->name);\n \t\tif (!branch->merge_nr || !branch->merge || !branch->remote_name)\n \t\t\tdie(_(\"The current branch %s has no upstream branch.\\n\"\n \t\t\t \"To push the current branch and set the remote as upstream, use\\n\"\n@@ -243,20 +244,14 @@ static void setup_push_simple(struct remote *remote, struct branch *branch, int\n \t\tif (branch->merge_nr != 1)\n \t\t\tdie(_(\"The current branch %s has multiple upstream branches, \"\n \t\t\t \"refusing to push.\"), branch->name);\n-\t\tif (triangular)\n-\t\t\tdie(_(\"You are pushing to remote '%s', which is not the upstream of\\n\"\n-\t\t\t \"your current branch '%s', without telling me what to push\\n\"\n-\t\t\t \"to update which remote branch.\"),\n-\t\t\t remote->name, branch->name);\n-\n-\t\tif (1) {\n-\t\t\t/* Additional safety */\n-\t\t\tif (strcmp(branch->refname, branch->merge[0]->src))\n-\t\t\t\tdie_push_simple(branch, remote);\n-\t\t}\n \n-\t\trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->merge[0]->src);\n+\t\t/* Additional safety */\n+\t\tif (strcmp(branch->refname, branch->merge[0]->src))\n+\t\t\tdie_push_simple(branch, remote);\n+\n+\t\tdst = branch->merge[0]->src;\n \t}\n+\trefspec_appendf(&rs, \"%s:%s\", branch->refname, dst);\n }\n \n static int is_workflow_triangular(struct remote *remote)\n-- \n2.32.0.rc0\n\n"},{"id":"425856","messageId":"20210529071115.1908310-7-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH v2 6/6] doc: push: explain default=simple correctly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:11:15Z","receivedAt":"2021-05-29T07:11:39Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Now that the code has been simplified and it's clear what it's\nactually doing, update the documentation to reflect that.\n\nNamely; the simple mode only barfs when working on a centralized\nworkflow, and there's no configured upstream branch with the same name.\n\nCc: Elijah Newren \nSigned-off-by: Felipe Contreras \n---\n Documentation/config/push.txt | 13 ++++++-------\n 1 file changed, 6 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/config/push.txt b/Documentation/config/push.txt\nindex f2667b2689..632033638c 100644\n--- a/Documentation/config/push.txt\n+++ b/Documentation/config/push.txt\n@@ -24,15 +24,14 @@ push.default::\n \n * `tracking` - This is a deprecated synonym for `upstream`.\n \n-* `simple` - in centralized workflow, work like `upstream` with an\n- added safety to refuse to push if the upstream branch's name is\n- different from the local one.\n+* `simple` - pushes the current branch with the same name on the remote.\n +\n-When pushing to a remote that is different from the remote you normally\n-pull from, work as `current`. This is the safest option and is suited\n-for beginners.\n+If you are working on a centralized workflow (pushing to the same repository you\n+pull from, which is typically `origin`), then you need to configure an upstream\n+branch with the same name.\n +\n-This mode has become the default in Git 2.0.\n+This mode is the default since Git 2.0, and is the safest option suited for\n+beginners.\n \n * `matching` - push all branches having the same name on both ends.\n This makes the repository you are pushing to remember the set of\n-- \n2.32.0.rc0\n\n"},{"id":"425857","messageId":"20210529071115.1908310-6-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH v2 5/6] push: remove unused code in setup_push_upstream()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:11:14Z","receivedAt":"2021-05-29T07:11:39Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Now it's not used for the simple mode.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 10 ++--------\n 1 file changed, 2 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 73fe083682..0961826269 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -186,7 +186,7 @@ static const char message_detached_head_die[] =\n \t \" git push %s HEAD:\\n\");\n \n static void setup_push_upstream(struct remote *remote, struct branch *branch,\n-\t\t\t\tint triangular, int simple)\n+\t\t\t\tint triangular)\n {\n \tif (!branch)\n \t\tdie(_(message_detached_head_die), remote->name);\n@@ -207,12 +207,6 @@ static void setup_push_upstream(struct remote *remote, struct branch *branch,\n \t\t \"to update which remote branch.\"),\n \t\t remote->name, branch->name);\n \n-\tif (simple) {\n-\t\t/* Additional safety */\n-\t\tif (strcmp(branch->refname, branch->merge[0]->src))\n-\t\t\tdie_push_simple(branch, remote);\n-\t}\n-\n \trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->merge[0]->src);\n }\n \n@@ -271,7 +265,7 @@ static void setup_default_push_refspecs(struct remote *remote)\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_UPSTREAM:\n-\t\tsetup_push_upstream(remote, branch, triangular, 0);\n+\t\tsetup_push_upstream(remote, branch, triangular);\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_CURRENT:\n-- \n2.32.0.rc0\n\n"},{"id":"425858","messageId":"20210529074458.1916817-1-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH 00/15] push: revamp push.default","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:44:43Z","receivedAt":"2021-05-29T07:45:05Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"This patch series is the new second part of [1].\n\nIt borrows a few patches from v1, but it's mostly a ton of cleanups.\n\nIn theory there should be no functional changes.\n\nThere's too many changes to list them all, it's much easier to see the result:\n\n\nstatic const char *get_upstream_ref(struct branch *branch, const char *remote_name)\n{\n\tif (!branch->merge_nr || !branch->merge || !branch->remote_name)\n\t\tdie(_(\"The current branch %s has no upstream branch.\\n\"\n\t\t \"To push the current branch and set the remote as upstream, use\\n\"\n\t\t \"\\n\"\n\t\t \" git push --set-upstream %s %s\\n\"),\n\t\t branch->name,\n\t\t remote_name,\n\t\t branch->name);\n\tif (branch->merge_nr != 1)\n\t\tdie(_(\"The current branch %s has multiple upstream branches, \"\n\t\t \"refusing to push.\"), branch->name);\n\n\treturn branch->merge[0]->src;\n}\n\nstatic void setup_default_push_refspecs(struct remote *remote)\n{\n\tstruct branch *branch;\n\tconst char *dst;\n\tint same_remote;\n\n\tswitch (push_default) {\n\tcase PUSH_DEFAULT_MATCHING:\n\t\trefspec_append(&rs, \":\");\n\t\treturn;\n\n\tcase PUSH_DEFAULT_NOTHING:\n\t\tdie(_(\"You didn't specify any refspecs to push, and \"\n\t\t \"push.default is \\\"nothing\\\".\"));\n\t\treturn;\n\tdefault:\n\t}\n\n\tbranch = branch_get(NULL);\n\tif (!branch)\n\t\tdie(_(message_detached_head_die), remote->name);\n\n\tdst = branch->refname;\n\tsame_remote = !strcmp(remote->name, remote_for_branch(branch, NULL));\n\n\tswitch (push_default) {\n\tdefault:\n\tcase PUSH_DEFAULT_UNSPECIFIED:\n\tcase PUSH_DEFAULT_SIMPLE:\n\t\tif (!same_remote)\n\t\t\tbreak;\n\t\tif (strcmp(branch->refname, get_upstream_ref(branch, remote->name)))\n\t\t\tdie_push_simple(branch, remote);\n\t\tbreak;\n\n\tcase PUSH_DEFAULT_UPSTREAM:\n\t\tif (!same_remote)\n\t\t\tdie(_(\"You are pushing to remote '%s', which is not the upstream of\\n\"\n\t\t\t \"your current branch '%s', without telling me what to push\\n\"\n\t\t\t \"to update which remote branch.\"),\n\t\t\t remote->name, branch->name);\n\t\tdst = get_upstream_ref(branch, remote->name);\n\t\tbreak;\n\n\tcase PUSH_DEFAULT_CURRENT:\n\t\tbreak;\n\t}\n\n\trefspec_appendf(&rs, \"%s:%s\", branch->refname, dst);\n}\n\n[1] https://lore.kernel.org/git/20210529071115.1908310-1-felipe.contreras@gmail.com/\n\nFelipe Contreras (15):\n push: create new get_upstream_ref() helper\n push: return immediately in trivial switch case\n push: reorder switch cases\n push: factor out null branch check\n push: only get the branch when needed\n push: make setup_push_* return the dst\n push: trivial simplifications\n push: get rid of all the setup_push_* functions\n push: factor out the typical case\n push: remove redundant check\n push: fix Yoda condition\n push: remove trivial function\n push: only get triangular when needed\n push: don't get a full remote object\n push: rename !triangular to same_remote\n\n builtin/push.c | 92 ++++++++++++++++++--------------------------------\n 1 file changed, 33 insertions(+), 59 deletions(-)\n\n-- \n2.32.0.rc0\n\n"},{"id":"425859","messageId":"20210529074458.1916817-2-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH 01/15] push: create new get_upstream_ref() helper","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:44:44Z","receivedAt":"2021-05-29T07:45:06Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"This code is duplicated among multiple functions.\n\nNo functional changes.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 36 ++++++++++++++++++------------------\n 1 file changed, 18 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 0961826269..39a271bb27 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -185,29 +185,37 @@ static const char message_detached_head_die[] =\n \t \"\\n\"\n \t \" git push %s HEAD:\\n\");\n \n-static void setup_push_upstream(struct remote *remote, struct branch *branch,\n-\t\t\t\tint triangular)\n+static const char *get_upstream_ref(struct branch *branch, const char *remote_name)\n {\n-\tif (!branch)\n-\t\tdie(_(message_detached_head_die), remote->name);\n \tif (!branch->merge_nr || !branch->merge || !branch->remote_name)\n \t\tdie(_(\"The current branch %s has no upstream branch.\\n\"\n \t\t \"To push the current branch and set the remote as upstream, use\\n\"\n \t\t \"\\n\"\n \t\t \" git push --set-upstream %s %s\\n\"),\n \t\t branch->name,\n-\t\t remote->name,\n+\t\t remote_name,\n \t\t branch->name);\n \tif (branch->merge_nr != 1)\n \t\tdie(_(\"The current branch %s has multiple upstream branches, \"\n \t\t \"refusing to push.\"), branch->name);\n+\n+\treturn branch->merge[0]->src;\n+}\n+\n+static void setup_push_upstream(struct remote *remote, struct branch *branch,\n+\t\t\t\tint triangular)\n+{\n+\tconst char *upstream_ref;\n+\tif (!branch)\n+\t\tdie(_(message_detached_head_die), remote->name);\n+\tupstream_ref = get_upstream_ref(branch, remote->name);\n \tif (triangular)\n \t\tdie(_(\"You are pushing to remote '%s', which is not the upstream of\\n\"\n \t\t \"your current branch '%s', without telling me what to push\\n\"\n \t\t \"to update which remote branch.\"),\n \t\t remote->name, branch->name);\n \n-\trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->merge[0]->src);\n+\trefspec_appendf(&rs, \"%s:%s\", branch->refname, upstream_ref);\n }\n \n static void setup_push_current(struct remote *remote, struct branch *branch)\n@@ -223,20 +231,12 @@ static void setup_push_simple(struct remote *remote, struct branch *branch, int\n \t\tdie(_(message_detached_head_die), remote->name);\n \n \tif (!triangular) {\n-\t\tif (!branch->merge_nr || !branch->merge || !branch->remote_name)\n-\t\t\tdie(_(\"The current branch %s has no upstream branch.\\n\"\n-\t\t\t \"To push the current branch and set the remote as upstream, use\\n\"\n-\t\t\t \"\\n\"\n-\t\t\t \" git push --set-upstream %s %s\\n\"),\n-\t\t\t branch->name,\n-\t\t\t remote->name,\n-\t\t\t branch->name);\n-\t\tif (branch->merge_nr != 1)\n-\t\t\tdie(_(\"The current branch %s has multiple upstream branches, \"\n-\t\t\t \"refusing to push.\"), branch->name);\n+\t\tconst char *upstream_ref;\n+\n+\t\tupstream_ref = get_upstream_ref(branch, remote->name);\n \n \t\t/* Additional safety */\n-\t\tif (strcmp(branch->refname, branch->merge[0]->src))\n+\t\tif (strcmp(branch->refname, upstream_ref))\n \t\t\tdie_push_simple(branch, remote);\n \t}\n \trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n-- \n2.32.0.rc0\n\n"},{"id":"425860","messageId":"20210529074458.1916817-3-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH 02/15] push: return immediately in trivial switch case","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:44:45Z","receivedAt":"2021-05-29T07:45:08Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"There's no need to break when nothing else will be executed.\n\nWill help next patches.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 39a271bb27..f4e919450d 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -257,25 +257,25 @@ static void setup_default_push_refspecs(struct remote *remote)\n \tdefault:\n \tcase PUSH_DEFAULT_MATCHING:\n \t\trefspec_append(&rs, \":\");\n-\t\tbreak;\n+\t\treturn;\n \n \tcase PUSH_DEFAULT_UNSPECIFIED:\n \tcase PUSH_DEFAULT_SIMPLE:\n \t\tsetup_push_simple(remote, branch, triangular);\n-\t\tbreak;\n+\t\treturn;\n \n \tcase PUSH_DEFAULT_UPSTREAM:\n \t\tsetup_push_upstream(remote, branch, triangular);\n-\t\tbreak;\n+\t\treturn;\n \n \tcase PUSH_DEFAULT_CURRENT:\n \t\tsetup_push_current(remote, branch);\n-\t\tbreak;\n+\t\treturn;\n \n \tcase PUSH_DEFAULT_NOTHING:\n \t\tdie(_(\"You didn't specify any refspecs to push, and \"\n \t\t \"push.default is \\\"nothing\\\".\"));\n-\t\tbreak;\n+\t\treturn;\n \t}\n }\n \n-- \n2.32.0.rc0\n\n"},{"id":"425861","messageId":"20210529074458.1916817-4-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH 03/15] push: reorder switch cases","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:44:46Z","receivedAt":"2021-05-29T07:45:10Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"We want all the cases that don't do anything with a branch first, and\nthen the rest.\n\nWill help further patches.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 15 +++++++++------\n 1 file changed, 9 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex f4e919450d..c19321bb9d 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -254,11 +254,19 @@ static void setup_default_push_refspecs(struct remote *remote)\n \tint triangular = is_workflow_triangular(remote);\n \n \tswitch (push_default) {\n-\tdefault:\n \tcase PUSH_DEFAULT_MATCHING:\n \t\trefspec_append(&rs, \":\");\n \t\treturn;\n \n+\tcase PUSH_DEFAULT_NOTHING:\n+\t\tdie(_(\"You didn't specify any refspecs to push, and \"\n+\t\t \"push.default is \\\"nothing\\\".\"));\n+\t\treturn;\n+\tdefault:\n+\t}\n+\n+\tswitch (push_default) {\n+\tdefault:\n \tcase PUSH_DEFAULT_UNSPECIFIED:\n \tcase PUSH_DEFAULT_SIMPLE:\n \t\tsetup_push_simple(remote, branch, triangular);\n@@ -271,11 +279,6 @@ static void setup_default_push_refspecs(struct remote *remote)\n \tcase PUSH_DEFAULT_CURRENT:\n \t\tsetup_push_current(remote, branch);\n \t\treturn;\n-\n-\tcase PUSH_DEFAULT_NOTHING:\n-\t\tdie(_(\"You didn't specify any refspecs to push, and \"\n-\t\t \"push.default is \\\"nothing\\\".\"));\n-\t\treturn;\n \t}\n }\n \n-- \n2.32.0.rc0\n\n"},{"id":"425862","messageId":"20210529074458.1916817-5-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH 04/15] push: factor out null branch check","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:44:47Z","receivedAt":"2021-05-29T07:45:11Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"No need to do it in every single function.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 10 +++-------\n 1 file changed, 3 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex c19321bb9d..a2abacf64d 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -206,8 +206,6 @@ static void setup_push_upstream(struct remote *remote, struct branch *branch,\n \t\t\t\tint triangular)\n {\n \tconst char *upstream_ref;\n-\tif (!branch)\n-\t\tdie(_(message_detached_head_die), remote->name);\n \tupstream_ref = get_upstream_ref(branch, remote->name);\n \tif (triangular)\n \t\tdie(_(\"You are pushing to remote '%s', which is not the upstream of\\n\"\n@@ -220,16 +218,11 @@ static void setup_push_upstream(struct remote *remote, struct branch *branch,\n \n static void setup_push_current(struct remote *remote, struct branch *branch)\n {\n-\tif (!branch)\n-\t\tdie(_(message_detached_head_die), remote->name);\n \trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n }\n \n static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n {\n-\tif (!branch)\n-\t\tdie(_(message_detached_head_die), remote->name);\n-\n \tif (!triangular) {\n \t\tconst char *upstream_ref;\n \n@@ -265,6 +258,9 @@ static void setup_default_push_refspecs(struct remote *remote)\n \tdefault:\n \t}\n \n+\tif (!branch)\n+\t\tdie(_(message_detached_head_die), remote->name);\n+\n \tswitch (push_default) {\n \tdefault:\n \tcase PUSH_DEFAULT_UNSPECIFIED:\n-- \n2.32.0.rc0\n\n"},{"id":"425863","messageId":"20210529074458.1916817-6-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH 05/15] push: only get the branch when needed","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:44:48Z","receivedAt":"2021-05-29T07:45:12Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras \n---\n builtin/push.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex a2abacf64d..4b3a14278a 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -243,7 +243,7 @@ static int is_workflow_triangular(struct remote *remote)\n \n static void setup_default_push_refspecs(struct remote *remote)\n {\n-\tstruct branch *branch = branch_get(NULL);\n+\tstruct branch *branch;\n \tint triangular = is_workflow_triangular(remote);\n \n \tswitch (push_default) {\n@@ -258,6 +258,7 @@ static void setup_default_push_refspecs(struct remote *remote)\n \tdefault:\n \t}\n \n+\tbranch = branch_get(NULL);\n \tif (!branch)\n \t\tdie(_(message_detached_head_die), remote->name);\n \n-- \n2.32.0.rc0\n\n"},{"id":"425864","messageId":"20210529074458.1916817-7-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH 06/15] push: make setup_push_* return the dst","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:44:49Z","receivedAt":"2021-05-29T07:45:22Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"All of the setup_push_* functions are appending a refspec. Do this only\nonce in the parent function.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 30 ++++++++++++++++--------------\n 1 file changed, 16 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 4b3a14278a..23b1908a5c 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -202,8 +202,8 @@ static const char *get_upstream_ref(struct branch *branch, const char *remote_na\n \treturn branch->merge[0]->src;\n }\n \n-static void setup_push_upstream(struct remote *remote, struct branch *branch,\n-\t\t\t\tint triangular)\n+static const char *setup_push_upstream(struct remote *remote, struct branch *branch,\n+\tint triangular)\n {\n \tconst char *upstream_ref;\n \tupstream_ref = get_upstream_ref(branch, remote->name);\n@@ -212,16 +212,15 @@ static void setup_push_upstream(struct remote *remote, struct branch *branch,\n \t\t \"your current branch '%s', without telling me what to push\\n\"\n \t\t \"to update which remote branch.\"),\n \t\t remote->name, branch->name);\n-\n-\trefspec_appendf(&rs, \"%s:%s\", branch->refname, upstream_ref);\n+\treturn upstream_ref;\n }\n \n-static void setup_push_current(struct remote *remote, struct branch *branch)\n+static const char *setup_push_current(struct remote *remote, struct branch *branch)\n {\n-\trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n+\treturn branch->refname;\n }\n \n-static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n+static const char *setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n {\n \tif (!triangular) {\n \t\tconst char *upstream_ref;\n@@ -232,7 +231,7 @@ static void setup_push_simple(struct remote *remote, struct branch *branch, int\n \t\tif (strcmp(branch->refname, upstream_ref))\n \t\t\tdie_push_simple(branch, remote);\n \t}\n-\trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n+\treturn branch->refname;\n }\n \n static int is_workflow_triangular(struct remote *remote)\n@@ -245,6 +244,7 @@ static void setup_default_push_refspecs(struct remote *remote)\n {\n \tstruct branch *branch;\n \tint triangular = is_workflow_triangular(remote);\n+\tconst char *dst;\n \n \tswitch (push_default) {\n \tcase PUSH_DEFAULT_MATCHING:\n@@ -266,17 +266,19 @@ static void setup_default_push_refspecs(struct remote *remote)\n \tdefault:\n \tcase PUSH_DEFAULT_UNSPECIFIED:\n \tcase PUSH_DEFAULT_SIMPLE:\n-\t\tsetup_push_simple(remote, branch, triangular);\n-\t\treturn;\n+\t\tdst = setup_push_simple(remote, branch, triangular);\n+\t\tbreak;\n \n \tcase PUSH_DEFAULT_UPSTREAM:\n-\t\tsetup_push_upstream(remote, branch, triangular);\n-\t\treturn;\n+\t\tdst = setup_push_upstream(remote, branch, triangular);\n+\t\tbreak;\n \n \tcase PUSH_DEFAULT_CURRENT:\n-\t\tsetup_push_current(remote, branch);\n-\t\treturn;\n+\t\tdst = setup_push_current(remote, branch);\n+\t\tbreak;\n \t}\n+\n+\trefspec_appendf(&rs, \"%s:%s\", branch->refname, dst);\n }\n \n static const char message_advice_pull_before_push[] =\n-- \n2.32.0.rc0\n\n"},{"id":"425865","messageId":"20210529074458.1916817-11-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH 10/15] push: remove redundant check","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:44:53Z","receivedAt":"2021-05-29T07:45:24Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"If fetch_remote is NULL (i.e. the branch remote is invalid), then it\ncan't possibly be same as remote, which can't be NULL.\n\nThe check is redundant, and so is the extra variable.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 94cb2eda63..7485522807 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -204,8 +204,7 @@ static const char *get_upstream_ref(struct branch *branch, const char *remote_na\n \n static int is_workflow_triangular(struct remote *remote)\n {\n-\tstruct remote *fetch_remote = remote_get(NULL);\n-\treturn (fetch_remote && fetch_remote != remote);\n+\treturn remote_get(NULL) != remote;\n }\n \n static void setup_default_push_refspecs(struct remote *remote)\n-- \n2.32.0.rc0\n\n"},{"id":"425866","messageId":"20210529074458.1916817-9-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH 08/15] push: get rid of all the setup_push_* functions","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:44:51Z","receivedAt":"2021-05-29T07:45:25Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Their code is much simpler now and can move into the parent function.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 38 +++++++++++---------------------------\n 1 file changed, 11 insertions(+), 27 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 21968abf6e..dbb4f78e61 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -202,30 +202,6 @@ static const char *get_upstream_ref(struct branch *branch, const char *remote_na\n \treturn branch->merge[0]->src;\n }\n \n-static const char *setup_push_upstream(struct remote *remote, struct branch *branch,\n-\tint triangular)\n-{\n-\tif (triangular)\n-\t\tdie(_(\"You are pushing to remote '%s', which is not the upstream of\\n\"\n-\t\t \"your current branch '%s', without telling me what to push\\n\"\n-\t\t \"to update which remote branch.\"),\n-\t\t remote->name, branch->name);\n-\treturn get_upstream_ref(branch, remote->name);\n-}\n-\n-static const char *setup_push_current(struct remote *remote, struct branch *branch)\n-{\n-\treturn branch->refname;\n-}\n-\n-static const char *setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n-{\n-\tif (!triangular)\n-\t\tif (strcmp(branch->refname, get_upstream_ref(branch, remote->name)))\n-\t\t\tdie_push_simple(branch, remote);\n-\treturn branch->refname;\n-}\n-\n static int is_workflow_triangular(struct remote *remote)\n {\n \tstruct remote *fetch_remote = remote_get(NULL);\n@@ -258,15 +234,23 @@ static void setup_default_push_refspecs(struct remote *remote)\n \tdefault:\n \tcase PUSH_DEFAULT_UNSPECIFIED:\n \tcase PUSH_DEFAULT_SIMPLE:\n-\t\tdst = setup_push_simple(remote, branch, triangular);\n+\t\tif (!triangular)\n+\t\t\tif (strcmp(branch->refname, get_upstream_ref(branch, remote->name)))\n+\t\t\t\tdie_push_simple(branch, remote);\n+\t\tdst = branch->refname;\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_UPSTREAM:\n-\t\tdst = setup_push_upstream(remote, branch, triangular);\n+\t\tif (triangular)\n+\t\t\tdie(_(\"You are pushing to remote '%s', which is not the upstream of\\n\"\n+\t\t\t \"your current branch '%s', without telling me what to push\\n\"\n+\t\t\t \"to update which remote branch.\"),\n+\t\t\t remote->name, branch->name);\n+\t\tdst = get_upstream_ref(branch, remote->name);\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_CURRENT:\n-\t\tdst = setup_push_current(remote, branch);\n+\t\tdst = branch->refname;\n \t\tbreak;\n \t}\n \n-- \n2.32.0.rc0\n\n"},{"id":"425867","messageId":"20210529074458.1916817-8-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH 07/15] push: trivial simplifications","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:44:50Z","receivedAt":"2021-05-29T07:45:26Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras \n---\n builtin/push.c | 14 +++-----------\n 1 file changed, 3 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 23b1908a5c..21968abf6e 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -205,14 +205,12 @@ static const char *get_upstream_ref(struct branch *branch, const char *remote_na\n static const char *setup_push_upstream(struct remote *remote, struct branch *branch,\n \tint triangular)\n {\n-\tconst char *upstream_ref;\n-\tupstream_ref = get_upstream_ref(branch, remote->name);\n \tif (triangular)\n \t\tdie(_(\"You are pushing to remote '%s', which is not the upstream of\\n\"\n \t\t \"your current branch '%s', without telling me what to push\\n\"\n \t\t \"to update which remote branch.\"),\n \t\t remote->name, branch->name);\n-\treturn upstream_ref;\n+\treturn get_upstream_ref(branch, remote->name);\n }\n \n static const char *setup_push_current(struct remote *remote, struct branch *branch)\n@@ -222,15 +220,9 @@ static const char *setup_push_current(struct remote *remote, struct branch *bran\n \n static const char *setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n {\n-\tif (!triangular) {\n-\t\tconst char *upstream_ref;\n-\n-\t\tupstream_ref = get_upstream_ref(branch, remote->name);\n-\n-\t\t/* Additional safety */\n-\t\tif (strcmp(branch->refname, upstream_ref))\n+\tif (!triangular)\n+\t\tif (strcmp(branch->refname, get_upstream_ref(branch, remote->name)))\n \t\t\tdie_push_simple(branch, remote);\n-\t}\n \treturn branch->refname;\n }\n \n-- \n2.32.0.rc0\n\n"},{"id":"425868","messageId":"20210529074458.1916817-10-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH 09/15] push: factor out the typical case","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:44:52Z","receivedAt":"2021-05-29T07:45:27Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Only override dst on the odd case.\n\nThis allows a preemptive break on the `simple` case.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 11 ++++++-----\n 1 file changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex dbb4f78e61..94cb2eda63 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -230,14 +230,16 @@ static void setup_default_push_refspecs(struct remote *remote)\n \tif (!branch)\n \t\tdie(_(message_detached_head_die), remote->name);\n \n+\tdst = branch->refname;\n+\n \tswitch (push_default) {\n \tdefault:\n \tcase PUSH_DEFAULT_UNSPECIFIED:\n \tcase PUSH_DEFAULT_SIMPLE:\n-\t\tif (!triangular)\n-\t\t\tif (strcmp(branch->refname, get_upstream_ref(branch, remote->name)))\n-\t\t\t\tdie_push_simple(branch, remote);\n-\t\tdst = branch->refname;\n+\t\tif (triangular)\n+\t\t\tbreak;\n+\t\tif (strcmp(branch->refname, get_upstream_ref(branch, remote->name)))\n+\t\t\tdie_push_simple(branch, remote);\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_UPSTREAM:\n@@ -250,7 +252,6 @@ static void setup_default_push_refspecs(struct remote *remote)\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_CURRENT:\n-\t\tdst = branch->refname;\n \t\tbreak;\n \t}\n \n-- \n2.32.0.rc0\n\n"},{"id":"425869","messageId":"20210529074458.1916817-12-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH 11/15] push: fix Yoda condition","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:44:54Z","receivedAt":"2021-05-29T07:45:28Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"We want to check if remote is the same as the branch remote, not the\nother way around.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 7485522807..468ccc1067 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -204,7 +204,7 @@ static const char *get_upstream_ref(struct branch *branch, const char *remote_na\n \n static int is_workflow_triangular(struct remote *remote)\n {\n-\treturn remote_get(NULL) != remote;\n+\treturn remote != remote_get(NULL);\n }\n \n static void setup_default_push_refspecs(struct remote *remote)\n-- \n2.32.0.rc0\n\n"},{"id":"425870","messageId":"20210529074458.1916817-15-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH 14/15] push: don't get a full remote object","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:44:57Z","receivedAt":"2021-05-29T07:45:29Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"All we need to know is that their names are different.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 818ca4c5c9..2f30a97b97 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -225,7 +225,7 @@ static void setup_default_push_refspecs(struct remote *remote)\n \t\tdie(_(message_detached_head_die), remote->name);\n \n \tdst = branch->refname;\n-\ttriangular = remote != remote_get(NULL);\n+\ttriangular = strcmp(remote->name, remote_for_branch(branch, NULL));\n \n \tswitch (push_default) {\n \tdefault:\n-- \n2.32.0.rc0\n\n"},{"id":"425871","messageId":"20210529074458.1916817-14-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH 13/15] push: only get triangular when needed","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:44:56Z","receivedAt":"2021-05-29T07:45:30Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras \n---\n builtin/push.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex c220f70795..818ca4c5c9 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -205,8 +205,8 @@ static const char *get_upstream_ref(struct branch *branch, const char *remote_na\n static void setup_default_push_refspecs(struct remote *remote)\n {\n \tstruct branch *branch;\n-\tint triangular = remote != remote_get(NULL);\n \tconst char *dst;\n+\tint triangular;\n \n \tswitch (push_default) {\n \tcase PUSH_DEFAULT_MATCHING:\n@@ -225,6 +225,7 @@ static void setup_default_push_refspecs(struct remote *remote)\n \t\tdie(_(message_detached_head_die), remote->name);\n \n \tdst = branch->refname;\n+\ttriangular = remote != remote_get(NULL);\n \n \tswitch (push_default) {\n \tdefault:\n-- \n2.32.0.rc0\n\n"},{"id":"425872","messageId":"20210529074458.1916817-13-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH 12/15] push: remove trivial function","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:44:55Z","receivedAt":"2021-05-29T07:45:31Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"It's a single line that is used in a single place, and the variable has\nthe same name as the function.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 7 +------\n 1 file changed, 1 insertion(+), 6 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 468ccc1067..c220f70795 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -202,15 +202,10 @@ static const char *get_upstream_ref(struct branch *branch, const char *remote_na\n \treturn branch->merge[0]->src;\n }\n \n-static int is_workflow_triangular(struct remote *remote)\n-{\n-\treturn remote != remote_get(NULL);\n-}\n-\n static void setup_default_push_refspecs(struct remote *remote)\n {\n \tstruct branch *branch;\n-\tint triangular = is_workflow_triangular(remote);\n+\tint triangular = remote != remote_get(NULL);\n \tconst char *dst;\n \n \tswitch (push_default) {\n-- \n2.32.0.rc0\n\n"},{"id":"425873","messageId":"20210529074458.1916817-16-felipe.contreras@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"[PATCH 15/15] push: rename !triangular to same_remote","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-29T07:44:58Z","receivedAt":"2021-05-29T07:45:33Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The typical case is what git was designed for: distributed remotes.\n\nIt's only the atypical case--fetching and pushing to the same\nremote--that we need to keep an eye on.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/push.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 2f30a97b97..f1ac531252 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -206,7 +206,7 @@ static void setup_default_push_refspecs(struct remote *remote)\n {\n \tstruct branch *branch;\n \tconst char *dst;\n-\tint triangular;\n+\tint same_remote;\n \n \tswitch (push_default) {\n \tcase PUSH_DEFAULT_MATCHING:\n@@ -225,20 +225,20 @@ static void setup_default_push_refspecs(struct remote *remote)\n \t\tdie(_(message_detached_head_die), remote->name);\n \n \tdst = branch->refname;\n-\ttriangular = strcmp(remote->name, remote_for_branch(branch, NULL));\n+\tsame_remote = !strcmp(remote->name, remote_for_branch(branch, NULL));\n \n \tswitch (push_default) {\n \tdefault:\n \tcase PUSH_DEFAULT_UNSPECIFIED:\n \tcase PUSH_DEFAULT_SIMPLE:\n-\t\tif (triangular)\n+\t\tif (!same_remote)\n \t\t\tbreak;\n \t\tif (strcmp(branch->refname, get_upstream_ref(branch, remote->name)))\n \t\t\tdie_push_simple(branch, remote);\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_UPSTREAM:\n-\t\tif (triangular)\n+\t\tif (!same_remote)\n \t\t\tdie(_(\"You are pushing to remote '%s', which is not the upstream of\\n\"\n \t\t\t \"your current branch '%s', without telling me what to push\\n\"\n \t\t\t \"to update which remote branch.\"),\n-- \n2.32.0.rc0\n\n"},{"id":"425891","messageId":"ad0751c3-654f-04d4-5ad1-c0aea91b46b8@iee.email","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 0/6] Unconvolutize push.default=simple","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2021-05-29T20:37:24Z","receivedAt":"2021-05-29T20:37:30Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 29/05/2021 08:11, Felipe Contreras wrote:\n> Tired of jumping through hoops trying to understand what the \"simple\"\n> mode does, I decided to reorganize it up for good so it's crystal\n> clear.\n>\n> There are no functional changes.\n>\n> Basically the simple mode pushes the current branch with the same name\n> on the remote.\n>\n> Except... when there's no upstream branch configured with the same name.\n>\n> Now the code and the documentation are clear.\n\nNot your problem, but I do note, as a general point, that we don't\nexplain the different variants of Triangular workflow anywhere [just two\nmentions in gitrevisions.txt] (e.g. patch flow versus server-side\nmerges, etc.). \n\n\nPhilip\n>\n> This is basically the same as v1, except I removed a bunch of patches which are now part of a\n> different series. Also, I updated some commit messages with suggestions from Elijah Newren.\n>\n> The result of this series is in short this function:\n>\n> static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n> {\n> \tif (!branch)\n> \t\tdie(_(message_detached_head_die), remote->name);\n>\n> \tif (!triangular) {\n> \t\tif (!branch->merge_nr || !branch->merge || !branch->remote_name)\n> \t\t\tdie(_(\"The current branch %s has no upstream branch.\\n\"\n> \t\t\t \"To push the current branch and set the remote as upstream, use\\n\"\n> \t\t\t \"\\n\"\n> \t\t\t \" git push --set-upstream %s %s\\n\"),\n> \t\t\t branch->name,\n> \t\t\t remote->name,\n> \t\t\t branch->name);\n> \t\tif (branch->merge_nr != 1)\n> \t\t\tdie(_(\"The current branch %s has multiple upstream branches, \"\n> \t\t\t \"refusing to push.\"), branch->name);\n>\n> \t\t/* Additional safety */\n> \t\tif (strcmp(branch->refname, branch->merge[0]->src))\n> \t\t\tdie_push_simple(branch, remote);\n> \t}\n> \trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n> }\n>\n> Felipe Contreras (6):\n> push: hedge code of default=simple\n> push: move code to setup_push_simple()\n> push: reorganize setup_push_simple()\n> push: simplify setup_push_simple()\n> push: remove unused code in setup_push_upstream()\n> doc: push: explain default=simple correctly\n>\n> Documentation/config/push.txt | 13 ++++++------\n> builtin/push.c | 40 ++++++++++++++++++++++++-----------\n> 2 files changed, 34 insertions(+), 19 deletions(-)\n>\n> Range-diff against v1:\n> 1: 2856711eb3 ! 1: f1f42bda32 push: hedge code of default=simple\n> @@ Commit message\n> `simple` is the most important mode so move the relevant code to its own\n> function to make it easier to see what it's doing.\n> \n> + Reviewed-by: Elijah Newren \n> Signed-off-by: Felipe Contreras \n> \n> ## builtin/push.c ##\n> 2: 33acb09e82 ! 2: bb6d810011 push: move code to setup_push_simple()\n> @@ Commit message\n> \n> The code is copied exactly as-is; no functional changes.\n> \n> + Reviewed-by: Elijah Newren \n> Signed-off-by: Felipe Contreras \n> \n> ## builtin/push.c ##\n> 3: de1b621b7e ! 3: d66a442fba push: reorganize setup_push_simple()\n> @@ Metadata\n> ## Commit message ##\n> push: reorganize setup_push_simple()\n> \n> - Simply move the code around.\n> + Simply move the code around and remove dead code. In particular the\n> + 'trivial' conditional is a no-op since that part of the code is the\n> + !trivial leg of the conditional beforehand.\n> \n> No functional changes.\n> \n> + Suggestions-by: Elijah Newren \n> Signed-off-by: Felipe Contreras \n> \n> ## builtin/push.c ##\n> 4: 83efcad143 ! 4: eaae6a826a push: simplify setup_push_simple()\n> @@ Metadata\n> ## Commit message ##\n> push: simplify setup_push_simple()\n> \n> - branch->refname can never be different from branch->merge[0]->src.\n> + There's a safety check to make sure branch->refname is not different\n> + from branch->merge[0]->src, otherwise we die().\n> \n> + Therefore we always push to branch->refname.\n> +\n> + Suggestions-by: Elijah Newren \n> Signed-off-by: Felipe Contreras \n> \n> ## builtin/push.c ##\n> 5: d7489e9545 = 5: 8d9ae5e552 push: remove unused code in setup_push_upstream()\n> 6: e693341403 < -: ---------- push: merge current and simple\n> 7: 830a57c867 < -: ---------- push: remove redundant check\n> 8: d2dded5998 < -: ---------- push: fix Yoda condition\n> 9: 7528738091 < -: ---------- push: remove trivial function\n> 10: 1ae0546df6 < -: ---------- push: flip !triangular for centralized\n> 11: 3acd42e385 ! 6: b35bdf14dc doc: push: explain default=simple correctly\n> @@ Metadata\n> ## Commit message ##\n> doc: push: explain default=simple correctly\n> \n> - Now that the code has been unconvolutized and it's clear what it's\n> + Now that the code has been simplified and it's clear what it's\n> actually doing, update the documentation to reflect that.\n> \n> Namely; the simple mode only barfs when working on a centralized\n\n"},{"id":"425913","messageId":"87a6oc318s.fsf@evledraar.gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-1-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 0/6] Unconvolutize push.default=simple","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-05-30T11:31:21Z","receivedAt":"2021-05-30T11:33:15Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, May 29 2021, Felipe Contreras wrote:\n\n> Tired of jumping through hoops trying to understand what the \"simple\"\n> mode does, I decided to reorganize it up for good so it's crystal\n> clear.\n\nI'd find the end-state even more readable if this \"triangular\" wasn't\npassed as a parameter but we just did the top-level dispatch based on\nthat, i.e. \"simple\" is really internally SIMPLE_SIMPLE and\nSIMPLE_TRIANGULAR, why not dispatch on that? Our internal enum doesn't\nneed to 1=1 map to the config setting.\n\nPart of that's that I prefer reading code in the current \"master\" which\nis if -> die -> most of the rest of function, v.s. your end-state of if\n-> stuff -> else -> most of the function being indented to that \"else\".\n"},{"id":"425927","messageId":"60b3bbd48b00b_b937208d7@natae.notmuch","threadId":"55796","inReplyTo":"87a6oc318s.fsf@evledraar.gmail.com","subject":"Re: [PATCH v2 0/6] Unconvolutize push.default=simple","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-30T16:22:44Z","receivedAt":"2021-05-30T16:22:49Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n> \n> On Sat, May 29 2021, Felipe Contreras wrote:\n> \n> > Tired of jumping through hoops trying to understand what the \"simple\"\n> > mode does, I decided to reorganize it up for good so it's crystal\n> > clear.\n> \n> I'd find the end-state even more readable if this \"triangular\" wasn't\n> passed as a parameter but we just did the top-level dispatch based on\n> that, i.e. \"simple\" is really internally SIMPLE_SIMPLE and\n> SIMPLE_TRIANGULAR, why not dispatch on that? Our internal enum doesn't\n> need to 1=1 map to the config setting.\n\nYes, but where are you going to make sure you are in SIMPLE, and not in\nCURRENT? Surely you are not suggesting to modify\ngit_default_push_config(), which is pretty straightforward. So we would\nneed yet another enum.\n\nI don't think there's a need for that, as you can see on the patch\nseries on top of this, there's not even a need to dispatch anything.\n\n> Part of that's that I prefer reading code in the current \"master\" which\n> is if -> die -> most of the rest of function, v.s. your end-state of if\n> -> stuff -> else -> most of the function being indented to that \"else\".\n\nI prefer that as well, but unfortunately in this case we need to do\nsomething after the checks to die(). I could add a goto, but for just\none conditional seems like it's not worth it.\n\nEither way this is a temporary thing, because in the next patch series\nthe code becomes pretty quickly:\n\n if (!triangular && strcmp(branch->refname, get_upstream_ref(branch, remote->name)))\n die_push_simple(branch, remote);\n\nAnd then:\n\n if (triangular)\n break;\n if (strcmp(branch->refname, get_upstream_ref(branch, remote->name)))\n die_push_simple(branch, remote);\n\nCheers.\n\n-- \nFelipe Contreras"},{"id":"425928","messageId":"60b3bd0fb6d4c_b937208dc@natae.notmuch","threadId":"55796","inReplyTo":"ad0751c3-654f-04d4-5ad1-c0aea91b46b8@iee.email","subject":"Re: [PATCH v2 0/6] Unconvolutize push.default=simple","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-30T16:27:59Z","receivedAt":"2021-05-30T16:28:05Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Philip Oakley wrote:\n> On 29/05/2021 08:11, Felipe Contreras wrote:\n> > Tired of jumping through hoops trying to understand what the \"simple\"\n> > mode does, I decided to reorganize it up for good so it's crystal\n> > clear.\n> >\n> > There are no functional changes.\n> >\n> > Basically the simple mode pushes the current branch with the same name\n> > on the remote.\n> >\n> > Except... when there's no upstream branch configured with the same name.\n> >\n> > Now the code and the documentation are clear.\n> \n> Not your problem, but I do note, as a general point, that we don't\n> explain the different variants of Triangular workflow anywhere [just two\n> mentions in gitrevisions.txt] (e.g. patch flow versus server-side\n> merges, etc.). \n\nAll my documentation improvements get stuck in tar, so somebody else\nwould need to do that.\n\nIf and when they do, it will become clear the big pit in functionality\nthere is.\n\n-- \nFelipe Contreras"},{"id":"425934","messageId":"acfafe6e-b0f3-ba2e-71f0-9bcc09b9d612@gmail.com","threadId":"55796","inReplyTo":"20210529071115.1908310-7-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 6/6] doc: push: explain default=simple correctly","fromName":"Mathias Kunter","fromEmail":"mathiaskunter@gmail.com","sentAt":"2021-05-30T18:19:03Z","receivedAt":"2021-05-30T18:19:10Z","isPatch":true,"sender":{"key":"mathiaskunter@gmail.com","avatar":"https://gravatar.com/avatar/131a813ba8d2cb36edb0d102971af5b523bad01aa9c842822e0d068b350bc4e8?d=mp&s=160"},"body":"Am 29.05.21 um 09:11 schrieb Felipe Contreras:\n> Now that the code has been simplified and it's clear what it's\n> actually doing, update the documentation to reflect that.\n> \n> Namely; the simple mode only barfs when working on a centralized\n> workflow, and there's no configured upstream branch with the same name.\n> \n> [...]\n> \n> +* `simple` - pushes the current branch with the same name on the remote.\n> +If you are working on a centralized workflow (pushing to the same repository you\n> +pull from, which is typically `origin`), then you need to configure an upstream\n> +branch with the same name.\n\nI'd like to remark that I personally find the following description of \n`push.default=simple`, taken from the git push man page [1], easier to \nunderstand:\n\n> The current branch is pushed to the corresponding upstream branch, but\n> as a safety measure, the push is aborted if the upstream branch does not\n> have the same name as the local one.\n\n[1] https://git-scm.com/docs/git-push#_description\n"},{"id":"425935","messageId":"60b3e2ceee840_ee4c20823@natae.notmuch","threadId":"55796","inReplyTo":"acfafe6e-b0f3-ba2e-71f0-9bcc09b9d612@gmail.com","subject":"Re: [PATCH v2 6/6] doc: push: explain default=simple correctly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-30T19:09:02Z","receivedAt":"2021-05-30T19:09:09Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Mathias Kunter wrote:\n> Am 29.05.21 um 09:11 schrieb Felipe Contreras:\n> > Now that the code has been simplified and it's clear what it's\n> > actually doing, update the documentation to reflect that.\n> > \n> > Namely; the simple mode only barfs when working on a centralized\n> > workflow, and there's no configured upstream branch with the same name.\n> > \n> > [...]\n> > \n> > +* `simple` - pushes the current branch with the same name on the remote.\n> > +If you are working on a centralized workflow (pushing to the same repository you\n> > +pull from, which is typically `origin`), then you need to configure an upstream\n> > +branch with the same name.\n> \n> I'd like to remark that I personally find the following description of \n> `push.default=simple`, taken from the git push man page [1], easier to \n> understand:\n> \n> > The current branch is pushed to the corresponding upstream branch, but\n> > as a safety measure, the push is aborted if the upstream branch does not\n> > have the same name as the local one.\n\nExcept that isn't accurate.\n\n git clone $url\n git checkout -b fix-1\n # do commits\n git push\n\nDoes that push the current branch to the corresponding upstream branch?\n\n-- \nFelipe Contreras\n"},{"id":"425960","messageId":"xmqqim2zy0jg.fsf@gitster.g","threadId":"55796","inReplyTo":"20210529071115.1908310-2-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 1/6] push: hedge code of default=simple","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-31T04:44:51Z","receivedAt":"2021-05-31T04:44:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> `simple` is the most important mode so move the relevant code to its own\n> function to make it easier to see what it's doing.\n\nThe change and the above description makes sense to me.\n\nI didn't, and still don't, understand the use of verb \"to hedge\" in\nthe title, though. Apparently it isn't \"to evade the risk of\ncommitment\", \"to protect oneself finantially\", and of course not \"to\nplant, form, or trim a hedge\".\n\n> Reviewed-by: Elijah Newren \n\nI trust Elijah would complain and/or clarify if this footer is\ninappropriate (I didn't see an explicit \"You can have my\nReviewed-by\", but only \"this looks good to me\", and didn't know\nwhat he meant).\n\nI do like the change of the phrasing from triangular to same-remote\nat the end of the extended series, by the way. It makes the code\nsimpler to read and much easier to reason about, and it would be\nnice to have it even before this step ;-)\n\n\n> Signed-off-by: Felipe Contreras \n> ---\n> builtin/push.c | 13 +++++++++----\n> 1 file changed, 9 insertions(+), 4 deletions(-)\n>\n> diff --git a/builtin/push.c b/builtin/push.c\n> index 194967ed79..7045e4ef0c 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -223,6 +223,14 @@ static void setup_push_current(struct remote *remote, struct branch *branch)\n> \trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n> }\n> \n> +static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n> +{\n> +\tif (triangular)\n> +\t\tsetup_push_current(remote, branch);\n> +\telse\n> +\t\tsetup_push_upstream(remote, branch, triangular, 1);\n> +}\n> +\n> static int is_workflow_triangular(struct remote *remote)\n> {\n> \tstruct remote *fetch_remote = remote_get(NULL);\n> @@ -242,10 +250,7 @@ static void setup_default_push_refspecs(struct remote *remote)\n> \n> \tcase PUSH_DEFAULT_UNSPECIFIED:\n> \tcase PUSH_DEFAULT_SIMPLE:\n> -\t\tif (triangular)\n> -\t\t\tsetup_push_current(remote, branch);\n> -\t\telse\n> -\t\t\tsetup_push_upstream(remote, branch, triangular, 1);\n> +\t\tsetup_push_simple(remote, branch, triangular);\n> \t\tbreak;\n> \n> \tcase PUSH_DEFAULT_UPSTREAM:\n"},{"id":"425961","messageId":"xmqqczt7xzm4.fsf@gitster.g","threadId":"55796","inReplyTo":"20210529071115.1908310-3-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 2/6] push: move code to setup_push_simple()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-31T05:04:51Z","receivedAt":"2021-05-31T05:04:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> In order to avoid doing unnecessary things and simplify it in further\n> patches.\n>\n> The code is copied exactly as-is; no functional changes.\n\nThe title says \"move\" and I expected to see corresponding deletions,\nbut it seems that this (possibly temporarily) duplicates what these\ntwo functions do, without removing them, which (possibly\ntemporarily) risks the two to drift apart. So, the resulting code\nfrom this step will do nothing wrong (it's just two function's\nbodies inlined in duplicated form), but depending on how the code\nevolves, it might turn out to be a good change or a bad change---we\ncannot judge by this step alone.\n\nFrom a quick scanning of the remainder of the series, it seems that\n3/6 and 4/6 improve the copied code without changing the behaviour,\nand at the end these two functions remain, so we have duplicated\nlogic for these two functions and improvements only live in one of\nthe copies (namely, in the setup_push_simple())? Would that be a\nproblem, or it is too much work to do better, I wonder?\n\nLet's keep reading.\n\n> Reviewed-by: Elijah Newren \n> Signed-off-by: Felipe Contreras \n> ---\n> builtin/push.c | 36 ++++++++++++++++++++++++++++++++----\n> 1 file changed, 32 insertions(+), 4 deletions(-)\n>\n> diff --git a/builtin/push.c b/builtin/push.c\n> index 7045e4ef0c..d173c39283 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -225,10 +225,38 @@ static void setup_push_current(struct remote *remote, struct branch *branch)\n> \n> static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n> {\n> -\tif (triangular)\n> -\t\tsetup_push_current(remote, branch);\n> -\telse\n> -\t\tsetup_push_upstream(remote, branch, triangular, 1);\n> +\tif (triangular) {\n> +\t\tif (!branch)\n> +\t\t\tdie(_(message_detached_head_die), remote->name);\n> +\t\trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n> +\t} else {\n> +\t\tif (!branch)\n> +\t\t\tdie(_(message_detached_head_die), remote->name);\n> +\t\tif (!branch->merge_nr || !branch->merge || !branch->remote_name)\n> +\t\t\tdie(_(\"The current branch %s has no upstream branch.\\n\"\n> +\t\t\t \"To push the current branch and set the remote as upstream, use\\n\"\n> +\t\t\t \"\\n\"\n> +\t\t\t \" git push --set-upstream %s %s\\n\"),\n> +\t\t\t branch->name,\n> +\t\t\t remote->name,\n> +\t\t\t branch->name);\n> +\t\tif (branch->merge_nr != 1)\n> +\t\t\tdie(_(\"The current branch %s has multiple upstream branches, \"\n> +\t\t\t \"refusing to push.\"), branch->name);\n> +\t\tif (triangular)\n> +\t\t\tdie(_(\"You are pushing to remote '%s', which is not the upstream of\\n\"\n> +\t\t\t \"your current branch '%s', without telling me what to push\\n\"\n> +\t\t\t \"to update which remote branch.\"),\n> +\t\t\t remote->name, branch->name);\n> +\n> +\t\tif (1) {\n> +\t\t\t/* Additional safety */\n> +\t\t\tif (strcmp(branch->refname, branch->merge[0]->src))\n> +\t\t\t\tdie_push_simple(branch, remote);\n> +\t\t}\n> +\n> +\t\trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->merge[0]->src);\n> +\t}\n> }\n> \n> static int is_workflow_triangular(struct remote *remote)\n"},{"id":"425962","messageId":"xmqq7djfxzm0.fsf@gitster.g","threadId":"55796","inReplyTo":"20210529071115.1908310-4-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 3/6] push: reorganize setup_push_simple()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-31T05:04:55Z","receivedAt":"2021-05-31T05:05:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> Simply move the code around and remove dead code. In particular the\n> 'trivial' conditional is a no-op since that part of the code is the\n> !trivial leg of the conditional beforehand.\n>\n> No functional changes.\n>\n> Suggestions-by: Elijah Newren \n> Signed-off-by: Felipe Contreras \n> ---\n> builtin/push.c | 29 ++++++++++++-----------------\n> 1 file changed, 12 insertions(+), 17 deletions(-)\n\nMuch cleaner. Nice.\n\n> diff --git a/builtin/push.c b/builtin/push.c\n> index d173c39283..9c807ed707 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -225,13 +225,14 @@ static void setup_push_current(struct remote *remote, struct branch *branch)\n> \n> static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n> {\n> +\tconst char *dst;\n> +\n> +\tif (!branch)\n> +\t\tdie(_(message_detached_head_die), remote->name);\n> +\n> \tif (triangular) {\n> -\t\tif (!branch)\n> -\t\t\tdie(_(message_detached_head_die), remote->name);\n> -\t\trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n> +\t\tdst = branch->refname;\n> \t} else {\n> -\t\tif (!branch)\n> -\t\t\tdie(_(message_detached_head_die), remote->name);\n> \t\tif (!branch->merge_nr || !branch->merge || !branch->remote_name)\n> \t\t\tdie(_(\"The current branch %s has no upstream branch.\\n\"\n> \t\t\t \"To push the current branch and set the remote as upstream, use\\n\"\n> @@ -243,20 +244,14 @@ static void setup_push_simple(struct remote *remote, struct branch *branch, int\n> \t\tif (branch->merge_nr != 1)\n> \t\t\tdie(_(\"The current branch %s has multiple upstream branches, \"\n> \t\t\t \"refusing to push.\"), branch->name);\n> -\t\tif (triangular)\n> -\t\t\tdie(_(\"You are pushing to remote '%s', which is not the upstream of\\n\"\n> -\t\t\t \"your current branch '%s', without telling me what to push\\n\"\n> -\t\t\t \"to update which remote branch.\"),\n> -\t\t\t remote->name, branch->name);\n> -\n> -\t\tif (1) {\n> -\t\t\t/* Additional safety */\n> -\t\t\tif (strcmp(branch->refname, branch->merge[0]->src))\n> -\t\t\t\tdie_push_simple(branch, remote);\n> -\t\t}\n> \n> -\t\trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->merge[0]->src);\n> +\t\t/* Additional safety */\n> +\t\tif (strcmp(branch->refname, branch->merge[0]->src))\n> +\t\t\tdie_push_simple(branch, remote);\n> +\n> +\t\tdst = branch->merge[0]->src;\n> \t}\n> +\trefspec_appendf(&rs, \"%s:%s\", branch->refname, dst);\n> }\n> \n> static int is_workflow_triangular(struct remote *remote)\n"},{"id":"425963","messageId":"xmqq1r9nxzly.fsf@gitster.g","threadId":"55796","inReplyTo":"20210529071115.1908310-5-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 4/6] push: simplify setup_push_simple()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-31T05:04:57Z","receivedAt":"2021-05-31T05:05:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> There's a safety check to make sure branch->refname is not different\n> from branch->merge[0]->src, otherwise we die().\n>\n> Therefore we always push to branch->refname.\n>\n> Suggestions-by: Elijah Newren \n> Signed-off-by: Felipe Contreras \n> ---\n> builtin/push.c | 10 ++--------\n> 1 file changed, 2 insertions(+), 8 deletions(-)\n\nSimpler and nicer. Good.\n\n\n> diff --git a/builtin/push.c b/builtin/push.c\n> index 9c807ed707..73fe083682 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -225,14 +225,10 @@ static void setup_push_current(struct remote *remote, struct branch *branch)\n> \n> static void setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n> {\n> -\tconst char *dst;\n> -\n> \tif (!branch)\n> \t\tdie(_(message_detached_head_die), remote->name);\n> \n> -\tif (triangular) {\n> -\t\tdst = branch->refname;\n> -\t} else {\n> +\tif (!triangular) {\n> \t\tif (!branch->merge_nr || !branch->merge || !branch->remote_name)\n> \t\t\tdie(_(\"The current branch %s has no upstream branch.\\n\"\n> \t\t\t \"To push the current branch and set the remote as upstream, use\\n\"\n> @@ -248,10 +244,8 @@ static void setup_push_simple(struct remote *remote, struct branch *branch, int\n> \t\t/* Additional safety */\n> \t\tif (strcmp(branch->refname, branch->merge[0]->src))\n> \t\t\tdie_push_simple(branch, remote);\n> -\n> -\t\tdst = branch->merge[0]->src;\n> \t}\n> -\trefspec_appendf(&rs, \"%s:%s\", branch->refname, dst);\n> +\trefspec_appendf(&rs, \"%s:%s\", branch->refname, branch->refname);\n> }\n> \n> static int is_workflow_triangular(struct remote *remote)\n"},{"id":"425964","messageId":"xmqqv96zwkl0.fsf@gitster.g","threadId":"55796","inReplyTo":"20210529071115.1908310-7-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 6/6] doc: push: explain default=simple correctly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-31T05:14:51Z","receivedAt":"2021-05-31T05:15:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> +If you are working on a centralized workflow (pushing to the same repository you\n> +pull from, which is typically `origin`), then you need to configure an upstream\n> +branch with the same name.\n\nI suspect it would be simpler to read and easier to understand to\nbring the parethesized part front, e.g.\n\n If you are pushing to the same repository you pull from (which\n is typically `origin`), then you need to ...\n\nas it would avoid \"the project is not centralized, but I push to my\nown repository and pull from it---what should I do?\" questions.\n\n> +\n> +This mode is the default since Git 2.0, and is the safest option suited for\n> +beginners.\n\nNice.\n"},{"id":"425970","messageId":"xmqqeednwgvo.fsf@gitster.g","threadId":"55796","inReplyTo":"20210529074458.1916817-4-felipe.contreras@gmail.com","subject":"Re: [PATCH 03/15] push: reorder switch cases","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-31T06:34:51Z","receivedAt":"2021-05-31T06:34:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> We want all the cases that don't do anything with a branch first, and\n> then the rest.\n>\n> Will help further patches.\n>\n> Signed-off-by: Felipe Contreras \n> ---\n> builtin/push.c | 15 +++++++++------\n> 1 file changed, 9 insertions(+), 6 deletions(-)\n>\n> diff --git a/builtin/push.c b/builtin/push.c\n> index f4e919450d..c19321bb9d 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -254,11 +254,19 @@ static void setup_default_push_refspecs(struct remote *remote)\n> \tint triangular = is_workflow_triangular(remote);\n> \n> \tswitch (push_default) {\n> -\tdefault:\n> \tcase PUSH_DEFAULT_MATCHING:\n> \t\trefspec_append(&rs, \":\");\n> \t\treturn;\n> \n> +\tcase PUSH_DEFAULT_NOTHING:\n> +\t\tdie(_(\"You didn't specify any refspecs to push, and \"\n> +\t\t \"push.default is \\\"nothing\\\".\"));\n> +\t\treturn;\n> +\tdefault:\n> +\t}\n> +\n> +\tswitch (push_default) {\n> +\tdefault:\n\nThis is not quite \"reorder\" but split into two. It is not yet clear\nhow it helps, but hopefully we'll find out why splitting the switch\ninto two switches is a good idea soon in a later step, so the title\nneeds updating to sell that aspect of the change (unless it is a\npointless change made by mistake that the originally-single switch\ngot split into two, but that is unlikely the case ;-).\n\n> \tcase PUSH_DEFAULT_UNSPECIFIED:\n> \tcase PUSH_DEFAULT_SIMPLE:\n> \t\tsetup_push_simple(remote, branch, triangular);\n> @@ -271,11 +279,6 @@ static void setup_default_push_refspecs(struct remote *remote)\n> \tcase PUSH_DEFAULT_CURRENT:\n> \t\tsetup_push_current(remote, branch);\n> \t\treturn;\n> -\n> -\tcase PUSH_DEFAULT_NOTHING:\n> -\t\tdie(_(\"You didn't specify any refspecs to push, and \"\n> -\t\t \"push.default is \\\"nothing\\\".\"));\n> -\t\treturn;\n> \t}\n> }\n"},{"id":"425973","messageId":"xmqq8s3vwgf0.fsf@gitster.g","threadId":"55796","inReplyTo":"20210529074458.1916817-6-felipe.contreras@gmail.com","subject":"Re: [PATCH 05/15] push: only get the branch when needed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-31T06:44:51Z","receivedAt":"2021-05-31T06:44:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> Signed-off-by: Felipe Contreras \n> ---\n> builtin/push.c | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n\n> diff --git a/builtin/push.c b/builtin/push.c\n> index a2abacf64d..4b3a14278a 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -243,7 +243,7 @@ static int is_workflow_triangular(struct remote *remote)\n> \n> static void setup_default_push_refspecs(struct remote *remote)\n> {\n> -\tstruct branch *branch = branch_get(NULL);\n> +\tstruct branch *branch;\n> \tint triangular = is_workflow_triangular(remote);\n> \n> \tswitch (push_default) {\n> @@ -258,6 +258,7 @@ static void setup_default_push_refspecs(struct remote *remote)\n> \tdefault:\n\nNot a fault of this step, but please make it a habit to have\n\"break;\" here. case label with absolutely no body just looks\nstrange and is distracting to the eyes.\n\n> \t}\n> \n> +\tbranch = branch_get(NULL);\n> \tif (!branch)\n> \t\tdie(_(message_detached_head_die), remote->name);\n\nThis step is the true justification for the splitting of a single\nswitch into two switches done in [03/15]. Makes quite a lot of\nsense.\n\n"},{"id":"425974","messageId":"xmqq35u3wgew.fsf@gitster.g","threadId":"55796","inReplyTo":"20210529074458.1916817-9-felipe.contreras@gmail.com","subject":"Re: [PATCH 08/15] push: get rid of all the setup_push_* functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-31T06:44:55Z","receivedAt":"2021-05-31T06:45:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> Their code is much simpler now and can move into the parent function.\n>\n> Signed-off-by: Felipe Contreras \n> ---\n> builtin/push.c | 38 +++++++++++---------------------------\n> 1 file changed, 11 insertions(+), 27 deletions(-)\n\nFinally, this resolves the risk of deliberately duplicated code of\ndiverging, which was made in the earlier series. Without seeing\nthis step, it has been dubious if the earlier change was a good one,\nbut with this step, it is a very clear win.\n\nNicely done.\n\n> diff --git a/builtin/push.c b/builtin/push.c\n> index 21968abf6e..dbb4f78e61 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -202,30 +202,6 @@ static const char *get_upstream_ref(struct branch *branch, const char *remote_na\n> \treturn branch->merge[0]->src;\n> }\n> \n> -static const char *setup_push_upstream(struct remote *remote, struct branch *branch,\n> -\tint triangular)\n> -{\n> -\tif (triangular)\n> -\t\tdie(_(\"You are pushing to remote '%s', which is not the upstream of\\n\"\n> -\t\t \"your current branch '%s', without telling me what to push\\n\"\n> -\t\t \"to update which remote branch.\"),\n> -\t\t remote->name, branch->name);\n> -\treturn get_upstream_ref(branch, remote->name);\n> -}\n> -\n> -static const char *setup_push_current(struct remote *remote, struct branch *branch)\n> -{\n> -\treturn branch->refname;\n> -}\n> -\n> -static const char *setup_push_simple(struct remote *remote, struct branch *branch, int triangular)\n> -{\n> -\tif (!triangular)\n> -\t\tif (strcmp(branch->refname, get_upstream_ref(branch, remote->name)))\n> -\t\t\tdie_push_simple(branch, remote);\n> -\treturn branch->refname;\n> -}\n> -\n> static int is_workflow_triangular(struct remote *remote)\n> {\n> \tstruct remote *fetch_remote = remote_get(NULL);\n> @@ -258,15 +234,23 @@ static void setup_default_push_refspecs(struct remote *remote)\n> \tdefault:\n> \tcase PUSH_DEFAULT_UNSPECIFIED:\n> \tcase PUSH_DEFAULT_SIMPLE:\n> -\t\tdst = setup_push_simple(remote, branch, triangular);\n> +\t\tif (!triangular)\n> +\t\t\tif (strcmp(branch->refname, get_upstream_ref(branch, remote->name)))\n> +\t\t\t\tdie_push_simple(branch, remote);\n> +\t\tdst = branch->refname;\n> \t\tbreak;\n> \n> \tcase PUSH_DEFAULT_UPSTREAM:\n> -\t\tdst = setup_push_upstream(remote, branch, triangular);\n> +\t\tif (triangular)\n> +\t\t\tdie(_(\"You are pushing to remote '%s', which is not the upstream of\\n\"\n> +\t\t\t \"your current branch '%s', without telling me what to push\\n\"\n> +\t\t\t \"to update which remote branch.\"),\n> +\t\t\t remote->name, branch->name);\n> +\t\tdst = get_upstream_ref(branch, remote->name);\n> \t\tbreak;\n> \n> \tcase PUSH_DEFAULT_CURRENT:\n> -\t\tdst = setup_push_current(remote, branch);\n> +\t\tdst = branch->refname;\n> \t\tbreak;\n> \t}\n"},{"id":"425975","messageId":"xmqqwnrfv1uf.fsf@gitster.g","threadId":"55796","inReplyTo":"20210529074458.1916817-12-felipe.contreras@gmail.com","subject":"Re: [PATCH 11/15] push: fix Yoda condition","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-31T06:44:56Z","receivedAt":"2021-05-31T06:45:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> We want to check if remote is the same as the branch remote, not the\n> other way around.\n>\n> Signed-off-by: Felipe Contreras \n> ---\n> builtin/push.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n\nThis is 10/15's making, and squashing them together will make both\nthe entire series (because doing so means that the reviewers need to\nsee one fewer step) and 10/15 (because the end result of the conversion\nis more \"logical\") easier to review (and later read in \"git log -p\").\n\n>\n> diff --git a/builtin/push.c b/builtin/push.c\n> index 7485522807..468ccc1067 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -204,7 +204,7 @@ static const char *get_upstream_ref(struct branch *branch, const char *remote_na\n> \n> static int is_workflow_triangular(struct remote *remote)\n> {\n> -\treturn remote_get(NULL) != remote;\n> +\treturn remote != remote_get(NULL);\n> }\n> \n> static void setup_default_push_refspecs(struct remote *remote)\n"},{"id":"425977","messageId":"xmqqr1hnv1no.fsf@gitster.g","threadId":"55796","inReplyTo":"20210529074458.1916817-14-felipe.contreras@gmail.com","subject":"Re: [PATCH 13/15] push: only get triangular when needed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-31T06:48:59Z","receivedAt":"2021-05-31T06:49:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> Signed-off-by: Felipe Contreras \n> ---\n> builtin/push.c | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/push.c b/builtin/push.c\n> index c220f70795..818ca4c5c9 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -205,8 +205,8 @@ static const char *get_upstream_ref(struct branch *branch, const char *remote_na\n> static void setup_default_push_refspecs(struct remote *remote)\n> {\n> \tstruct branch *branch;\n> -\tint triangular = remote != remote_get(NULL);\n> \tconst char *dst;\n> +\tint triangular;\n> \n> \tswitch (push_default) {\n> \tcase PUSH_DEFAULT_MATCHING:\n> @@ -225,6 +225,7 @@ static void setup_default_push_refspecs(struct remote *remote)\n> \t\tdie(_(message_detached_head_die), remote->name);\n> \n> \tdst = branch->refname;\n> +\ttriangular = remote != remote_get(NULL);\n> \n> \tswitch (push_default) {\n> \tdefault:\n\nThis is another change that got helped by an earlier split.\nGood.\n"},{"id":"425980","messageId":"3e5c2303-030e-928e-af57-8e2be8f3ce86@gmail.com","threadId":"55796","inReplyTo":"60b3e2ceee840_ee4c20823@natae.notmuch","subject":"Re: [PATCH v2 6/6] doc: push: explain default=simple correctly","fromName":"Mathias Kunter","fromEmail":"mathiaskunter@gmail.com","sentAt":"2021-05-31T07:12:46Z","receivedAt":"2021-05-31T07:12:57Z","isPatch":true,"sender":{"key":"mathiaskunter@gmail.com","avatar":"https://gravatar.com/avatar/131a813ba8d2cb36edb0d102971af5b523bad01aa9c842822e0d068b350bc4e8?d=mp&s=160"},"body":"Am 30.05.21 um 21:09 schrieb Felipe Contreras:\n>>> The current branch is pushed to the corresponding upstream branch, but\n>>> as a safety measure, the push is aborted if the upstream branch does not\n>>> have the same name as the local one.\n> \n> Except that isn't accurate.\n> \n> git clone $url\n> git checkout -b fix-1\n> # do commits\n> git push\n> \n> Does that push the current branch to the corresponding upstream branch?\n\nI see. Then maybe reword to something like this:\n\n> The current branch is pushed to a branch with the same name on the\n> remote, but as a safety measure, the push is aborted if a corresponding\n> upstream branch does not have the same name as the local one.\n\nIn your above example, I'm in centralized workflow, but I can still push \nthe fix-1 branch to origin without having to configure an upstream \nbranch for it. So this seems to contradict with the currently proposed \nwording:\n\n> If you are working on a centralized workflow, then you need to configure\n> an upstream branch with the same name.\n"},{"id":"425984","messageId":"60b495442d1a0_24d2820857@natae.notmuch","threadId":"55796","inReplyTo":"xmqqim2zy0jg.fsf@gitster.g","subject":"Re: [PATCH v2 1/6] push: hedge code of default=simple","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-31T07:50:28Z","receivedAt":"2021-05-31T07:50:40Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Felipe Contreras writes:\n> \n> > `simple` is the most important mode so move the relevant code to its own\n> > function to make it easier to see what it's doing.\n> \n> The change and the above description makes sense to me.\n> \n> I didn't, and still don't, understand the use of verb \"to hedge\" in\n> the title, though.\n\n> Apparently it isn't \"to evade the risk of\n> commitment\", \"to protect oneself finantially\", and of course not \"to\n> plant, form, or trim a hedge\".\n\nThose appear to be the intransitive verb definitions of Merriam-Webster\n[1], there's an object in the sentence (code), so it's the transitive\nones that are applicable:\n\n * to enclose or protect with or as if with a dense row of shrubs or low\n trees: ENCIRCLE\n * to confine so as to prevent freedom of movement or action\n\nSome synonyms: block, border, cage, confine, coop, corral, edge, fence,\nrestrict, ring, surround.\n\nI think it fits.\n\n> > Reviewed-by: Elijah Newren \n> \n> I trust Elijah would complain and/or clarify if this footer is\n> inappropriate (I didn't see an explicit \"You can have my\n> Reviewed-by\", but only \"this looks good to me\", and didn't know\n> what he meant).\n\nBut he did review the entire series. So I think it's safe to say he\nreviewed this patch.\n\n> I do like the change of the phrasing from triangular to same-remote\n> at the end of the extended series, by the way. It makes the code\n> simpler to read and much easier to reason about, and it would be\n> nice to have it even before this step ;-)\n\nAll right, I'll keep in mind for the next round.\n\nCheers.\n\n[1] https://www.merriam-webster.com/dictionary/hedge\n\n-- \nFelipe Contreras\n"},{"id":"425986","messageId":"xmqqh7ijuylw.fsf@gitster.g","threadId":"55796","inReplyTo":"20210529074458.1916817-16-felipe.contreras@gmail.com","subject":"Re: [PATCH 15/15] push: rename !triangular to same_remote","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-31T07:54:51Z","receivedAt":"2021-05-31T07:54:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> The typical case is what git was designed for: distributed remotes.\n>\n> It's only the atypical case--fetching and pushing to the same\n> remote--that we need to keep an eye on.\n\nYup. Avoiding the phrase \"centralized\" and using \"same_remote\"\nmakes quite a lot of sense, too.\n\nOverall the end-result of the series is quite pleasant read, even\nthough some intermediate states in the middle risked readers to\nworry about \"are we going in the right direction?\"\n\nWell done.\n"},{"id":"425987","messageId":"60b498599fbde_24d28208d4@natae.notmuch","threadId":"55796","inReplyTo":"xmqqczt7xzm4.fsf@gitster.g","subject":"Re: [PATCH v2 2/6] push: move code to setup_push_simple()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-31T08:03:37Z","receivedAt":"2021-05-31T08:03:44Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Felipe Contreras writes:\n> \n> > In order to avoid doing unnecessary things and simplify it in further\n> > patches.\n> >\n> > The code is copied exactly as-is; no functional changes.\n> \n> The title says \"move\" and I expected to see corresponding deletions,\n> but it seems that this (possibly temporarily) duplicates what these\n> two functions do, without removing them, which (possibly\n> temporarily) risks the two to drift apart.\n\nOK. Copy then.\n\n> From a quick scanning of the remainder of the series, it seems that\n> 3/6 and 4/6 improve the copied code without changing the behaviour,\n> and at the end these two functions remain, so we have duplicated\n> logic for these two functions and improvements only live in one of\n> the copies (namely, in the setup_push_simple())?\n\nNot quite: setup_push_upstream() was simplified too, in 5/6.\n\nMaybe I can mention that part of the code of the `simple` mode lives in\nsetup_push_upstream() and by copying the code to the right function both\ncan be simplified.\n\n-- \nFelipe Contreras\n"},{"id":"425988","messageId":"60b49a3ae829b_24d28208fd@natae.notmuch","threadId":"55796","inReplyTo":"xmqqv96zwkl0.fsf@gitster.g","subject":"Re: [PATCH v2 6/6] doc: push: explain default=simple correctly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-31T08:11:38Z","receivedAt":"2021-05-31T08:11:45Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Felipe Contreras writes:\n> \n> > +If you are working on a centralized workflow (pushing to the same repository you\n> > +pull from, which is typically `origin`), then you need to configure an upstream\n> > +branch with the same name.\n> \n> I suspect it would be simpler to read and easier to understand to\n> bring the parethesized part front, e.g.\n> \n> If you are pushing to the same repository you pull from (which\n> is typically `origin`), then you need to ...\n> \n> as it would avoid \"the project is not centralized, but I push to my\n> own repository and pull from it---what should I do?\" questions.\n\nThe top of `push.default says:\n\n Different values are well-suited for specific workflows; for instance,\n in a purely central workflow (i.e. the fetch source is equal to the\n push destination), `upstream` is probably what you want.\n\nWe already brought up the central workflow, I think it's fine to reuse\nthe concept below.\n\n-- \nFelipe Contreras\n"},{"id":"425989","messageId":"60b49ae37803f_24d282085c@natae.notmuch","threadId":"55796","inReplyTo":"xmqqeednwgvo.fsf@gitster.g","subject":"Re: [PATCH 03/15] push: reorder switch cases","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-31T08:14:27Z","receivedAt":"2021-05-31T08:14:34Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Felipe Contreras writes:\n> \n> > We want all the cases that don't do anything with a branch first, and\n> > then the rest.\n> >\n> > Will help further patches.\n> >\n> > Signed-off-by: Felipe Contreras \n> > ---\n> > builtin/push.c | 15 +++++++++------\n> > 1 file changed, 9 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/builtin/push.c b/builtin/push.c\n> > index f4e919450d..c19321bb9d 100644\n> > --- a/builtin/push.c\n> > +++ b/builtin/push.c\n> > @@ -254,11 +254,19 @@ static void setup_default_push_refspecs(struct remote *remote)\n> > \tint triangular = is_workflow_triangular(remote);\n> > \n> > \tswitch (push_default) {\n> > -\tdefault:\n> > \tcase PUSH_DEFAULT_MATCHING:\n> > \t\trefspec_append(&rs, \":\");\n> > \t\treturn;\n> > \n> > +\tcase PUSH_DEFAULT_NOTHING:\n> > +\t\tdie(_(\"You didn't specify any refspecs to push, and \"\n> > +\t\t \"push.default is \\\"nothing\\\".\"));\n> > +\t\treturn;\n> > +\tdefault:\n> > +\t}\n> > +\n> > +\tswitch (push_default) {\n> > +\tdefault:\n> \n> This is not quite \"reorder\" but split into two.\n\nOK.\n\n> It is not yet clear\n> how it helps, but hopefully we'll find out why splitting the switch\n> into two switches is a good idea soon in a later step, so the title\n> needs updating to sell that aspect of the change\n\nHow about: By splitting the two kinds of modes we can return sooner in\nthe case the mode doesn't need to do anything else, and for the rest\nthat reuse quite a bit of code it can be set up after the first switch.\n\n-- \nFelipe Contreras\n"},{"id":"425990","messageId":"60b49b48de634_24d2820854@natae.notmuch","threadId":"55796","inReplyTo":"xmqq8s3vwgf0.fsf@gitster.g","subject":"Re: [PATCH 05/15] push: only get the branch when needed","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-31T08:16:08Z","receivedAt":"2021-05-31T08:16:15Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Felipe Contreras writes:\n> \n> > Signed-off-by: Felipe Contreras \n> > ---\n> > builtin/push.c | 3 ++-\n> > 1 file changed, 2 insertions(+), 1 deletion(-)\n> \n> > diff --git a/builtin/push.c b/builtin/push.c\n> > index a2abacf64d..4b3a14278a 100644\n> > --- a/builtin/push.c\n> > +++ b/builtin/push.c\n> > @@ -243,7 +243,7 @@ static int is_workflow_triangular(struct remote *remote)\n> > \n> > static void setup_default_push_refspecs(struct remote *remote)\n> > {\n> > -\tstruct branch *branch = branch_get(NULL);\n> > +\tstruct branch *branch;\n> > \tint triangular = is_workflow_triangular(remote);\n> > \n> > \tswitch (push_default) {\n> > @@ -258,6 +258,7 @@ static void setup_default_push_refspecs(struct remote *remote)\n> > \tdefault:\n> \n> Not a fault of this step, but please make it a habit to have\n> \"break;\" here. case label with absolutely no body just looks\n> strange and is distracting to the eyes.\n\nAll right.\n\n> > \t}\n> > \n> > +\tbranch = branch_get(NULL);\n> > \tif (!branch)\n> > \t\tdie(_(message_detached_head_die), remote->name);\n> \n> This step is the true justification for the splitting of a single\n> switch into two switches done in [03/15]. Makes quite a lot of\n> sense.\n\nIt is one, not the only one. But yeah, the most readily apparent.\n\n-- \nFelipe Contreras\n"},{"id":"426018","messageId":"60b4fd4db5dee_24d282088c@natae.notmuch","threadId":"55796","inReplyTo":"3e5c2303-030e-928e-af57-8e2be8f3ce86@gmail.com","subject":"Re: [PATCH v2 6/6] doc: push: explain default=simple correctly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-31T15:14:21Z","receivedAt":"2021-05-31T17:01:02Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Mathias Kunter wrote:\n> Am 30.05.21 um 21:09 schrieb Felipe Contreras:\n> >>> The current branch is pushed to the corresponding upstream branch, but\n> >>> as a safety measure, the push is aborted if the upstream branch does not\n> >>> have the same name as the local one.\n> > \n> > Except that isn't accurate.\n> > \n> > git clone $url\n> > git checkout -b fix-1\n> > # do commits\n> > git push\n> > \n> > Does that push the current branch to the corresponding upstream branch?\n> \n> I see. Then maybe reword to something like this:\n> \n> > The current branch is pushed to a branch with the same name on the\n> > remote, but as a safety measure, the push is aborted if a corresponding\n> > upstream branch does not have the same name as the local one.\n> \n> In your above example, I'm in centralized workflow, but I can still push \n> the fix-1 branch to origin without having to configure an upstream \n> branch for it.\n\nNo, you can't:\n\n % git push\n fatal: The current branch fix-1 has no upstream branch.\n To push the current branch and set the remote as upstream, use\n\n git push --set-upstream origin fix-1\n\nIsn't that problem the one you originally described [1]?\n\nThe behavior doesn't change if you don't specify the remote:\n`git push` == `git push origin`.\n\n> So this seems to contradict with the currently proposed \n> wording:\n> \n> > If you are working on a centralized workflow, then you need to configure\n> > an upstream branch with the same name.\n\nMy wording is correct, and that's precisely the problem.\n\nMaybe you are thinking this patch series implements the fix I proposed:\nit doesn't.\n\nThe two patch series merely reorganizes the code to make it simpler and\neasier to understand. No functionality changes.\n\nCheers.\n\n[1] https://lore.kernel.org/git/065528bf-b496-83d3-767d-2a78e50a9edf@gmail.com\n\n-- \nFelipe Contreras\n"},{"id":"426027","messageId":"5fa7e3f4-60a0-c3b1-83f2-054ed38e5683@gmail.com","threadId":"55796","inReplyTo":"60b4fd4db5dee_24d282088c@natae.notmuch","subject":"Re: [PATCH v2 6/6] doc: push: explain default=simple correctly","fromName":"Mathias Kunter","fromEmail":"mathiaskunter@gmail.com","sentAt":"2021-05-31T16:54:51Z","receivedAt":"2021-05-31T17:38:56Z","isPatch":true,"sender":{"key":"mathiaskunter@gmail.com","avatar":"https://gravatar.com/avatar/131a813ba8d2cb36edb0d102971af5b523bad01aa9c842822e0d068b350bc4e8?d=mp&s=160"},"body":"Am 31.05.21 um 17:14 schrieb Felipe Contreras:\n>> In your above example, I'm in centralized workflow, but I can still push\n>> the fix-1 branch to origin without having to configure an upstream\n>> branch for it.\n> \n> No, you can't:\n> \n> % git push\n> fatal: The current branch fix-1 has no upstream branch.\n> To push the current branch and set the remote as upstream, use\n> \n> git push --set-upstream origin fix-1\n> \n> Isn't that problem the one you originally described [1]?\n\nIt is.\n\n> Maybe you are thinking this patch series implements the fix I proposed:\n> it doesn't.\n\nYes, I thought so. Sorry for the confusion. When I asked \"will your \nprovided patch fix these failing push commands\" you answered \"It's not \nreally a patch (yet), but yeah: it will\". So that's why I thought so.\n\nMy point simply is: For the sake of easiness of use, I think the \nfollowing should work with the default settings of git:\n\n git clone $url\n git checkout -b fix-1\n # do commits\n git push # should push to origin/fix-1\n # others push to origin/fix-1 (but no local changes)\n git pull # should pull from origin/fix-1\n\nWill this be implemented?\n"},{"id":"426051","messageId":"60b51d6c93c7d_279c820856@natae.notmuch","threadId":"55796","inReplyTo":"5fa7e3f4-60a0-c3b1-83f2-054ed38e5683@gmail.com","subject":"Re: [PATCH v2 6/6] doc: push: explain default=simple correctly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-31T17:31:24Z","receivedAt":"2021-05-31T17:48:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Mathias Kunter wrote:\n> Am 31.05.21 um 17:14 schrieb Felipe Contreras:\n> >> In your above example, I'm in centralized workflow, but I can still push\n> >> the fix-1 branch to origin without having to configure an upstream\n> >> branch for it.\n> > \n> > No, you can't:\n> > \n> > % git push\n> > fatal: The current branch fix-1 has no upstream branch.\n> > To push the current branch and set the remote as upstream, use\n> > \n> > git push --set-upstream origin fix-1\n> > \n> > Isn't that problem the one you originally described [1]?\n> \n> It is.\n> \n> > Maybe you are thinking this patch series implements the fix I proposed:\n> > it doesn't.\n> \n> Yes, I thought so. Sorry for the confusion. When I asked \"will your \n> provided patch fix these failing push commands\" you answered \"It's not \n> really a patch (yet), but yeah: it will\". So that's why I thought so.\n\nI have not yet sent the patch to change the current behavior.\n\nFirst I sent a bunch of patches to reorganize the code so it's more\nclear what it's actually doing.\n\nOnce those patches are merged, then I will probably send the one to\nchange the behavior.\n\nI will include you in the Cc list when I do so.\n\n> My point simply is: For the sake of easiness of use, I think the \n> following should work with the default settings of git:\n> \n> git clone $url\n> git checkout -b fix-1\n> # do commits\n> git push # should push to origin/fix-1\n> # others push to origin/fix-1 (but no local changes)\n> git pull # should pull from origin/fix-1\n\nI agree.\n\n> Will this be implemented?\n\nIt's hard do say, but it probably won't.\n\nAll my suggestions for improvement are mostly disregarded, and\nconsidering *nobody* has said a word about whether or not the current\nbehavior makes sense, I don't think there are very good chances of\nchanging it.\n\nConsider my last mail \"git push default doesn't make sense\" [1]: nobody\nreplied.\n\nBut maybe that's my fault, because even though I broke the In-Reply-To\nin order to start a new thread, it seems mail handling software still\nuses References (incorrectly IMO).\n\nSo maybe if I start a new thread completely fresh people will notice,\nand reply.\n\nI will do so with a patch once I'm done with the reorganization.\n\n\nIn my experience you need to convince either Junio Hamano, or Jeff King\nfor any change in behavior to happen, and until they do comment on this\none it's fair to say it won't happen.\n\nCheers.\n\n[1] https://lore.kernel.org/git/60b15cd2c4136_2183bc20893@natae.notmuch/\n\n-- \nFelipe Contreras\n"},{"id":"426053","messageId":"c4bb335c-4909-8525-cbdb-bb214d18d8fd@gmail.com","threadId":"55796","inReplyTo":"60b51d6c93c7d_279c820856@natae.notmuch","subject":"Re: [PATCH v2 6/6] doc: push: explain default=simple correctly","fromName":"Mathias Kunter","fromEmail":"mathiaskunter@gmail.com","sentAt":"2021-05-31T18:38:56Z","receivedAt":"2021-05-31T18:39:02Z","isPatch":true,"sender":{"key":"mathiaskunter@gmail.com","avatar":"https://gravatar.com/avatar/131a813ba8d2cb36edb0d102971af5b523bad01aa9c842822e0d068b350bc4e8?d=mp&s=160"},"body":"Am 31.05.21 um 19:31 schrieb Felipe Contreras:\n> Once those patches are merged, then I will probably send the one to\n> change the behavior.\n> \n> I will include you in the Cc list when I do so.\n\nThank you very much.\n\n> In my experience you need to convince either Junio Hamano, or Jeff King\n> for any change in behavior to happen, and until they do comment on this\n> one it's fair to say it won't happen.\n\nI assume they are on this mailing list?\n\nI'd say it should be quite an argument if the related StackOverflow \nquestion dealing with this exact issue is one of the top-voted git \nquestions of all time. [1]\n\nIt seems that millions of developers (judging by the number of views of \nthat question) wonder what they need to do so that \"git pull and git \npush will work immediately\" on a newly created local branch.\n\nIf that would \"just work\" out of the box and with the default settings \nof git, without having to read up the solution in StackOverflow first, \nthen it would certainly be an improvement for a huge number of people. \nSo, to all whom it may concern, I think a behavioral change is advisable \nhere.\n\nThank you.\n\n[1] https://stackoverflow.com/q/2765421\n"},{"id":"426081","messageId":"60b55207adad1_36bff208c9@natae.notmuch","threadId":"55796","inReplyTo":"c4bb335c-4909-8525-cbdb-bb214d18d8fd@gmail.com","subject":"Re: [PATCH v2 6/6] doc: push: explain default=simple correctly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-31T21:15:51Z","receivedAt":"2021-05-31T21:16:02Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Mathias Kunter wrote:\n> Am 31.05.21 um 19:31 schrieb Felipe Contreras:\n> > Once those patches are merged, then I will probably send the one to\n> > change the behavior.\n> > \n> > I will include you in the Cc list when I do so.\n> \n> Thank you very much.\n> \n> > In my experience you need to convince either Junio Hamano, or Jeff King\n> > for any change in behavior to happen, and until they do comment on this\n> > one it's fair to say it won't happen.\n> \n> I assume they are on this mailing list?\n\nOf course, and they are also on the Cc list of this mail.\n\n> I'd say it should be quite an argument if the related StackOverflow \n> question dealing with this exact issue is one of the top-voted git \n> questions of all time. [1]\n\nThat seems to be a different issue. Related, but not quite the same.\n\nThat question is answered at the time the user tries to make a push:\n\n fatal: The current branch fix-1 has no upstream branch.\n To push the current branch and set the remote as upstream, use\n\n git push --set-upstream origin fix-1\n\nThat's literally the top answer in SO.\n\n> It seems that millions of developers (judging by the number of views of \n> that question) wonder what they need to do so that \"git pull and git \n> push will work immediately\" on a newly created local branch.\n\nYes, probably many people do wonder that, but not necessarily all of\nthem (at least of the ones that participatd in that SO question).\n\n> If that would \"just work\" out of the box and with the default settings \n> of git, without having to read up the solution in StackOverflow first, \n> then it would certainly be an improvement for a huge number of people. \n\nI agree, but I have already tried to improve the interface in a number\nof ways to no avail.\n\nInertia is a powerful force.\n\nWe'll see how this one goes.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"426082","messageId":"73fcc154-0a95-6569-e776-56bb2f200c5c@gmail.com","threadId":"55796","inReplyTo":"60b55207adad1_36bff208c9@natae.notmuch","subject":"Re: [PATCH v2 6/6] doc: push: explain default=simple correctly","fromName":"Mathias Kunter","fromEmail":"mathiaskunter@gmail.com","sentAt":"2021-05-31T21:54:57Z","receivedAt":"2021-05-31T21:55:05Z","isPatch":true,"sender":{"key":"mathiaskunter@gmail.com","avatar":"https://gravatar.com/avatar/131a813ba8d2cb36edb0d102971af5b523bad01aa9c842822e0d068b350bc4e8?d=mp&s=160"},"body":"Am 31.05.21 um 23:15 schrieb Felipe Contreras:\n>> I'd say it should be quite an argument if the related StackOverflow\n>> question dealing with this exact issue is one of the top-voted git\n>> questions of all time. [1]\n> \n> That seems to be a different issue. Related, but not quite the same.\n\nWell, what the OP actually wanted to know there is \"how can I use git \npull and git push on a new local branch\". The OP knows it can be done by \nconfiguring an upstream branch, and therefore he also specifically asks \nabout the details of how to do that.\n\n> We'll see how this one goes.\n\nThank you for your efforts.\n"},{"id":"426104","messageId":"xmqqfsy1udf0.fsf@gitster.g","threadId":"55796","inReplyTo":"60b49a3ae829b_24d28208fd@natae.notmuch","subject":"Re: [PATCH v2 6/6] doc: push: explain default=simple correctly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-01T09:44:51Z","receivedAt":"2021-06-01T09:45:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n>> I suspect it would be simpler to read and easier to understand to\n>> bring the parethesized part front, e.g.\n>> \n>> If you are pushing to the same repository you pull from (which\n>> is typically `origin`), then you need to ...\n>> \n>> as it would avoid \"the project is not centralized, but I push to my\n>> own repository and pull from it---what should I do?\" questions.\n>\n> The top of `push.default says:\n>\n> Different values are well-suited for specific workflows; for instance,\n> in a purely central workflow (i.e. the fetch source is equal to the\n> push destination), `upstream` is probably what you want.\n>\n> We already brought up the central workflow, I think it's fine to reuse\n> the concept below.\n\nOh, thanks for finding another instance to be corrected. Even in\nthat sentence, the more important point is that upstream would be\nappropriate if you push to the same place as you fetch from, and\nwe do not have to say \"purely central\" at all.\n"},{"id":"426117","messageId":"60b62420858c4_e409520828@natae.notmuch","threadId":"55796","inReplyTo":"xmqqfsy1udf0.fsf@gitster.g","subject":"Re: [PATCH v2 6/6] doc: push: explain default=simple correctly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-01T12:12:16Z","receivedAt":"2021-06-01T12:12:20Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Felipe Contreras writes:\n> \n> >> I suspect it would be simpler to read and easier to understand to\n> >> bring the parethesized part front, e.g.\n> >> \n> >> If you are pushing to the same repository you pull from (which\n> >> is typically `origin`), then you need to ...\n> >> \n> >> as it would avoid \"the project is not centralized, but I push to my\n> >> own repository and pull from it---what should I do?\" questions.\n> >\n> > The top of `push.default says:\n> >\n> > Different values are well-suited for specific workflows; for instance,\n> > in a purely central workflow (i.e. the fetch source is equal to the\n> > push destination), `upstream` is probably what you want.\n> >\n> > We already brought up the central workflow, I think it's fine to reuse\n> > the concept below.\n> \n> Oh, thanks for finding another instance to be corrected.\n\nIn fact there's many:\n\n This mode only makes sense if you are pushing to the same repository\n you would normally pull from (i.e. central workflow).\n\n> Even in that sentence, the more important point is that upstream would\n> be appropriate if you push to the same place as you fetch from, and we\n> do not have to say \"purely central\" at all.\n\nActually, I forgot the main reason I decided to rename centralized to\nsame_repo: they are not the same thing.\n\nYou can have a decentralized workflow where you have multiple\nrepositories configured, but every branch pulls and pushes to the same\nrepository (to them, not other branches):\n\n hotfix <-> dayjob/production\n cleanups <-> upstream/master\n experiment-1 <-> personal/experiment-1\n\nSo it's more like:\n\n centralized = ~decentralized\n triangular = ~two-way\n\nA centralized workflow consists of a single repository where branches\nare typically two-way, but not necessarily.\n\nA decentralized workflow consists of multiple repositories where\nbranches are typically triangular, but not necessarily.\n\nSo the triangularity is per branch, not per repository, and same_repo\nmeans a two-way branch, could be a centralized or decentralized\nworkflow.\n\nTherefore your proposal:\n\n If you are pushing to the same repository you pull from (which is\n typically `origin`), then you need to ...\n\nis actually better, I just had to remind myself that centralized and\nsame repo are not the same thing.\n\n\nOf course it would help to have a place in Documentation/ where\ntrianguar and centralized are explained.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"426146","messageId":"a385c279-5902-40af-f54d-85b950bbb497@iee.email","threadId":"55796","inReplyTo":"60b62420858c4_e409520828@natae.notmuch","subject":"Re: [PATCH v2 6/6] doc: push: explain default=simple correctly","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2021-06-01T15:57:01Z","receivedAt":"2021-06-01T15:57:05Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 01/06/2021 13:12, Felipe Contreras wrote:\n> So it's more like:\n>\n> centralized = ~decentralized\n> triangular = ~two-way\n>\n> A centralized workflow consists of a single repository where branches\n> are typically two-way, but not necessarily.\n>\n> A decentralized workflow consists of multiple repositories where\n> branches are typically triangular, but not necessarily.\n>\n> So the triangularity is per branch, not per repository, and same_repo\n> means a two-way branch, could be a centralized or decentralized\n> workflow.\nMy personal viewpoint is that triangular flow happens when you cannot\npush to the repo you consider as upstream.\n\nRather you typically have a publish/backup repo instead (semi-public,\nsemi-private - few are interested ;-).\n\nThat (can't push one way around the triangle) part of the flow is\nseparate from the distinction between patch flows and merge (Pull)\nrequest flows.\n\nE.g. My personal Git repo can be triangular with both git.git and\ngit-for-windows, plus a few (what I view as) fetch-only repos from other\ncollaborators/maintainers beyond the triangular 'golden' upstream repo.\n\nI often consider GitHub as a centraliser, but I don't think it's what is\nbeing considered above.\n\n--\nA thought did come to mind that a Git serve/repo (typically bare) should\nbe able to offer a 'refs/users/*' space (c.f. refs/remotes used by\nindividual users) that allows a type of 'centralised' operation (almost\nas if all the users used a common alternates repo). Users could only\npush to their own /user refs, but could pull from the main refs/heads,\nand their own refs/users/ space.\n\nThis would give flexibility to smaller corporate central operations to\noffer 'triangular flow' where each dev would feel like they have their\nown 'push' repo, when in reality it's really personalised branches. As\nusual the authentication of user names being handed off elsewhere;-). It\ncould avoid some of the --alternate management aspects.\n\nIt's a thought..\n\n--\n\nPhilip\n"},{"id":"426148","messageId":"60b661d5ee7cc_3c4208c1@natae.notmuch","threadId":"55796","inReplyTo":"a385c279-5902-40af-f54d-85b950bbb497@iee.email","subject":"Re: [PATCH v2 6/6] doc: push: explain default=simple correctly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-01T16:35:33Z","receivedAt":"2021-06-01T16:35:39Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Philip Oakley wrote:\n> On 01/06/2021 13:12, Felipe Contreras wrote:\n> > So it's more like:\n> >\n> > centralized = ~decentralized\n> > triangular = ~two-way\n> >\n> > A centralized workflow consists of a single repository where branches\n> > are typically two-way, but not necessarily.\n> >\n> > A decentralized workflow consists of multiple repositories where\n> > branches are typically triangular, but not necessarily.\n> >\n> > So the triangularity is per branch, not per repository, and same_repo\n> > means a two-way branch, could be a centralized or decentralized\n> > workflow.\n\n> My personal viewpoint is that triangular flow happens when you cannot\n> push to the repo you consider as upstream.\n\nIt's not about permissions. Even if I had permissions to push to git.git,\nI wouldn't do so. I do have permission to push to some public projects, but I\ninstead send patches/pull requests like everyone else.\n\nIt's more about ownership. In my personal repositories I can push\nwhatever I want, but on shared repositories I have to be more careful.\n\n> Rather you typically have a publish/backup repo instead (semi-public,\n> semi-private - few are interested ;-).\n> \n> That (can't push one way around the triangle) part of the flow is\n> separate from the distinction between patch flows and merge (Pull)\n> request flows.\n\nI think it's not separate, that is the thing that makes a triangular\nflow triangular: the flow of patches goes through a different repository,\nand then they get picked and merged into the upstream one.\n\n> E.g. My personal Git repo can be triangular with both git.git and\n> git-for-windows, plus a few (what I view as) fetch-only repos from other\n> collaborators/maintainers beyond the triangular 'golden' upstream repo.\n> \n> I often consider GitHub as a centraliser, but I don't think it's what is\n> being considered above.\n\nGitHub is all about pull requests, you fork a repository, you push your\nbranch into that personal fork, and then you request a pull from\nupstream.\n\nThat's triangular.\n\n> --\n> A thought did come to mind that a Git serve/repo (typically bare) should\n> be able to offer a 'refs/users/*' space (c.f. refs/remotes used by\n> individual users) that allows a type of 'centralised' operation (almost\n> as if all the users used a common alternates repo). Users could only\n> push to their own /user refs, but could pull from the main refs/heads,\n> and their own refs/users/ space.\n> \n> This would give flexibility to smaller corporate central operations to\n> offer 'triangular flow' where each dev would feel like they have their\n> own 'push' repo, when in reality it's really personalised branches. As\n> usual the authentication of user names being handed off elsewhere;-). It\n> could avoid some of the --alternate management aspects.\n> \n> It's a thought..\n\nYeah, and interesting thought. But it demonstrates what I said above:\nyou can have a central repository, and yet have triangular branches:\n\n feature-1 <= origin/master\n => origin/felipec/feature-1\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"426156","messageId":"xmqqpmx5s7j8.fsf@gitster.g","threadId":"55796","inReplyTo":"60b62420858c4_e409520828@natae.notmuch","subject":"Re: [PATCH v2 6/6] doc: push: explain default=simple correctly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-01T19:34:51Z","receivedAt":"2021-06-01T19:34:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> Therefore your proposal:\n>\n> If you are pushing to the same repository you pull from (which is\n> typically `origin`), then you need to ...\n>\n> is actually better, I just had to remind myself that centralized and\n> same repo are not the same thing.\n\nYes, that is exactly why I brought up\n\n>> as it would avoid \"the project is not centralized, but I push to my\n>> own repository and pull from it---what should I do?\" questions.\n\nas a reason why we want to phrase it that way, so we are on the same\npage ;-)\n\n"},{"id":"426157","messageId":"819f7ed4-ef38-32d4-2a64-a38ac37a5736@iee.email","threadId":"55796","inReplyTo":"60b661d5ee7cc_3c4208c1@natae.notmuch","subject":"Re: [PATCH v2 6/6] doc: push: explain default=simple correctly","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2021-06-01T19:39:13Z","receivedAt":"2021-06-01T19:39:18Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 01/06/2021 17:35, Felipe Contreras wrote:\n> Philip Oakley wrote:\n>> On 01/06/2021 13:12, Felipe Contreras wrote:\n>>> So it's more like:\n>>>\n>>> centralized = ~decentralized\n>>> triangular = ~two-way\n>>>\n>>> A centralized workflow consists of a single repository where branches\n>>> are typically two-way, but not necessarily.\n>>>\n>>> A decentralized workflow consists of multiple repositories where\n>>> branches are typically triangular, but not necessarily.\n>>>\n>>> So the triangularity is per branch, not per repository, and same_repo\n>>> means a two-way branch, could be a centralized or decentralized\n>>> workflow.\n>> My personal viewpoint is that triangular flow happens when you cannot\n>> push to the repo you consider as upstream.\n> It's not about permissions. Even if I had permissions to push to git.git,\n> I wouldn't do so. I do have permission to push to some public projects, but I\n> instead send patches/pull requests like everyone else.\n\nI had it that if you don't have permissions then you definitely need to\nuse a Triangular flow. Hence how I was presenting the view.\n>\n> It's more about ownership. In my personal repositories I can push\n> whatever I want, but on shared repositories I have to be more careful.\n\nTrue, there are social choices that are equivalent to a type of\n'permission'.\n>\n>> Rather you typically have a publish/backup repo instead (semi-public,\n>> semi-private - few are interested ;-).\n>>\n>> That (can't push one way around the triangle) part of the flow is\n>> separate from the distinction between patch flows and merge (Pull)\n>> request flows.\n> I think it's not separate, that is the thing that makes a triangular\n> flow triangular: the flow of patches goes through a different repository,\n> and then they get picked and merged into the upstream one.\nPull requests can do the same thing, just as in Git-for Windows..\n\n>\n>> E.g. My personal Git repo can be triangular with both git.git and\n>> git-for-windows, plus a few (what I view as) fetch-only repos from other\n>> collaborators/maintainers beyond the triangular 'golden' upstream repo.\n>>\n>> I often consider GitHub as a centraliser, but I don't think it's what is\n>> being considered above.\n> GitHub is all about pull requests, you fork a repository, you push your\n> branch into that personal fork, and then you request a pull from\n> upstream.\n>\n> That's triangular.\n\nYes.\n>\n>> --\n>> A thought did come to mind that a Git serve/repo (typically bare) should\n>> be able to offer a 'refs/users/*' space (c.f. refs/remotes used by\n>> individual users) that allows a type of 'centralised' operation (almost\n>> as if all the users used a common alternates repo). Users could only\n>> push to their own /user refs, but could pull from the main refs/heads,\n>> and their own refs/users/ space.\n>>\n>> This would give flexibility to smaller corporate central operations to\n>> offer 'triangular flow' where each dev would feel like they have their\n>> own 'push' repo, when in reality it's really personalised branches. As\n>> usual the authentication of user names being handed off elsewhere;-). It\n>> could avoid some of the --alternate management aspects.\n>>\n>> It's a thought..\n> Yeah, and interesting thought. But it demonstrates what I said above:\n> you can have a central repository, and yet have triangular branches:\n\nI see triangular being about repos, rather than branches. The suggestion\nabout was, essentially, about managing multiple user forks on the same\nserver without using alternates etc. It's not fully thought through ;-)\n\n>\n> feature-1 <= origin/master\n> => origin/felipec/feature-1\n>\n> Cheers.\n>\nI expect to be off-line for a week or so.\n\nPhilip\n"},{"id":"426179","messageId":"60b6c860e4a4e_40442081d@natae.notmuch","threadId":"55796","inReplyTo":"819f7ed4-ef38-32d4-2a64-a38ac37a5736@iee.email","subject":"Re: [PATCH v2 6/6] doc: push: explain default=simple correctly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-01T23:53:04Z","receivedAt":"2021-06-01T23:53:09Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Philip Oakley wrote:\n> On 01/06/2021 17:35, Felipe Contreras wrote:\n> > Philip Oakley wrote:\n> >> On 01/06/2021 13:12, Felipe Contreras wrote:\n> >>> So it's more like:\n> >>>\n> >>> centralized = ~decentralized\n> >>> triangular = ~two-way\n> >>>\n> >>> A centralized workflow consists of a single repository where branches\n> >>> are typically two-way, but not necessarily.\n> >>>\n> >>> A decentralized workflow consists of multiple repositories where\n> >>> branches are typically triangular, but not necessarily.\n> >>>\n> >>> So the triangularity is per branch, not per repository, and same_repo\n> >>> means a two-way branch, could be a centralized or decentralized\n> >>> workflow.\n> >> My personal viewpoint is that triangular flow happens when you cannot\n> >> push to the repo you consider as upstream.\n> > It's not about permissions. Even if I had permissions to push to git.git,\n> > I wouldn't do so. I do have permission to push to some public projects, but I\n> > instead send patches/pull requests like everyone else.\n> \n> I had it that if you don't have permissions then you definitely need to\n> use a Triangular flow. Hence how I was presenting the view.\n\nIf you don't have permissions you have no option but a triangular flow.\n\nIf you are in a triangular flow that doesn't necessarily mean you don't\nhave permissions.\n\n> >> A thought did come to mind that a Git serve/repo (typically bare) should\n> >> be able to offer a 'refs/users/*' space (c.f. refs/remotes used by\n> >> individual users) that allows a type of 'centralised' operation (almost\n> >> as if all the users used a common alternates repo). Users could only\n> >> push to their own /user refs, but could pull from the main refs/heads,\n> >> and their own refs/users/ space.\n> >>\n> >> This would give flexibility to smaller corporate central operations to\n> >> offer 'triangular flow' where each dev would feel like they have their\n> >> own 'push' repo, when in reality it's really personalised branches. As\n> >> usual the authentication of user names being handed off elsewhere;-). It\n> >> could avoid some of the --alternate management aspects.\n> >>\n> >> It's a thought..\n> > Yeah, and interesting thought. But it demonstrates what I said above:\n> > you can have a central repository, and yet have triangular branches:\n> \n> I see triangular being about repos, rather than branches.\n\nIf you have a feature-1 branch that fetches from origin, rebases onto\norigin/master, but pushes to origin/feature-1...\n\nDoes that qualify as triangular?\n\n-- \nFelipe Contreras\n"}]}