{"thread":{"id":"41881","subject":"[PATCH v3 0/4] Add an option to git-format-patch to record base tree info","startedAt":"2016-03-31T01:46:12Z","lastAt":"2016-04-09T15:56:30Z","messageCount":16,"participants":["Xiaolong Ye","Junio C Hamano","Ye Xiaolong"],"isPatch":true,"patchVersion":3,"patchTotal":4},"messages":[{"id":"282288","messageId":"1459388776-18066-1-git-send-email-xiaolong.ye@intel.com","threadId":"41881","inReplyTo":null,"subject":"[PATCH v3 0/4] Add an option to git-format-patch to record base tree info","fromName":"Xiaolong Ye","fromEmail":"xiaolong.ye@intel.com","sentAt":"2016-03-31T01:46:12Z","receivedAt":"2016-03-31T01:46:12Z","isPatch":true,"sender":{"key":"xiaolong.ye@intel.com","avatar":"https://avatars.githubusercontent.com/u/21098480?v=4"},"body":"V3 mainly improves the implementation according to Junio's comments,\nChanges vs v2 include:\n\n - Remove the unnecessary output line \"** base-commit-info **\".\n \n - Improve the traverse logic to handle not only linear topology, but more\n   general cases, it will start revision walk by setting the starting points\n   of the traversal to all elements in the rev list[], and skip the ones in \n   list[], only grab the patch-ids of prerequisite patches.\n\n - If --base=auto is set, it will get merge base of upstream and rev range\n   we specified and use it as base commit. If there is no upstream, we just\n   error out and suggest to use set-upstream-to to track a remote branch\n   as upstream.\n\nv1 can be found here: http://article.gmane.org/gmane.comp.version-control.git/286873\nv2 can be found here: http://article.gmane.org/gmane.comp.version-control.git/289603\n\nXiaolong Ye (4):\n  patch-ids: make commit_patch_id() a public helper function\n  format-patch: add '--base' option to record base tree info\n  format-patch: introduce --base=auto option\n  format-patch: introduce format.base configuration\n\n Documentation/git-format-patch.txt |  31 ++++++++++\n builtin/log.c                      | 119 +++++++++++++++++++++++++++++++++++++\n patch-ids.c                        |   2 +-\n patch-ids.h                        |   2 +\n 4 files changed, 153 insertions(+), 1 deletion(-)\n\n-- \n2.8.0.4.gcb5a9af\n\nbase-commit: 90f7b16b3adc78d4bbabbd426fb69aa78c714f71\n"},{"id":"282289","messageId":"1459388776-18066-2-git-send-email-xiaolong.ye@intel.com","threadId":"41881","inReplyTo":"1459388776-18066-1-git-send-email-xiaolong.ye@intel.com","subject":"[PATCH v3 1/4] patch-ids: make commit_patch_id() a public helper function","fromName":"Xiaolong Ye","fromEmail":"xiaolong.ye@intel.com","sentAt":"2016-03-31T01:46:13Z","receivedAt":"2016-03-31T01:46:13Z","isPatch":true,"sender":{"key":"xiaolong.ye@intel.com","avatar":"https://avatars.githubusercontent.com/u/21098480?v=4"},"body":"Make commit_patch_id() available to other builtins.\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Xiaolong Ye <xiaolong.ye@intel.com>\n---\n patch-ids.c | 2 +-\n patch-ids.h | 2 ++\n 2 files changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/patch-ids.c b/patch-ids.c\nindex b7b3e5a..a4d0016 100644\n--- a/patch-ids.c\n+++ b/patch-ids.c\n@@ -4,7 +4,7 @@\n #include \"sha1-lookup.h\"\n #include \"patch-ids.h\"\n \n-static int commit_patch_id(struct commit *commit, struct diff_options *options,\n+int commit_patch_id(struct commit *commit, struct diff_options *options,\n \t\t    unsigned char *sha1)\n {\n \tif (commit->parents)\ndiff --git a/patch-ids.h b/patch-ids.h\nindex c8c7ca1..eeb56b3 100644\n--- a/patch-ids.h\n+++ b/patch-ids.h\n@@ -13,6 +13,8 @@ struct patch_ids {\n \tstruct patch_id_bucket *patches;\n };\n \n+int commit_patch_id(struct commit *commit, struct diff_options *options,\n+\t\t    unsigned char *sha1);\n int init_patch_ids(struct patch_ids *);\n int free_patch_ids(struct patch_ids *);\n struct patch_id *add_commit_patch_id(struct commit *, struct patch_ids *);\n-- \n2.8.0.4.gcb5a9af\n"},{"id":"282292","messageId":"1459388776-18066-3-git-send-email-xiaolong.ye@intel.com","threadId":"41881","inReplyTo":"1459388776-18066-1-git-send-email-xiaolong.ye@intel.com","subject":"[PATCH v3 2/4] format-patch: add '--base' option to record base tree info","fromName":"Xiaolong Ye","fromEmail":"xiaolong.ye@intel.com","sentAt":"2016-03-31T01:46:14Z","receivedAt":"2016-03-31T01:46:14Z","isPatch":true,"sender":{"key":"xiaolong.ye@intel.com","avatar":"https://avatars.githubusercontent.com/u/21098480?v=4"},"body":"Maintainers or third party testers may want to know the exact base tree\nthe patch series applies to. Teach git format-patch a '--base' option to\nrecord the base tree info and append this information at the end of the\n_first_ message (either the cover letter or the first patch in the series).\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nHelped-by: Wu Fengguang <fengguang.wu@intel.com>\nSigned-off-by: Xiaolong Ye <xiaolong.ye@intel.com>\n---\n Documentation/git-format-patch.txt | 25 +++++++++++\n builtin/log.c                      | 89 ++++++++++++++++++++++++++++++++++++++\n 2 files changed, 114 insertions(+)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 6821441..067d562 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -265,6 +265,31 @@ you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`.\n   Output an all-zero hash in each patch's From header instead\n   of the hash of the commit.\n \n+--base=<commit>::\n+\tRecord the base tree information to identify the whole tree\n+\tthe patch series applies to. For example, the patch submitter\n+\thas a commit history of this shape:\n+\n+\t---P---X---Y---Z---A---B---C\n+\n+\twhere \"P\" is the well-known public commit (e.g. one in Linus's tree),\n+\t\"X\", \"Y\", \"Z\" are prerequisite patches in flight, and \"A\", \"B\", \"C\"\n+\tare the work being sent out, the submitter could say \"git format-patch\n+\t--base=P -3 C\" (or variants thereof, e.g. with \"--cover\" or using\n+\t\"Z..C\" instead of \"-3 C\" to specify the range), and the identifiers\n+\tfor P, X, Y, Z are appended at the end of the _first_ message (either\n+\tthe cover letter or the first patch in the series).\n+\n+\tFor non-linear topology, such as\n+\n+\t    ---P---X---A---M---C\n+\t\t\\         /\n+\t\t Y---Z---B\n+\n+\tthe submitter could also use \"git format-patch --base=P -3 C\" to generate\n+\tpatches for A, B and C, and the identifiers for P, X, Y, Z are appended\n+\tat the end of the _first_ message.\n+\n --root::\n \tTreat the revision argument as a <revision range>, even if it\n \tis just a single commit (that would normally be treated as a\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 0d738d6..03cbab0 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1185,6 +1185,82 @@ static int from_callback(const struct option *opt, const char *arg, int unset)\n \treturn 0;\n }\n \n+struct base_tree_info {\n+\tstruct object_id base_commit;\n+\tint nr_patch_id, alloc_patch_id;\n+\tstruct object_id *patch_id;\n+};\n+\n+static void prepare_bases(struct base_tree_info *bases,\n+\t\t\t  const char *base_commit,\n+\t\t\t  struct commit **list,\n+\t\t\t  int total)\n+{\n+\tstruct commit *base = NULL, *commit;\n+\tstruct rev_info revs;\n+\tstruct diff_options diffopt;\n+\tstruct object_id *patch_id;\n+\tunsigned char sha1[20];\n+\tint i;\n+\n+\tdiff_setup(&diffopt);\n+\tDIFF_OPT_SET(&diffopt, RECURSIVE);\n+\tdiff_setup_done(&diffopt);\n+\n+\tbase = lookup_commit_reference_by_name(base_commit);\n+\tif (!base)\n+\t\tdie(_(\"Unknown commit %s\"), base_commit);\n+\toidcpy(&bases->base_commit, &base->object.oid);\n+\n+\tinit_revisions(&revs, NULL);\n+\trevs.max_parents = 1;\n+\tbase->object.flags |= UNINTERESTING;\n+\tadd_pending_object(&revs, &base->object, \"base\");\n+\tfor (i = 0; i < total; i++) {\n+\t\tlist[i]->object.flags |= 0;\n+\t\tadd_pending_object(&revs, &list[i]->object, \"rev_list\");\n+\t\tlist[i]->util = (void *)1;\n+\t}\n+\n+\tif (prepare_revision_walk(&revs))\n+\t\tdie(_(\"revision walk setup failed\"));\n+\t/*\n+\t * Traverse the prerequisite commits list,\n+\t * get the patch ids and stuff them in bases structure.\n+\t */\n+\twhile ((commit = get_revision(&revs)) != NULL) {\n+\t\tif (commit->util)\n+\t\t\tcontinue;\n+\t\tif (commit_patch_id(commit, &diffopt, sha1))\n+\t\t\tdie(_(\"cannot get patch id\"));\n+\t\tALLOC_GROW(bases->patch_id, bases->nr_patch_id + 1, bases->alloc_patch_id);\n+\t\tpatch_id = bases->patch_id + bases->nr_patch_id;\n+\t\thashcpy(patch_id->hash, sha1);\n+\t\tbases->nr_patch_id++;\n+\t}\n+}\n+\n+static void print_bases(struct base_tree_info *bases)\n+{\n+\tint i;\n+\n+\t/* Only do this once, either for the cover or for the first one */\n+\tif (is_null_oid(&bases->base_commit))\n+\t\treturn;\n+\n+\t/* Show the base commit */\n+\tprintf(\"base-commit: %s\\n\", oid_to_hex(&bases->base_commit));\n+\n+\t/* Show the prerequisite patches */\n+\tfor (i = 0; i < bases->nr_patch_id; i++)\n+\t\tprintf(\"prerequisite-patch-id: %s\\n\", oid_to_hex(&bases->patch_id[i]));\n+\n+\tfree(bases->patch_id);\n+\tbases->nr_patch_id = 0;\n+\tbases->alloc_patch_id = 0;\n+\toidclr(&bases->base_commit);\n+}\n+\n int cmd_format_patch(int argc, const char **argv, const char *prefix)\n {\n \tstruct commit *commit;\n@@ -1209,6 +1285,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tint reroll_count = -1;\n \tchar *branch_name = NULL;\n \tchar *from = NULL;\n+\tchar *base_commit = NULL;\n+\tstruct base_tree_info bases;\n+\n \tconst struct option builtin_format_patch_options[] = {\n \t\t{ OPTION_CALLBACK, 'n', \"numbered\", &numbered, NULL,\n \t\t\t    N_(\"use [PATCH n/m] even with a single patch\"),\n@@ -1271,6 +1350,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t    PARSE_OPT_OPTARG, thread_callback },\n \t\tOPT_STRING(0, \"signature\", &signature, N_(\"signature\"),\n \t\t\t    N_(\"add a signature\")),\n+\t\tOPT_STRING(0, \"base\", &base_commit, N_(\"base-commit\"),\n+\t\t\t   N_(\"add prerequisite tree info to the patch series\")),\n \t\tOPT_FILENAME(0, \"signature-file\", &signature_file,\n \t\t\t\tN_(\"add a signature from a file\")),\n \t\tOPT__QUIET(&quiet, N_(\"don't print the patch filenames\")),\n@@ -1507,6 +1588,12 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tsignature = strbuf_detach(&buf, NULL);\n \t}\n \n+\tif (base_commit) {\n+\t\tmemset(&bases, 0, sizeof(bases));\n+\t\treset_revision_walk();\n+\t\tprepare_bases(&bases, base_commit, list, nr);\n+\t}\n+\n \tif (in_reply_to || thread || cover_letter)\n \t\trev.ref_message_ids = xcalloc(1, sizeof(struct string_list));\n \tif (in_reply_to) {\n@@ -1520,6 +1607,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\tgen_message_id(&rev, \"cover\");\n \t\tmake_cover_letter(&rev, use_stdout,\n \t\t\t\t  origin, nr, list, branch_name, quiet);\n+\t\tprint_bases(&bases);\n \t\ttotal++;\n \t\tstart_number--;\n \t}\n@@ -1585,6 +1673,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t\t       rev.mime_boundary);\n \t\t\telse\n \t\t\t\tprint_signature();\n+\t\t\tprint_bases(&bases);\n \t\t}\n \t\tif (!use_stdout)\n \t\t\tfclose(stdout);\n-- \n2.8.0.4.gcb5a9af\n"},{"id":"282290","messageId":"1459388776-18066-4-git-send-email-xiaolong.ye@intel.com","threadId":"41881","inReplyTo":"1459388776-18066-1-git-send-email-xiaolong.ye@intel.com","subject":"[PATCH v3 3/4] format-patch: introduce --base=auto option","fromName":"Xiaolong Ye","fromEmail":"xiaolong.ye@intel.com","sentAt":"2016-03-31T01:46:15Z","receivedAt":"2016-03-31T01:46:15Z","isPatch":true,"sender":{"key":"xiaolong.ye@intel.com","avatar":"https://avatars.githubusercontent.com/u/21098480?v=4"},"body":"Introduce --base=auto to record the base commit info automatically, the base_commit\nwill be the merge base of tip commit of the upstream branch and revision-range\nspecified in cmdline.\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nHelped-by: Wu Fengguang <fengguang.wu@intel.com>\nSigned-off-by: Xiaolong Ye <xiaolong.ye@intel.com>\n---\n Documentation/git-format-patch.txt |  4 ++++\n builtin/log.c                      | 31 +++++++++++++++++++++++++++----\n 2 files changed, 31 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 067d562..d8fe651 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -290,6 +290,10 @@ you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`.\n \tpatches for A, B and C, and the identifiers for P, X, Y, Z are appended\n \tat the end of the _first_ message.\n \n+\tIf set '--base=auto' in cmdline, it will track base commit automatically,\n+\tthe base commit will be the merge base of tip commit of the remote-tracking\n+\tbranch and revision-range specified in cmdline.\n+\n --root::\n \tTreat the revision argument as a <revision range>, even if it\n \tis just a single commit (that would normally be treated as a\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 03cbab0..c5efe73 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1200,6 +1200,9 @@ static void prepare_bases(struct base_tree_info *bases,\n \tstruct rev_info revs;\n \tstruct diff_options diffopt;\n \tstruct object_id *patch_id;\n+\tstruct branch *curr_branch;\n+\tstruct commit_list *base_list;\n+\tconst char *upstream;\n \tunsigned char sha1[20];\n \tint i;\n \n@@ -1207,10 +1210,30 @@ static void prepare_bases(struct base_tree_info *bases,\n \tDIFF_OPT_SET(&diffopt, RECURSIVE);\n \tdiff_setup_done(&diffopt);\n \n-\tbase = lookup_commit_reference_by_name(base_commit);\n-\tif (!base)\n-\t\tdie(_(\"Unknown commit %s\"), base_commit);\n-\toidcpy(&bases->base_commit, &base->object.oid);\n+\tif (!strcmp(base_commit, \"auto\")) {\n+\t\tcurr_branch = branch_get(NULL);\n+\t\tupstream = branch_get_upstream(curr_branch, NULL);\n+\t\tif (upstream) {\n+\t\t\tif (get_sha1(upstream, sha1))\n+\t\t\t\tdie(_(\"Failed to resolve '%s' as a valid ref.\"), upstream);\n+\t\t\tcommit = lookup_commit_or_die(sha1, \"upstream base\");\n+\t\t\tbase_list = get_merge_bases_many(commit, total, list);\n+\t\t\tif (!bases)\n+\t\t\t\tdie(_(\"Could not find merge base.\"));\n+\t\t\tbase = base_list->item;\n+\t\t\tfree_commit_list(base_list);\n+\t\t\toidcpy(&bases->base_commit, &base->object.oid);\n+\t\t} else {\n+\t\t\tdie(_(\"Failed to get upstream, if you want to record base commit automatically,\\n\"\n+\t\t\t      \"please use git branch --set-upstream-to to track a remote branch.\\n\"\n+\t\t\t      \"Or you could specify base commit by --base=<base-commit-id> manually.\"));\n+\t\t}\n+\t} else {\n+\t\tbase = lookup_commit_reference_by_name(base_commit);\n+\t\tif (!base)\n+\t\t\tdie(_(\"Unknown commit %s\"), base_commit);\n+\t\toidcpy(&bases->base_commit, &base->object.oid);\n+\t}\n \n \tinit_revisions(&revs, NULL);\n \trevs.max_parents = 1;\n-- \n2.8.0.4.gcb5a9af\n"},{"id":"282291","messageId":"1459388776-18066-5-git-send-email-xiaolong.ye@intel.com","threadId":"41881","inReplyTo":"1459388776-18066-1-git-send-email-xiaolong.ye@intel.com","subject":"[PATCH v3 4/4] format-patch: introduce format.base configuration","fromName":"Xiaolong Ye","fromEmail":"xiaolong.ye@intel.com","sentAt":"2016-03-31T01:46:16Z","receivedAt":"2016-03-31T01:46:16Z","isPatch":true,"sender":{"key":"xiaolong.ye@intel.com","avatar":"https://avatars.githubusercontent.com/u/21098480?v=4"},"body":"We can set format.base=auto to record the base commit info automatically,\nit is equivalent to set --base=auto in cmdline.\n\nThe format.base has lower priority than command line option, so if user\nset format.base=auto and pass the command line option in the meantime,\nbase_commit will be the one passed to command line option.\n\nSigned-off-by: Xiaolong Ye <xiaolong.ye@intel.com>\n---\n Documentation/git-format-patch.txt |  2 ++\n builtin/log.c                      | 21 ++++++++++++++-------\n 2 files changed, 16 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex d8fe651..10149ab 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -293,6 +293,8 @@ you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`.\n \tIf set '--base=auto' in cmdline, it will track base commit automatically,\n \tthe base commit will be the merge base of tip commit of the remote-tracking\n \tbranch and revision-range specified in cmdline.\n+\tIf 'format.base=auto' is set in configuration file, it is equivalent\n+\tto set '--base=auto' in cmdline.\n \n --root::\n \tTreat the revision argument as a <revision range>, even if it\ndiff --git a/builtin/log.c b/builtin/log.c\nindex c5efe73..821a778 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -699,6 +699,7 @@ static int do_signoff;\n static const char *signature = git_version_string;\n static const char *signature_file;\n static int config_cover_letter;\n+static int config_base_commit;\n static const char *config_output_directory;\n \n enum {\n@@ -780,6 +781,12 @@ static int git_format_config(const char *var, const char *value, void *cb)\n \t}\n \tif (!strcmp(var, \"format.outputdirectory\"))\n \t\treturn git_config_string(&config_output_directory, var, value);\n+\tif (!strcmp(var, \"format.base\")){\n+\t\tif (value && !strcasecmp(value, \"auto\")) {\n+\t\t\tconfig_base_commit = 1;\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n \n \treturn git_log_config(var, value, cb);\n }\n@@ -1210,7 +1217,12 @@ static void prepare_bases(struct base_tree_info *bases,\n \tDIFF_OPT_SET(&diffopt, RECURSIVE);\n \tdiff_setup_done(&diffopt);\n \n-\tif (!strcmp(base_commit, \"auto\")) {\n+\tif (base_commit && strcmp(base_commit, \"auto\")) {\n+\t\tbase = lookup_commit_reference_by_name(base_commit);\n+\t\tif (!base)\n+\t\t\tdie(_(\"Unknown commit %s\"), base_commit);\n+\t\toidcpy(&bases->base_commit, &base->object.oid);\n+\t} else if ((base_commit && !strcmp(base_commit, \"auto\")) || config_base_commit) {\n \t\tcurr_branch = branch_get(NULL);\n \t\tupstream = branch_get_upstream(curr_branch, NULL);\n \t\tif (upstream) {\n@@ -1228,11 +1240,6 @@ static void prepare_bases(struct base_tree_info *bases,\n \t\t\t      \"please use git branch --set-upstream-to to track a remote branch.\\n\"\n \t\t\t      \"Or you could specify base commit by --base=<base-commit-id> manually.\"));\n \t\t}\n-\t} else {\n-\t\tbase = lookup_commit_reference_by_name(base_commit);\n-\t\tif (!base)\n-\t\t\tdie(_(\"Unknown commit %s\"), base_commit);\n-\t\toidcpy(&bases->base_commit, &base->object.oid);\n \t}\n \n \tinit_revisions(&revs, NULL);\n@@ -1611,7 +1618,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tsignature = strbuf_detach(&buf, NULL);\n \t}\n \n-\tif (base_commit) {\n+\tif (base_commit || config_base_commit) {\n \t\tmemset(&bases, 0, sizeof(bases));\n \t\treset_revision_walk();\n \t\tprepare_bases(&bases, base_commit, list, nr);\n-- \n2.8.0.4.gcb5a9af\n"},{"id":"282352","messageId":"xmqqy48yo8eb.fsf@gitster.mtv.corp.google.com","threadId":"41881","inReplyTo":"1459388776-18066-3-git-send-email-xiaolong.ye@intel.com","subject":"Re: [PATCH v3 2/4] format-patch: add '--base' option to record base tree info","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-31T17:38:04Z","receivedAt":"2016-03-31T17:38:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Xiaolong Ye <xiaolong.ye@intel.com> writes:\n\n> Maintainers or third party testers may want to know the exact base tree\n> the patch series applies to. Teach git format-patch a '--base' option to\n> record the base tree info and append this information at the end of the\n> _first_ message (either the cover letter or the first patch in the series).\n\nYou'd need a description of what \"base tree info\" consists of as a\nseparate paragraph after the above paragraph.  I'd also suggest to\n\n\ts/and append this information/and append it/;\n\nBased on my understanding of what you consider \"base tree info\", it\nmay look like this, but you know your design better, so I'd expect\nyou to rewrite it to be more useful, or at least to fill in the\nblanks.\n\n\tThe base tree info consists of the \"base commit\", which is a\n\twell-known commit that is part of the stable part of the\n\tproject history everybody else works off of, and zero or\n\tmore \"prerequisite patches\", which are well-known patches in\n\tflight that is not yet part of the \"base commit\" that need\n\tto be applied on top of \"base commit\" ???IN WHAT ORDER???\n\tbefore the patches can be applied.\n\n\t\"base commit\" is shown as \"base-commit: \" followed by the\n\t40-hex of the commit object name.  A \"prerequisite patch\" is\n\tshown as \"prerequisite-patch-id: \" followed by the 40-hex\n\t\"patch id\", which can be obtained by ???DOING WHAT???\n\n> Helped-by: Junio C Hamano <gitster@pobox.com>\n> Helped-by: Wu Fengguang <fengguang.wu@intel.com>\n> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>\n> ---\n>  Documentation/git-format-patch.txt | 25 +++++++++++\n>  builtin/log.c                      | 89 ++++++++++++++++++++++++++++++++++++++\n>  2 files changed, 114 insertions(+)\n>\n> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\n> index 6821441..067d562 100644\n> --- a/Documentation/git-format-patch.txt\n> +++ b/Documentation/git-format-patch.txt\n> @@ -265,6 +265,31 @@ you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`.\n>    Output an all-zero hash in each patch's From header instead\n>    of the hash of the commit.\n>  \n> +--base=<commit>::\n> +\tRecord the base tree information to identify the whole tree\n> +\tthe patch series applies to. For example, the patch submitter\n> +\thas a commit history of this shape:\n> +\n> +\t---P---X---Y---Z---A---B---C\n> +\n> +\twhere \"P\" is the well-known public commit (e.g. one in Linus's tree),\n> +\t\"X\", \"Y\", \"Z\" are prerequisite patches in flight, and \"A\", \"B\", \"C\"\n> +\tare the work being sent out, the submitter could say \"git format-patch\n> +\t--base=P -3 C\" (or variants thereof, e.g. with \"--cover\" or using\n> +\t\"Z..C\" instead of \"-3 C\" to specify the range), and the identifiers\n> +\tfor P, X, Y, Z are appended at the end of the _first_ message (either\n> +\tthe cover letter or the first patch in the series).\n> +\n> +\tFor non-linear topology, such as\n> +\n> +\t    ---P---X---A---M---C\n> +\t\t\\         /\n> +\t\t Y---Z---B\n> +\n> +\tthe submitter could also use \"git format-patch --base=P -3 C\" to generate\n> +\tpatches for A, B and C, and the identifiers for P, X, Y, Z are appended\n> +\tat the end of the _first_ message.\n\nThe contents of this look OK, but does it format correctly via\nAsciiDoc?  I suspect that only the first paragraph up to \"of this\nshape:\" would appear correctly and all the rest would become funny.\n\nAlso the definition of \"base tree information\" you need to have in\nthe log message should be given somewhere in this documentation, not\nnecessarily in the documentation of --base=<commit> option.\n\nBecause the use of this new option is not an essential part of\nworkflow of all users of format-patch, it may be a good idea to have\nits own separate section, perhaps between the \"DISCUSSION\" and\n\"EXAMPLES\" sections, titled \"BASE TREE IDENTIFICATION\", move the\nbulk of text above there with the specification of what \"base tree\ninfo\" consists of there.\n\nAnd shorten the description of the option to something like:\n\n--base=<commit>::\n\tRecord the base tree information to identify the state the\n\tpatch series applies to.  See the BASE TREE IDENTIFICATION\n        section below for details.\n\nor something.\n\n> diff --git a/builtin/log.c b/builtin/log.c\n> index 0d738d6..03cbab0 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -1185,6 +1185,82 @@ static int from_callback(const struct option *opt, const char *arg, int unset)\n>  \treturn 0;\n>  }\n>  \n> +struct base_tree_info {\n> +\tstruct object_id base_commit;\n> +\tint nr_patch_id, alloc_patch_id;\n> +\tstruct object_id *patch_id;\n> +};\n> +\n> +static void prepare_bases(struct base_tree_info *bases,\n> +\t\t\t  const char *base_commit,\n> +\t\t\t  struct commit **list,\n> +\t\t\t  int total)\n> +{\n> +\tstruct commit *base = NULL, *commit;\n> +\tstruct rev_info revs;\n> +\tstruct diff_options diffopt;\n> +\tstruct object_id *patch_id;\n> +\tunsigned char sha1[20];\n> +\tint i;\n> +\n> +\tdiff_setup(&diffopt);\n> +\tDIFF_OPT_SET(&diffopt, RECURSIVE);\n> +\tdiff_setup_done(&diffopt);\n> +\n> +\tbase = lookup_commit_reference_by_name(base_commit);\n> +\tif (!base)\n> +\t\tdie(_(\"Unknown commit %s\"), base_commit);\n> +\toidcpy(&bases->base_commit, &base->object.oid);\n> +\n> +\tinit_revisions(&revs, NULL);\n> +\trevs.max_parents = 1;\n> +\tbase->object.flags |= UNINTERESTING;\n> +\tadd_pending_object(&revs, &base->object, \"base\");\n> +\tfor (i = 0; i < total; i++) {\n> +\t\tlist[i]->object.flags |= 0;\n\nWhat does this statement do, exactly?  Are you clearing some bits\nbut not others, and if so which ones?\n\n> +\t\tadd_pending_object(&revs, &list[i]->object, \"rev_list\");\n> +\t\tlist[i]->util = (void *)1;\n\nAre we sure commit objects not on the list have their ->util cleared?\nThe while() loop below seems to rely on that to correctly filter out\nthe ones that are on the list.\n\n> +\t}\n> +\n> +\tif (prepare_revision_walk(&revs))\n> +\t\tdie(_(\"revision walk setup failed\"));\n> +\t/*\n> +\t * Traverse the prerequisite commits list,\n> +\t * get the patch ids and stuff them in bases structure.\n> +\t */\n> +\twhile ((commit = get_revision(&revs)) != NULL) {\n> +\t\tif (commit->util)\n> +\t\t\tcontinue;\n> +\t\tif (commit_patch_id(commit, &diffopt, sha1))\n> +\t\t\tdie(_(\"cannot get patch id\"));\n> +\t\tALLOC_GROW(bases->patch_id, bases->nr_patch_id + 1, bases->alloc_patch_id);\n> +\t\tpatch_id = bases->patch_id + bases->nr_patch_id;\n> +\t\thashcpy(patch_id->hash, sha1);\n\nThe variable patch_id is used only once here.  Perhaps either write\n\n\thashcpy(bases->patch_id[bases->nr_patch_id]->hash, sha1);\n\nto get rid of the variable, or move its declaration inside the\nwhile() loop to limit its scope?\n\nHas this traversal been told, when setting up the &revs structure,\nto show commits in specific order (like \"topo order\")?  Should it\nbe?\n\n> +\t\tbases->nr_patch_id++;\n> +\t}\n> +}\n> +\n> +static void print_bases(struct base_tree_info *bases)\n> +{\n> +\tint i;\n> +\n> +\t/* Only do this once, either for the cover or for the first one */\n> +\tif (is_null_oid(&bases->base_commit))\n> +\t\treturn;\n> +\n> +\t/* Show the base commit */\n> +\tprintf(\"base-commit: %s\\n\", oid_to_hex(&bases->base_commit));\n> +\n> +\t/* Show the prerequisite patches */\n> +\tfor (i = 0; i < bases->nr_patch_id; i++)\n> +\t\tprintf(\"prerequisite-patch-id: %s\\n\", oid_to_hex(&bases->patch_id[i]));\n\nThis shows the patches in the order discovered by the revision\ntraversal, which typically is newer to older.  Is that intended?\nIs it assumed that the order of the patches does not matter?\n\n> @@ -1209,6 +1285,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n\nThe remainder of the patch looks very sensible, including the call\nto reset_revision_walk().\n\nThanks.\n"},{"id":"282353","messageId":"xmqqtwjmo84r.fsf@gitster.mtv.corp.google.com","threadId":"41881","inReplyTo":"1459388776-18066-4-git-send-email-xiaolong.ye@intel.com","subject":"Re: [PATCH v3 3/4] format-patch: introduce --base=auto option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-31T17:43:48Z","receivedAt":"2016-03-31T17:43:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Xiaolong Ye <xiaolong.ye@intel.com> writes:\n\n> Introduce --base=auto to record the base commit info automatically, the base_commit\n> will be the merge base of tip commit of the upstream branch and revision-range\n> specified in cmdline.\n\nThis line is probably a bit too long.\n\n>\n> Helped-by: Junio C Hamano <gitster@pobox.com>\n> Helped-by: Wu Fengguang <fengguang.wu@intel.com>\n> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>\n> ---\n>  Documentation/git-format-patch.txt |  4 ++++\n>  builtin/log.c                      | 31 +++++++++++++++++++++++++++----\n>  2 files changed, 31 insertions(+), 4 deletions(-)\n>\n> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\n> index 067d562..d8fe651 100644\n> --- a/Documentation/git-format-patch.txt\n> +++ b/Documentation/git-format-patch.txt\n> @@ -290,6 +290,10 @@ you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`.\n>  \tpatches for A, B and C, and the identifiers for P, X, Y, Z are appended\n>  \tat the end of the _first_ message.\n>  \n> +\tIf set '--base=auto' in cmdline, it will track base commit automatically,\n> +\tthe base commit will be the merge base of tip commit of the remote-tracking\n> +\tbranch and revision-range specified in cmdline.\n> +\n>  --root::\n>  \tTreat the revision argument as a <revision range>, even if it\n>  \tis just a single commit (that would normally be treated as a\n> diff --git a/builtin/log.c b/builtin/log.c\n> index 03cbab0..c5efe73 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -1200,6 +1200,9 @@ static void prepare_bases(struct base_tree_info *bases,\n>  \tstruct rev_info revs;\n>  \tstruct diff_options diffopt;\n>  \tstruct object_id *patch_id;\n> +\tstruct branch *curr_branch;\n> +\tstruct commit_list *base_list;\n> +\tconst char *upstream;\n>  \tunsigned char sha1[20];\n>  \tint i;\n>  \n> @@ -1207,10 +1210,30 @@ static void prepare_bases(struct base_tree_info *bases,\n>  \tDIFF_OPT_SET(&diffopt, RECURSIVE);\n>  \tdiff_setup_done(&diffopt);\n>  \n> -\tbase = lookup_commit_reference_by_name(base_commit);\n> -\tif (!base)\n> -\t\tdie(_(\"Unknown commit %s\"), base_commit);\n> -\toidcpy(&bases->base_commit, &base->object.oid);\n> +\tif (!strcmp(base_commit, \"auto\")) {\n> +\t\tcurr_branch = branch_get(NULL);\n\nCan branch_get() return NULL?  Which ...\n\n> +\t\tupstream = branch_get_upstream(curr_branch, NULL);\n\n... would cause branch_get_upstream() to give you an error (which\nyou ignore)?  I guess that is OK because upstream will safely be set\nto NULL in that case.\n\n> +\t\tif (upstream) {\n> +\t\t\tif (get_sha1(upstream, sha1))\n> +\t\t\t\tdie(_(\"Failed to resolve '%s' as a valid ref.\"), upstream);\n> +\t\t\tcommit = lookup_commit_or_die(sha1, \"upstream base\");\n> +\t\t\tbase_list = get_merge_bases_many(commit, total, list);\n> +\t\t\tif (!bases)\n> +\t\t\t\tdie(_(\"Could not find merge base.\"));\n> +\t\t\tbase = base_list->item;\n> +\t\t\tfree_commit_list(base_list);\n\nWhat should happen when there are multiple merge bases?  The code\npicks one at random and ignores the remainder, if I am reading this\ncorrectly.\n\n> +\t\t\toidcpy(&bases->base_commit, &base->object.oid);\n> +\t\t} else {\n> +\t\t\tdie(_(\"Failed to get upstream, if you want to record base commit automatically,\\n\"\n> +\t\t\t      \"please use git branch --set-upstream-to to track a remote branch.\\n\"\n> +\t\t\t      \"Or you could specify base commit by --base=<base-commit-id> manually.\"));\n> +\t\t}\n> +\t} else {\n> +\t\tbase = lookup_commit_reference_by_name(base_commit);\n> +\t\tif (!base)\n> +\t\t\tdie(_(\"Unknown commit %s\"), base_commit);\n> +\t\toidcpy(&bases->base_commit, &base->object.oid);\n> +\t}\n>  \n>  \tinit_revisions(&revs, NULL);\n>  \trevs.max_parents = 1;\n"},{"id":"282354","messageId":"xmqqpouao82p.fsf@gitster.mtv.corp.google.com","threadId":"41881","inReplyTo":"1459388776-18066-1-git-send-email-xiaolong.ye@intel.com","subject":"Re: [PATCH v3 0/4] Add an option to git-format-patch to record base tree info","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-31T17:45:02Z","receivedAt":"2016-03-31T17:45:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Xiaolong Ye <xiaolong.ye@intel.com> writes:\n\n> V3 mainly improves the implementation according to Junio's comments,\n> Changes vs v2 include:\n>\n>  - Remove the unnecessary output line \"** base-commit-info **\".\n>  \n>  - Improve the traverse logic to handle not only linear topology, but more\n>    general cases, it will start revision walk by setting the starting points\n>    of the traversal to all elements in the rev list[], and skip the ones in \n>    list[], only grab the patch-ids of prerequisite patches.\n\nThis looks much more sensible than the previous ones.  I sent a few\ncomments on remaining issues separately.\n\n\nThanks.\n"},{"id":"282458","messageId":"20160401133801.GA2915@yexl-desktop","threadId":"41881","inReplyTo":"xmqqy48yo8eb.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 2/4] format-patch: add '--base' option to record base tree info","fromName":"Ye Xiaolong","fromEmail":"xiaolong.ye@intel.com","sentAt":"2016-04-01T13:38:01Z","receivedAt":"2016-04-01T13:38:01Z","isPatch":true,"sender":{"key":"xiaolong.ye@intel.com","avatar":"https://avatars.githubusercontent.com/u/21098480?v=4"},"body":"On Thu, Mar 31, 2016 at 10:38:04AM -0700, Junio C Hamano wrote:\n>Xiaolong Ye <xiaolong.ye@intel.com> writes:\n>\n>> Maintainers or third party testers may want to know the exact base tree\n>> the patch series applies to. Teach git format-patch a '--base' option to\n>> record the base tree info and append this information at the end of the\n>> _first_ message (either the cover letter or the first patch in the series).\n>\n>You'd need a description of what \"base tree info\" consists of as a\n>separate paragraph after the above paragraph.  I'd also suggest to\n>\n>\ts/and append this information/and append it/;\n>\n>Based on my understanding of what you consider \"base tree info\", it\n>may look like this, but you know your design better, so I'd expect\n>you to rewrite it to be more useful, or at least to fill in the\n>blanks.\n>\n>\tThe base tree info consists of the \"base commit\", which is a\n>\twell-known commit that is part of the stable part of the\n>\tproject history everybody else works off of, and zero or\n>\tmore \"prerequisite patches\", which are well-known patches in\n>\tflight that is not yet part of the \"base commit\" that need\n>\tto be applied on top of \"base commit\" ???IN WHAT ORDER???\n>\tbefore the patches can be applied.\n>\n>\t\"base commit\" is shown as \"base-commit: \" followed by the\n>\t40-hex of the commit object name.  A \"prerequisite patch\" is\n>\tshown as \"prerequisite-patch-id: \" followed by the 40-hex\n>\t\"patch id\", which can be obtained by ???DOING WHAT???\n>\nThanks for the review.\n\nOk, I'll polish up the description of base tree info and add it to commit log\nas you suggested.\n\n>> Helped-by: Junio C Hamano <gitster@pobox.com>\n>> Helped-by: Wu Fengguang <fengguang.wu@intel.com>\n>> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>\n>> ---\n>>  Documentation/git-format-patch.txt | 25 +++++++++++\n>>  builtin/log.c                      | 89 ++++++++++++++++++++++++++++++++++++++\n>>  2 files changed, 114 insertions(+)\n>>\n>> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\n>> index 6821441..067d562 100644\n>> --- a/Documentation/git-format-patch.txt\n>> +++ b/Documentation/git-format-patch.txt\n>> @@ -265,6 +265,31 @@ you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`.\n>>    Output an all-zero hash in each patch's From header instead\n>>    of the hash of the commit.\n>>  \n>> +--base=<commit>::\n>> +\tRecord the base tree information to identify the whole tree\n>> +\tthe patch series applies to. For example, the patch submitter\n>> +\thas a commit history of this shape:\n>> +\n>> +\t---P---X---Y---Z---A---B---C\n>> +\n>> +\twhere \"P\" is the well-known public commit (e.g. one in Linus's tree),\n>> +\t\"X\", \"Y\", \"Z\" are prerequisite patches in flight, and \"A\", \"B\", \"C\"\n>> +\tare the work being sent out, the submitter could say \"git format-patch\n>> +\t--base=P -3 C\" (or variants thereof, e.g. with \"--cover\" or using\n>> +\t\"Z..C\" instead of \"-3 C\" to specify the range), and the identifiers\n>> +\tfor P, X, Y, Z are appended at the end of the _first_ message (either\n>> +\tthe cover letter or the first patch in the series).\n>> +\n>> +\tFor non-linear topology, such as\n>> +\n>> +\t    ---P---X---A---M---C\n>> +\t\t\\         /\n>> +\t\t Y---Z---B\n>> +\n>> +\tthe submitter could also use \"git format-patch --base=P -3 C\" to generate\n>> +\tpatches for A, B and C, and the identifiers for P, X, Y, Z are appended\n>> +\tat the end of the _first_ message.\n>\n>The contents of this look OK, but does it format correctly via\n>AsciiDoc?  I suspect that only the first paragraph up to \"of this\n>shape:\" would appear correctly and all the rest would become funny.\n\nSorry, just heard of AsciiDoc, I will try to use it to do the right format work.\n\n>\n>Also the definition of \"base tree information\" you need to have in\n>the log message should be given somewhere in this documentation, not\n>necessarily in the documentation of --base=<commit> option.\n>\n>Because the use of this new option is not an essential part of\n>workflow of all users of format-patch, it may be a good idea to have\n>its own separate section, perhaps between the \"DISCUSSION\" and\n>\"EXAMPLES\" sections, titled \"BASE TREE IDENTIFICATION\", move the\n>bulk of text above there with the specification of what \"base tree\n>info\" consists of there.\n>\n>And shorten the description of the option to something like:\n>\n>--base=<commit>::\n>\tRecord the base tree information to identify the state the\n>\tpatch series applies to.  See the BASE TREE IDENTIFICATION\n>        section below for details.\n>\n>or something.\n\nI'll restructure the descriptions in a resend.\n\n>\n>> diff --git a/builtin/log.c b/builtin/log.c\n>> index 0d738d6..03cbab0 100644\n>> --- a/builtin/log.c\n>> +++ b/builtin/log.c\n>> @@ -1185,6 +1185,82 @@ static int from_callback(const struct option *opt, const char *arg, int unset)\n>>  \treturn 0;\n>>  }\n>>  \n>> +struct base_tree_info {\n>> +\tstruct object_id base_commit;\n>> +\tint nr_patch_id, alloc_patch_id;\n>> +\tstruct object_id *patch_id;\n>> +};\n>> +\n>> +static void prepare_bases(struct base_tree_info *bases,\n>> +\t\t\t  const char *base_commit,\n>> +\t\t\t  struct commit **list,\n>> +\t\t\t  int total)\n>> +{\n>> +\tstruct commit *base = NULL, *commit;\n>> +\tstruct rev_info revs;\n>> +\tstruct diff_options diffopt;\n>> +\tstruct object_id *patch_id;\n>> +\tunsigned char sha1[20];\n>> +\tint i;\n>> +\n>> +\tdiff_setup(&diffopt);\n>> +\tDIFF_OPT_SET(&diffopt, RECURSIVE);\n>> +\tdiff_setup_done(&diffopt);\n>> +\n>> +\tbase = lookup_commit_reference_by_name(base_commit);\n>> +\tif (!base)\n>> +\t\tdie(_(\"Unknown commit %s\"), base_commit);\n>> +\toidcpy(&bases->base_commit, &base->object.oid);\n>> +\n>> +\tinit_revisions(&revs, NULL);\n>> +\trevs.max_parents = 1;\n>> +\tbase->object.flags |= UNINTERESTING;\n>> +\tadd_pending_object(&revs, &base->object, \"base\");\n>> +\tfor (i = 0; i < total; i++) {\n>> +\t\tlist[i]->object.flags |= 0;\n>\n>What does this statement do, exactly?  Are you clearing some bits\n>but not others, and if so which ones?\n\nMy mistake, it's useless and should be removed.\n\n>\n>> +\t\tadd_pending_object(&revs, &list[i]->object, \"rev_list\");\n>> +\t\tlist[i]->util = (void *)1;\n>\n>Are we sure commit objects not on the list have their ->util cleared?\n>The while() loop below seems to rely on that to correctly filter out\n>the ones that are on the list.\n>\n\nI'll need to check it.\n\n>> +\t}\n>> +\n>> +\tif (prepare_revision_walk(&revs))\n>> +\t\tdie(_(\"revision walk setup failed\"));\n>> +\t/*\n>> +\t * Traverse the prerequisite commits list,\n>> +\t * get the patch ids and stuff them in bases structure.\n>> +\t */\n>> +\twhile ((commit = get_revision(&revs)) != NULL) {\n>> +\t\tif (commit->util)\n>> +\t\t\tcontinue;\n>> +\t\tif (commit_patch_id(commit, &diffopt, sha1))\n>> +\t\t\tdie(_(\"cannot get patch id\"));\n>> +\t\tALLOC_GROW(bases->patch_id, bases->nr_patch_id + 1, bases->alloc_patch_id);\n>> +\t\tpatch_id = bases->patch_id + bases->nr_patch_id;\n>> +\t\thashcpy(patch_id->hash, sha1);\n>\n>The variable patch_id is used only once here.  Perhaps either write\n>\n>\thashcpy(bases->patch_id[bases->nr_patch_id]->hash, sha1);\n>\n>to get rid of the variable, or move its declaration inside the\n>while() loop to limit its scope?\n>\n\nSure, I'll move the patch_id declaration inside the loop.\n\n>Has this traversal been told, when setting up the &revs structure,\n>to show commits in specific order (like \"topo order\")?  Should it\n>be?\n\nThanks for the reminder, this traversal need to be in topo order,\nI'll set revs.topo_order to 1 explicitly.\n\n>\n>> +\t\tbases->nr_patch_id++;\n>> +\t}\n>> +}\n>> +\n>> +static void print_bases(struct base_tree_info *bases)\n>> +{\n>> +\tint i;\n>> +\n>> +\t/* Only do this once, either for the cover or for the first one */\n>> +\tif (is_null_oid(&bases->base_commit))\n>> +\t\treturn;\n>> +\n>> +\t/* Show the base commit */\n>> +\tprintf(\"base-commit: %s\\n\", oid_to_hex(&bases->base_commit));\n>> +\n>> +\t/* Show the prerequisite patches */\n>> +\tfor (i = 0; i < bases->nr_patch_id; i++)\n>> +\t\tprintf(\"prerequisite-patch-id: %s\\n\", oid_to_hex(&bases->patch_id[i]));\n>\n>This shows the patches in the order discovered by the revision\n>traversal, which typically is newer to older.  Is that intended?\n>Is it assumed that the order of the patches does not matter?\n\nThe prerequisite patches should show in topological order, thus robot\ncould parse them one by one and apply the patches in reverse order.\n\nThanks,\nXiaolong.\n>\n>> @@ -1209,6 +1285,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n>\n>The remainder of the patch looks very sensible, including the call\n>to reset_revision_walk().\n>\n>Thanks.\n"},{"id":"282460","messageId":"20160401135207.GB2915@yexl-desktop","threadId":"41881","inReplyTo":"xmqqtwjmo84r.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 3/4] format-patch: introduce --base=auto option","fromName":"Ye Xiaolong","fromEmail":"xiaolong.ye@intel.com","sentAt":"2016-04-01T13:52:07Z","receivedAt":"2016-04-01T13:52:07Z","isPatch":true,"sender":{"key":"xiaolong.ye@intel.com","avatar":"https://avatars.githubusercontent.com/u/21098480?v=4"},"body":"On Thu, Mar 31, 2016 at 10:43:48AM -0700, Junio C Hamano wrote:\n>Xiaolong Ye <xiaolong.ye@intel.com> writes:\n>\n>> Introduce --base=auto to record the base commit info automatically, the base_commit\n>> will be the merge base of tip commit of the upstream branch and revision-range\n>> specified in cmdline.\n>\n>This line is probably a bit too long.\n\nHow about simplifying it to \"the base_commit is the merge base of upstream and\nspecified revision-range.\"?\n\n>\n>>\n>> Helped-by: Junio C Hamano <gitster@pobox.com>\n>> Helped-by: Wu Fengguang <fengguang.wu@intel.com>\n>> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>\n>> ---\n>>  Documentation/git-format-patch.txt |  4 ++++\n>>  builtin/log.c                      | 31 +++++++++++++++++++++++++++----\n>>  2 files changed, 31 insertions(+), 4 deletions(-)\n>>\n>> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\n>> index 067d562..d8fe651 100644\n>> --- a/Documentation/git-format-patch.txt\n>> +++ b/Documentation/git-format-patch.txt\n>> @@ -290,6 +290,10 @@ you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`.\n>>  \tpatches for A, B and C, and the identifiers for P, X, Y, Z are appended\n>>  \tat the end of the _first_ message.\n>>  \n>> +\tIf set '--base=auto' in cmdline, it will track base commit automatically,\n>> +\tthe base commit will be the merge base of tip commit of the remote-tracking\n>> +\tbranch and revision-range specified in cmdline.\n>> +\n>>  --root::\n>>  \tTreat the revision argument as a <revision range>, even if it\n>>  \tis just a single commit (that would normally be treated as a\n>> diff --git a/builtin/log.c b/builtin/log.c\n>> index 03cbab0..c5efe73 100644\n>> --- a/builtin/log.c\n>> +++ b/builtin/log.c\n>> @@ -1200,6 +1200,9 @@ static void prepare_bases(struct base_tree_info *bases,\n>>  \tstruct rev_info revs;\n>>  \tstruct diff_options diffopt;\n>>  \tstruct object_id *patch_id;\n>> +\tstruct branch *curr_branch;\n>> +\tstruct commit_list *base_list;\n>> +\tconst char *upstream;\n>>  \tunsigned char sha1[20];\n>>  \tint i;\n>>  \n>> @@ -1207,10 +1210,30 @@ static void prepare_bases(struct base_tree_info *bases,\n>>  \tDIFF_OPT_SET(&diffopt, RECURSIVE);\n>>  \tdiff_setup_done(&diffopt);\n>>  \n>> -\tbase = lookup_commit_reference_by_name(base_commit);\n>> -\tif (!base)\n>> -\t\tdie(_(\"Unknown commit %s\"), base_commit);\n>> -\toidcpy(&bases->base_commit, &base->object.oid);\n>> +\tif (!strcmp(base_commit, \"auto\")) {\n>> +\t\tcurr_branch = branch_get(NULL);\n>\n>Can branch_get() return NULL?  Which ...\n>\n>> +\t\tupstream = branch_get_upstream(curr_branch, NULL);\n>\n>... would cause branch_get_upstream() to give you an error (which\n>you ignore)?  I guess that is OK because upstream will safely be set\n>to NULL in that case.\n>\nYes, branch_get_upstream(curr_branch, NULL) will safely return NULL if curr_branch\nis NULL, so I think there is no need to add error handling for branch_get().\n\n>> +\t\tif (upstream) {\n>> +\t\t\tif (get_sha1(upstream, sha1))\n>> +\t\t\t\tdie(_(\"Failed to resolve '%s' as a valid ref.\"), upstream);\n>> +\t\t\tcommit = lookup_commit_or_die(sha1, \"upstream base\");\n>> +\t\t\tbase_list = get_merge_bases_many(commit, total, list);\n>> +\t\t\tif (!bases)\n>> +\t\t\t\tdie(_(\"Could not find merge base.\"));\n>> +\t\t\tbase = base_list->item;\n>> +\t\t\tfree_commit_list(base_list);\n>\n>What should happen when there are multiple merge bases?  The code\n>picks one at random and ignores the remainder, if I am reading this\n>correctly.\n\nIf there is more than one merge base, commits in base_list should be sorted by date,\nif I am understanding it correctly, so base_list->item should be the lastest\nmerge base commit, it should be enough for us to used as base commit.\n\nThanks,\nXiaolong.\n>\n>> +\t\t\toidcpy(&bases->base_commit, &base->object.oid);\n>> +\t\t} else {\n>> +\t\t\tdie(_(\"Failed to get upstream, if you want to record base commit automatically,\\n\"\n>> +\t\t\t      \"please use git branch --set-upstream-to to track a remote branch.\\n\"\n>> +\t\t\t      \"Or you could specify base commit by --base=<base-commit-id> manually.\"));\n>> +\t\t}\n>> +\t} else {\n>> +\t\tbase = lookup_commit_reference_by_name(base_commit);\n>> +\t\tif (!base)\n>> +\t\t\tdie(_(\"Unknown commit %s\"), base_commit);\n>> +\t\toidcpy(&bases->base_commit, &base->object.oid);\n>> +\t}\n>>  \n>>  \tinit_revisions(&revs, NULL);\n>>  \trevs.max_parents = 1;\n"},{"id":"282470","messageId":"xmqqpou9jp4b.fsf@gitster.mtv.corp.google.com","threadId":"41881","inReplyTo":"20160401133801.GA2915@yexl-desktop","subject":"Re: [PATCH v3 2/4] format-patch: add '--base' option to record base tree info","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-01T16:00:20Z","receivedAt":"2016-04-01T16:00:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ye Xiaolong <xiaolong.ye@intel.com> writes:\n\n> On Thu, Mar 31, 2016 at 10:38:04AM -0700, Junio C Hamano wrote:\n>\n>>The contents of this look OK, but does it format correctly via\n>>AsciiDoc?  I suspect that only the first paragraph up to \"of this\n>>shape:\" would appear correctly and all the rest would become funny.\n>\n> Sorry, just heard of AsciiDoc, I will try to use it to do the right format work.\n\nPlease make sure \"make -C Documentation\" produces sensible output\nfor *.1 (manpage) and *.html.\n\n>>> +\tinit_revisions(&revs, NULL);\n>>> +\trevs.max_parents = 1;\n>>> +\tbase->object.flags |= UNINTERESTING;\n>>> +\tadd_pending_object(&revs, &base->object, \"base\");\n>>> +\tfor (i = 0; i < total; i++) {\n>>> +\t\tlist[i]->object.flags |= 0;\n>>\n>>What does this statement do, exactly?  Are you clearing some bits\n>>but not others, and if so which ones?\n>\n> My mistake, it's useless and should be removed.\n\nIt probably make sense to do \"&= ~UNINTERESTING\" there, though.  You\nare adding one UNINTERESTING object (i.e. the base) and adding\nobjects that are on the list[] as interesting.\n\n>>This shows the patches in the order discovered by the revision\n>>traversal, which typically is newer to older.  Is that intended?\n>>Is it assumed that the order of the patches does not matter?\n>\n> The prerequisite patches should show in topological order, thus robot\n> could parse them one by one and apply the patches in reverse order.\n\nIf you have history where base is B, with three prerequisites 1-2-3,\nbefore the patch series A-B-C, i.e.\n\n\tB---1---2---3---A---B---C\n\nif you are showing \"base-commit: B\" as the first line in the base\ntree information block, it would be natural to expect that the\nprerequisite patch ids are listed for 1 and then 2 and then finally\n3, i.e.\n\n\tbase-commit: B\n        prerequisite-patch-id: 1\n        prerequisite-patch-id: 2\n        prerequisite-patch-id: 3\n\nno?\n\nAlso I know _you_ intend to consume this by robot, but it makes me\nwonder if with a minimum update you can make the output also more\nuseful for bystander humans.  A mailing list participant may\n\n - see an early round of a series that interests her,\n - try to apply them to her tree,\n - find that the series does not apply, but\n - sees that a block to help identify to what tree the series is\n   meant to apply.\n\nWith a list of 40-hex alone, she may not be able to figure out the\nprerequisites, but if there is some other clue that helps her to\nidentify the base commit and these patches, she may be able to\nconstruct a tree that is close enough.  Maybe you can help her by\nappending the title of the commit and patches at the end of these\nlines?\n\nThis is not a strong suggestion (yet); I am thinking aloud at this\npoint, without knowing how much it would help in practice to do so.\n"},{"id":"282478","messageId":"xmqqlh4xjoub.fsf@gitster.mtv.corp.google.com","threadId":"41881","inReplyTo":"20160401135207.GB2915@yexl-desktop","subject":"Re: [PATCH v3 3/4] format-patch: introduce --base=auto option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-01T16:06:20Z","receivedAt":"2016-04-01T16:06:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ye Xiaolong <xiaolong.ye@intel.com> writes:\n\n> On Thu, Mar 31, 2016 at 10:43:48AM -0700, Junio C Hamano wrote:\n>>Xiaolong Ye <xiaolong.ye@intel.com> writes:\n>>\n>>> Introduce --base=auto to record the base commit info automatically, the base_commit\n>>> will be the merge base of tip commit of the upstream branch and revision-range\n>>> specified in cmdline.\n>>\n>>This line is probably a bit too long.\n>\n> How about simplifying it to \"the base_commit is the merge base of upstream and\n> specified revision-range.\"?\n\nWhat I meant was not that profound.  I just wanted you to wrap your\nlines a bit shorter so that quoting in the discussion thread like\nthis would not make the result overlong to fit on a 80-column\nterminal ;-)\n\n>>> +\t\t\tbase = base_list->item;\n>>> +\t\t\tfree_commit_list(base_list);\n>>\n>>What should happen when there are multiple merge bases?  The code\n>>picks one at random and ignores the remainder, if I am reading this\n>>correctly.\n>\n> If there is more than one merge base, commits in base_list should\n> be sorted by date, if I am understanding it correctly, so\n> base_list->item should be the lastest merge base commit, it should\n> be enough for us to used as base commit.\n\nBy definition, when there are multiple merge bases, there is no\nlatest one among them.\n\nWhen the history involves criss-cross merges, there can be more than\none 'best' common ancestor for two commits.  For example, with this\ntopology (note that X is not a commit; it merely denotes crossing of\ntwo lines):\n\n       ---1---o---A\n      /    \\ /\n  ---O      X\n      \\    / \\\n       ---2---o---o---B\n\nboth '1' and '2' are merge-bases of 'A' and 'B'.  And the timestamps\non one (be it committer or author timestamp) being later than those\nof the other do not make it any more suitable than the other one.\n"},{"id":"282683","messageId":"20160405055203.GA10110@yexl-desktop","threadId":"41881","inReplyTo":"xmqqpou9jp4b.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 2/4] format-patch: add '--base' option to record base tree info","fromName":"Ye Xiaolong","fromEmail":"xiaolong.ye@intel.com","sentAt":"2016-04-05T05:52:03Z","receivedAt":"2016-04-05T05:52:03Z","isPatch":true,"sender":{"key":"xiaolong.ye@intel.com","avatar":"https://avatars.githubusercontent.com/u/21098480?v=4"},"body":"On Fri, Apr 01, 2016 at 09:00:20AM -0700, Junio C Hamano wrote:\n>Ye Xiaolong <xiaolong.ye@intel.com> writes:\n>\n>> On Thu, Mar 31, 2016 at 10:38:04AM -0700, Junio C Hamano wrote:\n>>\n>>>The contents of this look OK, but does it format correctly via\n>>>AsciiDoc?  I suspect that only the first paragraph up to \"of this\n>>>shape:\" would appear correctly and all the rest would become funny.\n>>\n>> Sorry, just heard of AsciiDoc, I will try to use it to do the right format work.\n>\n>Please make sure \"make -C Documentation\" produces sensible output\n>for *.1 (manpage) and *.html.\nOK.\n\n>\n>>>> +\tinit_revisions(&revs, NULL);\n>>>> +\trevs.max_parents = 1;\n>>>> +\tbase->object.flags |= UNINTERESTING;\n>>>> +\tadd_pending_object(&revs, &base->object, \"base\");\n>>>> +\tfor (i = 0; i < total; i++) {\n>>>> +\t\tlist[i]->object.flags |= 0;\n>>>\n>>>What does this statement do, exactly?  Are you clearing some bits\n>>>but not others, and if so which ones?\n>>\n>> My mistake, it's useless and should be removed.\n>\n>It probably make sense to do \"&= ~UNINTERESTING\" there, though.  You\n>are adding one UNINTERESTING object (i.e. the base) and adding\n>objects that are on the list[] as interesting.\nYeah, it does make sense, I'll do the change.\n\n>\n>>>This shows the patches in the order discovered by the revision\n>>>traversal, which typically is newer to older.  Is that intended?\n>>>Is it assumed that the order of the patches does not matter?\n>>\n>> The prerequisite patches should show in topological order, thus robot\n>> could parse them one by one and apply the patches in reverse order.\n>\n>If you have history where base is B, with three prerequisites 1-2-3,\n>before the patch series A-B-C, i.e.\n>\n>\tB---1---2---3---A---B---C\n>\n>if you are showing \"base-commit: B\" as the first line in the base\n>tree information block, it would be natural to expect that the\n>prerequisite patch ids are listed for 1 and then 2 and then finally\n>3, i.e.\n>\n>\tbase-commit: B\n>        prerequisite-patch-id: 1\n>        prerequisite-patch-id: 2\n>        prerequisite-patch-id: 3\n>\n>no?\nI think this sounds more sensible than what I had thought, I'll adjust \nthe showing sequence accordingly.\n\n>\n>Also I know _you_ intend to consume this by robot, but it makes me\n>wonder if with a minimum update you can make the output also more\n>useful for bystander humans.  A mailing list participant may\n>\n> - see an early round of a series that interests her,\n> - try to apply them to her tree,\n> - find that the series does not apply, but\n> - sees that a block to help identify to what tree the series is\n>   meant to apply.\n>\n>With a list of 40-hex alone, she may not be able to figure out the\n>prerequisites, but if there is some other clue that helps her to\n>identify the base commit and these patches, she may be able to\n>construct a tree that is close enough.  Maybe you can help her by\n>appending the title of the commit and patches at the end of these\n>lines?\n>\n>This is not a strong suggestion (yet); I am thinking aloud at this\n>point, without knowing how much it would help in practice to do so.\nThanks for the suggestions, actually it is what we have planned for next\nstep to make the output info more friendly for human being, I'll\nfollow up on it.\n\nThanks,\nXiaolong.\n\n>--\n>To unsubscribe from this list: send the line \"unsubscribe git\" in\n>the body of a message to majordomo@vger.kernel.org\n>More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"282687","messageId":"20160405063609.GB10110@yexl-desktop","threadId":"41881","inReplyTo":"xmqqlh4xjoub.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 3/4] format-patch: introduce --base=auto option","fromName":"Ye Xiaolong","fromEmail":"xiaolong.ye@intel.com","sentAt":"2016-04-05T06:36:09Z","receivedAt":"2016-04-05T06:36:09Z","isPatch":true,"sender":{"key":"xiaolong.ye@intel.com","avatar":"https://avatars.githubusercontent.com/u/21098480?v=4"},"body":"On Fri, Apr 01, 2016 at 09:06:20AM -0700, Junio C Hamano wrote:\n>Ye Xiaolong <xiaolong.ye@intel.com> writes:\n>\n>> On Thu, Mar 31, 2016 at 10:43:48AM -0700, Junio C Hamano wrote:\n>>>Xiaolong Ye <xiaolong.ye@intel.com> writes:\n>>>\n>>>> Introduce --base=auto to record the base commit info automatically, the base_commit\n>>>> will be the merge base of tip commit of the upstream branch and revision-range\n>>>> specified in cmdline.\n>>>\n>>>This line is probably a bit too long.\n>>\n>> How about simplifying it to \"the base_commit is the merge base of upstream and\n>> specified revision-range.\"?\n>\n>What I meant was not that profound.  I just wanted you to wrap your\n>lines a bit shorter so that quoting in the discussion thread like\n>this would not make the result overlong to fit on a 80-column\n>terminal ;-)\n\nEmm, get your point now, I'll shorten the lines to fit in 80-column.\n\n>\n>>>> +\t\t\tbase = base_list->item;\n>>>> +\t\t\tfree_commit_list(base_list);\n>>>\n>>>What should happen when there are multiple merge bases?  The code\n>>>picks one at random and ignores the remainder, if I am reading this\n>>>correctly.\n>>\n>> If there is more than one merge base, commits in base_list should\n>> be sorted by date, if I am understanding it correctly, so\n>> base_list->item should be the lastest merge base commit, it should\n>> be enough for us to used as base commit.\n>\n>By definition, when there are multiple merge bases, there is no\n>latest one among them.\n>\n>When the history involves criss-cross merges, there can be more than\n>one 'best' common ancestor for two commits.  For example, with this\n>topology (note that X is not a commit; it merely denotes crossing of\n>two lines):\n>\n>       ---1---o---A\n>      /    \\ /\n>  ---O      X\n>      \\    / \\\n>       ---2---o---o---B\n>\n>both '1' and '2' are merge-bases of 'A' and 'B'.  And the timestamps\n>on one (be it committer or author timestamp) being later than those\n>of the other do not make it any more suitable than the other one.\n>\n\nFor this criss-cross merges, as neither merge base(like 1) is better\nthan the other(both 1 and 2 are 'best' merge bases), I think it should\nbe fine to pick a random one as base commit(Or you prefer to show all of\nthem?) and I'll add this part of discusstion into documentation.\n\nThanks,\nXiaolong.\n"},{"id":"282690","messageId":"xmqqk2kc34i6.fsf@gitster.mtv.corp.google.com","threadId":"41881","inReplyTo":"20160405063609.GB10110@yexl-desktop","subject":"Re: [PATCH v3 3/4] format-patch: introduce --base=auto option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-05T07:21:21Z","receivedAt":"2016-04-05T07:21:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ye Xiaolong <xiaolong.ye@intel.com> writes:\n\n>       ---1---o---A\n>      /    \\ /\n>  ---O      X\n>      \\    / \\\n>       ---2---o---o---B\n>\n> For this criss-cross merges, as neither merge base(like 1) is better\n> than the other(both 1 and 2 are 'best' merge bases), I think it should\n> be fine to pick a random one as base commit(Or you prefer to show all of\n> them?) and I'll add this part of discusstion into documentation.\n\nI think you should error out; it would give you blatantly wrong to\npick either one at random.\n\nSuppose A is where your remote-tracking branch is, and B is where\nthe user started working on her serie.  We are sending patches built\non top of B (not depicted) with \"format-patch --base=A B..\".\n\nIf you picked '1' as the base, you'll include commits on the ===\nstretch as prerequisite patches (think of any non-merge --o-- in the\npicture to consist of multiple commits), but you won't be showing\nwhat the merge 'M' between '1' and '2' did to the tree of '1' to\narrive at the resulting tree of 'M'.\n\n       ---1---o---A\n      /    \\ /\n  ---O      X\n      \\    / \\\n       ---2---M===o===B...your.patches.here...\n\nIf you picked '2' as the base, you'll include commits on the ===\nstretch as prerequisite patches, but again you won't be showing what\nthe merge 'M' between '1' and '2' did to the tree of '2' to arrive\nat the resulting tree of 'M'..\n\n       ---1---o---A\n      /    \\ /\n  ---O      X\n      \\    / \\\n       ---2===M===o===B...your.patches.here...\n\nSo either case, you cannot rebuild the tree of B by going from the\nbase and piling on patches in such a case, because you won't have\n\"patch\" for merge 'M'.\n"},{"id":"283022","messageId":"20160409155630.GA2046@yexl-desktop","threadId":"41881","inReplyTo":"xmqqy48yo8eb.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 2/4] format-patch: add '--base' option to record base tree info","fromName":"Ye Xiaolong","fromEmail":"xiaolong.ye@intel.com","sentAt":"2016-04-09T15:56:30Z","receivedAt":"2016-04-09T15:56:30Z","isPatch":true,"sender":{"key":"xiaolong.ye@intel.com","avatar":"https://avatars.githubusercontent.com/u/21098480?v=4"},"body":"On Thu, Mar 31, 2016 at 10:38:04AM -0700, Junio C Hamano wrote:\n>> diff --git a/builtin/log.c b/builtin/log.c\n>> index 0d738d6..03cbab0 100644\n>> --- a/builtin/log.c\n>> +++ b/builtin/log.c\n>> @@ -1185,6 +1185,82 @@ static int from_callback(const struct option *opt, const char *arg, int unset)\n>>  \treturn 0;\n>>  }\n>>  \n>> +struct base_tree_info {\n>> +\tstruct object_id base_commit;\n>> +\tint nr_patch_id, alloc_patch_id;\n>> +\tstruct object_id *patch_id;\n>> +};\n>> +\n>> +static void prepare_bases(struct base_tree_info *bases,\n>> +\t\t\t  const char *base_commit,\n>> +\t\t\t  struct commit **list,\n>> +\t\t\t  int total)\n>> +{\n>> +\tstruct commit *base = NULL, *commit;\n>> +\tstruct rev_info revs;\n>> +\tstruct diff_options diffopt;\n>> +\tstruct object_id *patch_id;\n>> +\tunsigned char sha1[20];\n>> +\tint i;\n>> +\n>> +\tdiff_setup(&diffopt);\n>> +\tDIFF_OPT_SET(&diffopt, RECURSIVE);\n>> +\tdiff_setup_done(&diffopt);\n>> +\n>> +\tbase = lookup_commit_reference_by_name(base_commit);\n>> +\tif (!base)\n>> +\t\tdie(_(\"Unknown commit %s\"), base_commit);\n>> +\toidcpy(&bases->base_commit, &base->object.oid);\n>> +\n>> +\tinit_revisions(&revs, NULL);\n>> +\trevs.max_parents = 1;\n>> +\tbase->object.flags |= UNINTERESTING;\n>> +\tadd_pending_object(&revs, &base->object, \"base\");\n>> +\tfor (i = 0; i < total; i++) {\n>> +\t\tlist[i]->object.flags |= 0;\n>\n>What does this statement do, exactly?  Are you clearing some bits\n>but not others, and if so which ones?\n>\n>> +\t\tadd_pending_object(&revs, &list[i]->object, \"rev_list\");\n>> +\t\tlist[i]->util = (void *)1;\n>\n>Are we sure commit objects not on the list have their ->util cleared?\n>The while() loop below seems to rely on that to correctly filter out\n>the ones that are on the list.\n>\nAfter some investigation and according to my understanding, the commit\nobject is allocated through alloc_commit_node->alloc_node, \n\nvoid *alloc_commit_node(void)\n{\n\tstruct commit *c = alloc_node(&commit_state, sizeof(struct commit));\n\tc->object.type = OBJ_COMMIT;\n\tc->index = alloc_commit_index();\n\treturn c;\n}\n\nstatic inline void *alloc_node(struct alloc_state *s, size_t node_size)\n{\n\tvoid *ret;\n\n\tif (!s->nr) {\n\t\ts->nr = BLOCKING;\n\t\ts->p = xmalloc(BLOCKING * node_size);\n\t}\n\ts->nr--;\n\ts->count++;\n\tret = s->p;\n\ts->p = (char *)s->p + node_size;\n\tmemset(ret, 0, node_size);\n\treturn ret;\n}\n\nSo the commit->util should be cleared after initialization, and it has\nnot been touched except above \"for\" loop in our code execution path, I think\nit is safe to rely on it to filter out commits that are on the rev list.\n\nThanks,\nXiaolong.\n>> +\t}\n>> +\n>> +\tif (prepare_revision_walk(&revs))\n>> +\t\tdie(_(\"revision walk setup failed\"));\n>> +\t/*\n>> +\t * Traverse the prerequisite commits list,\n>> +\t * get the patch ids and stuff them in bases structure.\n>> +\t */\n>> +\twhile ((commit = get_revision(&revs)) != NULL) {\n>> +\t\tif (commit->util)\n>> +\t\t\tcontinue;\n>> +\t\tif (commit_patch_id(commit, &diffopt, sha1))\n>> +\t\t\tdie(_(\"cannot get patch id\"));\n>> +\t\tALLOC_GROW(bases->patch_id, bases->nr_patch_id + 1, bases->alloc_patch_id);\n>> +\t\tpatch_id = bases->patch_id + bases->nr_patch_id;\n>> +\t\thashcpy(patch_id->hash, sha1);\n>\n"}]}