{"thread":{"id":"26448","subject":"[PATCH v2] Add support for merging from upstream by default.","startedAt":"2011-02-08T20:48:49Z","lastAt":"2011-02-09T23:27:32Z","messageCount":14,"participants":["Jared Hance","Jay Soffian","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"160711","messageId":"1297198129-3403-1-git-send-email-jaredhance@gmail.com","threadId":"26448","inReplyTo":null,"subject":"[PATCH v2] Add support for merging from upstream by default.","fromName":"Jared Hance","fromEmail":"jaredhance@gmail.com","sentAt":"2011-02-08T20:48:49Z","receivedAt":"2011-02-08T20:48:49Z","isPatch":true,"sender":{"key":"jaredhance@gmail.com","avatar":"https://avatars.githubusercontent.com/u/170192?v=4"},"body":"Adds the option merge.defaultupstream to add support for merging from the\nupstream branch by default. The upstream branch is found using\nbranch.[name].merge.\n\nAlso add helper functions `per_branch_config` and `setup_merge_commit` to\nreduce reduncancy and impove clarity.\n\nSigned-off-by: Jared Hance <jaredhance@gmail.com>\n---\n Documentation/config.txt |    5 +++\n builtin/merge.c          |   81 +++++++++++++++++++++++++++++++++-------------\n 2 files changed, 63 insertions(+), 23 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex c5e1835..25c8292 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1389,6 +1389,11 @@ man.<tool>.path::\n \n include::merge-config.txt[]\n \n+merge.defaultUpstream::\n+\tIf merge is called without any ref arguments, merge from the branch\n+\tspecified in branch.<current branch>.merge, which is considered to be\n+\tthe upstream branch for the current branch.\n+\n mergetool.<tool>.path::\n \tOverride the path for the given tool.  This is useful in case\n \tyour tool is not in the PATH.\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 42fff38..2b4fd4f 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -37,7 +37,7 @@ struct strategy {\n };\n \n static const char * const builtin_merge_usage[] = {\n-\t\"git merge [options] <remote>...\",\n+\t\"git merge [options] [<remote>...]\",\n \t\"git merge [options] <msg> HEAD <remote>\",\n \tNULL\n };\n@@ -58,6 +58,8 @@ static int option_renormalize;\n static int verbosity;\n static int allow_rerere_auto;\n static int abort_current_merge;\n+static int default_upstream;\n+static const char *upstream_branch;\n \n static struct strategy all_strategy[] = {\n \t{ \"recursive\",  DEFAULT_TWOHEAD | NO_TRIVIAL },\n@@ -498,11 +500,15 @@ cleanup:\n \tstrbuf_release(&bname);\n }\n \n-static int git_merge_config(const char *k, const char *v, void *cb)\n+static int per_branch_config(const char *k, const char *v, void *cb)\n {\n-\tif (branch && !prefixcmp(k, \"branch.\") &&\n-\t\t!prefixcmp(k + 7, branch) &&\n-\t\t!strcmp(k + 7 + strlen(branch), \".mergeoptions\")) {\n+\tconst char *variable;\n+\tif(!branch || prefixcmp(k, \"branch.\")\n+\t   || prefixcmp(k + 7, branch))\n+\t\treturn 1; /* ignore me */\n+\n+\tvariable = k + 7 + strlen(branch);\n+\tif(!strcmp(variable, \".mergeoptions\")) {\n \t\tconst char **argv;\n \t\tint argc;\n \t\tchar *buf;\n@@ -518,9 +524,26 @@ static int git_merge_config(const char *k, const char *v, void *cb)\n \t\tparse_options(argc, argv, NULL, builtin_merge_options,\n \t\t\t      builtin_merge_usage, 0);\n \t\tfree(buf);\n+\n+\t\treturn 0;\n \t}\n+\telse if(strcmp(variable, \".merge\")) {\n+\t\treturn git_config_string(&upstream_branch, k, v);\n+\t}\n+\n+\treturn 1; /* not what I handle */\n+}\n \n-\tif (!strcmp(k, \"merge.diffstat\") || !strcmp(k, \"merge.stat\"))\n+static int git_merge_config(const char *k, const char *v, void *cb)\n+{\n+\tint status = per_branch_config(k, v, cb);\n+\n+\tif (status <= 0)\n+\t\treturn status;\n+\n+\tif (!strcmp(k, \"merge.defaultupstream\"))\n+\t\tdefault_upstream = git_config_bool(k, v);\n+\telse if (!strcmp(k, \"merge.diffstat\") || !strcmp(k, \"merge.stat\"))\n \t\tshow_diffstat = git_config_bool(k, v);\n \telse if (!strcmp(k, \"pull.twohead\"))\n \t\treturn git_config_string(&pull_twohead, k, v);\n@@ -911,6 +934,24 @@ static int evaluate_result(void)\n \treturn cnt;\n }\n \n+static void setup_merge_commit(struct strbuf *buf,\n+\tstruct commit_list ***remotes, const char *s)\n+{\n+\tstruct object *o;\n+\tstruct commit *commit;\n+\n+\to = peel_to_type(s, 0, NULL, OBJ_COMMIT);\n+\tif (!o)\n+\t\tdie(\"%s - not something we can merge\", s);\n+\tcommit = lookup_commit(o->sha1);\n+\tcommit->util = (void *)s;\n+\t*remotes = &commit_list_insert(commit, *remotes)->next;\n+\n+\tstrbuf_addf(buf, \"GITHEAD_%s\", sha1_to_hex(o->sha1));\n+\tsetenv(buf->buf, s, 1);\n+\tstrbuf_reset(buf);\n+}\n+\n int cmd_merge(int argc, const char **argv, const char *prefix)\n {\n \tunsigned char result_tree[20];\n@@ -983,9 +1024,15 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \tif (!allow_fast_forward && fast_forward_only)\n \t\tdie(\"You cannot combine --no-ff with --ff-only.\");\n \n-\tif (!argc)\n-\t\tusage_with_options(builtin_merge_usage,\n-\t\t\tbuiltin_merge_options);\n+\tif (!argc) {\n+\t\tif(default_upstream && upstream_branch) {\n+\t\t\tsetup_merge_commit(&buf, &remotes, upstream_branch);\n+\t\t}\n+\t\telse {\n+\t\t\tusage_with_options(builtin_merge_usage,\n+\t\t\t\t\tbuiltin_merge_options);\n+\t\t}\n+\t}\n \n \t/*\n \t * This could be traditional \"merge <msg> HEAD <commit>...\"  and\n@@ -1048,7 +1095,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \n-\tif (head_invalid || !argc)\n+\tif (head_invalid || (!argc && !(default_upstream && upstream_branch)))\n \t\tusage_with_options(builtin_merge_usage,\n \t\t\tbuiltin_merge_options);\n \n@@ -1059,19 +1106,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \tstrbuf_reset(&buf);\n \n \tfor (i = 0; i < argc; i++) {\n-\t\tstruct object *o;\n-\t\tstruct commit *commit;\n-\n-\t\to = peel_to_type(argv[i], 0, NULL, OBJ_COMMIT);\n-\t\tif (!o)\n-\t\t\tdie(\"%s - not something we can merge\", argv[i]);\n-\t\tcommit = lookup_commit(o->sha1);\n-\t\tcommit->util = (void *)argv[i];\n-\t\tremotes = &commit_list_insert(commit, remotes)->next;\n-\n-\t\tstrbuf_addf(&buf, \"GITHEAD_%s\", sha1_to_hex(o->sha1));\n-\t\tsetenv(buf.buf, argv[i], 1);\n-\t\tstrbuf_reset(&buf);\n+\t\tsetup_merge_commit(&buf, &remotes, argv[i]);\n \t}\n \n \tif (!use_strategies) {\n-- \n1.7.4\n"},{"id":"160712","messageId":"AANLkTi=rZMfUiwr9m3BogaVFWfKCy6FQaHaqKKJrEsCY@mail.gmail.com","threadId":"26448","inReplyTo":"1297198129-3403-1-git-send-email-jaredhance@gmail.com","subject":"Re: [PATCH v2] Add support for merging from upstream by default.","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-02-08T21:38:07Z","receivedAt":"2011-02-08T21:38:07Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Tue, Feb 8, 2011 at 3:48 PM, Jared Hance <jaredhance@gmail.com> wrote:\n>\n> Adds the option merge.defaultupstream to add support for merging from the\n\nNit: I agree with Jeff[1] that this should be called merge.defaultToUpstream\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/165745\n\nj.\n"},{"id":"160714","messageId":"20110208223359.GB17981@elie","threadId":"26448","inReplyTo":"1297198129-3403-1-git-send-email-jaredhance@gmail.com","subject":"Re: [PATCH v2] Add support for merging from upstream by default.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-02-08T22:33:59Z","receivedAt":"2011-02-08T22:33:59Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jared Hance wrote:\n\n> Adds the option merge.defaultupstream to add support for merging from the\n> upstream branch by default.\n\nCould you give an example of breakage this configurability is designed\nto prevent?\n\nNot that it is a bad idea to be careful anyway, mind you --- just\nlooking for a clear answer for when people ask \"and why would I ever\nwant to set merge.defaultToUpstream to false?\"\n\n> reduce reduncancy and impove clarity.\n\nredundancy, improve :)\n\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1389,6 +1389,11 @@ man.<tool>.path::\n>  \n>  include::merge-config.txt[]\n>  \n> +merge.defaultUpstream::\n> +\tIf merge is called without any ref arguments, merge from the branch\n> +\tspecified in branch.<current branch>.merge, which is considered to be\n> +\tthe upstream branch for the current branch.\n\nI'd prefer to say \"merge from the branch configured with --track or\n--set-upstream\" (but that's just me).\n\n> +++ b/builtin/merge.c\n> @@ -37,7 +37,7 @@ struct strategy {\n>  };\n>  \n>  static const char * const builtin_merge_usage[] = {\n> -\t\"git merge [options] <remote>...\",\n> +\t\"git merge [options] [<remote>...]\",\n>  \t\"git merge [options] <msg> HEAD <remote>\",\n>  \tNULL\n\nSide note: these should probably say \"<commit>\" or \"<branch>\" rather\nthan \"<remote>\".  I'm guessing the usage string comes from the days\nbefore the separate-remotes ref layout...\n\n> @@ -58,6 +58,8 @@ static int option_renormalize;\n[...]\n> -static int git_merge_config(const char *k, const char *v, void *cb)\n> +static int per_branch_config(const char *k, const char *v, void *cb)\n>  {\n> -\tif (branch && !prefixcmp(k, \"branch.\") &&\n> -\t\t!prefixcmp(k + 7, branch) &&\n> -\t\t!strcmp(k + 7 + strlen(branch), \".mergeoptions\")) {\n> +\tconst char *variable;\n> +\tif(!branch || prefixcmp(k, \"branch.\")\n> +\t   || prefixcmp(k + 7, branch))\n> +\t\treturn 1; /* ignore me */\n\nStyle: missing space after \"if\" keyword.\n\nClarity: we are not supposed to _ignore_ the non-branch.*\nconfiguration, just leave it for other functions to handle, no?\nMaybe the comment should say \"let others handle it\" or something?\n\n> +\n> +\tvariable = k + 7 + strlen(branch);\n\n'7' can be written in a more self-explanatory way as 'strlen(\"branch.\")'\nand the optimizer will take care of translating it to 7.  Don't worry\nabout it if that makes the diff or code harder to read, though; I'm\njust mentioning the trick for future reference.\n\n> +\tif(!strcmp(variable, \".mergeoptions\")) {\n\nStyle: missing space after \"if\" keyword.\n\n> @@ -518,9 +524,26 @@ static int git_merge_config(const char *k, const char *v, void *cb)\n>  \t\tparse_options(argc, argv, NULL, builtin_merge_options,\n>  \t\t\t      builtin_merge_usage, 0);\n>  \t\tfree(buf);\n> +\n> +\t\treturn 0;\n\nI'd group the cleanup with the return.\n\n\tparse_options(...);\n\n\tfree(buf);\n\treturn 0;\n\n>  \t}\n> +\telse if(strcmp(variable, \".merge\")) {\n\nStyle: missing space after \"if\".  Uncuddled brace.\n\n[...]\n> @@ -911,6 +934,24 @@ static int evaluate_result(void)\n>  \treturn cnt;\n>  }\n>  \n> +static void setup_merge_commit(struct strbuf *buf,\n> +\tstruct commit_list ***remotes, const char *s)\n> +{\n> +\tstruct object *o;\n> +\tstruct commit *commit;\n> +\n> +\to = peel_to_type(s, 0, NULL, OBJ_COMMIT);\n> +\tif (!o)\n> +\t\tdie(\"%s - not something we can merge\", s);\n> +\tcommit = lookup_commit(o->sha1);\n> +\tcommit->util = (void *)s;\n> +\t*remotes = &commit_list_insert(commit, *remotes)->next;\n> +\n> +\tstrbuf_addf(buf, \"GITHEAD_%s\", sha1_to_hex(o->sha1));\n> +\tsetenv(buf->buf, s, 1);\n> +\tstrbuf_reset(buf);\n> +}\n\nWould be easier to review if this code movement were in a separate\npatch (separating cleanup from semantic changes).\n\n> @@ -983,9 +1024,15 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n>  \tif (!allow_fast_forward && fast_forward_only)\n>  \t\tdie(\"You cannot combine --no-ff with --ff-only.\");\n>  \n> -\tif (!argc)\n> -\t\tusage_with_options(builtin_merge_usage,\n> -\t\t\tbuiltin_merge_options);\n> +\tif (!argc) {\n> +\t\tif(default_upstream && upstream_branch) {\n\nStyle: missing space after \"if\".  Unnecessary braces.  To avoid\nkeeping the reader in suspense, it's usually best to handle the\n(simple) error case first, like so:\n\n\t\tif (!default_upstream || !upstream_branch)\n\t\t\tusage_with_options(...);\n\t\tsetup_merge_commit(...);\n\n[...]\n> @@ -1048,7 +1095,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n>  \t\t}\n>  \t}\n>  \n> -\tif (head_invalid || !argc)\n> +\tif (head_invalid || (!argc && !(default_upstream && upstream_branch)))\n\nMight be clearer to split the line?\n\n\tif (head_invalid\n\t    || (!argc && (!default_upstream || !upstream_branch)))\n\t\tusage_with_options(...);\n    \nEven better would be to use descriptive messages, like so:\n\n if (head_invalid)\n\tusage_msg_opt(\"cannot use old-style invocation from an unborn branch\",\n\t\t...);\n if (!argc && ...)\n\tusage_msg_opt(\"no commit to merge specified\", ...);\n\nThanks for making this happen. :)\nJonathan\n"},{"id":"160718","messageId":"7vpqr2oyy3.fsf@alter.siamese.dyndns.org","threadId":"26448","inReplyTo":"20110208223359.GB17981@elie","subject":"Re: [PATCH v2] Add support for merging from upstream by default.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-09T00:04:36Z","receivedAt":"2011-02-09T00:04:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Jared Hance wrote:\n\n>>> Subject: Re: [PATCH v2] Add support for merging from upstream by default.\n\nDrop full-stop '.' at the end.\n\n>> Adds the option merge.defaultupstream to add support for merging from the\n>> upstream branch by default.\n>\n> Could you give an example of breakage this configurability is designed\n> to prevent?\n\nI think there is no \"prevent\" or \"breakage\"; the patch is to give people a\nway to turn the feature on; without the configuration, \"git merge\" will\nkeep the traditional behaviour, no?\n\n>> +++ b/builtin/merge.c\n>> @@ -37,7 +37,7 @@ struct strategy {\n>>  };\n>>  \n>>  static const char * const builtin_merge_usage[] = {\n>> -\t\"git merge [options] <remote>...\",\n>> +\t\"git merge [options] [<remote>...]\",\n>>  \t\"git merge [options] <msg> HEAD <remote>\",\n>>  \tNULL\n>\n> Side note: these should probably say \"<commit>\" or \"<branch>\" rather\n> than \"<remote>\".  I'm guessing the usage string comes from the days\n> before the separate-remotes ref layout...\n\nYes, your guess is correct.\n\n>> @@ -911,6 +934,24 @@ static int evaluate_result(void)\n>>  \treturn cnt;\n>>  }\n>>  \n>> +static void setup_merge_commit(struct strbuf *buf,\n>> +\tstruct commit_list ***remotes, const char *s)\n>> +{\n>> +\tstruct object *o;\n>> +\tstruct commit *commit;\n>> +\n>> +\to = peel_to_type(s, 0, NULL, OBJ_COMMIT);\n>> +\tif (!o)\n>> +\t\tdie(\"%s - not something we can merge\", s);\n>> +\tcommit = lookup_commit(o->sha1);\n>> +\tcommit->util = (void *)s;\n>> +\t*remotes = &commit_list_insert(commit, *remotes)->next;\n>> +\n>> +\tstrbuf_addf(buf, \"GITHEAD_%s\", sha1_to_hex(o->sha1));\n>> +\tsetenv(buf->buf, s, 1);\n>> +\tstrbuf_reset(buf);\n>> +}\n>\n> Would be easier to review if this code movement were in a separate\n> patch (separating cleanup from semantic changes).\n\nProbably.  It is a very good idea to move this code out to its own helper\nfunction, nevertheless; I like this part of the patch.\n\n> Even better would be to use descriptive messages, like so:\n>\n>  if (head_invalid)\n> \tusage_msg_opt(\"cannot use old-style invocation from an unborn branch\",\n> \t\t...);\n>  if (!argc && ...)\n> \tusage_msg_opt(\"no commit to merge specified\", ...);\n\nMuch better.\n"},{"id":"160720","messageId":"1297211028-14382-1-git-send-email-jaredhance@gmail.com","threadId":"26448","inReplyTo":"1297198129-3403-1-git-send-email-jaredhance@gmail.com","subject":"[PATCH v3 0/3] Updated patch series for default upstream merge","fromName":"Jared Hance","fromEmail":"jaredhance@gmail.com","sentAt":"2011-02-09T00:23:45Z","receivedAt":"2011-02-09T00:23:45Z","isPatch":true,"sender":{"key":"jaredhance@gmail.com","avatar":"https://avatars.githubusercontent.com/u/170192?v=4"},"body":"This patch series allows for `git merge` to default to the upstream branch\nof the current branch.\n\nNotes/Complications:\n\t- I'm not sure whether the option should be merge.defaultUpstream\n\t  or merge.defaultToUpstream\n\t- Should [remotes] be changed to [branches]? I felt like it was\n\t  a completely different change and didn't belong in the patch series.\n\t- I left one of the ifs with unnecessary braces for clarity because\n\t  of a nested if-else: is this the preferred style?\n\nJared Hance (3):\n  Add setup_merge_commit function to merge/builtin.c.\n  Add function per_branch_config.\n  Add support for merging from upstream by default.\n\n Documentation/config.txt |    6 +++\n builtin/merge.c          |   87 ++++++++++++++++++++++++++++++++-------------\n 2 files changed, 68 insertions(+), 25 deletions(-)\n\n-- \n1.7.4\n"},{"id":"160721","messageId":"1297211028-14382-2-git-send-email-jaredhance@gmail.com","threadId":"26448","inReplyTo":"1297211028-14382-1-git-send-email-jaredhance@gmail.com","subject":"[PATCH v3 1/3] Add setup_merge_commit function to merge/builtin.c.","fromName":"Jared Hance","fromEmail":"jaredhance@gmail.com","sentAt":"2011-02-09T00:23:46Z","receivedAt":"2011-02-09T00:23:46Z","isPatch":true,"sender":{"key":"jaredhance@gmail.com","avatar":"https://avatars.githubusercontent.com/u/170192?v=4"},"body":"Adds a new function that will be used in a later patch as well as the\ncurrent code to reduce redundancy.\n\nSigned-off-by: Jared Hance <jaredhance@gmail.com>\n---\n builtin/merge.c |   32 +++++++++++++++++++-------------\n 1 files changed, 19 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 42fff38..bc1ae94 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -911,6 +911,24 @@ static int evaluate_result(void)\n \treturn cnt;\n }\n \n+static void setup_merge_commit(struct strbuf *buf,\n+\tstruct commit_list ***remotes, const char *s)\n+{\n+\tstruct object *o;\n+\tstruct commit *commit;\n+\n+\to = peel_to_type(s, 0, NULL, OBJ_COMMIT);\n+\tif (!o)\n+\t\tdie(\"%s - not something we can merge\", s);\n+\tcommit = lookup_commit(o->sha1);\n+\tcommit->util = (void *)s;\n+\t*remotes = &commit_list_insert(commit, *remotes)->next;\n+\n+\tstrbuf_addf(buf, \"GITHEAD_%s\", sha1_to_hex(o->sha1));\n+\tsetenv(buf->buf, s, 1);\n+\tstrbuf_reset(buf);\n+}\n+\n int cmd_merge(int argc, const char **argv, const char *prefix)\n {\n \tunsigned char result_tree[20];\n@@ -1059,19 +1077,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \tstrbuf_reset(&buf);\n \n \tfor (i = 0; i < argc; i++) {\n-\t\tstruct object *o;\n-\t\tstruct commit *commit;\n-\n-\t\to = peel_to_type(argv[i], 0, NULL, OBJ_COMMIT);\n-\t\tif (!o)\n-\t\t\tdie(\"%s - not something we can merge\", argv[i]);\n-\t\tcommit = lookup_commit(o->sha1);\n-\t\tcommit->util = (void *)argv[i];\n-\t\tremotes = &commit_list_insert(commit, remotes)->next;\n-\n-\t\tstrbuf_addf(&buf, \"GITHEAD_%s\", sha1_to_hex(o->sha1));\n-\t\tsetenv(buf.buf, argv[i], 1);\n-\t\tstrbuf_reset(&buf);\n+\t\tsetup_merge_commit(&buf, &remotes, argv[i]);\n \t}\n \n \tif (!use_strategies) {\n-- \n1.7.4\n"},{"id":"160722","messageId":"1297211028-14382-3-git-send-email-jaredhance@gmail.com","threadId":"26448","inReplyTo":"1297211028-14382-1-git-send-email-jaredhance@gmail.com","subject":"[PATCH v3 2/3] Add function per_branch_config.","fromName":"Jared Hance","fromEmail":"jaredhance@gmail.com","sentAt":"2011-02-09T00:23:47Z","receivedAt":"2011-02-09T00:23:47Z","isPatch":true,"sender":{"key":"jaredhance@gmail.com","avatar":"https://avatars.githubusercontent.com/u/170192?v=4"},"body":"Adds a configuration function to be filled up more in the next patch.\n\nSigned-off-by: Jared Hance <jaredhance@gmail.com>\n---\n builtin/merge.c |   24 ++++++++++++++++++++----\n 1 files changed, 20 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex bc1ae94..818bfc7 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -498,11 +498,15 @@ cleanup:\n \tstrbuf_release(&bname);\n }\n \n-static int git_merge_config(const char *k, const char *v, void *cb)\n+static int per_branch_config(const char *k, const char *v, void *cb)\n {\n-\tif (branch && !prefixcmp(k, \"branch.\") &&\n-\t\t!prefixcmp(k + 7, branch) &&\n-\t\t!strcmp(k + 7 + strlen(branch), \".mergeoptions\")) {\n+\tconst char *variable;\n+\tif (!branch || prefixcmp(k, \"branch.\")\n+\t   || prefixcmp(k + 7, branch))\n+\t\treturn 1; /* not what I handle */\n+\n+\tvariable = k + 7 + strlen(branch);\n+\tif (!strcmp(variable, \".mergeoptions\")) {\n \t\tconst char **argv;\n \t\tint argc;\n \t\tchar *buf;\n@@ -518,8 +522,20 @@ static int git_merge_config(const char *k, const char *v, void *cb)\n \t\tparse_options(argc, argv, NULL, builtin_merge_options,\n \t\t\t      builtin_merge_usage, 0);\n \t\tfree(buf);\n+\n+\t\treturn 0;\n \t}\n \n+\treturn 1; /* not what I handle */\n+}\n+\n+static int git_merge_config(const char *k, const char *v, void *cb)\n+{\n+\tint status = per_branch_config(k, v, cb);\n+\n+\tif (status <= 0)\n+\t\treturn status;\n+\n \tif (!strcmp(k, \"merge.diffstat\") || !strcmp(k, \"merge.stat\"))\n \t\tshow_diffstat = git_config_bool(k, v);\n \telse if (!strcmp(k, \"pull.twohead\"))\n-- \n1.7.4\n"},{"id":"160723","messageId":"1297211028-14382-4-git-send-email-jaredhance@gmail.com","threadId":"26448","inReplyTo":"1297211028-14382-1-git-send-email-jaredhance@gmail.com","subject":"[PATCH v3 3/3] Add support for merging from upstream by default.","fromName":"Jared Hance","fromEmail":"jaredhance@gmail.com","sentAt":"2011-02-09T00:23:48Z","receivedAt":"2011-02-09T00:23:48Z","isPatch":true,"sender":{"key":"jaredhance@gmail.com","avatar":"https://avatars.githubusercontent.com/u/170192?v=4"},"body":"Adds the option merge.defaultupstream to add support for merging from the\nupstream branch by default. The upstream branch is found using\nbranch.[name].merge.\n\nSigned-off-by: Jared Hance <jaredhance@gmail.com>\n---\n Documentation/config.txt |    6 ++++++\n builtin/merge.c          |   31 +++++++++++++++++++++++--------\n 2 files changed, 29 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex c5e1835..4415691 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1389,6 +1389,12 @@ man.<tool>.path::\n \n include::merge-config.txt[]\n \n+merge.defaultUpstream::\n+\tIf merge is called without any ref arguments, merge from the branch\n+\tspecified in branch.<current branch>.merge, which is considered to be\n+\tthe upstream branch for the current branch, possibly set by --track or\n+\t--set-upstream.\n+\n mergetool.<tool>.path::\n \tOverride the path for the given tool.  This is useful in case\n \tyour tool is not in the PATH.\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 818bfc7..cd646fb 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -37,7 +37,7 @@ struct strategy {\n };\n \n static const char * const builtin_merge_usage[] = {\n-\t\"git merge [options] <remote>...\",\n+\t\"git merge [options] [<remote>...]\",\n \t\"git merge [options] <msg> HEAD <remote>\",\n \tNULL\n };\n@@ -58,6 +58,8 @@ static int option_renormalize;\n static int verbosity;\n static int allow_rerere_auto;\n static int abort_current_merge;\n+static int default_upstream;\n+static const char *upstream_branch;\n \n static struct strategy all_strategy[] = {\n \t{ \"recursive\",  DEFAULT_TWOHEAD | NO_TRIVIAL },\n@@ -525,6 +527,8 @@ static int per_branch_config(const char *k, const char *v, void *cb)\n \n \t\treturn 0;\n \t}\n+\telse if (!strcmp(variable, \".merge\"))\n+\t\treturn git_config_string(&upstream_branch, k, v);\n \n \treturn 1; /* not what I handle */\n }\n@@ -536,7 +540,9 @@ static int git_merge_config(const char *k, const char *v, void *cb)\n \tif (status <= 0)\n \t\treturn status;\n \n-\tif (!strcmp(k, \"merge.diffstat\") || !strcmp(k, \"merge.stat\"))\n+\tif (!strcmp(k, \"merge.defaultupstream\"))\n+\t\tdefault_upstream = git_config_bool(k, v);\n+\telse if (!strcmp(k, \"merge.diffstat\") || !strcmp(k, \"merge.stat\"))\n \t\tshow_diffstat = git_config_bool(k, v);\n \telse if (!strcmp(k, \"pull.twohead\"))\n \t\treturn git_config_string(&pull_twohead, k, v);\n@@ -1017,9 +1023,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \tif (!allow_fast_forward && fast_forward_only)\n \t\tdie(\"You cannot combine --no-ff with --ff-only.\");\n \n-\tif (!argc)\n-\t\tusage_with_options(builtin_merge_usage,\n-\t\t\tbuiltin_merge_options);\n+\tif (!argc) {\n+\t\tif (!default_upstream || !upstream_branch)\n+\t\t\tusage_with_options(builtin_merge_usage,\n+\t\t\t\t\tbuiltin_merge_options);\n+\t\telse\n+\t\t\tsetup_merge_commit(&buf, &remotes, upstream_branch);\n+\t}\n \n \t/*\n \t * This could be traditional \"merge <msg> HEAD <commit>...\"  and\n@@ -1082,9 +1092,14 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \n-\tif (head_invalid || !argc)\n-\t\tusage_with_options(builtin_merge_usage,\n-\t\t\tbuiltin_merge_options);\n+\tif (head_invalid)\n+\t\tusage_msg_opt(\"cannot use old-style invocation from an unborn\"\n+\t\t\t\t\"branch\", \n+\t\t\t\tbuiltin_merge_usage, builtin_merge_options);\n+\n+\tif (!argc && !(default_upstream && upstream_branch))\n+\t\tusage_msg_opt(\"no commit to merge specified\",\n+\t\t\t\tbuiltin_merge_usage, builtin_merge_options);\n \n \tstrbuf_addstr(&buf, \"merge\");\n \tfor (i = 0; i < argc; i++)\n-- \n1.7.4\n"},{"id":"160725","messageId":"7vlj1qoxko.fsf@alter.siamese.dyndns.org","threadId":"26448","inReplyTo":"1297211028-14382-1-git-send-email-jaredhance@gmail.com","subject":"Re: [PATCH v3 0/3] Updated patch series for default upstream merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-09T00:34:15Z","receivedAt":"2011-02-09T00:34:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jared Hance <jaredhance@gmail.com> writes:\n\n> Notes/Complications:\n> \t- I'm not sure whether the option should be merge.defaultUpstream\n> \t  or merge.defaultToUpstream\n\nProbably the latter.\n\n> \t- Should [remotes] be changed to [branches]? I felt like it was\n> \t  a completely different change and didn't belong in the patch series.\n\nPerhaps at the beginning (just like your 1/3 refactoring) or at the end as\na separate patch?\n\n> \t- I left one of the ifs with unnecessary braces for clarity because\n> \t  of a nested if-else: is this the preferred style?\n\nIf you are talking about the one at @@ -1017,9 +1023,13 @@, it looks fine\nto me.  The new \"else\" you added to @@ -525,6 +527,8 @@ may probably want\nto start on the same line as closing \"}\" of the \"if\", though.  IOW, like\nthis:\n\n\tif (cond) {\n        \t...\n\t} else {\n        \t...\n\t}\n\nnot like this:\n\n\tif (cond) {\n        \t...\n\t}\n        else {\n        \t...\n\t}\n\nBut other than that looks reasonable.\n\nThanks.\n"},{"id":"160726","messageId":"20110209011441.GA24346@elie","threadId":"26448","inReplyTo":"1297211028-14382-3-git-send-email-jaredhance@gmail.com","subject":"Re: [PATCH v3 2/3] Add function per_branch_config.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-02-09T01:14:42Z","receivedAt":"2011-02-09T01:14:42Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jared Hance wrote:\n\n> [PATCH v3 2/3] Add function per_branch_config.\n>\n> Adds a configuration function to be filled up more in the next patch.\n\nMicronit: subject should be of the form\n\n\tsubsystem: do something really great\n\nwith no full stop.  So the series might be, roughly:\n\n - merge: introduce setup_merge_commit helper function\n - merge: introduce per-branch configuration helper function\n - merge: add support for merging from upstream by default\n\nCommit message bodies tend to be in the imperative mood, just like the\npatches themselves.\n\nAnyway, I like patch 1.  One tiny nit on this one:\n\n> +++ b/builtin/merge.c\n> @@ -498,11 +498,15 @@ cleanup:\n>  \tstrbuf_release(&bname);\n>  }\n>  \n> -static int git_merge_config(const char *k, const char *v, void *cb)\n> +static int per_branch_config(const char *k, const char *v, void *cb)\n>  {\n> -\tif (branch && !prefixcmp(k, \"branch.\") &&\n> -\t\t!prefixcmp(k + 7, branch) &&\n> -\t\t!strcmp(k + 7 + strlen(branch), \".mergeoptions\")) {\n> +\tconst char *variable;\n> +\tif (!branch || prefixcmp(k, \"branch.\")\n> +\t   || prefixcmp(k + 7, branch))\n> +\t\treturn 1; /* not what I handle */\n\nAlignment: isn't the \"||\" meant to be one space over?\n\nBased on a quick grep[1], it seems the prevailing style is rather\n\n\tif (!branch || prefixcmp(k, \"branch.\") ||\n\t    prefixcmp(k + 7, branch))\n\t\treturn 1;\n\nwith the || at the end of the line, though I'm not sure I care\nmuch.  :)\n\nWith or without an extra space before the ||,\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n\nThanks for a nice cleanup.\nJonathan\n\n[1]\naddup () {\n\tsum=0\n\twhile read term\n\tdo\n\t\t: $((sum = sum + term))\n\tdone\n\techo $sum\n}\n\n $ git grep --cached -c -e '||$' | cut -d: -f2 | addup\n 730\n $ git grep --cached -c -e '  ||' | cut -d: -f2 | addup\n 98\n $ git grep --cached -c -e '\t||' | cut -d: -f2 | addup\n 79\n"},{"id":"160728","messageId":"20110209014129.GB24346@elie","threadId":"26448","inReplyTo":"1297211028-14382-4-git-send-email-jaredhance@gmail.com","subject":"Re: [PATCH v3 3/3] Add support for merging from upstream by default.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-02-09T01:41:29Z","receivedAt":"2011-02-09T01:41:29Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jared Hance wrote:\n\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1389,6 +1389,12 @@ man.<tool>.path::\n> \n>  include::merge-config.txt[]\n> \n> +merge.defaultUpstream::\n[...]\n\nMight make sense to put this in merge-config.txt, to get documentation\nin git-merge(1) for free.\n\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -525,6 +527,8 @@ static int per_branch_config(const char *k, const char *v, void *cb)\n>  \n>  \t\treturn 0;\n>  \t}\n> +\telse if (!strcmp(variable, \".merge\"))\n> +\t\treturn git_config_string(&upstream_branch, k, v);\n\nStyle:\n\n\t} else if (...) {\n\t\treturn git_config_string(...);\n\t}\n\n> @@ -1017,9 +1023,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n>  \tif (!allow_fast_forward && fast_forward_only)\n>  \t\tdie(\"You cannot combine --no-ff with --ff-only.\");\n>  \n> -\tif (!argc)\n> -\t\tusage_with_options(builtin_merge_usage,\n> -\t\t\tbuiltin_merge_options);\n> +\tif (!argc) {\n> +\t\tif (!default_upstream || !upstream_branch)\n> +\t\t\tusage_with_options(builtin_merge_usage,\n> +\t\t\t\t\tbuiltin_merge_options);\n> +\t\telse\n> +\t\t\tsetup_merge_commit(&buf, &remotes, upstream_branch);\n\nWhy the \"else\"?  usage_msg_opt doesn't return to the caller.\n\nHopefully someone will chime in with some tests to complement\nt5520-pull (hint, hint). :)\n\nJonathan\n\ndiff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt\nindex c1efaaa..bc8ce25 100644\n--- a/Documentation/git-merge.txt\n+++ b/Documentation/git-merge.txt\n@@ -11,7 +11,7 @@ SYNOPSIS\n [verse]\n 'git merge' [-n] [--stat] [--no-commit] [--squash]\n \t[-s <strategy>] [-X <strategy-option>]\n-\t[--[no-]rerere-autoupdate] [-m <msg>] <commit>...\n+\t[--[no-]rerere-autoupdate] [-m <msg>] [<commit>...]\n 'git merge' <msg> HEAD <commit>...\n 'git merge' --abort\n \n@@ -97,7 +97,10 @@ commit or stash your changes before running 'git merge'.\n \tCommits, usually other branch heads, to merge into our branch.\n \tYou need at least one <commit>.  Specifying more than one\n \t<commit> obviously means you are trying an Octopus.\n-\n++\n+By default, if no commit is specified then git will error out.\n+However, the `merge.defaultToUpstream` configuration item (see below)\n+changes that.\n \n PRE-MERGE CHECKS\n ----------------\n"},{"id":"160729","messageId":"20110209014846.GC24346@elie","threadId":"26448","inReplyTo":"7vpqr2oyy3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] Add support for merging from upstream by default.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-02-09T01:48:46Z","receivedAt":"2011-02-09T01:48:46Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n\n>> Could you give an example of breakage this configurability is designed\n>> to prevent?\n>\n> I think there is no \"prevent\" or \"breakage\"; the patch is to give people a\n> way to turn the feature on; without the configuration, \"git merge\" will\n> keep the traditional behaviour, no?\n\nYes.  One answer to my question is that a person might try\n\n\tgit fetch git://some/random/repository branch\n\tgit merge\n\nand at least for now it seems sensible to make that error out by\ndefault rather than doing something unexpected.\n\nIn other words, another possible rule is \"default to the last fetched\nbranch\" (FETCH_HEAD), so the intent behind running \"git merge\" without\na branchname for the first time is not as obvious as I would hope.\n"},{"id":"160795","messageId":"7v8vxookop.fsf@alter.siamese.dyndns.org","threadId":"26448","inReplyTo":"1297211028-14382-2-git-send-email-jaredhance@gmail.com","subject":"Re: [PATCH v3 1/3] Add setup_merge_commit function to merge/builtin.c.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-09T23:24:54Z","receivedAt":"2011-02-09T23:24:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jared Hance <jaredhance@gmail.com> writes:\n\n> Subject: Re: [PATCH v3 1/3] Add setup_merge_commit function to merge/builtin.c.\n\nPlease spell the name of the file you are touching correctly ;-).\n\n\tSubject: builtin/merge.c: Add setup_merge_commit function\n\n> Adds a new function that will be used in a later patch as well as the\n> current code to reduce redundancy.\n\nRedandunt.  If you say \"add\", we know the function is \"new\" ;-)\n\nState what that function does instead of a vague \"reduce redundancy\",\nperhaps like...\n\n\tRefactor the loop to parse list of commits given from the command\n        line into a separate helper function.\n\nThe patch itself looks good.\n\nThanks.\n"},{"id":"160796","messageId":"7v4o8cokkb.fsf@alter.siamese.dyndns.org","threadId":"26448","inReplyTo":"1297211028-14382-3-git-send-email-jaredhance@gmail.com","subject":"Re: [PATCH v3 2/3] Add function per_branch_config.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-09T23:27:32Z","receivedAt":"2011-02-09T23:27:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jared Hance <jaredhance@gmail.com> writes:\n\n> Adds a configuration function to be filled up more in the next patch.\n\nAgain, \"to be filled up more\" is not the primary purpose of the new\nfunction that was refactored from the existing one.  State what it does,\nand what it is for instead, perhaps like...\n\n    Split a part of the configuration callback into a separate function\n    to handle per-branch configuration.\n"}]}