{"thread":{"id":"32175","subject":"[RFC/PATCH] Option to revert order of parents in merge commit","startedAt":"2012-11-23T08:35:53Z","lastAt":"2012-11-26T23:24:11Z","messageCount":5,"participants":["Kacper Kornet","Junio C Hamano","Aaron Schrab"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"203690","messageId":"20121123083550.GA702@camk.edu.pl","threadId":"32175","inReplyTo":null,"subject":"[RFC/PATCH] Option to revert order of parents in merge commit","fromName":"Kacper Kornet","fromEmail":"draenog@pld-linux.org","sentAt":"2012-11-23T08:35:53Z","receivedAt":"2012-11-23T08:35:53Z","isPatch":true,"sender":{"key":"draenog@pld-linux.org","avatar":"https://avatars.githubusercontent.com/u/608762?v=4"},"body":"When the changes are pushed upstream, and in the meantime someone else\nupdated upstream branch git advises to use git pull. This results in\nhistory:\n\n     ---A---B---C--\n         \\     /\n          D---E\n\nwhere B is my commit. D, E are commits pushed by someone else when I was\nworking on B. However sometimes the following history is preferable:\n\n    ---A---D---C'--\n        \\     /\n          -B-\n\nThe difference between C and C' is the order of parents. Presently to\nobtain it, instead of git pull, one needs to do (assuming that I am on\nthe master branch):\n\ngit fetch origin\ngit branch tmp_branch\ngit reset --hard origin/master\ngit merge tmp_branch\n\nReverting from wrong pull is more cumbersome. It would be simpler if git\nmerge learn an option to reverse order of parents in the produced\ncommits, so one could do:\n\ngit fetch origin\ngit merge --revert-order origin/master\n\nThe following patch is an attempt to implement this idea.\n\nSigned-off-by: Kacper Kornet <draenog@pld-linux.org>\n---\n\nI'm not 100% percent sure that it is a good idea. But it would make life\nof our developers easier and produced nicer history then using git pull.\nGit pull seems to written for a case of single maintainer who gathers\ncontributions from other developers and incorporates them in master\nbranch. However in my opinion it doesn't produce best history when many\ndevelopers modify the canonical repository. \n\n builtin/commit.c | 22 ++++++++++++++--------\n builtin/merge.c  | 16 ++++++++++++----\n commit.c         | 11 +++++++++++\n commit.h         |  2 ++\n 4 files changed, 39 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 1dd2ec5..ab2b844 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1427,7 +1427,6 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tunsigned char sha1[20];\n \tstruct ref_lock *ref_lock;\n \tstruct commit_list *parents = NULL, **pptr = &parents;\n-\tstruct stat statbuf;\n \tint allow_fast_forward = 1;\n \tstruct commit *current_head = NULL;\n \tstruct commit_extra_header *extra = NULL;\n@@ -1478,10 +1477,21 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t} else if (whence == FROM_MERGE) {\n \t\tstruct strbuf m = STRBUF_INIT;\n \t\tFILE *fp;\n+\t\tint reversed_order=0;\n \n \t\tif (!reflog_msg)\n \t\t\treflog_msg = \"commit (merge)\";\n-\t\tpptr = &commit_list_insert(current_head, pptr)->next;\n+\t\tif((fp = fopen(git_path(\"MERGE_MODE\"), \"r\"))) {\n+\t\t\twhile (strbuf_getline(&m, fp, '\\n') != EOF) {\n+\t\t\t\tif (!strcmp(m.buf, \"no-ff\"))\n+\t\t\t\t\tallow_fast_forward = 0;\n+\t\t\t\tif (!strcmp(m.buf, \"reversed-order\"))\n+\t\t\t\t\treversed_order = 1;\n+\t\t\t}\n+\t\t\tfclose(fp);\n+\t\t}\n+\t\tif (!reversed_order)\n+\t\t\tpptr = &commit_list_insert(current_head, pptr)->next;\n \t\tfp = fopen(git_path(\"MERGE_HEAD\"), \"r\");\n \t\tif (fp == NULL)\n \t\t\tdie_errno(_(\"could not open '%s' for reading\"),\n@@ -1496,12 +1506,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\t}\n \t\tfclose(fp);\n \t\tstrbuf_release(&m);\n-\t\tif (!stat(git_path(\"MERGE_MODE\"), &statbuf)) {\n-\t\t\tif (strbuf_read_file(&sb, git_path(\"MERGE_MODE\"), 0) < 0)\n-\t\t\t\tdie_errno(_(\"could not read MERGE_MODE\"));\n-\t\t\tif (!strcmp(sb.buf, \"no-ff\"))\n-\t\t\t\tallow_fast_forward = 0;\n-\t\t}\n+\t\tif (reversed_order)\n+\t\t\tpptr = &commit_list_insert(current_head, pptr)->next;\n \t\tif (allow_fast_forward)\n \t\t\tparents = reduce_heads(parents);\n \t} else {\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex a96e8ea..8d0ed18 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -65,6 +65,7 @@ static int abort_current_merge;\n static int show_progress = -1;\n static int default_to_upstream;\n static const char *sign_commit;\n+static int reversed_order=0;\n \n static struct strategy all_strategy[] = {\n \t{ \"recursive\",  DEFAULT_TWOHEAD | NO_TRIVIAL },\n@@ -213,6 +214,7 @@ static struct option builtin_merge_options[] = {\n \t{ OPTION_STRING, 'S', \"gpg-sign\", &sign_commit, N_(\"key id\"),\n \t  N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n \tOPT_BOOLEAN(0, \"overwrite-ignore\", &overwrite_ignore, N_(\"update ignored files (default)\")),\n+\tOPT_BOOLEAN(0, \"revert-order\", &reversed_order, N_(\"reverse order of parents\")),\n \tOPT_END()\n };\n \n@@ -822,9 +824,9 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads)\n \n \twrite_tree_trivial(result_tree);\n \tprintf(_(\"Wonderful.\\n\"));\n-\tparent->item = head;\n+\tparent->item = reversed_order ? remoteheads->item : head;\n \tparent->next = xmalloc(sizeof(*parent->next));\n-\tparent->next->item = remoteheads->item;\n+\tparent->next->item = reversed_order ? head : remoteheads->item;\n \tparent->next->next = NULL;\n \tprepare_to_commit(remoteheads);\n \tif (commit_tree(&merge_msg, result_tree, parent, result_commit, NULL,\n@@ -848,8 +850,12 @@ static int finish_automerge(struct commit *head,\n \n \tfree_commit_list(common);\n \tparents = remoteheads;\n-\tif (!head_subsumed || !allow_fast_forward)\n+\tif (!head_subsumed || !allow_fast_forward) {\n+\t    if (reversed_order )\n+\t\tcommit_list_insert_end(head, &parents);\n+\t    else\n \t\tcommit_list_insert(head, &parents);\n+\t}\n \tstrbuf_addch(&merge_msg, '\\n');\n \tprepare_to_commit(remoteheads);\n \tif (commit_tree(&merge_msg, result_tree, parents, result_commit,\n@@ -994,7 +1000,9 @@ static void write_merge_state(struct commit_list *remoteheads)\n \t\tdie_errno(_(\"Could not open '%s' for writing\"), filename);\n \tstrbuf_reset(&buf);\n \tif (!allow_fast_forward)\n-\t\tstrbuf_addf(&buf, \"no-ff\");\n+\t\tstrbuf_addf(&buf, \"no-ff\\n\");\n+\tif (reversed_order)\n+\t\tstrbuf_addf(&buf, \"reversed-order\\n\");\n \tif (write_in_full(fd, buf.buf, buf.len) != buf.len)\n \t\tdie_errno(_(\"Could not write to '%s'\"), filename);\n \tclose(fd);\ndiff --git a/commit.c b/commit.c\nindex e8eb0ae..6e58994 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -363,6 +363,17 @@ struct commit_list *commit_list_insert(struct commit *item, struct commit_list *\n \treturn new_list;\n }\n \n+struct commit_list *commit_list_insert_end(struct commit *item, struct commit_list **list_p)\n+{\n+\tstruct commit_list *list_iter = *list_p;\n+\twhile(list_iter->next)\n+\t\tlist_iter = list_iter->next;\n+\tlist_iter->next = xmalloc(sizeof(*list_iter->next));\n+\tlist_iter->next->item = item;\n+\tlist_iter->next->next = NULL;\n+\treturn *list_p;\n+}\n+\n unsigned commit_list_count(const struct commit_list *l)\n {\n \tunsigned c = 0;\ndiff --git a/commit.h b/commit.h\nindex b6ad8f3..17ae5e5 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -53,6 +53,8 @@ int find_commit_subject(const char *commit_buffer, const char **subject);\n \n struct commit_list *commit_list_insert(struct commit *item,\n \t\t\t\t\tstruct commit_list **list);\n+struct commit_list *commit_list_insert_end(struct commit *item,\n+\t\t\t\t\tstruct commit_list **list);\n struct commit_list **commit_list_append(struct commit *commit,\n \t\t\t\t\tstruct commit_list **next);\n unsigned commit_list_count(const struct commit_list *l);\n-- \n1.8.0\n"},{"id":"203726","messageId":"7vfw3zzoye.fsf@alter.siamese.dyndns.org","threadId":"32175","inReplyTo":"20121123083550.GA702@camk.edu.pl","subject":"Re: [RFC/PATCH] Option to revert order of parents in merge commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-24T02:58:49Z","receivedAt":"2012-11-24T02:58:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kacper Kornet <draenog@pld-linux.org> writes:\n\n> The following patch is an attempt to implement this idea.\n\nI think \"revert\" is a wrong word (implying you have already done\nsomething and you are trying to defeat the effect of that old\nsomething), and you meant to say \"reverse\" (i.e. the opposite of\nnormal) or something.\n\nI am unsure about the usefulness of this, though.\n\nAfter completing a topic on branch A, you would merge it to your own\ncopy of the integration branch (e.g. 'master') and try to push,\nwhich may be rejected due to non-fast-forwardness:\n\n    $ git checkout master\n    $ git merge A\n    $ git push\n\nAt that point, if you _care_ about the merge parent order, you could\ndo this (still on 'master'):\n\n    $ git fetch origin\n    $ git reset --hard origin/master\n    $ git merge A\n    $ test test test\n    $ git push\n\nWith --reverse-parents, it would become:\n\n    $ git pull --reverse-parents\n    $ test test test\n    $ git push\n\nwhich certainly is shorter and looks simpler.  The workflow however\nwould encourage people to work directly on the master branch, which\nis a bit of downside.\n\nIs there any interaction between this \"pull --reverse-parents\"\nchange and possible conflict resolution when the command stops and\nasks the user for help?  For example, whom should \"--ours\" and \"-X\nours\" refer to?  Us, or the upstream?\n"},{"id":"203872","messageId":"20121126124200.GA29859@camk.edu.pl","threadId":"32175","inReplyTo":"7vfw3zzoye.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] Option to revert order of parents in merge commit","fromName":"Kacper Kornet","fromEmail":"draenog@pld-linux.org","sentAt":"2012-11-26T12:42:01Z","receivedAt":"2012-11-26T12:42:01Z","isPatch":true,"sender":{"key":"draenog@pld-linux.org","avatar":"https://avatars.githubusercontent.com/u/608762?v=4"},"body":"On Fri, Nov 23, 2012 at 06:58:49PM -0800, Junio C Hamano wrote:\n> Kacper Kornet <draenog@pld-linux.org> writes:\n\n> > The following patch is an attempt to implement this idea.\n\n> I think \"revert\" is a wrong word (implying you have already done\n> something and you are trying to defeat the effect of that old\n> something), and you meant to say \"reverse\" (i.e. the opposite of\n> normal) or something.\n\nYou are right. Probably transpose is the best description what the patch\nreally does.\n\n> I am unsure about the usefulness of this, though.\n\n> After completing a topic on branch A, you would merge it to your own\n> copy of the integration branch (e.g. 'master') and try to push,\n> which may be rejected due to non-fast-forwardness:\n\n>     $ git checkout master\n>     $ git merge A\n>     $ git push\n\n> At that point, if you _care_ about the merge parent order, you could\n> do this (still on 'master'):\n\n>     $ git fetch origin\n>     $ git reset --hard origin/master\n>     $ git merge A\n>     $ test test test\n>     $ git push\n\n> With --reverse-parents, it would become:\n\n>     $ git pull --reverse-parents\n>     $ test test test\n>     $ git push\n\n> which certainly is shorter and looks simpler.  The workflow however\n> would encourage people to work directly on the master branch, which\n> is a bit of downside.\n\nOur developers work mainly on master branches. The project consists of\nmany thousands independent git repositories, and at the given time a\ndeveloper usually wants to make only one commit in the given repository\nand push his changes upstream. So he usually doesn't care to make a\nbranch.  Then after failed pushed, one needs to add creation and removal\nof temporary branch (see the commit message of the suggested patch).\nThe possibility to do git pull --reverse-parent would make the life\neasier in this case.\n\n> Is there any interaction between this \"pull --reverse-parents\"\n> change and possible conflict resolution when the command stops and\n> asks the user for help?  For example, whom should \"--ours\" and \"-X\n> ours\" refer to?  Us, or the upstream?\n\nThe change of order of parents happens at the very last moment, so\n\"ours\" in merge options is local version and \"theirs\" upstream.\n\n-- \n  Kacper Kornet\n"},{"id":"203897","messageId":"7v8v9otfdy.fsf@alter.siamese.dyndns.org","threadId":"32175","inReplyTo":"20121126124200.GA29859@camk.edu.pl","subject":"Re: [RFC/PATCH] Option to revert order of parents in merge commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-26T17:58:49Z","receivedAt":"2012-11-26T17:58:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kacper Kornet <draenog@pld-linux.org> writes:\n\n>> Is there any interaction between this \"pull --reverse-parents\"\n>> change and possible conflict resolution when the command stops and\n>> asks the user for help?  For example, whom should \"--ours\" and \"-X\n>> ours\" refer to?  Us, or the upstream?\n>\n> The change of order of parents happens at the very last moment, so\n> \"ours\" in merge options is local version and \"theirs\" upstream.\n\nThat may be something that wants to go to the proposed commit log\nmessage.  I am neutral on the \"feature\" (i.e. not against it but not\nextremely enthusiastic about it either).\n\nThanks.\n"},{"id":"203950","messageId":"20121126232411.GB3937@pug.qqx.org","threadId":"32175","inReplyTo":"7v8v9otfdy.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] Option to revert order of parents in merge commit","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2012-11-26T23:24:11Z","receivedAt":"2012-11-26T23:24:11Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"At 09:58 -0800 26 Nov 2012, Junio C Hamano <gitster@pobox.com> wrote:\n>Kacper Kornet <draenog@pld-linux.org> writes:\n>> The change of order of parents happens at the very last moment, so\n>> \"ours\" in merge options is local version and \"theirs\" upstream.\n>\n>That may be something that wants to go to the proposed commit log\n>message.  I am neutral on the \"feature\" (i.e. not against it but not\n>extremely enthusiastic about it either).\n\nThat should also be included in the (currently nonexistent) \ndocumentation of the proposed option.\n"}]}