{"thread":{"id":"31392","subject":"[PATCH 4/4] get_merge_bases_many(): walk from many tips in parallel","startedAt":"2012-08-30T23:13:04Z","lastAt":"2012-08-31T18:45:20Z","messageCount":9,"participants":["Junio C Hamano","Martin von Zweigbergk"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"198134","messageId":"1346368388-23576-1-git-send-email-gitster@pobox.com","threadId":"31392","inReplyTo":null,"subject":"[PATCH 0/4] \"merge-base\" updates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-30T23:13:04Z","receivedAt":"2012-08-30T23:13:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This replaces the topmost commit from the earlier series that tried\nto optimize N*(N-1) loop with N traversals.  With this, running the\nfollowing script in the kernel repository:\n\n-- >8 --\n#!/bin/sh\n\ngit rev-list --committer=torvalds@linux-foundation.org \\\n    --max-parents=2 --min-parents=2 --parents v3.5..v3.6-rc2 >RL\n\ncmd='\n\twhile read result parent1 parent2\n\tdo\n\t\t$GIT merge-base $parent1 $parent2\n\tdone <RL\n'\n\nGIT=\"rungit master\" time sh -c \"$cmd\" >:stock\nGIT=../git.git/git time sh -c \"$cmd\" >:optim\ncmp :stock :optim\n-- 8< --\n\nshows a slight but measurable boost in the performance of\nmerge-base itself.\n\n42.45user 3.79system 0:46.62elapsed 99%CPU (0avgtext+0avgdata 408176maxresident)k\n0inputs+24outputs (0major+2708085minor)pagefaults 0swaps\n39.99user 3.42system 0:43.27elapsed 100%CPU (0avgtext+0avgdata 408192maxresident)k\n0inputs+24outputs (0major+2179604minor)pagefaults 0swaps\n\nJunio C Hamano (4):\n  merge_bases_many(): split out the logic to paint history\n  merge-base: \"--is-ancestor A B\"\n  in_merge_bases(): use paint_down_to_common()\n  get_merge_bases_many(): walk from many tips in parallel\n\n builtin/merge-base.c |  21 ++++++++\n commit.c             | 143 +++++++++++++++++++++++++++++++++------------------\n 2 files changed, 114 insertions(+), 50 deletions(-)\n\n-- \n1.7.12.293.g6aeebca\n"},{"id":"198135","messageId":"1346368388-23576-2-git-send-email-gitster@pobox.com","threadId":"31392","inReplyTo":"1346368388-23576-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/4] merge_bases_many(): split out the logic to paint history","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-30T23:13:05Z","receivedAt":"2012-08-30T23:13:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Introduce a new helper function paint_down_to_common() that takes\nthe same parameters as merge_bases_many(), but without the first\noptimization of not painting anything when \"one\" is one of the\n\"twos\" (or vice versa), and the last clean-up of removing the common\nancestor that is known to be an ancestor of another common one.\n\nThis way, the caller of the new function could tell if \"one\" is\nreachable from any of the \"twos\" by simply looking at the flag bits\nof \"one\".  If (and only if) it is painted in PARENT2, it is\nreachable from one of the \"twos\".\n---\n commit.c | 47 ++++++++++++++++++++++++++++-------------------\n 1 file changed, 28 insertions(+), 19 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 12e5396..0058fa5 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -581,28 +581,12 @@ static struct commit *interesting(struct commit_list *list)\n \treturn NULL;\n }\n \n-static struct commit_list *merge_bases_many(struct commit *one, int n, struct commit **twos)\n+static struct commit_list *paint_down_to_common(struct commit *one, int n, struct commit **twos)\n {\n \tstruct commit_list *list = NULL;\n \tstruct commit_list *result = NULL;\n \tint i;\n \n-\tfor (i = 0; i < n; i++) {\n-\t\tif (one == twos[i])\n-\t\t\t/*\n-\t\t\t * We do not mark this even with RESULT so we do not\n-\t\t\t * have to clean it up.\n-\t\t\t */\n-\t\t\treturn commit_list_insert(one, &result);\n-\t}\n-\n-\tif (parse_commit(one))\n-\t\treturn NULL;\n-\tfor (i = 0; i < n; i++) {\n-\t\tif (parse_commit(twos[i]))\n-\t\t\treturn NULL;\n-\t}\n-\n \tone->object.flags |= PARENT1;\n \tcommit_list_insert_by_date(one, &list);\n \tfor (i = 0; i < n; i++) {\n@@ -643,9 +627,34 @@ static struct commit_list *merge_bases_many(struct commit *one, int n, struct co\n \t\t}\n \t}\n \n-\t/* Clean up the result to remove stale ones */\n \tfree_commit_list(list);\n-\tlist = result; result = NULL;\n+\treturn result;\n+}\n+\n+static struct commit_list *merge_bases_many(struct commit *one, int n, struct commit **twos)\n+{\n+\tstruct commit_list *list = NULL;\n+\tstruct commit_list *result = NULL;\n+\tint i;\n+\n+\tfor (i = 0; i < n; i++) {\n+\t\tif (one == twos[i])\n+\t\t\t/*\n+\t\t\t * We do not mark this even with RESULT so we do not\n+\t\t\t * have to clean it up.\n+\t\t\t */\n+\t\t\treturn commit_list_insert(one, &result);\n+\t}\n+\n+\tif (parse_commit(one))\n+\t\treturn NULL;\n+\tfor (i = 0; i < n; i++) {\n+\t\tif (parse_commit(twos[i]))\n+\t\t\treturn NULL;\n+\t}\n+\n+\tlist = paint_down_to_common(one, n, twos);\n+\n \twhile (list) {\n \t\tstruct commit_list *next = list->next;\n \t\tif (!(list->item->object.flags & STALE))\n-- \n1.7.12.293.g6aeebca\n"},{"id":"198137","messageId":"1346368388-23576-3-git-send-email-gitster@pobox.com","threadId":"31392","inReplyTo":"1346368388-23576-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/4] merge-base: \"--is-ancestor A B\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-30T23:13:06Z","receivedAt":"2012-08-30T23:13:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"In many scripted Porcelain commands, we find this idiom:\n\n    if test \"$(git rev-parse --verify A)\" = \"$(git merge-base A B)\"\n    then\n    \t... A is an ancestor of B ...\n    fi\n\nBut you do not have to compute exact merge-base only to see if A is\nan ancestor of B.  Give them a more direct way to use the underlying\nmachinery.\n---\n builtin/merge-base.c | 21 +++++++++++++++++++++\n 1 file changed, 21 insertions(+)\n\ndiff --git a/builtin/merge-base.c b/builtin/merge-base.c\nindex 4f30f1b..615aa04 100644\n--- a/builtin/merge-base.c\n+++ b/builtin/merge-base.c\n@@ -70,6 +70,20 @@ static int handle_octopus(int count, const char **args, int reduce, int show_all\n \treturn 0;\n }\n \n+static int show_is_ancestor(int argc, const char **argv)\n+{\n+\tstruct commit *one, *two;\n+\n+\tif (argc != 2)\n+\t\tdie(\"--is-ancestor takes exactly two commits\");\n+\tone = get_commit_reference(argv[0]);\n+\ttwo = get_commit_reference(argv[1]);\n+\tif (in_merge_bases(one, two))\n+\t\treturn 0;\n+\telse\n+\t\treturn 1;\n+}\n+\n int cmd_merge_base(int argc, const char **argv, const char *prefix)\n {\n \tstruct commit **rev;\n@@ -77,11 +91,14 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)\n \tint show_all = 0;\n \tint octopus = 0;\n \tint reduce = 0;\n+\tint is_ancestor = 0;\n \n \tstruct option options[] = {\n \t\tOPT_BOOLEAN('a', \"all\", &show_all, \"output all common ancestors\"),\n \t\tOPT_BOOLEAN(0, \"octopus\", &octopus, \"find ancestors for a single n-way merge\"),\n \t\tOPT_BOOLEAN(0, \"independent\", &reduce, \"list revs not reachable from others\"),\n+\t\tOPT_BOOLEAN(0, \"is-ancestor\", &is_ancestor,\n+\t\t\t    \"is the first one ancestor of the other?\"),\n \t\tOPT_END()\n \t};\n \n@@ -89,6 +106,10 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, options, merge_base_usage, 0);\n \tif (!octopus && !reduce && argc < 2)\n \t\tusage_with_options(merge_base_usage, options);\n+\tif (is_ancestor && (show_all | octopus | reduce))\n+\t\tdie(\"--is-reachable cannot be used with other options\");\n+\tif (is_ancestor)\n+\t\treturn show_is_ancestor(argc, argv);\n \tif (reduce && (show_all || octopus))\n \t\tdie(\"--independent cannot be used with other options\");\n \n-- \n1.7.12.293.g6aeebca\n"},{"id":"198136","messageId":"1346368388-23576-4-git-send-email-gitster@pobox.com","threadId":"31392","inReplyTo":"1346368388-23576-1-git-send-email-gitster@pobox.com","subject":"[PATCH 3/4] in_merge_bases(): use paint_down_to_common()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-30T23:13:07Z","receivedAt":"2012-08-30T23:13:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"With paint_down_to_common(), we can tell if \"commit\" is reachable\nfrom \"reference\" by simply looking at its object flag, instead of\niterating over the merge bases.\n---\n commit.c | 17 +++++++----------\n 1 file changed, 7 insertions(+), 10 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 0058fa5..d39a9e9 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -786,20 +786,17 @@ int is_descendant_of(struct commit *commit, struct commit_list *with_commit)\n  */\n int in_merge_bases(struct commit *commit, struct commit *reference)\n {\n-\tstruct commit_list *bases, *b;\n+\tstruct commit_list *bases;\n \tint ret = 0;\n \n-\tbases = merge_bases_many(commit, 1, &reference);\n+\tif (parse_commit(commit) || parse_commit(reference))\n+\t\treturn ret;\n+\n+\tbases = paint_down_to_common(commit, 1, &reference);\n+\tif (commit->object.flags & PARENT2)\n+\t\tret = 1;\n \tclear_commit_marks(commit, all_flags);\n \tclear_commit_marks(reference, all_flags);\n-\n-\tfor (b = bases; b; b = b->next) {\n-\t\tif (!hashcmp(commit->object.sha1, b->item->object.sha1)) {\n-\t\t\tret = 1;\n-\t\t\tbreak;\n-\t\t}\n-\t}\n-\n \tfree_commit_list(bases);\n \treturn ret;\n }\n-- \n1.7.12.293.g6aeebca\n"},{"id":"198133","messageId":"1346368388-23576-5-git-send-email-gitster@pobox.com","threadId":"31392","inReplyTo":"1346368388-23576-1-git-send-email-gitster@pobox.com","subject":"[PATCH 4/4] get_merge_bases_many(): walk from many tips in parallel","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-30T23:13:08Z","receivedAt":"2012-08-30T23:13:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The get_merge_bases_many() function reduces the result returned by\nthe merge_bases_many() function, which is a set of possible merge\nbases, by excluding commits that can be reached from other commits.\nWe used to do N*(N-1) traversals for this, but we can check if one\ncommit reaches which other (N-1) commits by a single traversal, and\nrepeat it for all the candidates to find the answer.\n\nIntroduce remove_redundant() helper function to do this painting; we\nshould be able to use it to reimplement reduce_heads() as well.\n---\n commit.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++-----------------\n 1 file changed, 58 insertions(+), 21 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex d39a9e9..2ff5061 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -692,6 +692,60 @@ struct commit_list *get_octopus_merge_bases(struct commit_list *in)\n \treturn ret;\n }\n \n+static int remove_redundant(struct commit **array, int cnt)\n+{\n+\t/*\n+\t * Some commit in the array may be an ancestor of\n+\t * another commit.  Move such commit to the end of\n+\t * the array, and return the number of commits that\n+\t * are independent from each other.\n+\t */\n+\tstruct commit **work;\n+\tunsigned char *redundant;\n+\tint *filled_index;\n+\tint i, j, filled;\n+\n+\twork = xcalloc(cnt, sizeof(*work));\n+\tredundant = xcalloc(cnt, 1);\n+\tfilled_index = xmalloc(sizeof(*filled_index) * (cnt - 1));\n+\n+\tfor (i = 0; i < cnt; i++) {\n+\t\tstruct commit_list *common;\n+\n+\t\tif (redundant[i])\n+\t\t\tcontinue;\n+\t\tfor (j = filled = 0; j < cnt; j++) {\n+\t\t\tif (i == j || redundant[j])\n+\t\t\t\tcontinue;\n+\t\t\tfilled_index[filled] = j;\n+\t\t\twork[filled++] = array[j];\n+\t\t}\n+\t\tcommon = paint_down_to_common(array[i], filled, work);\n+\t\tif (array[i]->object.flags & PARENT2)\n+\t\t\tredundant[i] = 1;\n+\t\tfor (j = 0; j < filled; j++)\n+\t\t\tif (work[j]->object.flags & PARENT1)\n+\t\t\t\tredundant[filled_index[j]] = 1;\n+\t\tclear_commit_marks(array[i], all_flags);\n+\t\tfor (j = 0; j < filled; j++)\n+\t\t\tclear_commit_marks(work[j], all_flags);\n+\t\tfree_commit_list(common);\n+\t}\n+\n+\t/* Now collect the result */\n+\tmemcpy(work, array, sizeof(*array) * cnt);\n+\tfor (i = filled = 0; i < cnt; i++)\n+\t\tif (!redundant[i])\n+\t\t\tarray[filled++] = work[i];\n+\tfor (j = filled, i = 0; i < cnt; i++)\n+\t\tif (redundant[i])\n+\t\t\tarray[j++] = work[i];\n+\tfree(work);\n+\tfree(redundant);\n+\tfree(filled_index);\n+\treturn filled;\n+}\n+\n struct commit_list *get_merge_bases_many(struct commit *one,\n \t\t\t\t\t int n,\n \t\t\t\t\t struct commit **twos,\n@@ -700,7 +754,7 @@ struct commit_list *get_merge_bases_many(struct commit *one,\n \tstruct commit_list *list;\n \tstruct commit **rslt;\n \tstruct commit_list *result;\n-\tint cnt, i, j;\n+\tint cnt, i;\n \n \tresult = merge_bases_many(one, n, twos);\n \tfor (i = 0; i < n; i++) {\n@@ -731,28 +785,11 @@ struct commit_list *get_merge_bases_many(struct commit *one,\n \tclear_commit_marks(one, all_flags);\n \tfor (i = 0; i < n; i++)\n \t\tclear_commit_marks(twos[i], all_flags);\n-\tfor (i = 0; i < cnt - 1; i++) {\n-\t\tfor (j = i+1; j < cnt; j++) {\n-\t\t\tif (!rslt[i] || !rslt[j])\n-\t\t\t\tcontinue;\n-\t\t\tresult = merge_bases_many(rslt[i], 1, &rslt[j]);\n-\t\t\tclear_commit_marks(rslt[i], all_flags);\n-\t\t\tclear_commit_marks(rslt[j], all_flags);\n-\t\t\tfor (list = result; list; list = list->next) {\n-\t\t\t\tif (rslt[i] == list->item)\n-\t\t\t\t\trslt[i] = NULL;\n-\t\t\t\tif (rslt[j] == list->item)\n-\t\t\t\t\trslt[j] = NULL;\n-\t\t\t}\n-\t\t}\n-\t}\n \n-\t/* Surviving ones in rslt[] are the independent results */\n+\tcnt = remove_redundant(rslt, cnt);\n \tresult = NULL;\n-\tfor (i = 0; i < cnt; i++) {\n-\t\tif (rslt[i])\n-\t\t\tcommit_list_insert_by_date(rslt[i], &result);\n-\t}\n+\tfor (i = 0; i < cnt; i++)\n+\t\tcommit_list_insert_by_date(rslt[i], &result);\n \tfree(rslt);\n \treturn result;\n }\n-- \n1.7.12.293.g6aeebca\n"},{"id":"198149","messageId":"CANiSa6iy60Ts5Rdss6EwGN5Vp9KCqn35BzMC+Aacm+zqWjp4-w@mail.gmail.com","threadId":"31392","inReplyTo":"1346368388-23576-3-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 2/4] merge-base: \"--is-ancestor A B\"","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2012-08-31T15:44:28Z","receivedAt":"2012-08-31T15:44:28Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"(Resending as plain text, sorry about duplicate, Junio.)\n\nOn Thu, Aug 30, 2012 at 4:13 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> In many scripted Porcelain commands, we find this idiom:\n>\n>     if test \"$(git rev-parse --verify A)\" = \"$(git merge-base A B)\"\n>     then\n>         ... A is an ancestor of B ...\n>     fi\n>\n> But you do not have to compute exact merge-base only to see if A is\n> an ancestor of B.  Give them a more direct way to use the underlying\n> machinery.\n\nThanks! It has bugged me ever since I first saw that idiom that there\nwas no way that better shows the intent.\n\n> +       if (argc != 2)\n> +               die(\"--is-ancestor takes exactly two commits\");\n\nI think git merge-base shows the usage message regardless if argc < 2,\nso this only happens when more than two arguments are given. Maybe\ninclude --is-ancestor in the usage message?\n\n> +       if (is_ancestor && (show_all | octopus | reduce))\n> +               die(\"--is-reachable cannot be used with other options\");\n\nI suppose --is-reachable should be --is-ancestor.\n"},{"id":"198156","messageId":"7vtxvjnfcu.fsf@alter.siamese.dyndns.org","threadId":"31392","inReplyTo":"CANiSa6iy60Ts5Rdss6EwGN5Vp9KCqn35BzMC+Aacm+zqWjp4-w@mail.gmail.com","subject":"Re: [PATCH 2/4] merge-base: \"--is-ancestor A B\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-31T17:25:21Z","receivedAt":"2012-08-31T17:25:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk <martinvonz@gmail.com> writes:\n\n>> +       if (argc != 2)\n>> +               die(\"--is-ancestor takes exactly two commits\");\n>\n> I think git merge-base shows the usage message regardless if argc < 2,\n> so this only happens when more than two arguments are given.\n\nYes, but it is a good discipline not to depend too much on what\nother parts of the code may have done, when you do not have to.\n\n> Maybe\n> include --is-ancestor in the usage message?\n\nI think \"merge-base -h\" will get this for free thanks to parseopt.\n\n>> +       if (is_ancestor && (show_all | octopus | reduce))\n>> +               die(\"--is-reachable cannot be used with other options\");\n>\n> I suppose --is-reachable should be --is-ancestor.\n\nThanks, that was a renaming error.\n\nI'll squash the fix in with documentation.\n\n Documentation/git-merge-base.txt | 28 ++++++++++++++++++++++++++++\n builtin/merge-base.c             |  6 +++---\n 2 files changed, 31 insertions(+), 3 deletions(-)\n\ndiff --git i/Documentation/git-merge-base.txt w/Documentation/git-merge-base.txt\nindex b295bf8..87842e3 100644\n--- i/Documentation/git-merge-base.txt\n+++ w/Documentation/git-merge-base.txt\n@@ -11,6 +11,7 @@ SYNOPSIS\n [verse]\n 'git merge-base' [-a|--all] <commit> <commit>...\n 'git merge-base' [-a|--all] --octopus <commit>...\n+'git merge-base' --is-ancestor <commit> <commit>\n 'git merge-base' --independent <commit>...\n \n DESCRIPTION\n@@ -50,6 +51,12 @@ from linkgit:git-show-branch[1] when used with the `--merge-base` option.\n \tfrom any other.  This mimics the behavior of 'git show-branch\n \t--independent'.\n \n+--is-ancestor::\n+\tCheck if the first <commit> is an ancestor of the second <commit>,\n+\tand exit with status 0 if true, or with status 1 if not.\n+\tErrors are signaled by a non-zero status that is not 1.\n+\n+\n OPTIONS\n -------\n -a::\n@@ -110,6 +117,27 @@ both '1' and '2' are merge-bases of A and B.  Neither one is better than\n the other (both are 'best' merge bases).  When the `--all` option is not given,\n it is unspecified which best one is output.\n \n+A common idiom to check \"fast-forward-ness\" between two commits A\n+and B is (or at least used to be) to compute the merge base between\n+A and B, and check if it is the same as A, in which case, A is an\n+ancestor of B.  You will see this idiom used often in older scripts.\n+\n+\tA=$(git rev-parse --verify A)\n+\tif test \"$A\" = \"$(git merge-base A B)\"\n+\tthen\n+\t\t... A is an ancestor of B ...\n+\tfi\n+\n+In modern git, you can say this in a more direct way:\n+\n+\tif git merge-base --is-ancestor A B\n+\tthen\n+\t\t... A is an ancestor of B ...\n+\tfi\n+\n+instead.\n+\n+\n See also\n --------\n linkgit:git-rev-list[1],\ndiff --git i/builtin/merge-base.c w/builtin/merge-base.c\nindex 615aa04..447ab7c 100644\n--- i/builtin/merge-base.c\n+++ w/builtin/merge-base.c\n@@ -70,7 +70,7 @@ static int handle_octopus(int count, const char **args, int reduce, int show_all\n \treturn 0;\n }\n \n-static int show_is_ancestor(int argc, const char **argv)\n+static int handle_is_ancestor(int argc, const char **argv)\n {\n \tstruct commit *one, *two;\n \n@@ -107,9 +107,9 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)\n \tif (!octopus && !reduce && argc < 2)\n \t\tusage_with_options(merge_base_usage, options);\n \tif (is_ancestor && (show_all | octopus | reduce))\n-\t\tdie(\"--is-reachable cannot be used with other options\");\n+\t\tdie(\"--is-ancestor cannot be used with other options\");\n \tif (is_ancestor)\n-\t\treturn show_is_ancestor(argc, argv);\n+\t\treturn handle_is_ancestor(argc, argv);\n \tif (reduce && (show_all || octopus))\n \t\tdie(\"--independent cannot be used with other options\");\n \n"},{"id":"198160","messageId":"CANiSa6i5tY_V_dhK0wSaKzy0xwvN9uTqRJUJ7ZvgqCG7Ta5rsQ@mail.gmail.com","threadId":"31392","inReplyTo":"7vtxvjnfcu.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/4] merge-base: \"--is-ancestor A B\"","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2012-08-31T18:03:28Z","receivedAt":"2012-08-31T18:03:28Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Fri, Aug 31, 2012 at 10:25 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Martin von Zweigbergk <martinvonz@gmail.com> writes:\n>\n>>> +       if (argc != 2)\n>>> +               die(\"--is-ancestor takes exactly two commits\");\n>>\n>> I think git merge-base shows the usage message regardless if argc < 2,\n>> so this only happens when more than two arguments are given.\n>\n> Yes, but it is a good discipline not to depend too much on what\n> other parts of the code may have done, when you do not have to.\n\nYes, I definitely agree. It was more that when I saw this line and\nthen tried it out, I was a little surprised that it didn't tell med\n\"--is-ancestor takes exactly two commits\" when I gave it only one. But\nsince few users read the code before running the command, it will\nprobably not be too surprising :-)\n\n>> Maybe\n>> include --is-ancestor in the usage message?\n>\n> I think \"merge-base -h\" will get this for free thanks to parseopt.\n\nIt seems not, or maybe I'm using the wrong terminology; I meant to\ninclude it in the first part of the below.\n\nusage: git merge-base [-a|--all] <commit> <commit>...\n   or: git merge-base [-a|--all] --octopus <commit>...\n   or: git merge-base --independent <commit>...\n\n    -a, --all             output all common ancestors\n    --octopus             find ancestors for a single n-way merge\n    --independent         list revs not reachable from others\n    --is-ancestor         is the first one ancestor of the other?\n\n\n>  'git merge-base' [-a|--all] --octopus <commit>...\n> +'git merge-base' --is-ancestor <commit> <commit>\n>  'git merge-base' --independent <commit>...\n\nAh, I guess this is what I was looking for. The remainder of the patch\nlooks good too. Thanks.\n"},{"id":"198163","messageId":"7v3932oq7z.fsf@alter.siamese.dyndns.org","threadId":"31392","inReplyTo":"CANiSa6i5tY_V_dhK0wSaKzy0xwvN9uTqRJUJ7ZvgqCG7Ta5rsQ@mail.gmail.com","subject":"Re: [PATCH 2/4] merge-base: \"--is-ancestor A B\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-31T18:45:20Z","receivedAt":"2012-08-31T18:45:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk <martinvonz@gmail.com> writes:\n\n> On Fri, Aug 31, 2012 at 10:25 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> ...\n>> I think \"merge-base -h\" will get this for free thanks to parseopt.\n>\n> It seems not, or maybe I'm using the wrong terminology; I meant to\n> include it in the first part of the below.\n>\n> usage: git merge-base [-a|--all] <commit> <commit>...\n>    or: git merge-base [-a|--all] --octopus <commit>...\n>    or: git merge-base --independent <commit>...\n>\n>     -a, --all             output all common ancestors\n>     --octopus             find ancestors for a single n-way merge\n>     --independent         list revs not reachable from others\n>     --is-ancestor         is the first one ancestor of the other?\n\nAhh.\n\nAs most of the modern parse-optified ones just say \"git cmd <args>\"\nor something like that, I wasn't paying attention to the upper part.\n\nWill amend again.  Thanks.\n"}]}