{"thread":{"id":"32217","subject":"[PATCH v2 2/3] Allow for MERGE_MODE to specify more then one mode","startedAt":"2012-11-27T23:00:14Z","lastAt":"2012-11-28T16:52:52Z","messageCount":11,"participants":["Kacper Kornet","Junio C Hamano","Johannes Sixt"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"204055","messageId":"1354057217-65886-1-git-send-email-draenog@pld-linux.org","threadId":"32217","inReplyTo":null,"subject":"[PATCH v2 0/3] Add option to change order of parents in merge commit","fromName":"Kacper Kornet","fromEmail":"draenog@pld-linux.org","sentAt":"2012-11-27T23:00:14Z","receivedAt":"2012-11-27T23:00:14Z","isPatch":true,"sender":{"key":"draenog@pld-linux.org","avatar":"https://avatars.githubusercontent.com/u/608762?v=4"},"body":"The second version of patches introducing option to change order\nof parents in merge commits. The changes in respect to the previous\nversion:\n\n- I have divided the changes to the preparatory ones, which\n  only refactore the code without introducing new functionality, and\n  the commit which introduces the new option\n\n- The documentation for the new options has been added\n\nThis is not yet a final version, as the tests are missing. But maybe while I'm working\non them there will be some comments.\n\nKacper Kornet (3):\n  Process MERGE_MODE before MERGE_HEAD\n  Allow for MERGE_MODE to specify more then one mode\n  Add option to transpose parents of merge commit\n\n Documentation/merge-options.txt |  7 +++++++\n builtin/commit.c                | 22 ++++++++++++++--------\n builtin/merge.c                 | 16 ++++++++++++----\n commit.c                        | 11 +++++++++++\n commit.h                        |  2 ++\n git-pull.sh                     |  4 +++-\n 6 files changed, 49 insertions(+), 13 deletions(-)\n\n-- \n1.8.0.1\n"},{"id":"204054","messageId":"1354057217-65886-2-git-send-email-draenog@pld-linux.org","threadId":"32217","inReplyTo":"1354057217-65886-1-git-send-email-draenog@pld-linux.org","subject":"[PATCH v2 1/3] Process MERGE_MODE before MERGE_HEAD","fromName":"Kacper Kornet","fromEmail":"draenog@pld-linux.org","sentAt":"2012-11-27T23:00:15Z","receivedAt":"2012-11-27T23:00:15Z","isPatch":true,"sender":{"key":"draenog@pld-linux.org","avatar":"https://avatars.githubusercontent.com/u/608762?v=4"},"body":"It is in preparation to introduce --transpose-parents option to\ngit-merge, when the content of MERGE_MODE will dictate how the\nMERGE_HEAD is interpreted.\n\nSigned-off-by: Kacper Kornet <draenog@pld-linux.org>\n---\n builtin/commit.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 1dd2ec5..273332f 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1481,6 +1481,12 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \n \t\tif (!reflog_msg)\n \t\t\treflog_msg = \"commit (merge)\";\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\tpptr = &commit_list_insert(current_head, pptr)->next;\n \t\tfp = fopen(git_path(\"MERGE_HEAD\"), \"r\");\n \t\tif (fp == NULL)\n@@ -1496,12 +1502,6 @@ 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 (allow_fast_forward)\n \t\t\tparents = reduce_heads(parents);\n \t} else {\n-- \n1.8.0.1\n"},{"id":"204053","messageId":"1354057217-65886-3-git-send-email-draenog@pld-linux.org","threadId":"32217","inReplyTo":"1354057217-65886-1-git-send-email-draenog@pld-linux.org","subject":"[PATCH v2 2/3] Allow for MERGE_MODE to specify more then one mode","fromName":"Kacper Kornet","fromEmail":"draenog@pld-linux.org","sentAt":"2012-11-27T23:00:16Z","receivedAt":"2012-11-27T23:00:16Z","isPatch":true,"sender":{"key":"draenog@pld-linux.org","avatar":"https://avatars.githubusercontent.com/u/608762?v=4"},"body":"Presently only one merge mode exists: non-fast-forward. But in future\nthe second one (transpose-parents) will be added, so the need to read\nall lines of MERGE_MODE.\n\nSigned-off-by: Kacper Kornet <draenog@pld-linux.org>\n---\n builtin/commit.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 273332f..ee0e884 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@@ -1481,11 +1480,12 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \n \t\tif (!reflog_msg)\n \t\t\treflog_msg = \"commit (merge)\";\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\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}\n+\t\t\tfclose(fp);\n \t\t}\n \t\tpptr = &commit_list_insert(current_head, pptr)->next;\n \t\tfp = fopen(git_path(\"MERGE_HEAD\"), \"r\");\n-- \n1.8.0.1\n"},{"id":"204056","messageId":"1354057217-65886-4-git-send-email-draenog@pld-linux.org","threadId":"32217","inReplyTo":"1354057217-65886-1-git-send-email-draenog@pld-linux.org","subject":"[PATCH v2 3/3] Add option to transpose parents of merge commit","fromName":"Kacper Kornet","fromEmail":"draenog@pld-linux.org","sentAt":"2012-11-27T23:00:17Z","receivedAt":"2012-11-27T23:00:17Z","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 the local commit. D, E are commits pushed by someone else\nwhen the developer was working on B. However sometimes the following\nhistory is preferable:\n\n    ---A---D---C'--\n        \\     /\n         '-B-'\n\nThe difference between C and C' is the order of parents. This change\nallow for easier way to obtain this effect by introducing the option\n--transpose-parents to git-merge and git-pull, which changes the order\nof parents in resulting commit moving the original first parent to\nthe very end of list of parents.\n\nThe transposition is done just before the commit is finalized, so the\nmeaning of \"our\" and \"their\" for conflict resolution is not changed, so\n\"ours\" denotes local version and \"theirs\" upstream.\n\nSigned-off-by: Kacper Kornet <draenog@pld-linux.org>\n---\n Documentation/merge-options.txt |  7 +++++++\n builtin/commit.c                |  8 +++++++-\n builtin/merge.c                 | 16 ++++++++++++----\n commit.c                        | 11 +++++++++++\n commit.h                        |  2 ++\n git-pull.sh                     |  4 +++-\n 6 files changed, 42 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt\nindex 0bcbe0a..b4fbfdc 100644\n--- a/Documentation/merge-options.txt\n+++ b/Documentation/merge-options.txt\n@@ -88,6 +88,13 @@ option can be used to override --squash.\n \tSynonyms to --stat and --no-stat; these are deprecated and will be\n \tremoved in the future.\n \n+--transpose-parents::\n+\tTranspose the parents in the final commit. The change is made\n+\tjust before the commit so the meaning of 'our' and 'their'\n+\tconcepts remains the same (i.e. 'our' means current branch before\n+\tthe merge).\n+\n+\n ifndef::git-pull[]\n -q::\n --quiet::\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex ee0e884..ab2b844 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1477,6 +1477,7 @@ 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@@ -1484,10 +1485,13 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\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\tpptr = &commit_list_insert(current_head, pptr)->next;\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@@ -1502,6 +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 (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..41738a5 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, \"transpose-parents\", &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);\ndiff --git a/git-pull.sh b/git-pull.sh\nindex 266e682..d9c7591 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -84,6 +84,8 @@ do\n \t\tno_ff=--no-ff ;;\n \t--ff-only)\n \t\tff_only=--ff-only ;;\n+\t--transpose-parents)\n+\t\ttranspose_parents=--transpose-parents;;\n \t-s=*|--s=*|--st=*|--str=*|--stra=*|--strat=*|--strate=*|\\\n \t\t--strateg=*|--strategy=*|\\\n \t-s|--s|--st|--str|--stra|--strat|--strate|--strateg|--strategy)\n@@ -283,7 +285,7 @@ true)\n \teval=\"$eval --onto $merge_head ${oldremoteref:-$merge_head}\"\n \t;;\n *)\n-\teval=\"git-merge $diffstat $no_commit $edit $squash $no_ff $ff_only\"\n+\teval=\"git-merge $diffstat $no_commit $edit $squash $no_ff $ff_only $transpose_parents\"\n \teval=\"$eval  $log_arg $strategy_args $merge_args $verbosity $progress\"\n \teval=\"$eval \\\"\\$merge_name\\\" HEAD $merge_head\"\n \t;;\n-- \n1.8.0.1\n"},{"id":"204101","messageId":"7v7gp6jwsn.fsf@alter.siamese.dyndns.org","threadId":"32217","inReplyTo":"1354057217-65886-3-git-send-email-draenog@pld-linux.org","subject":"Re: [PATCH v2 2/3] Allow for MERGE_MODE to specify more then one mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-28T02:17:28Z","receivedAt":"2012-11-28T02:17:28Z","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> Presently only one merge mode exists: non-fast-forward. But in future\n> the second one (transpose-parents) will be added, so the need to read\n> all lines of MERGE_MODE.\n>\n> Signed-off-by: Kacper Kornet <draenog@pld-linux.org>\n> ---\n>  builtin/commit.c | 12 ++++++------\n>  1 file changed, 6 insertions(+), 6 deletions(-)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 273332f..ee0e884 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> @@ -1481,11 +1480,12 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n>  \n>  \t\tif (!reflog_msg)\n>  \t\t\treflog_msg = \"commit (merge)\";\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\tif((fp = fopen(git_path(\"MERGE_MODE\"), \"r\"))) {\n\nStyle: s/if((fp/if ((fp/;\n\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}\n> +\t\t\tfclose(fp);\n\nThis needs a bit more careful planning for interacting with other\npeople's programs, I suspect.\n\nYour updated builtin/merge.c may write an extra LF after no-ff to\nmake this parser to grok it, but it is entirely plausible that\npeople have their own Porcelain that writes \"no-ff\" without LF\n(because that is what we read from this file, and I suspect the\ncurrent code would ignore \"no-ff\\n\").\n\nAt least strbuf_getline() would give us \"no-ff\" when either \"no-ff\"\nor \"no-ff\\n\" terminates the file, so updated code would be able to\ngrok what other people would write, but if other people want to read\nMERGE_MODE we write, at least we shouldn't break them when we only\nwrite no-ff in it (once you start writing \"reverse-parents\" in the\nfile, they will be broken anyway, as they do not currently expect\nsuch token in this file).\n\nI am starting to wonder if this is worth it, though...\n"},{"id":"204102","messageId":"7vzk22ii7b.fsf@alter.siamese.dyndns.org","threadId":"32217","inReplyTo":"1354057217-65886-4-git-send-email-draenog@pld-linux.org","subject":"Re: [PATCH v2 3/3] Add option to transpose parents of merge commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-28T02:18:00Z","receivedAt":"2012-11-28T02:18:00Z","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> +--transpose-parents::\n> +\tTranspose the parents in the final commit. The change is made\n> +\tjust before the commit so the meaning of 'our' and 'their'\n> +\tconcepts remains the same (i.e. 'our' means current branch before\n> +\tthe merge).\n> +\n\nHow does this interact with Octopus merges?\n\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index ee0e884..ab2b844 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -1477,6 +1477,7 @@ 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\nStyle. s/=/ = /;\n\n> +\tOPT_BOOLEAN(0, \"transpose-parents\", &reversed_order, N_(\"reverse order of parents\")\n\nIt smells more like \"--reverse-parents\" (if you deal only with\ntwo-head merges), no?\n"},{"id":"204131","messageId":"20121128043608.GA17470@camk.edu.pl","threadId":"32217","inReplyTo":"7v7gp6jwsn.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 2/3] Allow for MERGE_MODE to specify more then one mode","fromName":"Kacper Kornet","fromEmail":"draenog@pld-linux.org","sentAt":"2012-11-28T04:36:09Z","receivedAt":"2012-11-28T04:36:09Z","isPatch":true,"sender":{"key":"draenog@pld-linux.org","avatar":"https://avatars.githubusercontent.com/u/608762?v=4"},"body":"On Tue, Nov 27, 2012 at 06:17:28PM -0800, Junio C Hamano wrote:\n> Kacper Kornet <draenog@pld-linux.org> writes:\n\n> > Presently only one merge mode exists: non-fast-forward. But in future\n> > the second one (transpose-parents) will be added, so the need to read\n> > all lines of MERGE_MODE.\n\n> > Signed-off-by: Kacper Kornet <draenog@pld-linux.org>\n> > ---\n> >  builtin/commit.c | 12 ++++++------\n> >  1 file changed, 6 insertions(+), 6 deletions(-)\n\n> > diff --git a/builtin/commit.c b/builtin/commit.c\n> > index 273332f..ee0e884 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> > @@ -1481,11 +1480,12 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n\n> >  \t\tif (!reflog_msg)\n> >  \t\t\treflog_msg = \"commit (merge)\";\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\tif((fp = fopen(git_path(\"MERGE_MODE\"), \"r\"))) {\n\n> Style: s/if((fp/if ((fp/;\n\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}\n> > +\t\t\tfclose(fp);\n\n> This needs a bit more careful planning for interacting with other\n> people's programs, I suspect.\n\n> Your updated builtin/merge.c may write an extra LF after no-ff to\n> make this parser to grok it, but it is entirely plausible that\n> people have their own Porcelain that writes \"no-ff\" without LF\n> (because that is what we read from this file, and I suspect the\n> current code would ignore \"no-ff\\n\").\n\n> At least strbuf_getline() would give us \"no-ff\" when either \"no-ff\"\n> or \"no-ff\\n\" terminates the file, so updated code would be able to\n> grok what other people would write, but if other people want to read\n> MERGE_MODE we write, at least we shouldn't break them when we only\n> write no-ff in it (once you start writing \"reverse-parents\" in the\n> file, they will be broken anyway, as they do not currently expect\n> such token in this file).\n\nAt this stage in the patch series the format of MERGE_MODE is not\nchanged - \"no-ff\" is printed without \"\\n\". What should be changed is the\nnext patch. Relevant part should read:\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex a96e8ea..5ceb291 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -993,6 +999,8 @@ static void write_merge_state(struct commit_list *remoteheads)\n \tif (fd < 0)\n \t\tdie_errno(_(\"Could not open '%s' for writing\"), filename);\n \tstrbuf_reset(&buf);\n+\tif (reversed_order)\n+\t\tstrbuf_addf(&buf, \"reversed-order\\n\");\n \tif (!allow_fast_forward)\n \t\tstrbuf_addf(&buf, \"no-ff\");\n \tif (write_in_full(fd, buf.buf, buf.len) != buf.len)\n\nThis way when only no-ff is specified all parsers should be happy. If\nreversed-order is specified together no-ff the \"external\" parser\nprobably would fail. Which in my opinion is a good think at this point,\nas it can't correctly interpret MERGE_MODE anyway.\n\n-- \n  Kacper\n"},{"id":"204132","messageId":"20121128044354.GB17470@camk.edu.pl","threadId":"32217","inReplyTo":"7vzk22ii7b.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 3/3] Add option to transpose parents of merge commit","fromName":"Kacper Kornet","fromEmail":"draenog@pld-linux.org","sentAt":"2012-11-28T04:43:54Z","receivedAt":"2012-11-28T04:43:54Z","isPatch":true,"sender":{"key":"draenog@pld-linux.org","avatar":"https://avatars.githubusercontent.com/u/608762?v=4"},"body":"On Tue, Nov 27, 2012 at 06:18:00PM -0800, Junio C Hamano wrote:\n> Kacper Kornet <draenog@pld-linux.org> writes:\n\n> > +--transpose-parents::\n> > +\tTranspose the parents in the final commit. The change is made\n> > +\tjust before the commit so the meaning of 'our' and 'their'\n> > +\tconcepts remains the same (i.e. 'our' means current branch before\n> > +\tthe merge).\n> > +\n\n> How does this interact with Octopus merges?\n\nIt moves the original first parent to the last position. And nothing\nmore. I have forgotten to mention it in the documentation.\n\n> > diff --git a/builtin/commit.c b/builtin/commit.c\n> > index ee0e884..ab2b844 100644\n> > --- a/builtin/commit.c\n> > +++ b/builtin/commit.c\n> > @@ -1477,6 +1477,7 @@ 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> Style. s/=/ = /;\n\n> > +\tOPT_BOOLEAN(0, \"transpose-parents\", &reversed_order, N_(\"reverse order of parents\")\n\n> It smells more like \"--reverse-parents\" (if you deal only with\n> two-head merges), no?\n\nI have changes to --transpose-parents because of the octopus merges.\nAlthough it is not a mathematical transposition in this case, but a cycle\npermutation. \n\n-- \n  Kacper\n"},{"id":"204136","messageId":"50B5B599.3020105@viscovery.net","threadId":"32217","inReplyTo":"1354057217-65886-4-git-send-email-draenog@pld-linux.org","subject":"Re: [PATCH v2 3/3] Add option to transpose parents of merge commit","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-11-28T06:56:25Z","receivedAt":"2012-11-28T06:56:25Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 11/28/2012 0:00, schrieb Kacper Kornet:\n> When the changes are pushed upstream, and in the meantime someone else\n> updated upstream branch git advises to use git pull. This results in\n> history:\n> \n>      ---A---B---C--\n>          \\     /\n>           D---E\n\nThe commit message will say:\n\n  Merge branch 'master' of /that/remote\n\n  * 'master' of /that/remote:\n    E\n    D\n\n> where B is the local commit. D, E are commits pushed by someone else\n> when the developer was working on B. However sometimes the following\n> history is preferable:\n> \n>     ---A---D---C'--\n>         \\     /\n>          '-B-'\n\nBetter:\n\n     ---A--D--E--C'\n         \\      /\n          `----B\n\nIn this case, the commit message should say... what? Certainly not the\nsame thing. But I do not see that you changed anything in this regard.\n\n-- Hannes\n"},{"id":"204140","messageId":"7v38zui3q2.fsf@alter.siamese.dyndns.org","threadId":"32217","inReplyTo":"20121128043608.GA17470@camk.edu.pl","subject":"Re: [PATCH v2 2/3] Allow for MERGE_MODE to specify more then one mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-28T07:30:45Z","receivedAt":"2012-11-28T07:30:45Z","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> This way when only no-ff is specified all parsers should be happy. If\n> reversed-order is specified together no-ff the \"external\" parser\n> probably would fail. Which in my opinion is a good think at this point,\n> as it can't correctly interpret MERGE_MODE anyway.\n\nAmen to that.\n"},{"id":"204165","messageId":"7vr4ndhdp7.fsf@alter.siamese.dyndns.org","threadId":"32217","inReplyTo":"50B5B599.3020105@viscovery.net","subject":"Re: [PATCH v2 3/3] Add option to transpose parents of merge commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-28T16:52:52Z","receivedAt":"2012-11-28T16:52:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> writes:\n\n> Am 11/28/2012 0:00, schrieb Kacper Kornet:\n>> When the changes are pushed upstream, and in the meantime someone else\n>> updated upstream branch git advises to use git pull. This results in\n>> history:\n>> \n>>      ---A---B---C--\n>>          \\     /\n>>           D---E\n>\n> The commit message will say:\n>\n>   Merge branch 'master' of /that/remote\n>\n>   * 'master' of /that/remote:\n>     E\n>     D\n>\n>> where B is the local commit. D, E are commits pushed by someone else\n>> when the developer was working on B. However sometimes the following\n>> history is preferable:\n>> \n>>     ---A---D---C'--\n>>         \\     /\n>>          '-B-'\n>\n> Better:\n>\n>      ---A--D--E--C'\n>          \\      /\n>           `----B\n\nYup, that topology is what Kacper's workflow wants.\n\nStepping back a bit, however, I am not sure if that is really true.\nThe goal of this topic seems to be to keep one integration branch\nand always merge *into* that integration branch, never *from* it,\nbut for what purpose?  Making the \"log --first-parent\" express the\nintegration branch as a linear series of progress?  If so, I suspect\na project with such a policy would dictate that D and E also be on a\nside branch, i.e. the history would look more like this:\n\n      D---E\n     /     \\\n  --A-------X---C---\n     \\         /\n      `-------B\n\nwith X being a --no-ff merge of the topic that consists of these two\ncommits.\n\n> In this case, the commit message should say... what? Certainly not the\n> same thing. But I do not see that you changed anything in this regard.\n\nTrue.  If the goal is to emulate a merge of B from a side branch\ninto _the_ integration branch, the summary should also emulate the\nmessage that would be given when the remote pulled from your current\nbranch.\n"}]}