{"thread":{"id":"29944","subject":"[PATCH] push: Provide situational hints for non-fast-forward errors","startedAt":"2012-03-13T23:22:57Z","lastAt":"2012-03-19T00:15:09Z","messageCount":28,"participants":["Christopher Tiwald","Junio C Hamano","Matthieu Moy","Zbigniew Jędrzejewski-Szmek","Clemens Buchacher"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"186914","messageId":"20120313232256.GA49626@democracyinaction.org","threadId":"29944","inReplyTo":null,"subject":"[PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Christopher Tiwald","fromEmail":"christiwald@gmail.com","sentAt":"2012-03-13T23:22:57Z","receivedAt":"2012-03-13T23:22:57Z","isPatch":true,"sender":{"key":"christiwald@gmail.com","avatar":"https://avatars.githubusercontent.com/u/667276?v=4"},"body":"Pushing a non-fast-forward update to a remote repository will result in\nan error, but the hint text doesn't provide the correct resolution in\nevery case. Three scenarios may arise depending on your workflow, each\nwith a different resolution:\n\n1) If you push a non-fast-forward update to HEAD, you should merge\nremote changes with 'git pull' before pushing again.\n\n2) If you push to a shared repository others push to, and your local\ntracking branches are not kept up to date, the 'matching refs' default\nwill generate non-fast-forward errors on outdated branches. If this is\nyour workflow, the 'matching refs' default is not for you. Consider\nsetting the 'push.default' configuration variable to 'upstream' to\nensure only your checked-out branch is pushed.\n\n3) If you push with explicit ref matching (e.g. 'git push ... topic:topic')\nwhile checked out on another branch (e.g. 'master'), the correct\nresolution is checking out the local branch, issuing git pull, and\nmerging remote changes before pushing again.\n\nMake nonfastforward an enum and teach transport.c to detect the\nscenarios described above. Give situation-specific resolution advice\nwhen pushes are rejected due to non-fast-forward updates. Finally,\nupdate other instances of nonfastforward to use the proper enum option.\n\nSigned-off-by: Christopher Tiwald \nBased-on-patch-by: Junio C Hamano \n---\nThis is a reroll of jc/advise-push-default (2011-12-18). Apologies if\nit is out of band. I saw some chatter recently about 1.7.10.rc*. I\nwasn't sure if that meant \"Don't submit patches at this point in the\ncycle\" or \"Feel free to submit patches, but they might not be\nacknowledged for a while.\" Mostly, I wanted to move this topic out of\nstalled in the \"What's Cooking\" emails. I'm happy to resubmit at a\nbetter time.\n\nThe patch is based on jc/advise-push-default and attempts to\nimplement Peff's logic in [1]. It would also require a change if the\npush default behavior changes [2], but I think the core logic is\nsound. `git push` should be smart enough to distinguish different\ntypes of non-fast-forward updates and advise accordingly regardless of\nits default.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/187079/focus=187447\n[2] http://thread.gmane.org/gmane.comp.version-control.git/192547/focus=192694\n\n Documentation/config.txt | 15 ++++++++++\n advice.c | 6 ++++\n advice.h | 3 ++\n builtin/push.c | 72 ++++++++++++++++++++++++++++++++++++++++++----\n builtin/send-pack.c | 2 +-\n cache.h | 9 ++++--\n environment.c | 2 +-\n transport.c | 17 ++++++++---\n 8 files changed, 112 insertions(+), 14 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex c081657..50d9249 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -158,6 +158,21 @@ advice.*::\n \t\tAdvice shown when you used linkgit:git-checkout[1] to\n \t\tmove to the detach HEAD state, to instruct how to create\n \t\ta local branch after the fact.\n+\tpullBeforePush::\n+\t\tAdvice shown when you ran linkgit:git-push[1] and pushed\n+\t\ta non-fast-forward update to HEAD, instructing you to\n+\t\tlinkgit:git-pull[1] before pushing again.\n+\tuseUpstream::\n+\t\tAdvice to set 'push.default' to 'upstream' when you ran\n+\t\tlinkgit:git-push[1] and pushed 'matching refs' by default\n+\t\t(i.e. you did not have any explicit refspec on the command\n+\t\tline, and no 'push.default' configuration was set) and it\n+\t\tresulted in a non-fast-forward error.\n+\tcheckoutPullPush::\n+\t\tAdvice shown when you ran linkgit:git-push[1] and pushed\n+\t\ta non-fast-forward update to a non-HEAD branch, instructing\n+\t\tyou to checkout the branch and run linkgit:git-pull[1]\n+\t\tbefore pushing again.\n --\n \n core.fileMode::\ndiff --git a/advice.c b/advice.c\nindex 01130e5..608e90d 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -6,6 +6,9 @@ int advice_commit_before_merge = 1;\n int advice_resolve_conflict = 1;\n int advice_implicit_identity = 1;\n int advice_detached_head = 1;\n+int advice_pull_before_push = 1;\n+int advice_use_upstream = 1;\n+int advice_checkout_pull_push = 1;\n \n static struct {\n \tconst char *name;\n@@ -17,6 +20,9 @@ static struct {\n \t{ \"resolveconflict\", &advice_resolve_conflict },\n \t{ \"implicitidentity\", &advice_implicit_identity },\n \t{ \"detachedhead\", &advice_detached_head },\n+\t{ \"pullbeforepush\", &advice_pull_before_push },\n+\t{ \"useupstream\", &advice_use_upstream },\n+\t{ \"checkoutpullpush\", &advice_checkout_pull_push }\n };\n \n void advise(const char *advice, ...)\ndiff --git a/advice.h b/advice.h\nindex 7bda45b..ac07a44 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -9,6 +9,9 @@ extern int advice_commit_before_merge;\n extern int advice_resolve_conflict;\n extern int advice_implicit_identity;\n extern int advice_detached_head;\n+extern int advice_use_upstream;\n+extern int advice_pull_before_push;\n+extern int advice_checkout_pull_push;\n \n int git_default_advice_config(const char *var, const char *value);\n void advise(const char *advice, ...);\ndiff --git a/builtin/push.c b/builtin/push.c\nindex d315475..0fecf06 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -24,6 +24,7 @@ static int progress = -1;\n static const char **refspec;\n static int refspec_nr;\n static int refspec_alloc;\n+static int default_matching_used;\n \n static void add_refspec(const char *ref)\n {\n@@ -95,6 +96,9 @@ static void setup_default_push_refspecs(struct remote *remote)\n {\n \tswitch (push_default) {\n \tdefault:\n+\tcase PUSH_DEFAULT_UNSPECIFIED:\n+\t\tdefault_matching_used = 1;\n+\t\t/* fallthru */\n \tcase PUSH_DEFAULT_MATCHING:\n \t\tadd_refspec(\":\");\n \t\tbreak;\n@@ -114,6 +118,59 @@ static void setup_default_push_refspecs(struct remote *remote)\n \t}\n }\n \n+static const char *message_advice_pull_before_push[] = {\n+\t\"To prevent you from losing history, non-fast-forward updates to HEAD\",\n+\t\"were rejected. Merge the remote changes (e.g. 'git pull') before\",\n+\t\"pushing again. See the 'Note about fast-forwards' section of\",\n+\t\"'git push --help' for details.\"\n+};\n+\n+static const char *message_advice_use_upstream[] = {\n+\t\"By default, git pushes all branches that have a matching counterpart\",\n+\t\"on the remote. In this case, some of your local branches were stale\",\n+\t\"with respect to their remote counterparts. If you did not intend to\",\n+\t\"push these branches, you may want to set the 'push.default'\",\n+\t\"configuration variable to 'upstream' to push only the current branch.\"\n+};\n+\n+static const char *message_advice_checkout_pull_push[] = {\n+\t\"To prevent you from losing history, your non-fast-forward branch\",\n+\t\"updates were rejected. Checkout the branch and merge the remote\",\n+\t\"changes (e.g. 'git pull') before pushing again. See the\",\n+\t\"'Note about fast-forwards' section of 'git push --help' for\",\n+\t\"details.\"\n+};\n+\n+static void advise_pull_before_push(void)\n+{\n+\tint i;\n+\n+\tif (!advice_pull_before_push)\n+\t\treturn;\n+\tfor (i = 0; i < ARRAY_SIZE(message_advice_pull_before_push); i++)\n+\t\tadvise(message_advice_pull_before_push[i]);\n+}\n+\n+static void advise_use_upstream(void)\n+{\n+\tint i;\n+\n+\tif (!advice_use_upstream)\n+\t\treturn;\n+\tfor (i = 0; i < ARRAY_SIZE(message_advice_use_upstream); i++)\n+\t\tadvise(message_advice_use_upstream[i]);\n+}\n+\n+static void advise_checkout_pull_push(void)\n+{\n+\tint i;\n+\n+\tif (!advice_checkout_pull_push)\n+\t\treturn;\n+\tfor (i = 0; i < ARRAY_SIZE(message_advice_checkout_pull_push); i++)\n+\t\tadvise(message_advice_checkout_pull_push[i]);\n+}\n+\n static int push_with_options(struct transport *transport, int flags)\n {\n \tint err;\n@@ -136,15 +193,18 @@ static int push_with_options(struct transport *transport, int flags)\n \n \terr |= transport_disconnect(transport);\n \n+\tif (nonfastforward == NONFASTFORWARD_HEAD) {\n+\t\tadvise_pull_before_push();\n+\t} else if (nonfastforward == NONFASTFORWARD_OTHER) {\n+\t\tif (default_matching_used)\n+\t\t\tadvise_use_upstream();\n+\t\telse\n+\t\t\tadvise_checkout_pull_push();\n+\t}\n+\n \tif (!err)\n \t\treturn 0;\n \n-\tif (nonfastforward && advice_push_nonfastforward) {\n-\t\tfprintf(stderr, _(\"To prevent you from losing history, non-fast-forward updates were rejected\\n\"\n-\t\t\t\t\"Merge the remote changes (e.g. 'git pull') before pushing again. See the\\n\"\n-\t\t\t\t\"'Note about fast-forwards' section of 'git push --help' for details.\\n\"));\n-\t}\n-\n \treturn 1;\n }\n \ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 9df341c..09895b9 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -409,7 +409,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \tint send_all = 0;\n \tconst char *receivepack = \"git-receive-pack\";\n \tint flags;\n-\tint nonfastforward = 0;\n+\tint nonfastforward = NONFASTFORWARD_NONE;\n \n \targv++;\n \tfor (i = 1; i < argc; i++, argv++) {\ndiff --git a/cache.h b/cache.h\nindex e5e1aa4..14bc305 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -625,7 +625,8 @@ enum push_default_type {\n \tPUSH_DEFAULT_NOTHING = 0,\n \tPUSH_DEFAULT_MATCHING,\n \tPUSH_DEFAULT_UPSTREAM,\n-\tPUSH_DEFAULT_CURRENT\n+\tPUSH_DEFAULT_CURRENT,\n+\tPUSH_DEFAULT_UNSPECIFIED\n };\n \n extern enum branch_track git_branch_track;\n@@ -1008,7 +1009,6 @@ struct ref {\n \tchar *symref;\n \tunsigned int force:1,\n \t\tmerge:1,\n-\t\tnonfastforward:1,\n \t\tdeletion:1;\n \tenum {\n \t\tREF_STATUS_NONE = 0,\n@@ -1019,6 +1019,11 @@ struct ref {\n \t\tREF_STATUS_REMOTE_REJECT,\n \t\tREF_STATUS_EXPECTING_REPORT\n \t} status;\n+\tenum {\n+\t\tNONFASTFORWARD_NONE = 0,\n+\t\tNONFASTFORWARD_HEAD,\n+\t\tNONFASTFORWARD_OTHER\n+\t} nonfastforward;\n \tchar *remote_status;\n \tstruct ref *peer_ref; /* when renaming */\n \tchar name[FLEX_ARRAY]; /* more */\ndiff --git a/environment.c b/environment.c\nindex c93b8f4..d7e6c65 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -52,7 +52,7 @@ enum safe_crlf safe_crlf = SAFE_CRLF_WARN;\n unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;\n enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;\n enum rebase_setup_type autorebase = AUTOREBASE_NEVER;\n-enum push_default_type push_default = PUSH_DEFAULT_MATCHING;\n+enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;\n #ifndef OBJECT_CREATION_MODE\n #define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS\n #endif\ndiff --git a/transport.c b/transport.c\nindex 181f8f2..23210d5 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -721,6 +721,10 @@ void transport_print_push_status(const char *dest, struct ref *refs,\n {\n \tstruct ref *ref;\n \tint n = 0;\n+\tunsigned char head_sha1[20];\n+\tchar *head;\n+\n+\thead = resolve_refdup(\"HEAD\", head_sha1, 1, NULL);\n \n \tif (verbose) {\n \t\tfor (ref = refs; ref; ref = ref->next)\n@@ -732,14 +736,19 @@ void transport_print_push_status(const char *dest, struct ref *refs,\n \t\tif (ref->status == REF_STATUS_OK)\n \t\t\tn += print_one_push_status(ref, dest, n, porcelain);\n \n-\t*nonfastforward = 0;\n+\t*nonfastforward = NONFASTFORWARD_NONE;\n \tfor (ref = refs; ref; ref = ref->next) {\n \t\tif (ref->status != REF_STATUS_NONE &&\n \t\t ref->status != REF_STATUS_UPTODATE &&\n \t\t ref->status != REF_STATUS_OK)\n \t\t\tn += print_one_push_status(ref, dest, n, porcelain);\n-\t\tif (ref->status == REF_STATUS_REJECT_NONFASTFORWARD)\n-\t\t\t*nonfastforward = 1;\n+\t\tif (ref->status == REF_STATUS_REJECT_NONFASTFORWARD &&\n+\t\t *nonfastforward != NONFASTFORWARD_HEAD) {\n+\t\t\tif (!strcmp(head, ref->name))\n+\t\t\t\t*nonfastforward = NONFASTFORWARD_HEAD;\n+\t\t\telse\n+\t\t\t\t*nonfastforward = NONFASTFORWARD_OTHER;\n+\t\t}\n \t}\n }\n \n@@ -1008,7 +1017,7 @@ int transport_push(struct transport *transport,\n \t\t int refspec_nr, const char **refspec, int flags,\n \t\t int *nonfastforward)\n {\n-\t*nonfastforward = 0;\n+\t*nonfastforward = NONFASTFORWARD_NONE;\n \ttransport_verify_remote_names(refspec_nr, refspec);\n \n \tif (transport->push) {\n-- \n1.7.10.rc0.42.gefc97.dirty\n"},{"id":"186916","messageId":"7vobrzst7n.fsf@alter.siamese.dyndns.org","threadId":"29944","inReplyTo":"20120313232256.GA49626@democracyinaction.org","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-14T04:27:08Z","receivedAt":"2012-03-14T04:27:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"An off-topic administrivia. Please do not try to deflect responses meant\nfor you by setting Mail-Followup-To.\n\nChristopher Tiwald writes:\n\n> Pushing a non-fast-forward update to a remote repository will result in\n> an error, but the hint text doesn't provide the correct resolution in\n> every case. Three scenarios may arise depending on your workflow, each\n> with a different resolution:\n\nAre we sure there are only three, or is this just \"we do not say anything\nconcrete, but at least we know common three cases, and there may be more\"?\n\nI am mostly interested in making sure that we do not give a bad advice.\nGiving an advice that is mostly accurate and relevant for 95% of the time\nis perfectly fine, as long as following the advice in the remaining 5%\ndoes not result in a disaster.\n\n> 1) If you push a non-fast-forward update to HEAD, you should merge\n> remote changes with 'git pull' before pushing again.\n\nYou said \"to HEAD\", but I think you meant the case you push your current\nbranch (i.e. HEAD) to update any ref on the other side. In other words,\nthe push does not have to be \"*to*\" HEAD over there. Am I mistaken?\n\n> 2) If you push to a shared repository others push to, and your local\n> tracking branches are not kept up to date, the 'matching refs' default\n> will generate non-fast-forward errors on outdated branches. If this is\n> your workflow, the 'matching refs' default is not for you. Consider\n> setting the 'push.default' configuration variable to 'upstream' to\n> ensure only your checked-out branch is pushed.\n\nOK.\n\n> 3) If you push with explicit ref matching (e.g. 'git push ... topic:topic')\n> while checked out on another branch (e.g. 'master'), the correct\n> resolution is checking out the local branch, issuing git pull, and\n> merging remote changes before pushing again.\n\nOr you may have misspelled the source side of the refspec and tried to\npush a wrong branch.\n\n> Make nonfastforward an enum and teach transport.c to detect the\n> scenarios described above. Give situation-specific resolution advice\n> when pushes are rejected due to non-fast-forward updates. Finally,\n> update other instances of nonfastforward to use the proper enum option.\n\nI think the overall direction of the implemention is good, modulo minor\ndesign nits.\n\n * I do not particularly find NONFASTFORWARD_NONE that is defined to be 0\n a useful readability measure. Plain vanilla constant 0 says that there\n is nothing magical going on to the readers clearly already.\n\n * Also NONFASTFORWARD_FROTZ is way too long. Wouldn't NONFF_FROTZ be\n sufficient and clear?\n\n * I can see there are three kinds of advices, but I do not see why users\n need to acknowledge that they understand them one by one with separate\n advice configuration. Isn't it better to have only one variable, \"OK,\n I know how to deal with a failed push due to non-fast-forward\"?\n\n> Signed-off-by: Christopher Tiwald \n> Based-on-patch-by: Junio C Hamano \n\nThese two lines are chronologically swapped.\n\n> ---\n> This is a reroll of jc/advise-push-default (2011-12-18).\n\nI lost track of this topic during the last round. Thanks for picking it\nup.\n\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index c081657..50d9249 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -158,6 +158,21 @@ advice.*::\n> \t\tAdvice shown when you used linkgit:git-checkout[1] to\n> \t\tmove to the detach HEAD state, to instruct how to create\n> \t\ta local branch after the fact.\n> +\tpullBeforePush::\n> +\t\tAdvice shown when you ran linkgit:git-push[1] and pushed\n> +\t\ta non-fast-forward update to HEAD, instructing you to\n> +\t\tlinkgit:git-pull[1] before pushing again.\n> +\tuseUpstream::\n> +\t\tAdvice to set 'push.default' to 'upstream' when you ran\n> +\t\tlinkgit:git-push[1] and pushed 'matching refs' by default\n> +\t\t(i.e. you did not have any explicit refspec on the command\n> +\t\tline, and no 'push.default' configuration was set) and it\n> +\t\tresulted in a non-fast-forward error.\n> +\tcheckoutPullPush::\n> +\t\tAdvice shown when you ran linkgit:git-push[1] and pushed\n> +\t\ta non-fast-forward update to a non-HEAD branch, instructing\n> +\t\tyou to checkout the branch and run linkgit:git-pull[1]\n> +\t\tbefore pushing again.\n\nI would prefer to see these consolidated into a single advice.pushNonFF\nvariable, but I may be missing why it could be a good idea to allow them\nturned off selectively.\n\n> diff --git a/builtin/push.c b/builtin/push.c\n> index d315475..0fecf06 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> \t}\n> }\n> \n> +static const char *message_advice_pull_before_push[] = {\n> +\t\"To prevent you from losing history, non-fast-forward updates to HEAD\",\n> +\t\"were rejected. Merge the remote changes (e.g. 'git pull') before\",\n> +\t\"pushing again. See the 'Note about fast-forwards' section of\",\n> +\t\"'git push --help' for details.\"\n> +};\n\nThis again says \"*to* HEAD\". If this should be \"a non-fast-forward update\nto send the current branch was rejected\" as I suspected above, the message\nneeds to be rephrased accordingly.\n\n> +static const char *message_advice_use_upstream[] = {\n> +\t\"By default, git pushes all branches that have a matching counterpart\",\n> +\t\"on the remote. In this case, some of your local branches were stale\",\n> +\t\"with respect to their remote counterparts. If you did not intend to\",\n> +\t\"push these branches, you may want to set the 'push.default'\",\n> +\t\"configuration variable to 'upstream' to push only the current branch.\"\n> +};\n\nIf you drop everything up to and including \"In this case, \", the advice\nmessage still teaches exactly what the user needs to learn.\n\n> +static const char *message_advice_checkout_pull_push[] = {\n> +\t\"To prevent you from losing history, your non-fast-forward branch\",\n> +\t\"updates were rejected. Checkout the branch and merge the remote\",\n> +\t\"changes (e.g. 'git pull') before pushing again. See the\",\n> +\t\"'Note about fast-forwards' section of 'git push --help' for\",\n> +\t\"details.\"\n> +};\n\nOK.\n\n> @@ -136,15 +193,18 @@ static int push_with_options(struct transport *transport, int flags)\n> \n> \terr |= transport_disconnect(transport);\n> \n> +\tif (nonfastforward == NONFASTFORWARD_HEAD) {\n> +\t\tadvise_pull_before_push();\n> +\t} else if (nonfastforward == NONFASTFORWARD_OTHER) {\n> +\t\tif (default_matching_used)\n> +\t\t\tadvise_use_upstream();\n> +\t\telse\n> +\t\t\tadvise_checkout_pull_push();\n> +\t}\n\t\t\nI suspect that we may find more cases not just three, so\n\n\tswitch (nonfastforward) {\n\tdefault:\n \tbreak;\n\tcase NONFF_HEAD:\n \tadvice_pull_before_push();\n\t\tbreak;\n\tcase NONFF_OTHER:\n\t\t...\n\t}\n\nwould be a more forward-looking way to write it.\n\nAlso, shouldn't we be doing this only when err is true, or is it too\ndefensive?\n\n> \tif (!err)\n> \t\treturn 0;\n> \n> -\tif (nonfastforward && advice_push_nonfastforward) {\n> -\t\tfprintf(stderr, _(\"To prevent you from losing history,...\n\nThat is, I am wondering why your \"more detailed diag & advice\" code is not\nhere, i.e. after \"if (!err) return 0\".\n"},{"id":"186924","messageId":"vpqipi7zh3n.fsf@bauges.imag.fr","threadId":"29944","inReplyTo":"20120313232256.GA49626@democracyinaction.org","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-03-14T09:06:52Z","receivedAt":"2012-03-14T09:06:52Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Christopher Tiwald writes:\n\n> 2) If you push to a shared repository others push to, and your local\n> tracking branches are not kept up to date, the 'matching refs' default\n> will generate non-fast-forward errors on outdated branches. If this is\n> your workflow, the 'matching refs' default is not for you. Consider\n> setting the 'push.default' configuration variable to 'upstream' to\n> ensure only your checked-out branch is pushed.\n\nVery good point.\n\nDepending on the outcome of the discussion in the thread about\n'push.default', you may want to suggest 'current' instead of upstream:\nhttp://thread.gmane.org/gmane.comp.version-control.git/192547/focus=192694\n\nActually, if the user has 'push.default=matching', the least surprising\nmove from this value is 'push.default=current', that will push a subset\nof what used to be pushed, and won't change the target branch.\n\n> +static const char *message_advice_pull_before_push[] = {\n> +\t\"To prevent you from losing history, non-fast-forward updates to HEAD\",\n> +\t\"were rejected. Merge the remote changes (e.g. 'git pull') before\",\n> +\t\"pushing again. See the 'Note about fast-forwards' section of\",\n> +\t\"'git push --help' for details.\"\n> +};\n\nYour patch removes the _(...) around the string, which breaks the\ninternationalization.\n\n> +static const char *message_advice_use_upstream[] = {\n> +\t\"By default, git pushes all branches that have a matching counterpart\",\n> +\t\"on the remote. In this case, some of your local branches were stale\",\n> +\t\"with respect to their remote counterparts. If you did not intend to\",\n> +\t\"push these branches, you may want to set the 'push.default'\",\n> +\t\"configuration variable to 'upstream' to push only the current branch.\"\n> +};\n\nI'd give the full cut-and-paste ready command to set the variable, to\nhelp the user who doesn't know what \"configuration variable\" really\nmeans in the context of Git:\n\n... you may want to run\n\n git config push.default upstream (or current, if you like my remark above)\n\nto ask Git to push only the current branch from now on.\n\n(I'd advise \"git config\" without \"--global\" here, because the user may\nwant to do something else in other repositories)\n\n> +\tfor (i = 0; i < ARRAY_SIZE(message_advice_pull_before_push); i++)\n> +\t\tadvise(message_advice_pull_before_push[i]);\n\nI'm no expert in gettext, but I think the internationalization people\nwill have a hard time dealing with a single message split accross an\narray.\n\nActually, I prefer the effect of a single advise() call (i.e. say\n\"hint:\" just once, not for each line), but this part is subjective.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"186934","messageId":"20120314121434.GB28595@in.waw.pl","threadId":"29944","inReplyTo":"7vobrzst7n.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Zbigniew Jędrzejewski-Szmek","fromEmail":"zbyszek@in.waw.pl","sentAt":"2012-03-14T12:14:34Z","receivedAt":"2012-03-14T12:14:34Z","isPatch":true,"sender":{"key":"zbyszek@in.waw.pl","avatar":"https://avatars.githubusercontent.com/u/349618?v=4"},"body":"On Tue, Mar 13, 2012 at 09:27:08PM -0700, Junio C Hamano wrote:\n> Christopher Tiwald writes:\n> * I can see there are three kinds of advices, but I do not see why users\n> need to acknowledge that they understand them one by one with separate\n> advice configuration. Isn't it better to have only one variable, \"OK,\n> I know how to deal with a failed push due to non-fast-forward\"?\n\nHi,\n\nI think that having three different config keys for the three\ndifferent advices makes sense, because the advices will be displayed\nat different times. E.g. the user starts with the simplest one-branch\nworkflow, triggers the first alternative, reads \"pullBeforePush\" and\nthen disables the hint. Then the team upgrades the workflow to use\nseveral branches and the user triggers the second alternative. At this\npoint, git should hint to \"useUpstream\". If the user disabled all the\nnon-FF hints at the first advice, she would miss the second, different\none, later.\n\n> > diff --git a/Documentation/config.txt b/Documentation/config.txt\n> > index c081657..50d9249 100644\n> > --- a/Documentation/config.txt\n> > +++ b/Documentation/config.txt\n> > @@ -158,6 +158,21 @@ advice.*::\n> > \t\tAdvice shown when you used linkgit:git-checkout[1] to\n> > \t\tmove to the detach HEAD state, to instruct how to create\n> > \t\ta local branch after the fact.\n> > +\tpullBeforePush::\n> > +\t\tAdvice shown when you ran linkgit:git-push[1] and pushed\n> > +\t\ta non-fast-forward update to HEAD, instructing you to\n> > +\t\tlinkgit:git-pull[1] before pushing again.\n> > +\tuseUpstream::\n> > +\t\tAdvice to set 'push.default' to 'upstream' when you ran\n> > +\t\tlinkgit:git-push[1] and pushed 'matching refs' by default\n> > +\t\t(i.e. you did not have any explicit refspec on the command\n> > +\t\tline, and no 'push.default' configuration was set) and it\n> > +\t\tresulted in a non-fast-forward error.\n> > +\tcheckoutPullPush::\n> > +\t\tAdvice shown when you ran linkgit:git-push[1] and pushed\n> > +\t\ta non-fast-forward update to a non-HEAD branch, instructing\n> > +\t\tyou to checkout the branch and run linkgit:git-pull[1]\n> > +\t\tbefore pushing again.\n> \n> I would prefer to see these consolidated into a single advice.pushNonFF\n> variable, but I may be missing why it could be a good idea to allow them\n> turned off selectively.\n\nZbyszek\n"},{"id":"186938","messageId":"vpqobrzgww9.fsf@bauges.imag.fr","threadId":"29944","inReplyTo":"20120314121434.GB28595@in.waw.pl","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-03-14T13:00:38Z","receivedAt":"2012-03-14T13:00:38Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Zbigniew Jędrzejewski-Szmek writes:\n\n> I think that having three different config keys for the three\n> different advices makes sense, because the advices will be displayed\n> at different times.\n\nI don't think it really makes sense to be such fine-grained. We already\nhave 6 different advices, so an advance user who do not want them need\nto set these 6 variables. I think we want to keep this number relatively\nlow.\n\nThe advice messages do not point explicitely to the way to disable them,\nso users who know how to set advice.* are users who know a little about\nconfiguration files, and who read the docs. Instead of having too\nfine-grained configuration variables, we can have a better doc,\nexplaining shortly the 3 possible cases under advice.nonfastforward in\nconfig.txt. The user who disable the advice can read the doc (I usually\nthink that \"users don't read documentation\" is a better assumption, but\nsince the user knows about the name of the variable, it is OK here).\n\nAlso, if I read correctly the patch, the old variable is left in the doc\nand in advice.{c,h}, but is no longer used. This means old-timers who\nhave set it will see the message poping-up again after they upgrade,\nwhich I think is inconveinient for them.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"186948","messageId":"20120314142752.GD28595@in.waw.pl","threadId":"29944","inReplyTo":"vpqobrzgww9.fsf@bauges.imag.fr","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Zbigniew Jędrzejewski-Szmek","fromEmail":"zbyszek@in.waw.pl","sentAt":"2012-03-14T14:27:52Z","receivedAt":"2012-03-14T14:27:52Z","isPatch":true,"sender":{"key":"zbyszek@in.waw.pl","avatar":"https://avatars.githubusercontent.com/u/349618?v=4"},"body":"On Wed, Mar 14, 2012 at 02:00:38PM +0100, Matthieu Moy wrote:\n> Zbigniew Jędrzejewski-Szmek writes:\n> \n> > I think that having three different config keys for the three\n> > different advices makes sense, because the advices will be displayed\n> > at different times.\n> \n> I don't think it really makes sense to be such fine-grained. We already\n> have 6 different advices, so an advance user who do not want them need\n> to set these 6 variables. I think we want to keep this number relatively\n> low.\n> \n> The advice messages do not point explicitely to the way to disable them,\n> so users who know how to set advice.* are users who know a little about\n> configuration files, and who read the docs. \n\nElsewhere in this thread it was proposed to add an actual 'git config'\ncommand to the advice.\n\n> Instead of having too\n> fine-grained configuration variables, we can have a better doc,\n> explaining shortly the 3 possible cases under advice.nonfastforward in\n> config.txt. The user who disable the advice can read the doc (I usually\n> think that \"users don't read documentation\" is a better assumption, but\n> since the user knows about the name of the variable, it is OK here).\n> \n> Also, if I read correctly the patch, the old variable is left in the doc\n> and in advice.{c,h}, but is no longer used. This means old-timers who\n> have set it will see the message poping-up again after they upgrade,\n> which I think is inconveinient for them.\n\nSo it seems that the old variable should be respected, not to annoy\n\"old-timers\".\n\nZbyszek\n"},{"id":"186952","messageId":"20120314144802.GA3558@gmail.com","threadId":"29944","inReplyTo":"7vobrzst7n.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Christopher Tiwald","fromEmail":"christiwald@gmail.com","sentAt":"2012-03-14T14:48:03Z","receivedAt":"2012-03-14T14:48:03Z","isPatch":true,"sender":{"key":"christiwald@gmail.com","avatar":"https://avatars.githubusercontent.com/u/667276?v=4"},"body":"On Tue, Mar 13, 2012 at 09:27:08PM -0700, Junio C Hamano wrote:\n> An off-topic administrivia. Please do not try to deflect responses meant\n> for you by setting Mail-Followup-To.\n\nThanks for catching this. Truth be told I downloaded a command line MUA\nspecifically to send this patch and read in others from this list. I've\nbeen wrestling with the config and will wrestle it further.\n\n> Christopher Tiwald writes:\n> \n> > Pushing a non-fast-forward update to a remote repository will result in\n> > an error, but the hint text doesn't provide the correct resolution in\n> > every case. Three scenarios may arise depending on your workflow, each\n> > with a different resolution:\n> \n> Are we sure there are only three, or is this just \"we do not say anything\n> concrete, but at least we know common three cases, and there may be more\"?\n> \n> I am mostly interested in making sure that we do not give a bad advice.\n> Giving an advice that is mostly accurate and relevant for 95% of the time\n> is perfectly fine, as long as following the advice in the remaining 5%\n> does not result in a disaster.\n> \n> > 1) If you push a non-fast-forward update to HEAD, you should merge\n> > remote changes with 'git pull' before pushing again.\n> \n> You said \"to HEAD\", but I think you meant the case you push your current\n> branch (i.e. HEAD) to update any ref on the other side. In other words,\n> the push does not have to be \"*to*\" HEAD over there. Am I mistaken?\n> \n> > 3) If you push with explicit ref matching (e.g. 'git push ... topic:topic')\n> > while checked out on another branch (e.g. 'master'), the correct\n> > resolution is checking out the local branch, issuing git pull, and\n> > merging remote changes before pushing again.\n> \n> Or you may have misspelled the source side of the refspec and tried to\n> push a wrong branch.\n> \n> > Make nonfastforward an enum and teach transport.c to detect the\n> > scenarios described above. Give situation-specific resolution advice\n> > when pushes are rejected due to non-fast-forward updates. Finally,\n> > update other instances of nonfastforward to use the proper enum option.\n> \n> I think the overall direction of the implemention is good, modulo minor\n> design nits.\n> \n> * I do not particularly find NONFASTFORWARD_NONE that is defined to be 0\n> a useful readability measure. Plain vanilla constant 0 says that there\n> is nothing magical going on to the readers clearly already.\n> \n> * Also NONFASTFORWARD_FROTZ is way too long. Wouldn't NONFF_FROTZ be\n> sufficient and clear?\n\nThese notes make sense and I will reroll v2 with them in mind, as well\nas the other comments about the advice wording, sign-off line, and making\nnonfastforward a switch, not quoted here.\n\n> * I can see there are three kinds of advices, but I do not see why users\n> need to acknowledge that they understand them one by one with separate\n> advice configuration. Isn't it better to have only one variable, \"OK,\n> I know how to deal with a failed push due to non-fast-forward\"?\n> \n> > diff --git a/Documentation/config.txt b/Documentation/config.txt\n> > index c081657..50d9249 100644\n> > --- a/Documentation/config.txt\n> > +++ b/Documentation/config.txt\n> > @@ -158,6 +158,21 @@ advice.*::\n> > \t\tAdvice shown when you used linkgit:git-checkout[1] to\n> > \t\tmove to the detach HEAD state, to instruct how to create\n> > \t\ta local branch after the fact.\n> > +\tpullBeforePush::\n> > +\t\tAdvice shown when you ran linkgit:git-push[1] and pushed\n> > +\t\ta non-fast-forward update to HEAD, instructing you to\n> > +\t\tlinkgit:git-pull[1] before pushing again.\n> > +\tuseUpstream::\n> > +\t\tAdvice to set 'push.default' to 'upstream' when you ran\n> > +\t\tlinkgit:git-push[1] and pushed 'matching refs' by default\n> > +\t\t(i.e. you did not have any explicit refspec on the command\n> > +\t\tline, and no 'push.default' configuration was set) and it\n> > +\t\tresulted in a non-fast-forward error.\n> > +\tcheckoutPullPush::\n> > +\t\tAdvice shown when you ran linkgit:git-push[1] and pushed\n> > +\t\ta non-fast-forward update to a non-HEAD branch, instructing\n> > +\t\tyou to checkout the branch and run linkgit:git-pull[1]\n> > +\t\tbefore pushing again.\n> \n> I would prefer to see these consolidated into a single advice.pushNonFF\n> variable, but I may be missing why it could be a good idea to allow them\n> turned off selectively.\n\nAfter mulling over it, I tend to agree with this, but will address\nfurther down the thread.\n\n> Also, shouldn't we be doing this only when err is true, or is it too\n> defensive?\n\nThis was an oversight on my part. Given that the current error message\nhas been in place since 07436e4 in 2009 and doesn't seem to have caused\ntrouble (other than it not being applicable in some 'git push'\nsituations), I'll move the code in v2.\n\nThanks for the comments. They are much appreciated.\n\n--\nChristopher Tiwald\n"},{"id":"186951","messageId":"20120314145355.GB3558@gmail.com","threadId":"29944","inReplyTo":"20120314144802.GA3558@gmail.com","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Christopher Tiwald","fromEmail":"christiwald@gmail.com","sentAt":"2012-03-14T14:53:55Z","receivedAt":"2012-03-14T14:53:55Z","isPatch":true,"sender":{"key":"christiwald@gmail.com","avatar":"https://avatars.githubusercontent.com/u/667276?v=4"},"body":"On Wed, Mar 14, 2012 at 10:48:03AM -0400, Christopher Tiwald wrote:\n> \n\nWhoops. Also apologies for not following the correct To: and Cc:\nconvention in my most recent response.\n\n--\nChristopher Tiwald\n"},{"id":"186953","messageId":"20120314155204.GC3558@gmail.com","threadId":"29944","inReplyTo":"vpqipi7zh3n.fsf@bauges.imag.fr","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Christopher Tiwald","fromEmail":"christiwald@gmail.com","sentAt":"2012-03-14T15:52:04Z","receivedAt":"2012-03-14T15:52:04Z","isPatch":true,"sender":{"key":"christiwald@gmail.com","avatar":"https://avatars.githubusercontent.com/u/667276?v=4"},"body":"On Wed, Mar 14, 2012 at 10:06:52AM +0100, Matthieu Moy wrote:\n> Depending on the outcome of the discussion in the thread about\n> 'push.default', you may want to suggest 'current' instead of upstream:\n> http://thread.gmane.org/gmane.comp.version-control.git/192547/focus=192694\n> \n> Actually, if the user has 'push.default=matching', the least surprising\n> move from this value is 'push.default=current', that will push a subset\n> of what used to be pushed, and won't change the target branch.\n\nMy only concern about 'push.default=current' vs. 'upstream' is the\ncase where a developer might push to a central shared repository, but\nhas a local branch tracking a remote branch with a different name.\nThat might be too deep in the edge-case weeds, but it seems like for\n'centralized' git users, 'upstream' covers more cases without any\ndistraction to their workflows.\n\n> Your patch removes the _(...) around the string, which breaks the\n> internationalization.\n> ...\n> > +\tfor (i = 0; i < ARRAY_SIZE(message_advice_pull_before_push); i++)\n> > +\t\tadvise(message_advice_pull_before_push[i]);\n> \n> I'm no expert in gettext, but I think the internationalization people\n> will have a hard time dealing with a single message split accross an\n> array.\n> \n> Actually, I prefer the effect of a single advise() call (i.e. say\n> \"hint:\" just once, not for each line), but this part is subjective.\n\nThe lack of support for internationalization is an oversight. I'll\ncorrect it in v2.\n\n> I'd give the full cut-and-paste ready command to set the variable, to\n> help the user who doesn't know what \"configuration variable\" really\n> means in the context of Git.\n\nMakes a lot of sense. I remember struggling with setting config\nvariables when I was new to git. I'll make that change.\n\n--\nChristopher Tiwald\n"},{"id":"186964","messageId":"20120314164057.GD3558@gmail.com","threadId":"29944","inReplyTo":"20120314142752.GD28595@in.waw.pl","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Christopher Tiwald","fromEmail":"christiwald@gmail.com","sentAt":"2012-03-14T16:40:57Z","receivedAt":"2012-03-14T16:40:57Z","isPatch":true,"sender":{"key":"christiwald@gmail.com","avatar":"https://avatars.githubusercontent.com/u/667276?v=4"},"body":"On Wed, Mar 14, 2012 at 03:27:52PM +0100, Zbigniew Jędrzejewski-Szmek wrote:\n> On Wed, Mar 14, 2012 at 02:00:38PM +0100, Matthieu Moy wrote:\n> > Zbigniew Jędrzejewski-Szmek writes:\n> > \n> > > I think that having three different config keys for the three\n> > > different advices makes sense, because the advices will be displayed\n> > > at different times.\n> > \n> > I don't think it really makes sense to be such fine-grained. We already\n> > have 6 different advices, so an advance user who do not want them need\n> > to set these 6 variables. I think we want to keep this number relatively\n> > low.\n> > \n> > The advice messages do not point explicitely to the way to disable them,\n> > so users who know how to set advice.* are users who know a little about\n> > configuration files, and who read the docs. \n> \n> Elsewhere in this thread it was proposed to add an actual 'git config'\n> command to the advice.\n\nAfter considering it, I tend to agree that three different config keys\nis overkill. I feel like users who disable advice are doing it because\nthey find the messages annoying, not because they've mastered that\nparticular situation and no longer need the reminder. Forcing them to\ndisable three different options to get an advice-less 'git push' seems\nlike it'd just be irritating.\n\nI could be wrong about that. Perhaps users who graduate workflows as you\ndescribed above are more common? I don't disable any advice locally and\nthus can't speak well to what motivates that decision.\n> \n> > Instead of having too\n> > fine-grained configuration variables, we can have a better doc,\n> > explaining shortly the 3 possible cases under advice.nonfastforward in\n> > config.txt. The user who disable the advice can read the doc (I usually\n> > think that \"users don't read documentation\" is a better assumption, but\n> > since the user knows about the name of the variable, it is OK here).\n> > \n> > Also, if I read correctly the patch, the old variable is left in the doc\n> > and in advice.{c,h}, but is no longer used. This means old-timers who\n> > have set it will see the message poping-up again after they upgrade,\n> > which I think is inconveinient for them.\n> \n> So it seems that the old variable should be respected, not to annoy\n> \"old-timers\".\n\nI hadn't considered users who already have the variable set. I'll\ncorrect for that. I'll also attempt to improve the doc for\nadvice.nonfastforward.\n\n--\nChristopher Tiwald\n"},{"id":"186967","messageId":"7vty1rqek5.fsf@alter.siamese.dyndns.org","threadId":"29944","inReplyTo":"vpqipi7zh3n.fsf@bauges.imag.fr","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-14T17:26:34Z","receivedAt":"2012-03-14T17:26:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> I'm no expert in gettext, but I think the internationalization people\n> will have a hard time dealing with a single message split accross an\n> array.\n\nMy original patch on this topic predates the i18n adjustment we made to\nadvice infrastructure in 23cb5bf (i18n of multi-line advice messages,\n2011-12-22), so that is an understandable oversight.\n\nThanks for catching this.\n\n> Actually, I prefer the effect of a single advise() call (i.e. say\n> \"hint:\" just once, not for each line), but this part is subjective.\n\nThe way advice.c::error_resolve_conflict() uses multi-line advice message\nshould be a good template. The choice between \"hint\" for once or for\nevery line can later be adjusted in advice.c::advice() if we want to and\nsuch a change will convert all the users of advice API consistently.\n"},{"id":"187034","messageId":"20120315085426.GA11003@ecki","threadId":"29944","inReplyTo":"20120314142752.GD28595@in.waw.pl","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-03-15T08:54:26Z","receivedAt":"2012-03-15T08:54:26Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Wed, Mar 14, 2012 at 03:27:52PM +0100, Zbigniew Jędrzejewski-Szmek wrote:\n> On Wed, Mar 14, 2012 at 02:00:38PM +0100, Matthieu Moy wrote:\n> > Zbigniew Jędrzejewski-Szmek writes:\n> > \n> > > I think that having three different config keys for the three\n> > > different advices makes sense, because the advices will be displayed\n> > > at different times.\n> > \n> > I don't think it really makes sense to be such fine-grained. We already\n> > have 6 different advices, so an advance user who do not want them need\n> > to set these 6 variables. I think we want to keep this number relatively\n> > low.\n> > \n> > The advice messages do not point explicitely to the way to disable them,\n> > so users who know how to set advice.* are users who know a little about\n> > configuration files, and who read the docs. \n> \n> Elsewhere in this thread it was proposed to add an actual 'git config'\n> command to the advice.\n\nThe proposed command does not turn off the advice. It only changes\npush.default. The advice about push.default is effectively disabled once\nthey change push.default, but the other warnings are still in effect.\n"},{"id":"187051","messageId":"7vfwd9kacd.fsf@alter.siamese.dyndns.org","threadId":"29944","inReplyTo":"20120315085426.GA11003@ecki","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-15T18:06:26Z","receivedAt":"2012-03-15T18:06:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher writes:\n\n> On Wed, Mar 14, 2012 at 03:27:52PM +0100, Zbigniew Jędrzejewski-Szmek wrote:\n>> On Wed, Mar 14, 2012 at 02:00:38PM +0100, Matthieu Moy wrote:\n>> ...\n>> > The advice messages do not point explicitely to the way to disable them,\n>> > so users who know how to set advice.* are users who know a little about\n>> > configuration files, and who read the docs. \n>> \n>> Elsewhere in this thread it was proposed to add an actual 'git config'\n>> command to the advice.\n>\n> The proposed command does not turn off the advice. It only changes\n> push.default. The advice about push.default is effectively disabled once\n> they change push.default, but the other warnings are still in effect.\n\nTrue.\n\nI looked to see if some existing message that is triggered by advice.* has\nan extra comment at the end to suggest setting advice.* to false to\ndecline seeing the advice in the future, as it feels like a sensible thing\nto do and also I vaguely recalled us actually doing such a patch, but I do\nnot seem to be able to find such a message in the current codebase.\n\nNothing from a quick \"git log --no-merges --grep=advice --grep=advise\"\npops at me telling that we used to have instructions on how to decline but\nwe deliberately removed them, so I probably is misremembering things.\n\nWe do mention them in git-config(1), but it may be hard to match the\nvariables to situations from the description there UNLESS the user already\nunderstands what the annoying \"I know what I am doing, no need for this\nadvice anymore\" advice is about.\n\nOh, wait. Perhaps the advice messages are designed to be declined only by\nthe user who do understand, so perhaps it is a *good* think that we do not\nmention how to squelch in the message. In a twisted way, the logic sort\nof makes sense.\n\nI dunno.\n"},{"id":"187074","messageId":"7vlin1gl9l.fsf@alter.siamese.dyndns.org","threadId":"29944","inReplyTo":"7vty1rqek5.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-16T05:36:22Z","receivedAt":"2012-03-16T05:36:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Here is what I'll queue on top of your patch in 'pu', based on the review\ncomments in the thread.\n\nThis message is primarily to make sure everybody is on the same page,\nand ask eyeballs of people to make sure that I did not screw-up.\n\n Documentation/config.txt | 19 ++----------\n advice.c | 6 ----\n advice.h | 3 --\n builtin/push.c | 76 +++++++++++++++++++++-------------------------\n builtin/send-pack.c | 2 +-\n cache.h | 5 ++-\n transport.c | 10 +++---\n 7 files changed, 44 insertions(+), 77 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 50d9249..6e86681 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -138,8 +138,8 @@ advice.*::\n +\n --\n \tpushNonFastForward::\n-\t\tAdvice shown when linkgit:git-push[1] refuses\n-\t\tnon-fast-forward refs.\n+\t\tAdvice shown when linkgit:git-push[1] fails due to a\n+\t\tnon-fast-forward update.\n \tstatusHints::\n \t\tDirections on how to stage/unstage/add shown in the\n \t\toutput of linkgit:git-status[1] and the template shown\n@@ -158,21 +158,6 @@ advice.*::\n \t\tAdvice shown when you used linkgit:git-checkout[1] to\n \t\tmove to the detach HEAD state, to instruct how to create\n \t\ta local branch after the fact.\n-\tpullBeforePush::\n-\t\tAdvice shown when you ran linkgit:git-push[1] and pushed\n-\t\ta non-fast-forward update to HEAD, instructing you to\n-\t\tlinkgit:git-pull[1] before pushing again.\n-\tuseUpstream::\n-\t\tAdvice to set 'push.default' to 'upstream' when you ran\n-\t\tlinkgit:git-push[1] and pushed 'matching refs' by default\n-\t\t(i.e. you did not have any explicit refspec on the command\n-\t\tline, and no 'push.default' configuration was set) and it\n-\t\tresulted in a non-fast-forward error.\n-\tcheckoutPullPush::\n-\t\tAdvice shown when you ran linkgit:git-push[1] and pushed\n-\t\ta non-fast-forward update to a non-HEAD branch, instructing\n-\t\tyou to checkout the branch and run linkgit:git-pull[1]\n-\t\tbefore pushing again.\n --\n \n core.fileMode::\ndiff --git a/advice.c b/advice.c\nindex 608e90d..01130e5 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -6,9 +6,6 @@ int advice_commit_before_merge = 1;\n int advice_resolve_conflict = 1;\n int advice_implicit_identity = 1;\n int advice_detached_head = 1;\n-int advice_pull_before_push = 1;\n-int advice_use_upstream = 1;\n-int advice_checkout_pull_push = 1;\n \n static struct {\n \tconst char *name;\n@@ -20,9 +17,6 @@ static struct {\n \t{ \"resolveconflict\", &advice_resolve_conflict },\n \t{ \"implicitidentity\", &advice_implicit_identity },\n \t{ \"detachedhead\", &advice_detached_head },\n-\t{ \"pullbeforepush\", &advice_pull_before_push },\n-\t{ \"useupstream\", &advice_use_upstream },\n-\t{ \"checkoutpullpush\", &advice_checkout_pull_push }\n };\n \n void advise(const char *advice, ...)\ndiff --git a/advice.h b/advice.h\nindex ac07a44..7bda45b 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -9,9 +9,6 @@ extern int advice_commit_before_merge;\n extern int advice_resolve_conflict;\n extern int advice_implicit_identity;\n extern int advice_detached_head;\n-extern int advice_use_upstream;\n-extern int advice_pull_before_push;\n-extern int advice_checkout_pull_push;\n \n int git_default_advice_config(const char *var, const char *value);\n void advise(const char *advice, ...);\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 0fecf06..d7587d7 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -118,57 +118,45 @@ static void setup_default_push_refspecs(struct remote *remote)\n \t}\n }\n \n-static const char *message_advice_pull_before_push[] = {\n-\t\"To prevent you from losing history, non-fast-forward updates to HEAD\",\n-\t\"were rejected. Merge the remote changes (e.g. 'git pull') before\",\n-\t\"pushing again. See the 'Note about fast-forwards' section of\",\n-\t\"'git push --help' for details.\"\n-};\n-\n-static const char *message_advice_use_upstream[] = {\n-\t\"By default, git pushes all branches that have a matching counterpart\",\n-\t\"on the remote. In this case, some of your local branches were stale\",\n-\t\"with respect to their remote counterparts. If you did not intend to\",\n-\t\"push these branches, you may want to set the 'push.default'\",\n-\t\"configuration variable to 'upstream' to push only the current branch.\"\n-};\n-\n-static const char *message_advice_checkout_pull_push[] = {\n-\t\"To prevent you from losing history, your non-fast-forward branch\",\n-\t\"updates were rejected. Checkout the branch and merge the remote\",\n-\t\"changes (e.g. 'git pull') before pushing again. See the\",\n-\t\"'Note about fast-forwards' section of 'git push --help' for\",\n-\t\"details.\"\n-};\n+static const char message_advice_pull_before_push[] =\n+\tN_(\"Update was rejected because the tip of your current branch is behind\\n\"\n+\t \"the remote. Merge the remote changes (e.g. 'git pull') before\\n\"\n+\t \"pushing again. See the 'Note about fast-forwards' section of\\n\"\n+\t \"'git push --help' for details.\");\n+\n+\n+static const char message_advice_use_upstream[] =\n+\tN_(\"Some of your local branches were stale with respect to their\\n\"\n+\t \"remote counterparts. If you did not intend to push these branches,\\n\"\n+\t \"you may want to set the 'push.default' configuration variable to\\n\"\n+\t \"'current' or 'upstream' to push only the current branch.\");\n+\n+static const char message_advice_checkout_pull_push[] =\n+\tN_(\"Updates were rejected because the tip of some of your branches are\\n\"\n+\t \"behind the remote. Check out the branch and merge the remote\\n\"\n+\t \"changes (e.g. 'git pull') before pushing again. See the\\n\"\n+\t \"'Note about fast-forwards' section of 'git push --help'\\n\"\n+\t \"for details.\");\n \n static void advise_pull_before_push(void)\n {\n-\tint i;\n-\n-\tif (!advice_pull_before_push)\n+\tif (!advice_push_nonfastforward)\n \t\treturn;\n-\tfor (i = 0; i < ARRAY_SIZE(message_advice_pull_before_push); i++)\n-\t\tadvise(message_advice_pull_before_push[i]);\n+\tadvise(_(message_advice_pull_before_push));\n }\n \n static void advise_use_upstream(void)\n {\n-\tint i;\n-\n-\tif (!advice_use_upstream)\n+\tif (!advice_push_nonfastforward)\n \t\treturn;\n-\tfor (i = 0; i < ARRAY_SIZE(message_advice_use_upstream); i++)\n-\t\tadvise(message_advice_use_upstream[i]);\n+\tadvise(_(message_advice_use_upstream));\n }\n \n static void advise_checkout_pull_push(void)\n {\n-\tint i;\n-\n-\tif (!advice_checkout_pull_push)\n+\tif (!advice_push_nonfastforward)\n \t\treturn;\n-\tfor (i = 0; i < ARRAY_SIZE(message_advice_checkout_pull_push); i++)\n-\t\tadvise(message_advice_checkout_pull_push[i]);\n+\tadvise(_(message_advice_checkout_pull_push));\n }\n \n static int push_with_options(struct transport *transport, int flags)\n@@ -192,19 +180,23 @@ static int push_with_options(struct transport *transport, int flags)\n \t\terror(_(\"failed to push some refs to '%s'\"), transport->url);\n \n \terr |= transport_disconnect(transport);\n+\tif (!err)\n+\t\treturn 0;\n \n-\tif (nonfastforward == NONFASTFORWARD_HEAD) {\n+\tswitch (nonfastforward) {\n+\tdefault:\n+\t\tbreak;\n+\tcase NON_FF_HEAD:\n \t\tadvise_pull_before_push();\n-\t} else if (nonfastforward == NONFASTFORWARD_OTHER) {\n+\t\tbreak;\n+\tcase NON_FF_OTHER:\n \t\tif (default_matching_used)\n \t\t\tadvise_use_upstream();\n \t\telse\n \t\t\tadvise_checkout_pull_push();\n+\t\tbreak;\n \t}\n \n-\tif (!err)\n-\t\treturn 0;\n-\n \treturn 1;\n }\n \ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 09895b9..9df341c 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -409,7 +409,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \tint send_all = 0;\n \tconst char *receivepack = \"git-receive-pack\";\n \tint flags;\n-\tint nonfastforward = NONFASTFORWARD_NONE;\n+\tint nonfastforward = 0;\n \n \targv++;\n \tfor (i = 1; i < argc; i++, argv++) {\ndiff --git a/cache.h b/cache.h\nindex 14bc305..427b600 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1020,9 +1020,8 @@ struct ref {\n \t\tREF_STATUS_EXPECTING_REPORT\n \t} status;\n \tenum {\n-\t\tNONFASTFORWARD_NONE = 0,\n-\t\tNONFASTFORWARD_HEAD,\n-\t\tNONFASTFORWARD_OTHER\n+\t\tNON_FF_HEAD = 1,\n+\t\tNON_FF_OTHER\n \t} nonfastforward;\n \tchar *remote_status;\n \tstruct ref *peer_ref; /* when renaming */\ndiff --git a/transport.c b/transport.c\nindex 23210d5..7864007 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -736,18 +736,18 @@ void transport_print_push_status(const char *dest, struct ref *refs,\n \t\tif (ref->status == REF_STATUS_OK)\n \t\t\tn += print_one_push_status(ref, dest, n, porcelain);\n \n-\t*nonfastforward = NONFASTFORWARD_NONE;\n+\t*nonfastforward = 0;\n \tfor (ref = refs; ref; ref = ref->next) {\n \t\tif (ref->status != REF_STATUS_NONE &&\n \t\t ref->status != REF_STATUS_UPTODATE &&\n \t\t ref->status != REF_STATUS_OK)\n \t\t\tn += print_one_push_status(ref, dest, n, porcelain);\n \t\tif (ref->status == REF_STATUS_REJECT_NONFASTFORWARD &&\n-\t\t *nonfastforward != NONFASTFORWARD_HEAD) {\n+\t\t *nonfastforward != NON_FF_HEAD) {\n \t\t\tif (!strcmp(head, ref->name))\n-\t\t\t\t*nonfastforward = NONFASTFORWARD_HEAD;\n+\t\t\t\t*nonfastforward = NON_FF_HEAD;\n \t\t\telse\n-\t\t\t\t*nonfastforward = NONFASTFORWARD_OTHER;\n+\t\t\t\t*nonfastforward = NON_FF_OTHER;\n \t\t}\n \t}\n }\n@@ -1017,7 +1017,7 @@ int transport_push(struct transport *transport,\n \t\t int refspec_nr, const char **refspec, int flags,\n \t\t int *nonfastforward)\n {\n-\t*nonfastforward = NONFASTFORWARD_NONE;\n+\t*nonfastforward = 0;\n \ttransport_verify_remote_names(refspec_nr, refspec);\n \n \tif (transport->push) {\n-- \n1.7.10.rc1.22.g07e85\n"},{"id":"187075","messageId":"vpqzkbh555h.fsf@bauges.imag.fr","threadId":"29944","inReplyTo":"7vfwd9kacd.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-03-16T08:19:54Z","receivedAt":"2012-03-16T08:19:54Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Oh, wait. Perhaps the advice messages are designed to be declined only by\n> the user who do understand, so perhaps it is a *good* think that we do not\n> mention how to squelch in the message. In a twisted way, the logic sort\n> of makes sense.\n\nI'd be against having a detailed message with the cut-and-paste ready\ncommand to decline the message, as the messages would become long and\nannoying, so people would disable it too early.\n\nBut having a short mention like \"(to squelch this message, set\nadvice.bla)\", short enough not to be disturbing, and vague enough to\nforce people to read the docs.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"187077","messageId":"20120316091019.GB22273@ecki","threadId":"29944","inReplyTo":"7vlin1gl9l.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-03-16T09:10:19Z","receivedAt":"2012-03-16T09:10:19Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Thu, Mar 15, 2012 at 10:36:22PM -0700, Junio C Hamano wrote:\n>\n> +static const char message_advice_pull_before_push[] =\n> +\tN_(\"Update was rejected because the tip of your current branch is behind\\n\"\n> +\t \"the remote. Merge the remote changes (e.g. 'git pull') before\\n\"\n> +\t \"pushing again. See the 'Note about fast-forwards' section of\\n\"\n> +\t \"'git push --help' for details.\");\n> +\n> +\n> +static const char message_advice_use_upstream[] =\n> +\tN_(\"Some of your local branches were stale with respect to their\\n\"\n> +\t \"remote counterparts. If you did not intend to push these branches,\\n\"\n> +\t \"you may want to set the 'push.default' configuration variable to\\n\"\n> +\t \"'current' or 'upstream' to push only the current branch.\");\n> +\n> +static const char message_advice_checkout_pull_push[] =\n> +\tN_(\"Updates were rejected because the tip of some of your branches are\\n\"\n> +\t \"behind the remote. Check out the branch and merge the remote\\n\"\n> +\t \"changes (e.g. 'git pull') before pushing again. See the\\n\"\n> +\t \"'Note about fast-forwards' section of 'git push --help'\\n\"\n> +\t \"for details.\");\n\nThe first sentence of the above two warnings state the same thing, but\nin different ways. Yet the difference does not reflect the different\nsituations. They should be the same, or maybe the first one should be\nchanged to the following variant of the second:\n\n \"Updates were rejected because the tip of some of your branches are\n behind the remote branches with matching names.\"\n\nI like that you changed the advice to 'current' _or_ 'upstream'. But\nmaybe the variable name should change from message_advice_use_upstream\nto message_advice_push_default.\n\n> -\tif (nonfastforward == NONFASTFORWARD_HEAD) {\n> +\tswitch (nonfastforward) {\n> +\tdefault:\n> +\t\tbreak;\n> +\tcase NON_FF_HEAD:\n> \t\tadvise_pull_before_push();\n> -\t} else if (nonfastforward == NONFASTFORWARD_OTHER) {\n> +\t\tbreak;\n> +\tcase NON_FF_OTHER:\n> \t\tif (default_matching_used)\n> \t\t\tadvise_use_upstream();\n> \t\telse\n> \t\t\tadvise_checkout_pull_push();\n> +\t\tbreak;\n> \t}\n\nWe should not give advise_use_upstream if the user specified git push\n--all. The advice_checkout_pull_push would make more sense in that case.\n\nActually, if the user decides that matching branches is indeed the\ndefault they want to use, advise_checkout_pull_push would still be\nhelpful. So I think advise_checkout_pull_push should be given in any\ncase, while advise_use_upstream should be added if push.default=matching\nand the user did not say git push --all.\n"},{"id":"187086","messageId":"7v3998kb0x.fsf@alter.siamese.dyndns.org","threadId":"29944","inReplyTo":"20120316091019.GB22273@ecki","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-16T12:03:58Z","receivedAt":"2012-03-16T12:03:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher writes:\n\n> On Thu, Mar 15, 2012 at 10:36:22PM -0700, Junio C Hamano wrote:\n>>\n>> +static const char message_advice_pull_before_push[] =\n>> +\tN_(\"Update was rejected because the tip of your current branch is behind\\n\"\n>> +\t \"the remote. Merge the remote changes (e.g. 'git pull') before\\n\"\n>> +\t \"pushing again. See the 'Note about fast-forwards' section of\\n\"\n>> +\t \"'git push --help' for details.\");\n>> +\n>> +\n>> +static const char message_advice_use_upstream[] =\n>> +\tN_(\"Some of your local branches were stale with respect to their\\n\"\n>> +\t \"remote counterparts. If you did not intend to push these branches,\\n\"\n>> +\t \"you may want to set the 'push.default' configuration variable to\\n\"\n>> +\t \"'current' or 'upstream' to push only the current branch.\");\n>> +\n>> +static const char message_advice_checkout_pull_push[] =\n>> +\tN_(\"Updates were rejected because the tip of some of your branches are\\n\"\n>> +\t \"behind the remote. Check out the branch and merge the remote\\n\"\n>> +\t \"changes (e.g. 'git pull') before pushing again. See the\\n\"\n>> +\t \"'Note about fast-forwards' section of 'git push --help'\\n\"\n>> +\t \"for details.\");\n>\n> The first sentence of the above two warnings state the same thing, but\n> in different ways. Yet the difference does not reflect the different\n> situations. They should be the same, or maybe the first one should be\n> changed to the following variant of the second:\n>\n> \"Updates were rejected because the tip of some of your branches are\n> behind the remote branches with matching names.\"\n\nThat defeats the whole point of Christpher's patch and suggestion by Peff\nin the original discussion.\n\nThey apply to two different situations. If your current branch is behind,\nyou get the first one, if your current branch is *NOT* behind, but some\nothers are, you get the second one. The suggested solutions are different.\n\nRead each of them in isolation, imagining that you just saw your action\nresulted in a corresponding error condition. I thought they are clear\nenough (that is why I sent the pach), but the wording may still need to be\npolished, and updates are welcome.\n\n> We should not give advise_use_upstream if the user specified git push\n> --all. The advice_checkout_pull_push would make more sense in that case.\n\nYeah, \"default_matching_used\" variable should be looked at somewhere\naround that, but I *think* the approach Christpher and Peff took (and I\nagree with them) is to help solving the immediate problem the user has and\ncan address. Deal with the current branch first (which would solve \"the\ncurrent branch is behind\" problem). The next push may then show that\nother branches are behind, and at that time the other advice will tell him\nhow to deal with it (\"check out and fix them, and then push\").\n\nAgain, updates are welcome.\n"},{"id":"187105","messageId":"20120316172013.GA8119@gmail.com","threadId":"29944","inReplyTo":"7v3998kb0x.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Christopher Tiwald","fromEmail":"christiwald@gmail.com","sentAt":"2012-03-16T17:20:13Z","receivedAt":"2012-03-16T17:20:13Z","isPatch":true,"sender":{"key":"christiwald@gmail.com","avatar":"https://avatars.githubusercontent.com/u/667276?v=4"},"body":"On Fri, Mar 16, 2012 at 05:03:58AM -0700, Junio C Hamano wrote:\n> > We should not give advise_use_upstream if the user specified git push\n> > --all. The advice_checkout_pull_push would make more sense in that case.\n> \n> Yeah, \"default_matching_used\" variable should be looked at somewhere\n> around that, but I *think* the approach Christpher and Peff took (and I\n> agree with them) is to help solving the immediate problem the user has and\n> can address.\n\nYeah, this was how I interpretted Peff's original suggestion. It seemed\nlike a nice compromise between advice that was inapplicable and advice\nthat was too complex (\"There are 3 different non-ff errors in your push.\nHere are the four resolution processes required to fix them...\").\n\nThanks for the additional patching. The language / logic changes make\nsense. One quick, slightly-off-topic question: I'd like\nto take another crack at the patch's commit message, to implement\nsome of your language suggestions and clean it up further. Is it\nreasonable for me to wait a few days for additional comments or\nupdates, squash together these fixups into a single v2 patch (assuming\none patch is a logical unit for it), then resubmit?\n\nJust wanted to clarify the workflow,\n\n--\nChristopher Tiwald\n"},{"id":"187107","messageId":"7vk42kh11k.fsf@alter.siamese.dyndns.org","threadId":"29944","inReplyTo":"20120316172013.GA8119@gmail.com","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-16T18:07:51Z","receivedAt":"2012-03-16T18:07:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christopher Tiwald writes:\n\n> One quick, slightly-off-topic question: I'd like to take another crack\n> at the patch's commit message, to implement some of your language\n> suggestions and clean it up further. Is it reasonable for me to wait a\n> few days for additional comments or updates, squash together these\n> fixups into a single v2 patch (assuming one patch is a logical unit for\n> it), then resubmit?\n\nSurely.\n\nAfter reading the fix-up patch again, I actually have a couple of\ncomments/reservations myself.\n\n (1) I suggested (and the fix-up patch does so) to use a single existing\n advice configuration, but if you read the description in my response\n to Clemens carefully, you may realize that at least the configuration\n for \"Here is how to deal with your current branch\" and \"Here is how\n for the rest of your branch\" might be better if they are separate\n variables. The user may fix current branch (i.e. \"pull then push\"),\n set the advice.pushNonFastForward to false thinking that he learned\n everything there to know about non fast-forward, and then get another\n failure from \"git push\" because other branches are still behind, but\n with my \"fix-up\" patch, we would no longer give advice to him.\n\n (2) The advice to \"Your current branch is OK but you are also pushing\n others that do not fast-forward\" only talks about \"check out, pull\n and then push\", but an equally plausible solution may be \"don't push\n other branches---you are not working on them right now\". Both of our\n versions have this issue.\n\n I didn't trace the logic flow, though. If this advice is issued only\n to the user who explicitly said he wants to push these other branches\n (e.g. has \"push.default = matching\" in the config and gave no command\n line options, or gave refspec on the command line to tell us to push\n these other branches), then the wording is OK.\n\n But for the purpose of helping people who may be surprised by the\n current \"matching\" default, I think we should detect this very narrow\n case:\n\n - The user did not give us any refspec from the command line; and\n\n - The user does not have push.default set to matching (either the\n user does not have any push.default, or it is set to something\n else); and\n\n - The remote.$name.push would not push the branch other than the\n current branch.\n\n When these three conditions hold, we can be sure that the user worked\n on more than one branch and did \"git push $there\" without telling us\n what to push, and we defaulted to push \"matching\" and failed on stale\n branches that the user hasn't been working on. In that case, \"don't\n push other branches---perhaps push.default needs to be set\" may be a\n far more appropriate advice.\n\n So, the third case may have to be split further into two.\n"},{"id":"187110","messageId":"20120316214151.GA25092@ecki","threadId":"29944","inReplyTo":"7v3998kb0x.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-03-16T21:41:52Z","receivedAt":"2012-03-16T21:41:52Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Fri, Mar 16, 2012 at 05:03:58AM -0700, Junio C Hamano wrote:\n> Clemens Buchacher writes:\n> \n> > On Thu, Mar 15, 2012 at 10:36:22PM -0700, Junio C Hamano wrote:\n> >>\n> >> +static const char message_advice_pull_before_push[] =\n> >> +\tN_(\"Update was rejected because the tip of your current branch is behind\\n\"\n> >> +\t \"the remote. Merge the remote changes (e.g. 'git pull') before\\n\"\n> >> +\t \"pushing again. See the 'Note about fast-forwards' section of\\n\"\n> >> +\t \"'git push --help' for details.\");\n> >> +\n> >> +\n> >> +static const char message_advice_use_upstream[] =\n> >> +\tN_(\"Some of your local branches were stale with respect to their\\n\"\n> >> +\t \"remote counterparts. If you did not intend to push these branches,\\n\"\n> >> +\t \"you may want to set the 'push.default' configuration variable to\\n\"\n> >> +\t \"'current' or 'upstream' to push only the current branch.\");\n> >> +\n> >> +static const char message_advice_checkout_pull_push[] =\n> >> +\tN_(\"Updates were rejected because the tip of some of your branches are\\n\"\n> >> +\t \"behind the remote. Check out the branch and merge the remote\\n\"\n> >> +\t \"changes (e.g. 'git pull') before pushing again. See the\\n\"\n> >> +\t \"'Note about fast-forwards' section of 'git push --help'\\n\"\n> >> +\t \"for details.\");\n> >\n> > The first sentence of the above two warnings state the same thing, but\n> > in different ways. Yet the difference does not reflect the different\n> > situations. They should be the same, or maybe the first one should be\n> > changed to the following variant of the second:\n> >\n> > \"Updates were rejected because the tip of some of your branches are\n> > behind the remote branches with matching names.\"\n> \n> That defeats the whole point of Christpher's patch and suggestion by Peff\n> in the original discussion.\n> \n> They apply to two different situations. If your current branch is behind,\n> you get the first one, if your current branch is *NOT* behind, but some\n> others are, you get the second one. The suggested solutions are different.\n\nSorry if I did not express myself well. I should have deleted the first\nmessage. I was not talking about the case where the current branch is\nrejected. I mean the two cases where other branches are rejected.\n\nAnd the suggested solutions may be different for those too. I did not\nmean to object to that either. I only object to those two sentences,\nwhich basically say the same thing in different ways:\n\n \"Some of your local branches were stale with respect to their\\n\"\n \"remote counterparts.\n\n \"Updates were rejected because the tip of some of your branches are\\n\"\n \"behind the remote. Check out the branch and merge the remote\\n\"\n\n\n> > We should not give advise_use_upstream if the user specified git push\n> > --all. The advice_checkout_pull_push would make more sense in that case.\n> \n> Yeah, \"default_matching_used\" variable should be looked at somewhere\n> around that, but I *think* the approach Christpher and Peff took (and I\n> agree with them) is to help solving the immediate problem the user has and\n> can address. Deal with the current branch first (which would solve \"the\n> current branch is behind\" problem). The next push may then show that\n> other branches are behind, and at that time the other advice will tell him\n> how to deal with it (\"check out and fix them, and then push\").\n\nAgain, I am not talking about the current branch situation at all. I\ndon't understand what you mean here.\n"},{"id":"187112","messageId":"7v1uosfc0p.fsf@alter.siamese.dyndns.org","threadId":"29944","inReplyTo":"20120316214151.GA25092@ecki","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-16T21:53:42Z","receivedAt":"2012-03-16T21:53:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher writes:\n\n> On Fri, Mar 16, 2012 at 05:03:58AM -0700, Junio C Hamano wrote:\n>> Clemens Buchacher writes:\n>> ...\n>> >> +static const char message_advice_use_upstream[] =\n>> >> +\tN_(\"Some of your local branches were stale with respect to their\\n\"\n>> >> +\t \"remote counterparts. If you did not intend to push these branches,\\n\"\n>> >> +\t \"you may want to set the 'push.default' configuration variable to\\n\"\n>> >> +\t \"'current' or 'upstream' to push only the current branch.\");\n>> >> +\n>> >> +static const char message_advice_checkout_pull_push[] =\n>> >> +\tN_(\"Updates were rejected because the tip of some of your branches are\\n\"\n>> >> +\t \"behind the remote. Check out the branch and merge the remote\\n\"\n>> >> +\t \"changes (e.g. 'git pull') before pushing again. See the\\n\"\n>> >> +\t \"'Note about fast-forwards' section of 'git push --help'\\n\"\n>> >> +\t \"for details.\");\n>> >\n> ...\n> Sorry if I did not express myself well. I should have deleted the first\n> message. I was not talking about the case where the current branch is\n> rejected. I mean the two cases where other branches are rejected.\n\nOh, I see. My reply ended up being very similar, though ;-)\n\nThese two apply to two different situations. In the code, you can see\nwe switch between them like this:\n\n> +\tcase NON_FF_OTHER:\n> \t\tif (default_matching_used)\n> \t\t\tadvise_use_upstream();\n> \t\telse\n> \t\t\tadvise_checkout_pull_push();\n> +\t\tbreak;\n\nThis distinguishes the two cases _why_ you ended up pushing branches that\nare not your current branch. default_matching_used is set (eh, at least,\n\"designed to be set\"; the patch is based on my oooold patch whose details\nI do not recall offhand) only when the user said either \"git push\" or \"git\npush $there\" without explicit pathspec (i.e. \"git push $there other\" does\nnot set it to true) and we end up using the \"matching\" semantics as it is\nthe current built-in default.\n\nIf you tried to push other branch because you weren't aware of the\nmatching default, you get the first advice, if you explicitly tried to\npush other branch, you get the second one. The suggested solutions are\ndifferent.\n\nRead each of them in isolation, imagining that you just saw your action\nresulted in a corresponding error condition. I think they are clear\nenough (that is why I sent the pach), but the wording may still need to be\npolished, and updates are welcome.\n"},{"id":"187113","messageId":"7vty1odx46.fsf@alter.siamese.dyndns.org","threadId":"29944","inReplyTo":"7vk42kh11k.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-16T22:00:57Z","receivedAt":"2012-03-16T22:00:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> After reading the fix-up patch again, I actually have a couple of\n> comments/reservations myself.\n>\n> (1) I suggested (and the fix-up patch does so) to use a single existing\n> advice configuration, but ...\n>\n> (2) The advice to \"Your current branch is OK but you are also pushing\n\nWell, (2) is untrue. Between message-advice-checkout-pull-push and\nmessage-advice-use-upstream, we already capture this difference just fine.\n\nSorry about the noise.\n"},{"id":"187114","messageId":"20120316220117.GA25624@ecki","threadId":"29944","inReplyTo":"7v1uosfc0p.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-03-16T22:01:19Z","receivedAt":"2012-03-16T22:01:19Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Fri, Mar 16, 2012 at 02:53:42PM -0700, Junio C Hamano wrote:\n> Clemens Buchacher writes:\n> \n> > On Fri, Mar 16, 2012 at 05:03:58AM -0700, Junio C Hamano wrote:\n> >> Clemens Buchacher writes:\n> >> ...\n> >> >> +static const char message_advice_use_upstream[] =\n> >> >> +\tN_(\"Some of your local branches were stale with respect to their\\n\"\n> >> >> +\t \"remote counterparts. If you did not intend to push these branches,\\n\"\n> >> >> +\t \"you may want to set the 'push.default' configuration variable to\\n\"\n> >> >> +\t \"'current' or 'upstream' to push only the current branch.\");\n> >> >> +\n> >> >> +static const char message_advice_checkout_pull_push[] =\n> >> >> +\tN_(\"Updates were rejected because the tip of some of your branches are\\n\"\n> >> >> +\t \"behind the remote. Check out the branch and merge the remote\\n\"\n> >> >> +\t \"changes (e.g. 'git pull') before pushing again. See the\\n\"\n> >> >> +\t \"'Note about fast-forwards' section of 'git push --help'\\n\"\n> >> >> +\t \"for details.\");\n> >> >\n> > ...\n> > Sorry if I did not express myself well. I should have deleted the first\n> > message. I was not talking about the case where the current branch is\n> > rejected. I mean the two cases where other branches are rejected.\n> \n> Oh, I see. My reply ended up being very similar, though ;-)\n\nI am afraid we are still not talking about the same thing...\n"},{"id":"187115","messageId":"7vpqccdwof.fsf@alter.siamese.dyndns.org","threadId":"29944","inReplyTo":"20120316220117.GA25624@ecki","subject":"Re: [PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-16T22:10:24Z","receivedAt":"2012-03-16T22:10:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher writes:\n\n> I am afraid we are still not talking about the same thing...\n\nThen I give up and shut up, at least for now, as it would not help you if\nI rephrase what I said in a different way.\n\nIt's your turn to try explaining what is different better.\n"},{"id":"187167","messageId":"4F64C58B.4000207@in.waw.pl","threadId":"29944","inReplyTo":"7vlin1gl9l.fsf@alter.siamese.dyndns.org","subject":"[fixup PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Zbigniew Jędrzejewski-Szmek","fromEmail":"zbyszek@in.waw.pl","sentAt":"2012-03-17T17:10:35Z","receivedAt":"2012-03-17T17:10:35Z","isPatch":true,"sender":{"key":"zbyszek@in.waw.pl","avatar":"https://avatars.githubusercontent.com/u/349618?v=4"},"body":"On 03/16/2012 06:36 AM, Junio C Hamano wrote:\n> +static const char message_advice_pull_before_push[] =\n> +\tN_(\"Update was rejected because the tip of your current branch is behind\\n\"\n> +\t \"the remote. Merge the remote changes (e.g. 'git pull') before\\n\"\n> +\t \"pushing again. See the 'Note about fast-forwards' section of\\n\"\n> +\t \"'git push --help' for details.\");\n> +\n> +\n> +static const char message_advice_use_upstream[] =\n> +\tN_(\"Some of your local branches were stale with respect to their\\n\"\n> +\t \"remote counterparts. If you did not intend to push these branches,\\n\"\n> +\t \"you may want to set the 'push.default' configuration variable to\\n\"\n> +\t \"'current' or 'upstream' to push only the current branch.\");\n> +\n> +static const char message_advice_checkout_pull_push[] =\n> +\tN_(\"Updates were rejected because the tip of some of your branches are\\n\"\n> +\t \"behind the remote. Check out the branch and merge the remote\\n\"\n> +\t \"changes (e.g. 'git pull') before pushing again. See the\\n\"\n> +\t \"'Note about fast-forwards' section of 'git push --help'\\n\"\n> +\t \"for details.\");\n\nHi,\n\nClemens' observation that there are unnecessary differences between\n\"message_advice_use_upstream\" and \"message_advice_checkout_pull_push\"\nis valid. There also was a grammatical error in message_advice_checkout_pull_push\n(\"the tip ... are behind\") and some tense/number inconsistencies.\n\nI think the following can be squashed into 'fixup push-non-ff advice':\n\n- always start with \"Updates were rejected\", i.e. explain what is why\n git is talking\n- consistently use present tense to talk about stuff which is still true\n- mention that branches to be pushed can be specified (add\n \" explicitly specify branches to push or\" in\n \"you may want to set the 'push.default' configuration variable\")\n- use the simpler \"tip of your branch is behind the remote\" instead of the more \n complicated and longer \"some of your branches are stale with respect to their \n remote counterparts\".\n- resolve the \"tip ... are\" problem by using singular and talking about\n a single branch. This way there is no conflict with the following \n sentence which talks about checking out a single branch.\n- rewrap the text to 72 lines (standard TeX paragraph width).\n (One line is 73 characters, but it seems better than the \n alternative which makes the text take an extra line).\n\n[I know that this mixes whitespace/layout changes with the rest, but the texts were mostly rewritten anyway.]\n\nZbyszek\n\n------ 8< --------\nFrom ef8d15494d518df809e4a822af0d0e1c4008c91e Mon Sep 17 00:00:00 2001\nFrom: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= \nDate: Sat, 17 Mar 2012 18:00:42 +0100\nSubject: [PATCH] fixup! fixup push-non-ff advice\n\n---\n builtin/push.c | 25 +++++++++++--------------\n 1 file changed, 11 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 511a3ba..4c5b52b 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -142,24 +142,21 @@ static void setup_default_push_refspecs(struct remote *remote)\n }\n \n static const char message_advice_pull_before_push[] =\n-\tN_(\"Update was rejected because the tip of your current branch is behind\\n\"\n-\t \"the remote. Merge the remote changes (e.g. 'git pull') before\\n\"\n-\t \"pushing again. See the 'Note about fast-forwards' section of\\n\"\n-\t \"'git push --help' for details.\");\n-\n+\tN_(\"Update was rejected because the tip of your current branch is behind the\\n\"\n+\t \"remote. Merge the remote changes (e.g. 'git pull') before pushing again.\\n\"\n+\t \"See the 'Note about fast-forwards' in 'git push --help' for details.\");\n \n static const char message_advice_use_upstream[] =\n-\tN_(\"Some of your local branches were stale with respect to their\\n\"\n-\t \"remote counterparts. If you did not intend to push these branches,\\n\"\n-\t \"you may want to set the 'push.default' configuration variable to\\n\"\n-\t \"'current' or 'upstream' to push only the current branch.\");\n+\tN_(\"Updates were rejected because a tip of your branch is behind the remote.\\n\"\n+\t \"If you did not intend to push that branch, you may want to explicitly\\n\"\n+\t \"specify branches to push or set the 'push.default' configuration variable\"\n+\t \"to 'current' or 'upstream' to always push only the current branch.\");\n \n static const char message_advice_checkout_pull_push[] =\n-\tN_(\"Updates were rejected because the tip of some of your branches are\\n\"\n-\t \"behind the remote. Check out the branch and merge the remote\\n\"\n-\t \"changes (e.g. 'git pull') before pushing again. See the\\n\"\n-\t \"'Note about fast-forwards' section of 'git push --help'\\n\"\n-\t \"for details.\");\n+\tN_(\"Updates were rejected because a tip of your branch is behind the remote.\\n\"\n+\t \"Check out this branch and merge the remote changes (e.g. 'git pull')\\n\"\n+\t \"before pushing again.\\n\"\n+\t \"See the 'Note about fast-forwards' in 'git push --help' for details.\");\n \n static void advise_pull_before_push(void)\n {\n-- \n1.7.10.rc0.162.g5dce3\n\n------ >8 --------\n"},{"id":"187171","messageId":"20120317184649.GA320@gmail.com","threadId":"29944","inReplyTo":"4F64C58B.4000207@in.waw.pl","subject":"Re: [fixup PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Christopher Tiwald","fromEmail":"christiwald@gmail.com","sentAt":"2012-03-17T18:46:51Z","receivedAt":"2012-03-17T18:46:51Z","isPatch":true,"sender":{"key":"christiwald@gmail.com","avatar":"https://avatars.githubusercontent.com/u/667276?v=4"},"body":"On Sat, Mar 17, 2012 at 06:10:35PM +0100, Zbigniew Jędrzejewski-Szmek wrote:\n> static const char message_advice_use_upstream[] =\n> -\tN_(\"Some of your local branches were stale with respect to their\\n\"\n> -\t \"remote counterparts. If you did not intend to push these branches,\\n\"\n> -\t \"you may want to set the 'push.default' configuration variable to\\n\"\n> -\t \"'current' or 'upstream' to push only the current branch.\");\n> +\tN_(\"Updates were rejected because a tip of your branch is behind the remote.\\n\"\n> +\t \"If you did not intend to push that branch, you may want to explicitly\\n\"\n> +\t \"specify branches to push or set the 'push.default' configuration variable\"\n> +\t \"to 'current' or 'upstream' to always push only the current branch.\");\n\nI prefer the \"Some of your local...\" language to \"Updates were\nrejected...\" as a reader, but I think you're right about providing the\nreason git rejected the push up front.\n\nMy concern about this particular message is \"tip of your branch is behind\nthe remote\" reads to me like my _current_ branch is the offender, when\nthat cannot be the case (it'd hit message_advice_pull_before_push\nfirst). Maybe something like this might make it clearer?\n\n\"Updates were rejected because a pushed branch tip is behind its remote\ncounterpart. If you did not intend to push that branch, you may want to\nexplicitly specify branches to push or set the 'push.default' configuration\nvariable to 'current' or 'upstream' to always push only the current branch.\"\n\n--\nChristopher Tiwald\n"},{"id":"187172","messageId":"4F64E920.7010008@in.waw.pl","threadId":"29944","inReplyTo":"20120317184649.GA320@gmail.com","subject":"Re: [fixup PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Zbigniew Jędrzejewski-Szmek","fromEmail":"zbyszek@in.waw.pl","sentAt":"2012-03-17T19:42:24Z","receivedAt":"2012-03-17T19:42:24Z","isPatch":true,"sender":{"key":"zbyszek@in.waw.pl","avatar":"https://avatars.githubusercontent.com/u/349618?v=4"},"body":"On 03/17/2012 07:46 PM, Christopher Tiwald wrote:\n> On Sat, Mar 17, 2012 at 06:10:35PM +0100, Zbigniew Jędrzejewski-Szmek wrote:\n>> static const char message_advice_use_upstream[] =\n>> -\tN_(\"Some of your local branches were stale with respect to their\\n\"\n>> -\t \"remote counterparts. If you did not intend to push these branches,\\n\"\n>> -\t \"you may want to set the 'push.default' configuration variable to\\n\"\n>> -\t \"'current' or 'upstream' to push only the current branch.\");\n>> +\tN_(\"Updates were rejected because a tip of your branch is behind the remote.\\n\"\n>> +\t \"If you did not intend to push that branch, you may want to explicitly\\n\"\n>> +\t \"specify branches to push or set the 'push.default' configuration variable\"\n>> +\t \"to 'current' or 'upstream' to always push only the current branch.\");\n>\n> I prefer the \"Some of your local...\" language to \"Updates were\n> rejected...\" as a reader, but I think you're right about providing the\n> reason git rejected the push up front.\n>\n> My concern about this particular message is \"tip of your branch is behind\n> the remote\" reads to me like my _current_ branch is the offender, when\n> that cannot be the case (it'd hit message_advice_pull_before_push\n> first). Maybe something like this might make it clearer?\n>\n> \"Updates were rejected because a pushed branch tip is behind its remote\n> counterpart. If you did not intend to push that branch, you may want to\n> explicitly specify branches to push or set the 'push.default' configuration\n> variable to 'current' or 'upstream' to always push only the current branch.\"\nYeah, that's better.\n\nZbyszek\n"},{"id":"187226","messageId":"7v7gyhcupe.fsf@alter.siamese.dyndns.org","threadId":"29944","inReplyTo":"20120317184649.GA320@gmail.com","subject":"Re: [fixup PATCH] push: Provide situational hints for non-fast-forward errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-19T00:15:09Z","receivedAt":"2012-03-19T00:15:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christopher Tiwald writes:\n\n> I prefer the \"Some of your local...\" language to \"Updates were\n> rejected...\" as a reader, but I think you're right about providing the\n> reason git rejected the push up front.\n\nOk.\n\n> My concern about this particular message is \"tip of your branch is behind\n> the remote\" reads to me like my _current_ branch is the offender, when\n> that cannot be the case (it'd hit message_advice_pull_before_push\n> first). Maybe something like this might make it clearer?\n>\n> \"Updates were rejected because a pushed branch tip is behind its remote\n> counterpart. If you did not intend to push that branch, you may want to\n> explicitly specify branches to push or set the 'push.default' configuration\n> variable to 'current' or 'upstream' to always push only the current branch.\"\n\nSounds good.\n"}]}