{"thread":{"id":"43907","subject":"[PATCH v10 1/9] Git 2.10-rc1","startedAt":"2016-08-22T23:43:52Z","lastAt":"2016-08-26T18:27:02Z","messageCount":26,"participants":["Jacob Keller","Stefan Beller","Junio C Hamano","Keller, Jacob E"],"isPatch":true,"patchVersion":10,"patchTotal":9},"messages":[{"id":"299878","messageId":"20160822234344.22797-2-jacob.e.keller@intel.com","threadId":"43907","inReplyTo":"20160822234344.22797-1-jacob.e.keller@intel.com","subject":"[PATCH v10 1/9] Git 2.10-rc1","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2016-08-22T23:43:36Z","receivedAt":"2016-08-22T23:43:52Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Junio C Hamano \n\nSigned-off-by: Junio C Hamano \n---\n Documentation/RelNotes/2.10.0.txt | 31 +++++++++++++++++++++++++++++++\n GIT-VERSION-GEN | 2 +-\n 2 files changed, 32 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/RelNotes/2.10.0.txt b/Documentation/RelNotes/2.10.0.txt\nindex 4f7460be3914..0ef70fd9b1eb 100644\n--- a/Documentation/RelNotes/2.10.0.txt\n+++ b/Documentation/RelNotes/2.10.0.txt\n@@ -118,6 +118,15 @@ UI, Workflows & Features\n \"git branch --delete/--move [--remote]\".\n (merge 2703c22 vs/completion-branch-fully-spelled-d-m-r later to maint).\n \n+ * \"git rev-parse --git-path hooks/\" learned to take\n+ core.hooksPath configuration variable (introduced during 2.9 cycle)\n+ into account.\n+ (merge 9445b49 ab/hooks later to maint).\n+\n+ * \"git log --show-signature\" and other commands that display the\n+ verification status of PGP signature now shows the longer key-id,\n+ as 32-bit key-id is so last century.\n+\n \n Performance, Internal Implementation, Development Support etc.\n \n@@ -600,6 +609,28 @@ notes for details).\n arises).\n (merge c2cafd3 js/test-lint-pathname later to maint).\n \n+ * When \"git merge-recursive\" works on history with many criss-cross\n+ merges in \"verbose\" mode, the names the command assigns to the\n+ virtual merge bases could have overwritten each other by unintended\n+ reuse of the same piece of memory.\n+ (merge 5447a76 rs/pull-signed-tag later to maint).\n+\n+ * \"git checkout --detach \" used to give the same advice\n+ message as that is issued when \"git checkout \" (or anything\n+ that is not a branch name) is given, but asking with \"--detach\" is\n+ an explicit enough sign that the user knows what is going on. The\n+ advice message has been squelched in this case.\n+ (merge 779b88a sb/checkout-explit-detach-no-advice later to maint).\n+\n+ * \"git difftool\" by default ignores the error exit from the backend\n+ commands it spawns, because often they signal that they found\n+ differences by exiting with a non-zero status code just like \"diff\"\n+ does; the exit status codes 126 and above however are special in\n+ that they are used to signal that the command is not executable,\n+ does not exist, or killed by a signal. \"git difftool\" has been\n+ taught to notice these exit status codes.\n+ (merge 45a4f5d jk/difftool-command-not-found later to maint).\n+\n * Other minor clean-ups and documentation updates\n (merge 02a8cfa rs/merge-add-strategies-simplification later to maint).\n (merge af4941d rs/merge-recursive-string-list-init later to maint).\ndiff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN\nindex eea85c340454..702c067a78c0 100755\n--- a/GIT-VERSION-GEN\n+++ b/GIT-VERSION-GEN\n@@ -1,7 +1,7 @@\n #!/bin/sh\n \n GVF=GIT-VERSION-FILE\n-DEF_VER=v2.10.0-rc0\n+DEF_VER=v2.10.0-rc1\n \n LF='\n '\n-- \n2.10.0.rc0.259.g83512d9\n\n"},{"id":"299879","messageId":"20160822234344.22797-8-jacob.e.keller@intel.com","threadId":"43907","inReplyTo":"20160822234344.22797-1-jacob.e.keller@intel.com","subject":"[PATCH v10 7/9] submodule: convert show_submodule_summary to use struct object_id *","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2016-08-22T23:43:42Z","receivedAt":"2016-08-22T23:43:57Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nSince we're going to be changing this function in a future patch, lets\ngo ahead and convert this to use object_id now.\n\nSigned-off-by: Jacob Keller \n---\n diff.c | 2 +-\n submodule.c | 16 ++++++++--------\n submodule.h | 2 +-\n 3 files changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex d6b321da3d1d..16253b191f53 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2307,7 +2307,7 @@ static void builtin_diff(const char *name_a,\n \t\tconst char *add = diff_get_color_opt(o, DIFF_FILE_NEW);\n \t\tshow_submodule_summary(o->file, one->path ? one->path : two->path,\n \t\t\t\tline_prefix,\n-\t\t\t\tone->oid.hash, two->oid.hash,\n+\t\t\t\t&one->oid, &two->oid,\n \t\t\t\ttwo->dirty_submodule,\n \t\t\t\tmeta, del, add, reset);\n \t\treturn;\ndiff --git a/submodule.c b/submodule.c\nindex e1a51b7506ff..422353ccf6cc 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -335,7 +335,7 @@ static void print_submodule_summary(struct rev_info *rev, FILE *f,\n \n void show_submodule_summary(FILE *f, const char *path,\n \t\tconst char *line_prefix,\n-\t\tunsigned char one[20], unsigned char two[20],\n+\t\tstruct object_id *one, struct object_id *two,\n \t\tunsigned dirty_submodule, const char *meta,\n \t\tconst char *del, const char *add, const char *reset)\n {\n@@ -345,14 +345,14 @@ void show_submodule_summary(FILE *f, const char *path,\n \tstruct strbuf sb = STRBUF_INIT;\n \tint fast_forward = 0, fast_backward = 0;\n \n-\tif (is_null_sha1(two))\n+\tif (is_null_oid(two))\n \t\tmessage = \"(submodule deleted)\";\n \telse if (add_submodule_odb(path))\n \t\tmessage = \"(not initialized)\";\n-\telse if (is_null_sha1(one))\n+\telse if (is_null_oid(one))\n \t\tmessage = \"(new submodule)\";\n-\telse if (!(left = lookup_commit_reference(one)) ||\n-\t\t !(right = lookup_commit_reference(two)))\n+\telse if (!(left = lookup_commit_reference(one->hash)) ||\n+\t\t !(right = lookup_commit_reference(two->hash)))\n \t\tmessage = \"(commits not present)\";\n \telse if (prepare_submodule_summary(&rev, path, left, right,\n \t\t\t\t\t &fast_forward, &fast_backward))\n@@ -365,16 +365,16 @@ void show_submodule_summary(FILE *f, const char *path,\n \t\tfprintf(f, \"%sSubmodule %s contains modified content\\n\",\n \t\t\tline_prefix, path);\n \n-\tif (!hashcmp(one, two)) {\n+\tif (!oidcmp(one, two)) {\n \t\tstrbuf_release(&sb);\n \t\treturn;\n \t}\n \n \tstrbuf_addf(&sb, \"%s%sSubmodule %s %s..\", line_prefix, meta, path,\n-\t\t\tfind_unique_abbrev(one, DEFAULT_ABBREV));\n+\t\t\tfind_unique_abbrev(one->hash, DEFAULT_ABBREV));\n \tif (!fast_backward && !fast_forward)\n \t\tstrbuf_addch(&sb, '.');\n-\tstrbuf_addf(&sb, \"%s\", find_unique_abbrev(two, DEFAULT_ABBREV));\n+\tstrbuf_addf(&sb, \"%s\", find_unique_abbrev(two->hash, DEFAULT_ABBREV));\n \tif (message)\n \t\tstrbuf_addf(&sb, \" %s%s\\n\", message, reset);\n \telse\ndiff --git a/submodule.h b/submodule.h\nindex 2af939099819..d83df57e24ff 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -43,7 +43,7 @@ const char *submodule_strategy_to_string(const struct submodule_update_strategy\n void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);\n void show_submodule_summary(FILE *f, const char *path,\n \t\tconst char *line_prefix,\n-\t\tunsigned char one[20], unsigned char two[20],\n+\t\tstruct object_id *one, struct object_id *two,\n \t\tunsigned dirty_submodule, const char *meta,\n \t\tconst char *del, const char *add, const char *reset);\n void set_config_fetch_recurse_submodules(int value);\n-- \n2.10.0.rc0.259.g83512d9\n\n"},{"id":"299880","messageId":"20160822234344.22797-4-jacob.e.keller@intel.com","threadId":"43907","inReplyTo":"20160822234344.22797-1-jacob.e.keller@intel.com","subject":"[PATCH v10 3/9] diff.c: remove output_prefix_length field","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2016-08-22T23:43:38Z","receivedAt":"2016-08-22T23:44:06Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Junio C Hamano \n\n\"diff/log --stat\" has a logic that determines the display columns\navailable for the diffstat part of the output and apportions it for\npathnames and diffstat graph automatically.\n\n5e71a84a (Add output_prefix_length to diff_options, 2012-04-16)\nadded the output_prefix_length field to diff_options structure to\nallow this logic to subtract the display columns used for the\nhistory graph part from the total \"terminal width\"; this matters\nwhen the \"git log --graph -p\" option is in use.\n\nThe field must be set to the number of display columns needed to\nshow the output from the output_prefix() callback, which is error\nprone. As there is only one user of the field, and the user has the\nactual value of the prefix string, let's get rid of the field and\nhave the user count the display width itself.\n\nSigned-off-by: Junio C Hamano \n---\n diff.c | 2 +-\n diff.h | 1 -\n graph.c | 2 --\n 3 files changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 534c12e28ea8..50bef1f07658 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1625,7 +1625,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t */\n \n \tif (options->stat_width == -1)\n-\t\twidth = term_columns() - options->output_prefix_length;\n+\t\twidth = term_columns() - strlen(line_prefix);\n \telse\n \t\twidth = options->stat_width ? options->stat_width : 80;\n \tnumber_width = decimal_width(max_change) > number_width ?\ndiff --git a/diff.h b/diff.h\nindex 7883729edf10..747a204d75a4 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -174,7 +174,6 @@ struct diff_options {\n \tdiff_format_fn_t format_callback;\n \tvoid *format_callback_data;\n \tdiff_prefix_fn_t output_prefix;\n-\tint output_prefix_length;\n \tvoid *output_prefix_data;\n \n \tint diff_path_counter;\ndiff --git a/graph.c b/graph.c\nindex dd1720148dc5..a46803840511 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -197,7 +197,6 @@ static struct strbuf *diff_output_prefix_callback(struct diff_options *opt, void\n \tassert(opt);\n \tassert(graph);\n \n-\topt->output_prefix_length = graph->width;\n \tstrbuf_reset(&msgbuf);\n \tgraph_padding_line(graph, &msgbuf);\n \treturn &msgbuf;\n@@ -245,7 +244,6 @@ struct git_graph *graph_init(struct rev_info *opt)\n \t */\n \topt->diffopt.output_prefix = diff_output_prefix_callback;\n \topt->diffopt.output_prefix_data = graph;\n-\topt->diffopt.output_prefix_length = 0;\n \n \treturn graph;\n }\n-- \n2.10.0.rc0.259.g83512d9\n\n"},{"id":"299881","messageId":"20160822234344.22797-7-jacob.e.keller@intel.com","threadId":"43907","inReplyTo":"20160822234344.22797-1-jacob.e.keller@intel.com","subject":"[PATCH v10 6/9] allow do_submodule_path to work even if submodule isn't checked out","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2016-08-22T23:43:41Z","receivedAt":"2016-08-22T23:44:09Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nCurrently, do_submodule_path will attempt locating the .git directory by\nusing read_gitfile on /.git. If this fails it just assumes the\n/.git is actually a git directory.\n\nThis is good because it allows for handling submodules which were cloned\nin a regular manner first before being added to the parent project.\n\nUnfortunately this fails if the is not actually checked out any\nlonger, such as by removing the directory.\n\nFix this by checking if the directory we found is actually a gitdir. In\nthe case it is not, attempt to lookup the submodule configuration and\nfind the name of where it is stored in the .git/modules/ folder of the\nparent project.\n\nIf we can't find this, just leave it alone and let the caller of\ndo_submodule_path handle a missing directory normally.\n\nIt might be worth adding a die() call here, but that seems a bit\noverkill.\n\nBecause this change fixes add_submodule_odb to work even if the\nsubmodule is not checked out, update the wording of the submodule log\ndiff format to correctly display that the submodule is \"not initialized\"\ninstead of \"not checked out\"\n\nAdd tests to ensure this change works as expected.\n\nSigned-off-by: Jacob Keller \n---\n path.c | 11 +++\n submodule.c | 2 +-\n t/t4059-diff-submodule-not-initialized.sh | 127 ++++++++++++++++++++++++++++++\n 3 files changed, 139 insertions(+), 1 deletion(-)\n create mode 100755 t/t4059-diff-submodule-not-initialized.sh\n\ndiff --git a/path.c b/path.c\nindex fe3c4d96c6d8..07dd0f62eb82 100644\n--- a/path.c\n+++ b/path.c\n@@ -6,6 +6,7 @@\n #include \"string-list.h\"\n #include \"dir.h\"\n #include \"worktree.h\"\n+#include \"submodule-config.h\"\n \n static int get_st_mode_bits(const char *path, int *mode)\n {\n@@ -474,6 +475,7 @@ static void do_submodule_path(struct strbuf *buf, const char *path,\n \tconst char *git_dir;\n \tstruct strbuf git_submodule_common_dir = STRBUF_INIT;\n \tstruct strbuf git_submodule_dir = STRBUF_INIT;\n+\tconst struct submodule *sub;\n \n \tstrbuf_addstr(buf, path);\n \tstrbuf_complete(buf, '/');\n@@ -484,6 +486,15 @@ static void do_submodule_path(struct strbuf *buf, const char *path,\n \t\tstrbuf_reset(buf);\n \t\tstrbuf_addstr(buf, git_dir);\n \t}\n+\tif (!is_git_directory(buf->buf)) {\n+\t\tsub = submodule_from_path(null_sha1, path);\n+\t\tif (sub) {\n+\t\t\tstrbuf_reset(buf);\n+\t\t\tstrbuf_git_path(buf, \"%s/%s\", \"modules\",\n+\t\t\t\t\tsub->name);\n+\t\t}\n+\t}\n+\n \tstrbuf_addch(buf, '/');\n \tstrbuf_addbuf(&git_submodule_dir, buf);\n \ndiff --git a/submodule.c b/submodule.c\nindex 1b5cdfb7e784..e1a51b7506ff 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -348,7 +348,7 @@ void show_submodule_summary(FILE *f, const char *path,\n \tif (is_null_sha1(two))\n \t\tmessage = \"(submodule deleted)\";\n \telse if (add_submodule_odb(path))\n-\t\tmessage = \"(not checked out)\";\n+\t\tmessage = \"(not initialized)\";\n \telse if (is_null_sha1(one))\n \t\tmessage = \"(new submodule)\";\n \telse if (!(left = lookup_commit_reference(one)) ||\ndiff --git a/t/t4059-diff-submodule-not-initialized.sh b/t/t4059-diff-submodule-not-initialized.sh\nnew file mode 100755\nindex 000000000000..c8775854d3c2\n--- /dev/null\n+++ b/t/t4059-diff-submodule-not-initialized.sh\n@@ -0,0 +1,127 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2016 Jacob Keller, based on t4041 by Jens Lehmann\n+#\n+\n+test_description='Test for submodule diff on non-checked out submodule\n+\n+This test tries to verify that add_submodule_odb works when the submodule was\n+initialized previously but the checkout has since been removed.\n+'\n+\n+. ./test-lib.sh\n+\n+# Tested non-UTF-8 encoding\n+test_encoding=\"ISO8859-1\"\n+\n+# String \"added\" in German (translated with Google Translate), encoded in UTF-8,\n+# used in sample commit log messages in add_file() function below.\n+added=$(printf \"hinzugef\\303\\274gt\")\n+\n+add_file () {\n+\t(\n+\t\tcd \"$1\" &&\n+\t\tshift &&\n+\t\tfor name\n+\t\tdo\n+\t\t\techo \"$name\" >\"$name\" &&\n+\t\t\tgit add \"$name\" &&\n+\t\t\ttest_tick &&\n+\t\t\t# \"git commit -m\" would break MinGW, as Windows refuse to pass\n+\t\t\t# $test_encoding encoded parameter to git.\n+\t\t\techo \"Add $name ($added $name)\" | iconv -f utf-8 -t $test_encoding |\n+\t\t\tgit -c \"i18n.commitEncoding=$test_encoding\" commit -F -\n+\t\tdone >/dev/null &&\n+\t\tgit rev-parse --short --verify HEAD\n+\t)\n+}\n+\n+commit_file () {\n+\ttest_tick &&\n+\tgit commit \"$@\" -m \"Commit $*\" >/dev/null\n+}\n+\n+test_expect_success 'setup - submodules' '\n+\ttest_create_repo sm2 &&\n+\tadd_file . foo &&\n+\tadd_file sm2 foo1 foo2 &&\n+\tsmhead1=$(git -C sm2 rev-parse --short --verify HEAD)\n+'\n+\n+test_expect_success 'setup - git submodule add' '\n+\tgit submodule add ./sm2 sm1 &&\n+\tcommit_file sm1 &&\n+\tgit diff-tree -p --no-commit-id --submodule=log HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 0000000...$smhead1 (new submodule)\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'submodule directory removed' '\n+\trm -rf sm1 &&\n+\tgit diff-tree -p --no-commit-id --submodule=log HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 0000000...$smhead1 (new submodule)\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'setup - submodule multiple commits' '\n+\tgit submodule update --checkout sm1 &&\n+\tsmhead2=$(add_file sm1 foo3 foo4) &&\n+\tcommit_file sm1 &&\n+\tgit diff-tree -p --no-commit-id --submodule=log HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 $smhead1..$smhead2:\n+\t > Add foo4 ($added foo4)\n+\t > Add foo3 ($added foo3)\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'submodule removed multiple commits' '\n+\trm -rf sm1 &&\n+\tgit diff-tree -p --no-commit-id --submodule=log HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 $smhead1..$smhead2:\n+\t > Add foo4 ($added foo4)\n+\t > Add foo3 ($added foo3)\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'submodule not initialized in new clone' '\n+\tgit clone . sm3 &&\n+\tgit -C sm3 diff-tree -p --no-commit-id --submodule=log HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 $smhead1...$smhead2 (not initialized)\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'setup submodule moved' '\n+\tgit submodule update --checkout sm1 &&\n+\tgit mv sm1 sm4 &&\n+\tcommit_file sm4 &&\n+\tgit diff-tree -p --no-commit-id --submodule=log HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm4 0000000...$smhead2 (new submodule)\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'submodule moved then removed' '\n+\tsmhead3=$(add_file sm4 foo6 foo7) &&\n+\tcommit_file sm4 &&\n+\trm -rf sm4 &&\n+\tgit diff-tree -p --no-commit-id --submodule=log HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm4 $smhead2..$smhead3:\n+\t > Add foo7 ($added foo7)\n+\t > Add foo6 ($added foo6)\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_done\n-- \n2.10.0.rc0.259.g83512d9\n\n"},{"id":"299882","messageId":"20160822234344.22797-5-jacob.e.keller@intel.com","threadId":"43907","inReplyTo":"20160822234344.22797-1-jacob.e.keller@intel.com","subject":"[PATCH v10 4/9] graph: add support for --line-prefix on all graph-aware output","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2016-08-22T23:43:39Z","receivedAt":"2016-08-22T23:44:11Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nAdd an extension to git-diff and git-log (and any other graph-aware\ndisplayable output) such that \"--line-prefix=\" will print the\nadditional line-prefix on every line of output.\n\nTo make this work, we have to fix a few bugs in the graph API that force\ngraph_show_commit_msg to be used only when you have a valid graph.\nAdditionally, we extend the default_diff_output_prefix handler to work\neven when no graph is enabled.\n\nThis is somewhat of a hack on top of the graph API, but I think it\nshould be acceptable here.\n\nThis will be used by a future extension of submodule display which\ndisplays the submodule diff as the actual diff between the pre and post\ncommit in the submodule project.\n\nAdd some tests for both git-log and git-diff to ensure that the prefix\nis honored correctly.\n\nSigned-off-by: Jacob Keller \n---\n Documentation/diff-options.txt | 3 +\n builtin/rev-list.c | 70 ++---\n diff.c | 7 +\n diff.h | 2 +\n graph.c | 98 ++++---\n graph.h | 22 +-\n log-tree.c | 5 +-\n t/t4013-diff-various.sh | 6 +\n ...diff.diff_--line-prefix=abc_master_master^_side | 29 ++\n t/t4013/diff.diff_--line-prefix_--cached_--_file0 | 15 +\n t/t4202-log.sh | 323 +++++++++++++++++++++\n 11 files changed, 502 insertions(+), 78 deletions(-)\n create mode 100644 t/t4013/diff.diff_--line-prefix=abc_master_master^_side\n create mode 100644 t/t4013/diff.diff_--line-prefix_--cached_--_file0\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 705a87394200..cc4342e2034d 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -569,5 +569,8 @@ endif::git-format-patch[]\n --no-prefix::\n \tDo not show any source or destination prefix.\n \n+--line-prefix=::\n+\tPrepend an additional prefix to every line of output.\n+\n For more detailed explanation on these common options, see also\n linkgit:gitdiffcore[7].\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 0ba82b1635b6..8479f6ed28aa 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -122,48 +122,40 @@ static void show_commit(struct commit *commit, void *data)\n \t\tctx.fmt = revs->commit_format;\n \t\tctx.output_encoding = get_log_output_encoding();\n \t\tpretty_print_commit(&ctx, commit, &buf);\n-\t\tif (revs->graph) {\n-\t\t\tif (buf.len) {\n-\t\t\t\tif (revs->commit_format != CMIT_FMT_ONELINE)\n-\t\t\t\t\tgraph_show_oneline(revs->graph);\n+\t\tif (buf.len) {\n+\t\t\tif (revs->commit_format != CMIT_FMT_ONELINE)\n+\t\t\t\tgraph_show_oneline(revs->graph);\n \n-\t\t\t\tgraph_show_commit_msg(revs->graph, &buf);\n+\t\t\tgraph_show_commit_msg(revs->graph, stdout, &buf);\n \n-\t\t\t\t/*\n-\t\t\t\t * Add a newline after the commit message.\n-\t\t\t\t *\n-\t\t\t\t * Usually, this newline produces a blank\n-\t\t\t\t * padding line between entries, in which case\n-\t\t\t\t * we need to add graph padding on this line.\n-\t\t\t\t *\n-\t\t\t\t * However, the commit message may not end in a\n-\t\t\t\t * newline. In this case the newline simply\n-\t\t\t\t * ends the last line of the commit message,\n-\t\t\t\t * and we don't need any graph output. (This\n-\t\t\t\t * always happens with CMIT_FMT_ONELINE, and it\n-\t\t\t\t * happens with CMIT_FMT_USERFORMAT when the\n-\t\t\t\t * format doesn't explicitly end in a newline.)\n-\t\t\t\t */\n-\t\t\t\tif (buf.len && buf.buf[buf.len - 1] == '\\n')\n-\t\t\t\t\tgraph_show_padding(revs->graph);\n-\t\t\t\tputchar('\\n');\n-\t\t\t} else {\n-\t\t\t\t/*\n-\t\t\t\t * If the message buffer is empty, just show\n-\t\t\t\t * the rest of the graph output for this\n-\t\t\t\t * commit.\n-\t\t\t\t */\n-\t\t\t\tif (graph_show_remainder(revs->graph))\n-\t\t\t\t\tputchar('\\n');\n-\t\t\t\tif (revs->commit_format == CMIT_FMT_ONELINE)\n-\t\t\t\t\tputchar('\\n');\n-\t\t\t}\n+\t\t\t/*\n+\t\t\t * Add a newline after the commit message.\n+\t\t\t *\n+\t\t\t * Usually, this newline produces a blank\n+\t\t\t * padding line between entries, in which case\n+\t\t\t * we need to add graph padding on this line.\n+\t\t\t *\n+\t\t\t * However, the commit message may not end in a\n+\t\t\t * newline. In this case the newline simply\n+\t\t\t * ends the last line of the commit message,\n+\t\t\t * and we don't need any graph output. (This\n+\t\t\t * always happens with CMIT_FMT_ONELINE, and it\n+\t\t\t * happens with CMIT_FMT_USERFORMAT when the\n+\t\t\t * format doesn't explicitly end in a newline.)\n+\t\t\t */\n+\t\t\tif (buf.len && buf.buf[buf.len - 1] == '\\n')\n+\t\t\t\tgraph_show_padding(revs->graph);\n+\t\t\tputchar('\\n');\n \t\t} else {\n-\t\t\tif (revs->commit_format != CMIT_FMT_USERFORMAT ||\n-\t\t\t buf.len) {\n-\t\t\t\tfwrite(buf.buf, 1, buf.len, stdout);\n-\t\t\t\tputchar(info->hdr_termination);\n-\t\t\t}\n+\t\t\t/*\n+\t\t\t * If the message buffer is empty, just show\n+\t\t\t * the rest of the graph output for this\n+\t\t\t * commit.\n+\t\t\t */\n+\t\t\tif (graph_show_remainder(revs->graph))\n+\t\t\t\tputchar('\\n');\n+\t\t\tif (revs->commit_format == CMIT_FMT_ONELINE)\n+\t\t\t\tputchar('\\n');\n \t\t}\n \t\tstrbuf_release(&buf);\n \t} else {\ndiff --git a/diff.c b/diff.c\nindex 50bef1f07658..e57cf39ad109 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -18,6 +18,7 @@\n #include \"ll-merge.h\"\n #include \"string-list.h\"\n #include \"argv-array.h\"\n+#include \"graph.h\"\n \n #ifdef NO_FAST_WORKING_DIRECTORY\n #define FAST_WORKING_DIRECTORY 0\n@@ -3966,6 +3967,12 @@ int diff_opt_parse(struct diff_options *options,\n \t\toptions->a_prefix = optarg;\n \t\treturn argcount;\n \t}\n+\telse if ((argcount = parse_long_opt(\"line-prefix\", av, &optarg))) {\n+\t\toptions->line_prefix = optarg;\n+\t\toptions->line_prefix_length = strlen(options->line_prefix);\n+\t\tgraph_setup_line_prefix(options);\n+\t\treturn argcount;\n+\t}\n \telse if ((argcount = parse_long_opt(\"dst-prefix\", av, &optarg))) {\n \t\toptions->b_prefix = optarg;\n \t\treturn argcount;\ndiff --git a/diff.h b/diff.h\nindex 747a204d75a4..1f57aad25c71 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -115,6 +115,8 @@ struct diff_options {\n \tconst char *pickaxe;\n \tconst char *single_follow;\n \tconst char *a_prefix, *b_prefix;\n+\tconst char *line_prefix;\n+\tsize_t line_prefix_length;\n \tunsigned flags;\n \tunsigned touched_flags;\n \ndiff --git a/graph.c b/graph.c\nindex a46803840511..06f1139f2e20 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -2,7 +2,6 @@\n #include \"commit.h\"\n #include \"color.h\"\n #include \"graph.h\"\n-#include \"diff.h\"\n #include \"revision.h\"\n \n /* Internal API */\n@@ -28,8 +27,15 @@ static void graph_padding_line(struct git_graph *graph, struct strbuf *sb);\n * responsible for printing this line's graph (perhaps via\n * graph_show_commit() or graph_show_oneline()) before calling\n * graph_show_strbuf().\n+ *\n+ * Note that unlike some other graph display functions, you must pass the file\n+ * handle directly. It is assumed that this is the same file handle as the\n+ * file specified by the graph diff options. This is necessary so that\n+ * graph_show_strbuf can be called even with a NULL graph.\n */\n-static void graph_show_strbuf(struct git_graph *graph, struct strbuf const *sb);\n+static void graph_show_strbuf(struct git_graph *graph,\n+\t\t\t FILE *file,\n+\t\t\t struct strbuf const *sb);\n \n /*\n * TODO:\n@@ -59,6 +65,17 @@ enum graph_state {\n \tGRAPH_COLLAPSING\n };\n \n+static void graph_show_line_prefix(const struct diff_options *diffopt)\n+{\n+\tif (!diffopt || !diffopt->line_prefix)\n+\t\treturn;\n+\n+\tfwrite(diffopt->line_prefix,\n+\t sizeof(char),\n+\t diffopt->line_prefix_length,\n+\t diffopt->file);\n+}\n+\n static const char **column_colors;\n static unsigned short column_colors_max;\n \n@@ -195,13 +212,28 @@ static struct strbuf *diff_output_prefix_callback(struct diff_options *opt, void\n \tstatic struct strbuf msgbuf = STRBUF_INIT;\n \n \tassert(opt);\n-\tassert(graph);\n \n \tstrbuf_reset(&msgbuf);\n-\tgraph_padding_line(graph, &msgbuf);\n+\tif (opt->line_prefix)\n+\t\tstrbuf_add(&msgbuf, opt->line_prefix,\n+\t\t\t opt->line_prefix_length);\n+\tif (graph)\n+\t\tgraph_padding_line(graph, &msgbuf);\n \treturn &msgbuf;\n }\n \n+static const struct diff_options *default_diffopt;\n+\n+void graph_setup_line_prefix(struct diff_options *diffopt)\n+{\n+\tdefault_diffopt = diffopt;\n+\n+\t/* setup an output prefix callback if necessary */\n+\tif (diffopt && !diffopt->output_prefix)\n+\t\tdiffopt->output_prefix = diff_output_prefix_callback;\n+}\n+\n+\n struct git_graph *graph_init(struct rev_info *opt)\n {\n \tstruct git_graph *graph = xmalloc(sizeof(struct git_graph));\n@@ -1183,6 +1215,8 @@ void graph_show_commit(struct git_graph *graph)\n \tstruct strbuf msgbuf = STRBUF_INIT;\n \tint shown_commit_line = 0;\n \n+\tgraph_show_line_prefix(default_diffopt);\n+\n \tif (!graph)\n \t\treturn;\n \n@@ -1200,8 +1234,10 @@ void graph_show_commit(struct git_graph *graph)\n \t\tshown_commit_line = graph_next_line(graph, &msgbuf);\n \t\tfwrite(msgbuf.buf, sizeof(char), msgbuf.len,\n \t\t\tgraph->revs->diffopt.file);\n-\t\tif (!shown_commit_line)\n+\t\tif (!shown_commit_line) {\n \t\t\tputc('\\n', graph->revs->diffopt.file);\n+\t\t\tgraph_show_line_prefix(&graph->revs->diffopt);\n+\t\t}\n \t\tstrbuf_setlen(&msgbuf, 0);\n \t}\n \n@@ -1212,6 +1248,8 @@ void graph_show_oneline(struct git_graph *graph)\n {\n \tstruct strbuf msgbuf = STRBUF_INIT;\n \n+\tgraph_show_line_prefix(default_diffopt);\n+\n \tif (!graph)\n \t\treturn;\n \n@@ -1224,6 +1262,8 @@ void graph_show_padding(struct git_graph *graph)\n {\n \tstruct strbuf msgbuf = STRBUF_INIT;\n \n+\tgraph_show_line_prefix(default_diffopt);\n+\n \tif (!graph)\n \t\treturn;\n \n@@ -1237,6 +1277,8 @@ int graph_show_remainder(struct git_graph *graph)\n \tstruct strbuf msgbuf = STRBUF_INIT;\n \tint shown = 0;\n \n+\tgraph_show_line_prefix(default_diffopt);\n+\n \tif (!graph)\n \t\treturn 0;\n \n@@ -1250,27 +1292,24 @@ int graph_show_remainder(struct git_graph *graph)\n \t\tstrbuf_setlen(&msgbuf, 0);\n \t\tshown = 1;\n \n-\t\tif (!graph_is_commit_finished(graph))\n+\t\tif (!graph_is_commit_finished(graph)) {\n \t\t\tputc('\\n', graph->revs->diffopt.file);\n-\t\telse\n+\t\t\tgraph_show_line_prefix(&graph->revs->diffopt);\n+\t\t} else {\n \t\t\tbreak;\n+\t\t}\n \t}\n \tstrbuf_release(&msgbuf);\n \n \treturn shown;\n }\n \n-\n-static void graph_show_strbuf(struct git_graph *graph, struct strbuf const *sb)\n+static void graph_show_strbuf(struct git_graph *graph,\n+\t\t\t FILE *file,\n+\t\t\t struct strbuf const *sb)\n {\n \tchar *p;\n \n-\tif (!graph) {\n-\t\tfwrite(sb->buf, sizeof(char), sb->len,\n-\t\t\tgraph->revs->diffopt.file);\n-\t\treturn;\n-\t}\n-\n \t/*\n \t * Print the strbuf line by line,\n \t * and display the graph info before each line but the first.\n@@ -1285,7 +1324,7 @@ static void graph_show_strbuf(struct git_graph *graph, struct strbuf const *sb)\n \t\t} else {\n \t\t\tlen = (sb->buf + sb->len) - p;\n \t\t}\n-\t\tfwrite(p, sizeof(char), len, graph->revs->diffopt.file);\n+\t\tfwrite(p, sizeof(char), len, file);\n \t\tif (next_p && *next_p != '\\0')\n \t\t\tgraph_show_oneline(graph);\n \t\tp = next_p;\n@@ -1293,29 +1332,20 @@ static void graph_show_strbuf(struct git_graph *graph, struct strbuf const *sb)\n }\n \n void graph_show_commit_msg(struct git_graph *graph,\n+\t\t\t FILE *file,\n \t\t\t struct strbuf const *sb)\n {\n \tint newline_terminated;\n \n-\tif (!graph) {\n-\t\t/*\n-\t\t * If there's no graph, just print the message buffer.\n-\t\t *\n-\t\t * The message buffer for CMIT_FMT_ONELINE and\n-\t\t * CMIT_FMT_USERFORMAT are already missing a terminating\n-\t\t * newline. All of the other formats should have it.\n-\t\t */\n-\t\tfwrite(sb->buf, sizeof(char), sb->len,\n-\t\t\tgraph->revs->diffopt.file);\n-\t\treturn;\n-\t}\n-\n-\tnewline_terminated = (sb->len && sb->buf[sb->len - 1] == '\\n');\n-\n \t/*\n \t * Show the commit message\n \t */\n-\tgraph_show_strbuf(graph, sb);\n+\tgraph_show_strbuf(graph, file, sb);\n+\n+\tif (!graph)\n+\t\treturn;\n+\n+\tnewline_terminated = (sb->len && sb->buf[sb->len - 1] == '\\n');\n \n \t/*\n \t * If there is more output needed for this commit, show it now\n@@ -1327,7 +1357,7 @@ void graph_show_commit_msg(struct git_graph *graph,\n \t\t * new line.\n \t\t */\n \t\tif (!newline_terminated)\n-\t\t\tputc('\\n', graph->revs->diffopt.file);\n+\t\t\tputc('\\n', file);\n \n \t\tgraph_show_remainder(graph);\n \n@@ -1335,6 +1365,6 @@ void graph_show_commit_msg(struct git_graph *graph,\n \t\t * If sb ends with a newline, our output should too.\n \t\t */\n \t\tif (newline_terminated)\n-\t\t\tputc('\\n', graph->revs->diffopt.file);\n+\t\t\tputc('\\n', file);\n \t}\n }\ndiff --git a/graph.h b/graph.h\nindex 3f48c19b6208..af623390b605 100644\n--- a/graph.h\n+++ b/graph.h\n@@ -1,9 +1,22 @@\n #ifndef GRAPH_H\n #define GRAPH_H\n+#include \"diff.h\"\n \n /* A graph is a pointer to this opaque structure */\n struct git_graph;\n \n+/*\n+ * Called to setup global display of line_prefix diff option.\n+ *\n+ * Passed a diff_options structure which indicates the line_prefix and the\n+ * file to output the prefix to. This is sort of a hack used so that the\n+ * line_prefix will be honored by all flows which also honor \"--graph\"\n+ * regardless of whether a graph has actually been setup. The normal graph\n+ * flow will honor the exact diff_options passed, but a NULL graph will cause\n+ * display of a line_prefix to stdout.\n+ */\n+void graph_setup_line_prefix(struct diff_options *diffopt);\n+\n /*\n * Set up a custom scheme for column colors.\n *\n@@ -113,7 +126,14 @@ int graph_show_remainder(struct git_graph *graph);\n * missing a terminating newline (including if it is empty), the output\n * printed by graph_show_commit_msg() will also be missing a terminating\n * newline.\n+ *\n+ * Note that unlike some other graph display functions, you must pass the file\n+ * handle directly. It is assumed that this is the same file handle as the\n+ * file specified by the graph diff options. This is necessary so that\n+ * graph_show_commit_msg can be called even with a NULL graph.\n */\n-void graph_show_commit_msg(struct git_graph *graph, struct strbuf const *sb);\n+void graph_show_commit_msg(struct git_graph *graph,\n+\t\t\t FILE *file,\n+\t\t\t struct strbuf const *sb);\n \n #endif /* GRAPH_H */\ndiff --git a/log-tree.c b/log-tree.c\nindex bfb735c84556..8c2415747a26 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -715,10 +715,7 @@ void show_log(struct rev_info *opt)\n \telse\n \t\topt->missing_newline = 0;\n \n-\tif (opt->graph)\n-\t\tgraph_show_commit_msg(opt->graph, &msgbuf);\n-\telse\n-\t\tfwrite(msgbuf.buf, sizeof(char), msgbuf.len, opt->diffopt.file);\n+\tgraph_show_commit_msg(opt->graph, opt->diffopt.file, &msgbuf);\n \tif (opt->use_terminator && !commit_format_is_empty(opt->commit_format)) {\n \t\tif (!opt->missing_newline)\n \t\t\tgraph_show_padding(opt->graph);\ndiff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\nindex 94ef5000e787..566817e2efdc 100755\n--- a/t/t4013-diff-various.sh\n+++ b/t/t4013-diff-various.sh\n@@ -306,6 +306,8 @@ diff --no-index --name-status dir2 dir\n diff --no-index --name-status -- dir2 dir\n diff --no-index dir dir3\n diff master master^ side\n+# Can't use spaces...\n+diff --line-prefix=abc master master^ side\n diff --dirstat master~1 master~2\n diff --dirstat initial rearrange\n diff --dirstat-by-file initial rearrange\n@@ -325,6 +327,10 @@ test_expect_success 'diff --cached -- file on unborn branch' '\n \tgit diff --cached -- file0 >result &&\n \ttest_cmp \"$TEST_DIRECTORY/t4013/diff.diff_--cached_--_file0\" result\n '\n+test_expect_success 'diff --line-prefix with spaces' '\n+\tgit diff --line-prefix=\"| | | \" --cached -- file0 >result &&\n+\ttest_cmp \"$TEST_DIRECTORY/t4013/diff.diff_--line-prefix_--cached_--_file0\" result\n+'\n \n test_expect_success 'diff-tree --stdin with log formatting' '\n \tcat >expect <<-\\EOF &&\ndiff --git a/t/t4013/diff.diff_--line-prefix=abc_master_master^_side b/t/t4013/diff.diff_--line-prefix=abc_master_master^_side\nnew file mode 100644\nindex 000000000000..99f91e7f0e32\n--- /dev/null\n+++ b/t/t4013/diff.diff_--line-prefix=abc_master_master^_side\n@@ -0,0 +1,29 @@\n+$ git diff --line-prefix=abc master master^ side\n+abcdiff --cc dir/sub\n+abcindex cead32e,7289e35..992913c\n+abc--- a/dir/sub\n+abc+++ b/dir/sub\n+abc@@@ -1,6 -1,4 +1,8 @@@\n+abc A\n+abc B\n+abc +C\n+abc +D\n+abc +E\n+abc +F\n+abc+ 1\n+abc+ 2\n+abcdiff --cc file0\n+abcindex b414108,f4615da..10a8a9f\n+abc--- a/file0\n+abc+++ b/file0\n+abc@@@ -1,6 -1,6 +1,9 @@@\n+abc 1\n+abc 2\n+abc 3\n+abc +4\n+abc +5\n+abc +6\n+abc+ A\n+abc+ B\n+abc+ C\n+$\ndiff --git a/t/t4013/diff.diff_--line-prefix_--cached_--_file0 b/t/t4013/diff.diff_--line-prefix_--cached_--_file0\nnew file mode 100644\nindex 000000000000..f41ba4d36aa1\n--- /dev/null\n+++ b/t/t4013/diff.diff_--line-prefix_--cached_--_file0\n@@ -0,0 +1,15 @@\n+| | | diff --git a/file0 b/file0\n+| | | new file mode 100644\n+| | | index 0000000..10a8a9f\n+| | | --- /dev/null\n+| | | +++ b/file0\n+| | | @@ -0,0 +1,9 @@\n+| | | +1\n+| | | +2\n+| | | +3\n+| | | +4\n+| | | +5\n+| | | +6\n+| | | +A\n+| | | +B\n+| | | +C\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex e2db47c36e09..1ccbd5948a73 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -187,6 +187,16 @@ test_expect_success 'git log --no-walk=sorted sorts by commit time' '\n \ttest_cmp expect actual\n '\n \n+cat > expect << EOF\n+=== 804a787 sixth\n+=== 394ef78 fifth\n+=== 5d31159 fourth\n+EOF\n+test_expect_success 'git log --line-prefix=\"=== \" --no-walk sorts by commit time' '\n+\tgit log --line-prefix=\"=== \" --no-walk --oneline 5d31159 804a787 394ef78 > actual &&\n+\ttest_cmp expect actual\n+'\n+\n cat > expect << EOF\n 5d31159 fourth\n 804a787 sixth\n@@ -284,6 +294,21 @@ test_expect_success 'simple log --graph' '\n \ttest_cmp expect actual\n '\n \n+cat > expect <actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'set up merge history' '\n \tgit checkout -b side HEAD~4 &&\n \ttest_commit side-1 1 1 &&\n@@ -313,6 +338,27 @@ test_expect_success 'log --graph with merge' '\n \ttest_cmp expect actual\n '\n \n+cat > expect <<\\EOF\n+| | | * Merge branch 'side'\n+| | | |\\\n+| | | | * side-2\n+| | | | * side-1\n+| | | * | Second\n+| | | * | sixth\n+| | | * | fifth\n+| | | * | fourth\n+| | | |/\n+| | | * third\n+| | | * second\n+| | | * initial\n+EOF\n+\n+test_expect_success 'log --graph --line-prefix=\"| | | \" with merge' '\n+\tgit log --line-prefix=\"| | | \" --graph --date-order --pretty=tformat:%s |\n+\t\tsed \"s/ *\\$//\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'log --raw --graph -m with merge' '\n \tgit log --raw --graph --oneline -m master | head -n 500 >actual &&\n \tgrep \"initial\" actual\n@@ -867,6 +913,283 @@ test_expect_success 'log --graph with diff and stats' '\n \ttest_i18ncmp expect actual.sanitized\n '\n \n+cat >expect <<\\EOF\n+*** * commit COMMIT_OBJECT_NAME\n+*** |\\ Merge: MERGE_PARENTS\n+*** | | Author: A U Thor \n+*** | |\n+*** | | Merge HEADS DESCRIPTION\n+*** | |\n+*** | * commit COMMIT_OBJECT_NAME\n+*** | | Author: A U Thor \n+*** | |\n+*** | | reach\n+*** | | ---\n+*** | | reach.t | 1 +\n+*** | | 1 file changed, 1 insertion(+)\n+*** | |\n+*** | | diff --git a/reach.t b/reach.t\n+*** | | new file mode 100644\n+*** | | index 0000000..10c9591\n+*** | | --- /dev/null\n+*** | | +++ b/reach.t\n+*** | | @@ -0,0 +1 @@\n+*** | | +reach\n+*** | |\n+*** | \\\n+*** *-. \\ commit COMMIT_OBJECT_NAME\n+*** |\\ \\ \\ Merge: MERGE_PARENTS\n+*** | | | | Author: A U Thor \n+*** | | | |\n+*** | | | | Merge HEADS DESCRIPTION\n+*** | | | |\n+*** | | * | commit COMMIT_OBJECT_NAME\n+*** | | |/ Author: A U Thor \n+*** | | |\n+*** | | | octopus-b\n+*** | | | ---\n+*** | | | octopus-b.t | 1 +\n+*** | | | 1 file changed, 1 insertion(+)\n+*** | | |\n+*** | | | diff --git a/octopus-b.t b/octopus-b.t\n+*** | | | new file mode 100644\n+*** | | | index 0000000..d5fcad0\n+*** | | | --- /dev/null\n+*** | | | +++ b/octopus-b.t\n+*** | | | @@ -0,0 +1 @@\n+*** | | | +octopus-b\n+*** | | |\n+*** | * | commit COMMIT_OBJECT_NAME\n+*** | |/ Author: A U Thor \n+*** | |\n+*** | | octopus-a\n+*** | | ---\n+*** | | octopus-a.t | 1 +\n+*** | | 1 file changed, 1 insertion(+)\n+*** | |\n+*** | | diff --git a/octopus-a.t b/octopus-a.t\n+*** | | new file mode 100644\n+*** | | index 0000000..11ee015\n+*** | | --- /dev/null\n+*** | | +++ b/octopus-a.t\n+*** | | @@ -0,0 +1 @@\n+*** | | +octopus-a\n+*** | |\n+*** * | commit COMMIT_OBJECT_NAME\n+*** |/ Author: A U Thor \n+*** |\n+*** | seventh\n+*** | ---\n+*** | seventh.t | 1 +\n+*** | 1 file changed, 1 insertion(+)\n+*** |\n+*** | diff --git a/seventh.t b/seventh.t\n+*** | new file mode 100644\n+*** | index 0000000..9744ffc\n+*** | --- /dev/null\n+*** | +++ b/seventh.t\n+*** | @@ -0,0 +1 @@\n+*** | +seventh\n+*** |\n+*** * commit COMMIT_OBJECT_NAME\n+*** |\\ Merge: MERGE_PARENTS\n+*** | | Author: A U Thor \n+*** | |\n+*** | | Merge branch 'tangle'\n+*** | |\n+*** | * commit COMMIT_OBJECT_NAME\n+*** | |\\ Merge: MERGE_PARENTS\n+*** | | | Author: A U Thor \n+*** | | |\n+*** | | | Merge branch 'side' (early part) into tangle\n+*** | | |\n+*** | * | commit COMMIT_OBJECT_NAME\n+*** | |\\ \\ Merge: MERGE_PARENTS\n+*** | | | | Author: A U Thor \n+*** | | | |\n+*** | | | | Merge branch 'master' (early part) into tangle\n+*** | | | |\n+*** | * | | commit COMMIT_OBJECT_NAME\n+*** | | | | Author: A U Thor \n+*** | | | |\n+*** | | | | tangle-a\n+*** | | | | ---\n+*** | | | | tangle-a | 1 +\n+*** | | | | 1 file changed, 1 insertion(+)\n+*** | | | |\n+*** | | | | diff --git a/tangle-a b/tangle-a\n+*** | | | | new file mode 100644\n+*** | | | | index 0000000..7898192\n+*** | | | | --- /dev/null\n+*** | | | | +++ b/tangle-a\n+*** | | | | @@ -0,0 +1 @@\n+*** | | | | +a\n+*** | | | |\n+*** * | | | commit COMMIT_OBJECT_NAME\n+*** |\\ \\ \\ \\ Merge: MERGE_PARENTS\n+*** | | | | | Author: A U Thor \n+*** | | | | |\n+*** | | | | | Merge branch 'side'\n+*** | | | | |\n+*** | * | | | commit COMMIT_OBJECT_NAME\n+*** | | |_|/ Author: A U Thor \n+*** | |/| |\n+*** | | | | side-2\n+*** | | | | ---\n+*** | | | | 2 | 1 +\n+*** | | | | 1 file changed, 1 insertion(+)\n+*** | | | |\n+*** | | | | diff --git a/2 b/2\n+*** | | | | new file mode 100644\n+*** | | | | index 0000000..0cfbf08\n+*** | | | | --- /dev/null\n+*** | | | | +++ b/2\n+*** | | | | @@ -0,0 +1 @@\n+*** | | | | +2\n+*** | | | |\n+*** | * | | commit COMMIT_OBJECT_NAME\n+*** | | | | Author: A U Thor \n+*** | | | |\n+*** | | | | side-1\n+*** | | | | ---\n+*** | | | | 1 | 1 +\n+*** | | | | 1 file changed, 1 insertion(+)\n+*** | | | |\n+*** | | | | diff --git a/1 b/1\n+*** | | | | new file mode 100644\n+*** | | | | index 0000000..d00491f\n+*** | | | | --- /dev/null\n+*** | | | | +++ b/1\n+*** | | | | @@ -0,0 +1 @@\n+*** | | | | +1\n+*** | | | |\n+*** * | | | commit COMMIT_OBJECT_NAME\n+*** | | | | Author: A U Thor \n+*** | | | |\n+*** | | | | Second\n+*** | | | | ---\n+*** | | | | one | 1 +\n+*** | | | | 1 file changed, 1 insertion(+)\n+*** | | | |\n+*** | | | | diff --git a/one b/one\n+*** | | | | new file mode 100644\n+*** | | | | index 0000000..9a33383\n+*** | | | | --- /dev/null\n+*** | | | | +++ b/one\n+*** | | | | @@ -0,0 +1 @@\n+*** | | | | +case\n+*** | | | |\n+*** * | | | commit COMMIT_OBJECT_NAME\n+*** | |_|/ Author: A U Thor \n+*** |/| |\n+*** | | | sixth\n+*** | | | ---\n+*** | | | a/two | 1 -\n+*** | | | 1 file changed, 1 deletion(-)\n+*** | | |\n+*** | | | diff --git a/a/two b/a/two\n+*** | | | deleted file mode 100644\n+*** | | | index 9245af5..0000000\n+*** | | | --- a/a/two\n+*** | | | +++ /dev/null\n+*** | | | @@ -1 +0,0 @@\n+*** | | | -ni\n+*** | | |\n+*** * | | commit COMMIT_OBJECT_NAME\n+*** | | | Author: A U Thor \n+*** | | |\n+*** | | | fifth\n+*** | | | ---\n+*** | | | a/two | 1 +\n+*** | | | 1 file changed, 1 insertion(+)\n+*** | | |\n+*** | | | diff --git a/a/two b/a/two\n+*** | | | new file mode 100644\n+*** | | | index 0000000..9245af5\n+*** | | | --- /dev/null\n+*** | | | +++ b/a/two\n+*** | | | @@ -0,0 +1 @@\n+*** | | | +ni\n+*** | | |\n+*** * | | commit COMMIT_OBJECT_NAME\n+*** |/ / Author: A U Thor \n+*** | |\n+*** | | fourth\n+*** | | ---\n+*** | | ein | 1 +\n+*** | | 1 file changed, 1 insertion(+)\n+*** | |\n+*** | | diff --git a/ein b/ein\n+*** | | new file mode 100644\n+*** | | index 0000000..9d7e69f\n+*** | | --- /dev/null\n+*** | | +++ b/ein\n+*** | | @@ -0,0 +1 @@\n+*** | | +ichi\n+*** | |\n+*** * | commit COMMIT_OBJECT_NAME\n+*** |/ Author: A U Thor \n+*** |\n+*** | third\n+*** | ---\n+*** | ichi | 1 +\n+*** | one | 1 -\n+*** | 2 files changed, 1 insertion(+), 1 deletion(-)\n+*** |\n+*** | diff --git a/ichi b/ichi\n+*** | new file mode 100644\n+*** | index 0000000..9d7e69f\n+*** | --- /dev/null\n+*** | +++ b/ichi\n+*** | @@ -0,0 +1 @@\n+*** | +ichi\n+*** | diff --git a/one b/one\n+*** | deleted file mode 100644\n+*** | index 9d7e69f..0000000\n+*** | --- a/one\n+*** | +++ /dev/null\n+*** | @@ -1 +0,0 @@\n+*** | -ichi\n+*** |\n+*** * commit COMMIT_OBJECT_NAME\n+*** | Author: A U Thor \n+*** |\n+*** | second\n+*** | ---\n+*** | one | 2 +-\n+*** | 1 file changed, 1 insertion(+), 1 deletion(-)\n+*** |\n+*** | diff --git a/one b/one\n+*** | index 5626abf..9d7e69f 100644\n+*** | --- a/one\n+*** | +++ b/one\n+*** | @@ -1 +1 @@\n+*** | -one\n+*** | +ichi\n+*** |\n+*** * commit COMMIT_OBJECT_NAME\n+*** Author: A U Thor \n+***\n+*** initial\n+*** ---\n+*** one | 1 +\n+*** 1 file changed, 1 insertion(+)\n+***\n+*** diff --git a/one b/one\n+*** new file mode 100644\n+*** index 0000000..5626abf\n+*** --- /dev/null\n+*** +++ b/one\n+*** @@ -0,0 +1 @@\n+*** +one\n+EOF\n+\n+test_expect_success 'log --line-prefix=\"*** \" --graph with diff and stats' '\n+\tgit log --line-prefix=\"*** \" --no-renames --graph --pretty=short --stat -p >actual &&\n+\tsanitize_output >actual.sanitized expect &&\n-- \n2.10.0.rc0.259.g83512d9\n\n"},{"id":"299883","messageId":"20160822234344.22797-6-jacob.e.keller@intel.com","threadId":"43907","inReplyTo":"20160822234344.22797-1-jacob.e.keller@intel.com","subject":"[PATCH v10 5/9] diff: prepare for additional submodule formats","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2016-08-22T23:43:40Z","receivedAt":"2016-08-22T23:44:14Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nA future patch will add a new format for displaying the difference of\na submodule. Make it easier by changing how we store the current\nselected format. Replace the DIFF_OPT flag with an enumeration, as each\nformat will be mutually exclusive.\n\nSigned-off-by: Jacob Keller \n---\n diff.c | 12 ++++++------\n diff.h | 7 ++++++-\n 2 files changed, 12 insertions(+), 7 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex e57cf39ad109..d6b321da3d1d 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -132,9 +132,9 @@ static int parse_dirstat_params(struct diff_options *options, const char *params\n static int parse_submodule_params(struct diff_options *options, const char *value)\n {\n \tif (!strcmp(value, \"log\"))\n-\t\tDIFF_OPT_SET(options, SUBMODULE_LOG);\n+\t\toptions->submodule_format = DIFF_SUBMODULE_LOG;\n \telse if (!strcmp(value, \"short\"))\n-\t\tDIFF_OPT_CLR(options, SUBMODULE_LOG);\n+\t\toptions->submodule_format = DIFF_SUBMODULE_SHORT;\n \telse\n \t\treturn -1;\n \treturn 0;\n@@ -2300,9 +2300,9 @@ static void builtin_diff(const char *name_a,\n \tstruct strbuf header = STRBUF_INIT;\n \tconst char *line_prefix = diff_line_prefix(o);\n \n-\tif (DIFF_OPT_TST(o, SUBMODULE_LOG) &&\n-\t\t\t(!one->mode || S_ISGITLINK(one->mode)) &&\n-\t\t\t(!two->mode || S_ISGITLINK(two->mode))) {\n+\tif (o->submodule_format == DIFF_SUBMODULE_LOG &&\n+\t (!one->mode || S_ISGITLINK(one->mode)) &&\n+\t (!two->mode || S_ISGITLINK(two->mode))) {\n \t\tconst char *del = diff_get_color_opt(o, DIFF_FILE_OLD);\n \t\tconst char *add = diff_get_color_opt(o, DIFF_FILE_NEW);\n \t\tshow_submodule_summary(o->file, one->path ? one->path : two->path,\n@@ -3916,7 +3916,7 @@ int diff_opt_parse(struct diff_options *options,\n \t\tDIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);\n \t\thandle_ignore_submodules_arg(options, arg);\n \t} else if (!strcmp(arg, \"--submodule\"))\n-\t\tDIFF_OPT_SET(options, SUBMODULE_LOG);\n+\t\toptions->submodule_format = DIFF_SUBMODULE_LOG;\n \telse if (skip_prefix(arg, \"--submodule=\", &arg))\n \t\treturn parse_submodule_opt(options, arg);\n \telse if (skip_prefix(arg, \"--ws-error-highlight=\", &arg))\ndiff --git a/diff.h b/diff.h\nindex 1f57aad25c71..43b353aea091 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -83,7 +83,6 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n #define DIFF_OPT_DIRSTAT_BY_FILE (1 << 20)\n #define DIFF_OPT_ALLOW_TEXTCONV (1 << 21)\n #define DIFF_OPT_DIFF_FROM_CONTENTS (1 << 22)\n-#define DIFF_OPT_SUBMODULE_LOG (1 << 23)\n #define DIFF_OPT_DIRTY_SUBMODULES (1 << 24)\n #define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1 << 25)\n #define DIFF_OPT_IGNORE_DIRTY_SUBMODULES (1 << 26)\n@@ -110,6 +109,11 @@ enum diff_words_type {\n \tDIFF_WORDS_COLOR\n };\n \n+enum diff_submodule_format {\n+\tDIFF_SUBMODULE_SHORT = 0,\n+\tDIFF_SUBMODULE_LOG\n+};\n+\n struct diff_options {\n \tconst char *orderfile;\n \tconst char *pickaxe;\n@@ -157,6 +161,7 @@ struct diff_options {\n \tint stat_count;\n \tconst char *word_regex;\n \tenum diff_words_type word_diff;\n+\tenum diff_submodule_format submodule_format;\n \n \t/* this is set by diffcore for DIFF_FORMAT_PATCH */\n \tint found_changes;\n-- \n2.10.0.rc0.259.g83512d9\n\n"},{"id":"299884","messageId":"20160822234344.22797-1-jacob.e.keller@intel.com","threadId":"43907","inReplyTo":null,"subject":"[PATCH v10 0/9] submodule inline diff format","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2016-08-22T23:43:35Z","receivedAt":"2016-08-22T23:44:40Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nA few suggestions from Stefan in regards to falling back to\n.git/modules/ being a bad idea. I've chosen I think to avoid using\ndie() as we just stick with the current path if we can't find its name.\nI think this should be safe since we already do this today. The new flow\nonly changes if we are able to lookup the submodule, so I don't think\nit's worth adding a die() call.\n\ninterdiff between v9 and v10 is pretty small:\n\n\ndiff --git c/Documentation/RelNotes/2.10.0.txt w/Documentation/RelNotes/2.10.0.txt\nindex 4f7460be3914..0ef70fd9b1eb 100644\n--- c/Documentation/RelNotes/2.10.0.txt\n+++ w/Documentation/RelNotes/2.10.0.txt\n@@ -118,6 +118,15 @@ UI, Workflows & Features\n \"git branch --delete/--move [--remote]\".\n (merge 2703c22 vs/completion-branch-fully-spelled-d-m-r later to maint).\n \n+ * \"git rev-parse --git-path hooks/\" learned to take\n+ core.hooksPath configuration variable (introduced during 2.9 cycle)\n+ into account.\n+ (merge 9445b49 ab/hooks later to maint).\n+\n+ * \"git log --show-signature\" and other commands that display the\n+ verification status of PGP signature now shows the longer key-id,\n+ as 32-bit key-id is so last century.\n+\n \n Performance, Internal Implementation, Development Support etc.\n \n@@ -600,6 +609,28 @@ notes for details).\n arises).\n (merge c2cafd3 js/test-lint-pathname later to maint).\n \n+ * When \"git merge-recursive\" works on history with many criss-cross\n+ merges in \"verbose\" mode, the names the command assigns to the\n+ virtual merge bases could have overwritten each other by unintended\n+ reuse of the same piece of memory.\n+ (merge 5447a76 rs/pull-signed-tag later to maint).\n+\n+ * \"git checkout --detach \" used to give the same advice\n+ message as that is issued when \"git checkout \" (or anything\n+ that is not a branch name) is given, but asking with \"--detach\" is\n+ an explicit enough sign that the user knows what is going on. The\n+ advice message has been squelched in this case.\n+ (merge 779b88a sb/checkout-explit-detach-no-advice later to maint).\n+\n+ * \"git difftool\" by default ignores the error exit from the backend\n+ commands it spawns, because often they signal that they found\n+ differences by exiting with a non-zero status code just like \"diff\"\n+ does; the exit status codes 126 and above however are special in\n+ that they are used to signal that the command is not executable,\n+ does not exist, or killed by a signal. \"git difftool\" has been\n+ taught to notice these exit status codes.\n+ (merge 45a4f5d jk/difftool-command-not-found later to maint).\n+\n * Other minor clean-ups and documentation updates\n (merge 02a8cfa rs/merge-add-strategies-simplification later to maint).\n (merge af4941d rs/merge-recursive-string-list-init later to maint).\ndiff --git c/GIT-VERSION-GEN w/GIT-VERSION-GEN\nindex eea85c340454..702c067a78c0 100755\n--- c/GIT-VERSION-GEN\n+++ w/GIT-VERSION-GEN\n@@ -1,7 +1,7 @@\n #!/bin/sh\n \n GVF=GIT-VERSION-FILE\n-DEF_VER=v2.10.0-rc0\n+DEF_VER=v2.10.0-rc1\n \n LF='\n '\ndiff --git c/path.c w/path.c\nindex 081a22c1163c..07dd0f62eb82 100644\n--- c/path.c\n+++ w/path.c\n@@ -475,7 +475,7 @@ static void do_submodule_path(struct strbuf *buf, const char *path,\n \tconst char *git_dir;\n \tstruct strbuf git_submodule_common_dir = STRBUF_INIT;\n \tstruct strbuf git_submodule_dir = STRBUF_INIT;\n-\tconst struct submodule *submodule_config;\n+\tconst struct submodule *sub;\n \n \tstrbuf_addstr(buf, path);\n \tstrbuf_complete(buf, '/');\n@@ -487,17 +487,12 @@ static void do_submodule_path(struct strbuf *buf, const char *path,\n \t\tstrbuf_addstr(buf, git_dir);\n \t}\n \tif (!is_git_directory(buf->buf)) {\n-\t\tstrbuf_reset(buf);\n-\t\t/*\n-\t\t * Lookup the submodule name from the config. If that fails\n-\t\t * fall back to assuming the path is the name.\n-\t\t */\n-\t\tsubmodule_config = submodule_from_path(null_sha1, path);\n-\t\tif (submodule_config)\n+\t\tsub = submodule_from_path(null_sha1, path);\n+\t\tif (sub) {\n+\t\t\tstrbuf_reset(buf);\n \t\t\tstrbuf_git_path(buf, \"%s/%s\", \"modules\",\n-\t\t\t\t\tsubmodule_config->name);\n-\t\telse\n-\t\t\tstrbuf_git_path(buf, \"%s/%s\", \"modules\", path);\n+\t\t\t\t\tsub->name);\n+\t\t}\n \t}\n \n \tstrbuf_addch(buf, '/');\n\n-------->8\n\nJacob Keller (7):\n cache: add empty_tree_oid object and helper function\n graph: add support for --line-prefix on all graph-aware output\n diff: prepare for additional submodule formats\n allow do_submodule_path to work even if submodule isn't checked out\n submodule: convert show_submodule_summary to use struct object_id *\n submodule: refactor show_submodule_summary with helper function\n diff: teach diff to display submodule difference with an inline diff\n\nJunio C Hamano (2):\n Git 2.10-rc1\n diff.c: remove output_prefix_length field\n\n"},{"id":"299885","messageId":"20160822234344.22797-3-jacob.e.keller@intel.com","threadId":"43907","inReplyTo":"20160822234344.22797-1-jacob.e.keller@intel.com","subject":"[PATCH v10 2/9] cache: add empty_tree_oid object and helper function","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2016-08-22T23:43:37Z","receivedAt":"2016-08-22T23:44:43Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nSimilar to is_null_oid(), and is_empty_blob_sha1() add an\nempty_tree_oid along with helper function is_empty_tree_oid(). For\ncompleteness, also add an \"is_empty_tree_sha1()\",\n\"is_empty_blob_sha1()\", \"is_empty_tree_oid()\" and \"is_empty_blob_oid()\"\nhelpers.\n\nTo ensure we only get one singleton, implement EMPTY_BLOB_SHA1_BIN as\nsimply getting the hash of empty_blob_oid structure.\n\nSigned-off-by: Jacob Keller \n---\n cache.h | 25 +++++++++++++++++++++----\n sha1_file.c | 6 ++++++\n 2 files changed, 27 insertions(+), 4 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex f30a4417efdf..70428e92d7ed 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -953,22 +953,39 @@ static inline void oidclr(struct object_id *oid)\n #define EMPTY_TREE_SHA1_BIN_LITERAL \\\n \t \"\\x4b\\x82\\x5d\\xc6\\x42\\xcb\\x6e\\xb9\\xa0\\x60\" \\\n \t \"\\xe5\\x4b\\xf8\\xd6\\x92\\x88\\xfb\\xee\\x49\\x04\"\n-#define EMPTY_TREE_SHA1_BIN \\\n-\t ((const unsigned char *) EMPTY_TREE_SHA1_BIN_LITERAL)\n+extern const struct object_id empty_tree_oid;\n+#define EMPTY_TREE_SHA1_BIN (empty_tree_oid.hash)\n \n #define EMPTY_BLOB_SHA1_HEX \\\n \t\"e69de29bb2d1d6434b8b29ae775ad8c2e48c5391\"\n #define EMPTY_BLOB_SHA1_BIN_LITERAL \\\n \t\"\\xe6\\x9d\\xe2\\x9b\\xb2\\xd1\\xd6\\x43\\x4b\\x8b\" \\\n \t\"\\x29\\xae\\x77\\x5a\\xd8\\xc2\\xe4\\x8c\\x53\\x91\"\n-#define EMPTY_BLOB_SHA1_BIN \\\n-\t((const unsigned char *) EMPTY_BLOB_SHA1_BIN_LITERAL)\n+extern const struct object_id empty_blob_oid;\n+#define EMPTY_BLOB_SHA1_BIN (empty_blob_oid.hash)\n+\n \n static inline int is_empty_blob_sha1(const unsigned char *sha1)\n {\n \treturn !hashcmp(sha1, EMPTY_BLOB_SHA1_BIN);\n }\n \n+static inline int is_empty_blob_oid(const struct object_id *oid)\n+{\n+\treturn !hashcmp(oid->hash, EMPTY_BLOB_SHA1_BIN);\n+}\n+\n+static inline int is_empty_tree_sha1(const unsigned char *sha1)\n+{\n+\treturn !hashcmp(sha1, EMPTY_TREE_SHA1_BIN);\n+}\n+\n+static inline int is_empty_tree_oid(const struct object_id *oid)\n+{\n+\treturn !hashcmp(oid->hash, EMPTY_TREE_SHA1_BIN);\n+}\n+\n+\n int git_mkstemp(char *path, size_t n, const char *template);\n \n /* set default permissions by passing mode arguments to open(2) */\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 1e23fc186a02..21cf923bcf1f 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -38,6 +38,12 @@ static inline uintmax_t sz_fmt(size_t s) { return s; }\n \n const unsigned char null_sha1[20];\n const struct object_id null_oid;\n+const struct object_id empty_tree_oid = {\n+\tEMPTY_TREE_SHA1_BIN_LITERAL\n+};\n+const struct object_id empty_blob_oid = {\n+\tEMPTY_BLOB_SHA1_BIN_LITERAL\n+};\n \n /*\n * This is meant to hold a *small* number of objects that you would\n-- \n2.10.0.rc0.259.g83512d9\n\n"},{"id":"299886","messageId":"20160822234344.22797-10-jacob.e.keller@intel.com","threadId":"43907","inReplyTo":"20160822234344.22797-1-jacob.e.keller@intel.com","subject":"[PATCH v10 9/9] diff: teach diff to display submodule difference with an inline diff","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2016-08-22T23:43:44Z","receivedAt":"2016-08-22T23:44:45Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nTeach git-diff and friends a new format for displaying the difference\nof a submodule. The new format is an inline diff of the contents of the\nsubmodule between the commit range of the update. This allows the user\nto see the actual code change caused by a submodule update.\n\nAdd tests for the new format and option.\n\nSigned-off-by: Jacob Keller \n---\n Documentation/diff-config.txt | 9 +-\n Documentation/diff-options.txt | 17 +-\n diff.c | 31 +-\n diff.h | 3 +-\n submodule.c | 69 +++\n submodule.h | 6 +\n t/t4060-diff-submodule-option-diff-format.sh | 749 +++++++++++++++++++++++++++\n 7 files changed, 863 insertions(+), 21 deletions(-)\n create mode 100755 t/t4060-diff-submodule-option-diff-format.sh\n\ndiff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt\nindex d5a5b17d5088..0eded24034b5 100644\n--- a/Documentation/diff-config.txt\n+++ b/Documentation/diff-config.txt\n@@ -122,10 +122,11 @@ diff.suppressBlankEmpty::\n \n diff.submodule::\n \tSpecify the format in which differences in submodules are\n-\tshown. The \"log\" format lists the commits in the range like\n-\tlinkgit:git-submodule[1] `summary` does. The \"short\" format\n-\tformat just shows the names of the commits at the beginning\n-\tand end of the range. Defaults to short.\n+\tshown. The \"short\" format just shows the names of the commits\n+\tat the beginning and end of the range. The \"log\" format lists\n+\tthe commits in the range like linkgit:git-submodule[1] `summary`\n+\tdoes. The \"diff\" format shows an inline diff of the changed\n+\tcontents of the submodule. Defaults to \"short\".\n \n diff.wordRegex::\n \tA POSIX Extended Regular Expression used to determine what is a \"word\"\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex cc4342e2034d..7805a0ccadf2 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -210,13 +210,16 @@ any of those replacements occurred.\n \tof the `--diff-filter` option on what the status letters mean.\n \n --submodule[=]::\n-\tSpecify how differences in submodules are shown. When `--submodule`\n-\tor `--submodule=log` is given, the 'log' format is used. This format lists\n-\tthe commits in the range like linkgit:git-submodule[1] `summary` does.\n-\tOmitting the `--submodule` option or specifying `--submodule=short`,\n-\tuses the 'short' format. This format just shows the names of the commits\n-\tat the beginning and end of the range. Can be tweaked via the\n-\t`diff.submodule` configuration variable.\n+\tSpecify how differences in submodules are shown. When specifying\n+\t`--submodule=short` the 'short' format is used. This format just\n+\tshows the names of the commits at the beginning and end of the range.\n+\tWhen `--submodule` or `--submodule=log` is specified, the 'log'\n+\tformat is used. This format lists the commits in the range like\n+\tlinkgit:git-submodule[1] `summary` does. When `--submodule=diff`\n+\tis specified, the 'diff' format is used. This format shows an\n+\tinline diff of the changes in the submodule contents between the\n+\tcommit range. Defaults to `diff.submodule` or the 'short' format\n+\tif the config option is unset.\n \n --color[=]::\n \tShow colored diff.\ndiff --git a/diff.c b/diff.c\nindex 16253b191f53..b38d95eb249c 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -135,6 +135,8 @@ static int parse_submodule_params(struct diff_options *options, const char *valu\n \t\toptions->submodule_format = DIFF_SUBMODULE_LOG;\n \telse if (!strcmp(value, \"short\"))\n \t\toptions->submodule_format = DIFF_SUBMODULE_SHORT;\n+\telse if (!strcmp(value, \"diff\"))\n+\t\toptions->submodule_format = DIFF_SUBMODULE_INLINE_DIFF;\n \telse\n \t\treturn -1;\n \treturn 0;\n@@ -2300,6 +2302,15 @@ static void builtin_diff(const char *name_a,\n \tstruct strbuf header = STRBUF_INIT;\n \tconst char *line_prefix = diff_line_prefix(o);\n \n+\tdiff_set_mnemonic_prefix(o, \"a/\", \"b/\");\n+\tif (DIFF_OPT_TST(o, REVERSE_DIFF)) {\n+\t\ta_prefix = o->b_prefix;\n+\t\tb_prefix = o->a_prefix;\n+\t} else {\n+\t\ta_prefix = o->a_prefix;\n+\t\tb_prefix = o->b_prefix;\n+\t}\n+\n \tif (o->submodule_format == DIFF_SUBMODULE_LOG &&\n \t (!one->mode || S_ISGITLINK(one->mode)) &&\n \t (!two->mode || S_ISGITLINK(two->mode))) {\n@@ -2311,6 +2322,17 @@ static void builtin_diff(const char *name_a,\n \t\t\t\ttwo->dirty_submodule,\n \t\t\t\tmeta, del, add, reset);\n \t\treturn;\n+\t} else if (o->submodule_format == DIFF_SUBMODULE_INLINE_DIFF &&\n+\t\t (!one->mode || S_ISGITLINK(one->mode)) &&\n+\t\t (!two->mode || S_ISGITLINK(two->mode))) {\n+\t\tconst char *del = diff_get_color_opt(o, DIFF_FILE_OLD);\n+\t\tconst char *add = diff_get_color_opt(o, DIFF_FILE_NEW);\n+\t\tshow_submodule_inline_diff(o->file, one->path ? one->path : two->path,\n+\t\t\t\tline_prefix,\n+\t\t\t\t&one->oid, &two->oid,\n+\t\t\t\ttwo->dirty_submodule,\n+\t\t\t\tmeta, del, add, reset, o);\n+\t\treturn;\n \t}\n \n \tif (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) {\n@@ -2318,15 +2340,6 @@ static void builtin_diff(const char *name_a,\n \t\ttextconv_two = get_textconv(two);\n \t}\n \n-\tdiff_set_mnemonic_prefix(o, \"a/\", \"b/\");\n-\tif (DIFF_OPT_TST(o, REVERSE_DIFF)) {\n-\t\ta_prefix = o->b_prefix;\n-\t\tb_prefix = o->a_prefix;\n-\t} else {\n-\t\ta_prefix = o->a_prefix;\n-\t\tb_prefix = o->b_prefix;\n-\t}\n-\n \t/* Never use a non-valid filename anywhere if at all possible */\n \tname_a = DIFF_FILE_VALID(one) ? name_a : name_b;\n \tname_b = DIFF_FILE_VALID(two) ? name_b : name_a;\ndiff --git a/diff.h b/diff.h\nindex 43b353aea091..ec76a90522ea 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -111,7 +111,8 @@ enum diff_words_type {\n \n enum diff_submodule_format {\n \tDIFF_SUBMODULE_SHORT = 0,\n-\tDIFF_SUBMODULE_LOG\n+\tDIFF_SUBMODULE_LOG,\n+\tDIFF_SUBMODULE_INLINE_DIFF\n };\n \n struct diff_options {\ndiff --git a/submodule.c b/submodule.c\nindex 1fb753af3066..405fa9e4eb32 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -440,6 +440,75 @@ void show_submodule_summary(FILE *f, const char *path,\n \tclear_commit_marks(right, ~0);\n }\n \n+void show_submodule_inline_diff(FILE *f, const char *path,\n+\t\tconst char *line_prefix,\n+\t\tstruct object_id *one, struct object_id *two,\n+\t\tunsigned dirty_submodule, const char *meta,\n+\t\tconst char *del, const char *add, const char *reset,\n+\t\tconst struct diff_options *o)\n+{\n+\tconst struct object_id *old = &empty_tree_oid, *new = &empty_tree_oid;\n+\tstruct commit *left = NULL, *right = NULL;\n+\tstruct commit_list *merge_bases = NULL;\n+\tstruct strbuf submodule_dir = STRBUF_INIT;\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\n+\tshow_submodule_header(f, path, line_prefix, one, two, dirty_submodule,\n+\t\t\t meta, reset, &left, &right, &merge_bases);\n+\n+\t/* We need a valid left and right commit to display a difference */\n+\tif (!(left || is_null_oid(one)) ||\n+\t !(right || is_null_oid(two)))\n+\t\tgoto done;\n+\n+\tif (left)\n+\t\told = one;\n+\tif (right)\n+\t\tnew = two;\n+\n+\tfflush(f);\n+\tcp.git_cmd = 1;\n+\tcp.dir = path;\n+\tcp.out = dup(fileno(f));\n+\tcp.no_stdin = 1;\n+\n+\t/* TODO: other options may need to be passed here. */\n+\targv_array_push(&cp.args, \"diff\");\n+\targv_array_pushf(&cp.args, \"--line-prefix=%s\", line_prefix);\n+\tif (DIFF_OPT_TST(o, REVERSE_DIFF)) {\n+\t\targv_array_pushf(&cp.args, \"--src-prefix=%s%s/\",\n+\t\t\t\t o->b_prefix, path);\n+\t\targv_array_pushf(&cp.args, \"--dst-prefix=%s%s/\",\n+\t\t\t\t o->a_prefix, path);\n+\t} else {\n+\t\targv_array_pushf(&cp.args, \"--src-prefix=%s%s/\",\n+\t\t\t\t o->a_prefix, path);\n+\t\targv_array_pushf(&cp.args, \"--dst-prefix=%s%s/\",\n+\t\t\t\t o->b_prefix, path);\n+\t}\n+\targv_array_push(&cp.args, oid_to_hex(old));\n+\t/*\n+\t * If the submodule has modified content, we will diff against the\n+\t * work tree, under the assumption that the user has asked for the\n+\t * diff format and wishes to actually see all differences even if they\n+\t * haven't yet been committed to the submodule yet.\n+\t */\n+\tif (!(dirty_submodule & DIRTY_SUBMODULE_MODIFIED))\n+\t\targv_array_push(&cp.args, oid_to_hex(new));\n+\n+\tif (run_command(&cp))\n+\t\tfprintf(f, \"(diff failed)\\n\");\n+\n+done:\n+\tstrbuf_release(&submodule_dir);\n+\tif (merge_bases)\n+\t\tfree_commit_list(merge_bases);\n+\tif (left)\n+\t\tclear_commit_marks(left, ~0);\n+\tif (right)\n+\t\tclear_commit_marks(right, ~0);\n+}\n+\n void set_config_fetch_recurse_submodules(int value)\n {\n \tconfig_fetch_recurse_submodules = value;\ndiff --git a/submodule.h b/submodule.h\nindex d83df57e24ff..d9e197a948fd 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -46,6 +46,12 @@ void show_submodule_summary(FILE *f, const char *path,\n \t\tstruct object_id *one, struct object_id *two,\n \t\tunsigned dirty_submodule, const char *meta,\n \t\tconst char *del, const char *add, const char *reset);\n+void show_submodule_inline_diff(FILE *f, const char *path,\n+\t\tconst char *line_prefix,\n+\t\tstruct object_id *one, struct object_id *two,\n+\t\tunsigned dirty_submodule, const char *meta,\n+\t\tconst char *del, const char *add, const char *reset,\n+\t\tconst struct diff_options *opt);\n void set_config_fetch_recurse_submodules(int value);\n void check_for_new_submodule_commits(unsigned char new_sha1[20]);\n int fetch_populated_submodules(const struct argv_array *options,\ndiff --git a/t/t4060-diff-submodule-option-diff-format.sh b/t/t4060-diff-submodule-option-diff-format.sh\nnew file mode 100755\nindex 000000000000..7e23b55ea4c5\n--- /dev/null\n+++ b/t/t4060-diff-submodule-option-diff-format.sh\n@@ -0,0 +1,749 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2009 Jens Lehmann, based on t7401 by Ping Yin\n+# Copyright (c) 2011 Alexey Shumkin (+ non-UTF-8 commit encoding tests)\n+# Copyright (c) 2016 Jacob Keller (copy + convert to --submodule=diff)\n+#\n+\n+test_description='Support for diff format verbose submodule difference in git diff\n+\n+This test tries to verify the sanity of --submodule=diff option of git diff.\n+'\n+\n+. ./test-lib.sh\n+\n+# Tested non-UTF-8 encoding\n+test_encoding=\"ISO8859-1\"\n+\n+# String \"added\" in German (translated with Google Translate), encoded in UTF-8,\n+# used in sample commit log messages in add_file() function below.\n+added=$(printf \"hinzugef\\303\\274gt\")\n+\n+add_file () {\n+\t(\n+\t\tcd \"$1\" &&\n+\t\tshift &&\n+\t\tfor name\n+\t\tdo\n+\t\t\techo \"$name\" >\"$name\" &&\n+\t\t\tgit add \"$name\" &&\n+\t\t\ttest_tick &&\n+\t\t\t# \"git commit -m\" would break MinGW, as Windows refuse to pass\n+\t\t\t# $test_encoding encoded parameter to git.\n+\t\t\techo \"Add $name ($added $name)\" | iconv -f utf-8 -t $test_encoding |\n+\t\t\tgit -c \"i18n.commitEncoding=$test_encoding\" commit -F -\n+\t\tdone >/dev/null &&\n+\t\tgit rev-parse --short --verify HEAD\n+\t)\n+}\n+\n+commit_file () {\n+\ttest_tick &&\n+\tgit commit \"$@\" -m \"Commit $*\" >/dev/null\n+}\n+\n+test_expect_success 'setup repository' '\n+\ttest_create_repo sm1 &&\n+\tadd_file . foo &&\n+\thead1=$(add_file sm1 foo1 foo2) &&\n+\tfullhead1=$(git -C sm1 rev-parse --verify HEAD)\n+'\n+\n+test_expect_success 'added submodule' '\n+\tgit add sm1 &&\n+\tgit diff-index -p --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 0000000...$head1 (new submodule)\n+\tdiff --git a/sm1/foo1 b/sm1/foo1\n+\tnew file mode 100644\n+\tindex 0000000..1715acd\n+\t--- /dev/null\n+\t+++ b/sm1/foo1\n+\t@@ -0,0 +1 @@\n+\t+foo1\n+\tdiff --git a/sm1/foo2 b/sm1/foo2\n+\tnew file mode 100644\n+\tindex 0000000..54b060e\n+\t--- /dev/null\n+\t+++ b/sm1/foo2\n+\t@@ -0,0 +1 @@\n+\t+foo2\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'added submodule, set diff.submodule' '\n+\ttest_config diff.submodule log &&\n+\tgit add sm1 &&\n+\tgit diff-index -p --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 0000000...$head1 (new submodule)\n+\tdiff --git a/sm1/foo1 b/sm1/foo1\n+\tnew file mode 100644\n+\tindex 0000000..1715acd\n+\t--- /dev/null\n+\t+++ b/sm1/foo1\n+\t@@ -0,0 +1 @@\n+\t+foo1\n+\tdiff --git a/sm1/foo2 b/sm1/foo2\n+\tnew file mode 100644\n+\tindex 0000000..54b060e\n+\t--- /dev/null\n+\t+++ b/sm1/foo2\n+\t@@ -0,0 +1 @@\n+\t+foo2\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success '--submodule=short overrides diff.submodule' '\n+\ttest_config diff.submodule log &&\n+\tgit add sm1 &&\n+\tgit diff --submodule=short --cached >actual &&\n+\tcat >expected <<-EOF &&\n+\tdiff --git a/sm1 b/sm1\n+\tnew file mode 160000\n+\tindex 0000000..$head1\n+\t--- /dev/null\n+\t+++ b/sm1\n+\t@@ -0,0 +1 @@\n+\t+Subproject commit $fullhead1\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'diff.submodule does not affect plumbing' '\n+\ttest_config diff.submodule log &&\n+\tgit diff-index -p HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tdiff --git a/sm1 b/sm1\n+\tnew file mode 160000\n+\tindex 0000000..$head1\n+\t--- /dev/null\n+\t+++ b/sm1\n+\t@@ -0,0 +1 @@\n+\t+Subproject commit $fullhead1\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+commit_file sm1 &&\n+head2=$(add_file sm1 foo3)\n+\n+test_expect_success 'modified submodule(forward)' '\n+\tgit diff-index -p --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 $head1..$head2:\n+\tdiff --git a/sm1/foo3 b/sm1/foo3\n+\tnew file mode 100644\n+\tindex 0000000..c1ec6c6\n+\t--- /dev/null\n+\t+++ b/sm1/foo3\n+\t@@ -0,0 +1 @@\n+\t+foo3\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'modified submodule(forward)' '\n+\tgit diff --submodule=diff >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 $head1..$head2:\n+\tdiff --git a/sm1/foo3 b/sm1/foo3\n+\tnew file mode 100644\n+\tindex 0000000..c1ec6c6\n+\t--- /dev/null\n+\t+++ b/sm1/foo3\n+\t@@ -0,0 +1 @@\n+\t+foo3\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'modified submodule(forward) --submodule' '\n+\tgit diff --submodule >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 $head1..$head2:\n+\t > Add foo3 ($added foo3)\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+fullhead2=$(cd sm1; git rev-parse --verify HEAD)\n+test_expect_success 'modified submodule(forward) --submodule=short' '\n+\tgit diff --submodule=short >actual &&\n+\tcat >expected <<-EOF &&\n+\tdiff --git a/sm1 b/sm1\n+\tindex $head1..$head2 160000\n+\t--- a/sm1\n+\t+++ b/sm1\n+\t@@ -1 +1 @@\n+\t-Subproject commit $fullhead1\n+\t+Subproject commit $fullhead2\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+commit_file sm1 &&\n+head3=$(\n+\tcd sm1 &&\n+\tgit reset --hard HEAD~2 >/dev/null &&\n+\tgit rev-parse --short --verify HEAD\n+)\n+\n+test_expect_success 'modified submodule(backward)' '\n+\tgit diff-index -p --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 $head2..$head3 (rewind):\n+\tdiff --git a/sm1/foo2 b/sm1/foo2\n+\tdeleted file mode 100644\n+\tindex 54b060e..0000000\n+\t--- a/sm1/foo2\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-foo2\n+\tdiff --git a/sm1/foo3 b/sm1/foo3\n+\tdeleted file mode 100644\n+\tindex c1ec6c6..0000000\n+\t--- a/sm1/foo3\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-foo3\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+head4=$(add_file sm1 foo4 foo5)\n+test_expect_success 'modified submodule(backward and forward)' '\n+\tgit diff-index -p --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 $head2...$head4:\n+\tdiff --git a/sm1/foo2 b/sm1/foo2\n+\tdeleted file mode 100644\n+\tindex 54b060e..0000000\n+\t--- a/sm1/foo2\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-foo2\n+\tdiff --git a/sm1/foo3 b/sm1/foo3\n+\tdeleted file mode 100644\n+\tindex c1ec6c6..0000000\n+\t--- a/sm1/foo3\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-foo3\n+\tdiff --git a/sm1/foo4 b/sm1/foo4\n+\tnew file mode 100644\n+\tindex 0000000..a0016db\n+\t--- /dev/null\n+\t+++ b/sm1/foo4\n+\t@@ -0,0 +1 @@\n+\t+foo4\n+\tdiff --git a/sm1/foo5 b/sm1/foo5\n+\tnew file mode 100644\n+\tindex 0000000..d6f2413\n+\t--- /dev/null\n+\t+++ b/sm1/foo5\n+\t@@ -0,0 +1 @@\n+\t+foo5\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+commit_file sm1 &&\n+mv sm1 sm1-bak &&\n+echo sm1 >sm1 &&\n+head5=$(git hash-object sm1 | cut -c1-7) &&\n+git add sm1 &&\n+rm -f sm1 &&\n+mv sm1-bak sm1\n+\n+test_expect_success 'typechanged submodule(submodule->blob), --cached' '\n+\tgit diff --submodule=diff --cached >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 $head4...0000000 (submodule deleted)\n+\tdiff --git a/sm1/foo1 b/sm1/foo1\n+\tdeleted file mode 100644\n+\tindex 1715acd..0000000\n+\t--- a/sm1/foo1\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-foo1\n+\tdiff --git a/sm1/foo4 b/sm1/foo4\n+\tdeleted file mode 100644\n+\tindex a0016db..0000000\n+\t--- a/sm1/foo4\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-foo4\n+\tdiff --git a/sm1/foo5 b/sm1/foo5\n+\tdeleted file mode 100644\n+\tindex d6f2413..0000000\n+\t--- a/sm1/foo5\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-foo5\n+\tdiff --git a/sm1 b/sm1\n+\tnew file mode 100644\n+\tindex 0000000..9da5fb8\n+\t--- /dev/null\n+\t+++ b/sm1\n+\t@@ -0,0 +1 @@\n+\t+sm1\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'typechanged submodule(submodule->blob)' '\n+\tgit diff --submodule=diff >actual &&\n+\tcat >expected <<-EOF &&\n+\tdiff --git a/sm1 b/sm1\n+\tdeleted file mode 100644\n+\tindex 9da5fb8..0000000\n+\t--- a/sm1\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-sm1\n+\tSubmodule sm1 0000000...$head4 (new submodule)\n+\tdiff --git a/sm1/foo1 b/sm1/foo1\n+\tnew file mode 100644\n+\tindex 0000000..1715acd\n+\t--- /dev/null\n+\t+++ b/sm1/foo1\n+\t@@ -0,0 +1 @@\n+\t+foo1\n+\tdiff --git a/sm1/foo4 b/sm1/foo4\n+\tnew file mode 100644\n+\tindex 0000000..a0016db\n+\t--- /dev/null\n+\t+++ b/sm1/foo4\n+\t@@ -0,0 +1 @@\n+\t+foo4\n+\tdiff --git a/sm1/foo5 b/sm1/foo5\n+\tnew file mode 100644\n+\tindex 0000000..d6f2413\n+\t--- /dev/null\n+\t+++ b/sm1/foo5\n+\t@@ -0,0 +1 @@\n+\t+foo5\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+rm -rf sm1 &&\n+git checkout-index sm1\n+test_expect_success 'typechanged submodule(submodule->blob)' '\n+\tgit diff-index -p --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 $head4...0000000 (submodule deleted)\n+\tdiff --git a/sm1 b/sm1\n+\tnew file mode 100644\n+\tindex 0000000..9da5fb8\n+\t--- /dev/null\n+\t+++ b/sm1\n+\t@@ -0,0 +1 @@\n+\t+sm1\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+rm -f sm1 &&\n+test_create_repo sm1 &&\n+head6=$(add_file sm1 foo6 foo7)\n+fullhead6=$(cd sm1; git rev-parse --verify HEAD)\n+test_expect_success 'nonexistent commit' '\n+\tgit diff-index -p --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 $head4...$head6 (commits not present)\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+commit_file\n+test_expect_success 'typechanged submodule(blob->submodule)' '\n+\tgit diff-index -p --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tdiff --git a/sm1 b/sm1\n+\tdeleted file mode 100644\n+\tindex 9da5fb8..0000000\n+\t--- a/sm1\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-sm1\n+\tSubmodule sm1 0000000...$head6 (new submodule)\n+\tdiff --git a/sm1/foo6 b/sm1/foo6\n+\tnew file mode 100644\n+\tindex 0000000..462398b\n+\t--- /dev/null\n+\t+++ b/sm1/foo6\n+\t@@ -0,0 +1 @@\n+\t+foo6\n+\tdiff --git a/sm1/foo7 b/sm1/foo7\n+\tnew file mode 100644\n+\tindex 0000000..6e9262c\n+\t--- /dev/null\n+\t+++ b/sm1/foo7\n+\t@@ -0,0 +1 @@\n+\t+foo7\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+commit_file sm1 &&\n+test_expect_success 'submodule is up to date' '\n+\tgit diff-index -p --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'submodule contains untracked content' '\n+\techo new > sm1/new-file &&\n+\tgit diff-index -p --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 contains untracked content\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'submodule contains untracked content (untracked ignored)' '\n+\tgit diff-index -p --ignore-submodules=untracked --submodule=diff HEAD >actual &&\n+\t! test -s actual\n+'\n+\n+test_expect_success 'submodule contains untracked content (dirty ignored)' '\n+\tgit diff-index -p --ignore-submodules=dirty --submodule=diff HEAD >actual &&\n+\t! test -s actual\n+'\n+\n+test_expect_success 'submodule contains untracked content (all ignored)' '\n+\tgit diff-index -p --ignore-submodules=all --submodule=diff HEAD >actual &&\n+\t! test -s actual\n+'\n+\n+test_expect_success 'submodule contains untracked and modified content' '\n+\techo new > sm1/foo6 &&\n+\tgit diff-index -p --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 contains untracked content\n+\tSubmodule sm1 contains modified content\n+\tdiff --git a/sm1/foo6 b/sm1/foo6\n+\tindex 462398b..3e75765 100644\n+\t--- a/sm1/foo6\n+\t+++ b/sm1/foo6\n+\t@@ -1 +1 @@\n+\t-foo6\n+\t+new\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+# NOT OK\n+test_expect_success 'submodule contains untracked and modified content (untracked ignored)' '\n+\techo new > sm1/foo6 &&\n+\tgit diff-index -p --ignore-submodules=untracked --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 contains modified content\n+\tdiff --git a/sm1/foo6 b/sm1/foo6\n+\tindex 462398b..3e75765 100644\n+\t--- a/sm1/foo6\n+\t+++ b/sm1/foo6\n+\t@@ -1 +1 @@\n+\t-foo6\n+\t+new\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'submodule contains untracked and modified content (dirty ignored)' '\n+\techo new > sm1/foo6 &&\n+\tgit diff-index -p --ignore-submodules=dirty --submodule=diff HEAD >actual &&\n+\t! test -s actual\n+'\n+\n+test_expect_success 'submodule contains untracked and modified content (all ignored)' '\n+\techo new > sm1/foo6 &&\n+\tgit diff-index -p --ignore-submodules --submodule=diff HEAD >actual &&\n+\t! test -s actual\n+'\n+\n+test_expect_success 'submodule contains modified content' '\n+\trm -f sm1/new-file &&\n+\tgit diff-index -p --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 contains modified content\n+\tdiff --git a/sm1/foo6 b/sm1/foo6\n+\tindex 462398b..3e75765 100644\n+\t--- a/sm1/foo6\n+\t+++ b/sm1/foo6\n+\t@@ -1 +1 @@\n+\t-foo6\n+\t+new\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+(cd sm1; git commit -mchange foo6 >/dev/null) &&\n+head8=$(cd sm1; git rev-parse --short --verify HEAD) &&\n+test_expect_success 'submodule is modified' '\n+\tgit diff-index -p --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 17243c9..$head8:\n+\tdiff --git a/sm1/foo6 b/sm1/foo6\n+\tindex 462398b..3e75765 100644\n+\t--- a/sm1/foo6\n+\t+++ b/sm1/foo6\n+\t@@ -1 +1 @@\n+\t-foo6\n+\t+new\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'modified submodule contains untracked content' '\n+\techo new > sm1/new-file &&\n+\tgit diff-index -p --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 contains untracked content\n+\tSubmodule sm1 17243c9..$head8:\n+\tdiff --git a/sm1/foo6 b/sm1/foo6\n+\tindex 462398b..3e75765 100644\n+\t--- a/sm1/foo6\n+\t+++ b/sm1/foo6\n+\t@@ -1 +1 @@\n+\t-foo6\n+\t+new\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'modified submodule contains untracked content (untracked ignored)' '\n+\tgit diff-index -p --ignore-submodules=untracked --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 17243c9..$head8:\n+\tdiff --git a/sm1/foo6 b/sm1/foo6\n+\tindex 462398b..3e75765 100644\n+\t--- a/sm1/foo6\n+\t+++ b/sm1/foo6\n+\t@@ -1 +1 @@\n+\t-foo6\n+\t+new\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'modified submodule contains untracked content (dirty ignored)' '\n+\tgit diff-index -p --ignore-submodules=dirty --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 17243c9..cfce562:\n+\tdiff --git a/sm1/foo6 b/sm1/foo6\n+\tindex 462398b..3e75765 100644\n+\t--- a/sm1/foo6\n+\t+++ b/sm1/foo6\n+\t@@ -1 +1 @@\n+\t-foo6\n+\t+new\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'modified submodule contains untracked content (all ignored)' '\n+\tgit diff-index -p --ignore-submodules=all --submodule=diff HEAD >actual &&\n+\t! test -s actual\n+'\n+\n+test_expect_success 'modified submodule contains untracked and modified content' '\n+\techo modification >> sm1/foo6 &&\n+\tgit diff-index -p --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 contains untracked content\n+\tSubmodule sm1 contains modified content\n+\tSubmodule sm1 17243c9..cfce562:\n+\tdiff --git a/sm1/foo6 b/sm1/foo6\n+\tindex 462398b..dfda541 100644\n+\t--- a/sm1/foo6\n+\t+++ b/sm1/foo6\n+\t@@ -1 +1,2 @@\n+\t-foo6\n+\t+new\n+\t+modification\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'modified submodule contains untracked and modified content (untracked ignored)' '\n+\techo modification >> sm1/foo6 &&\n+\tgit diff-index -p --ignore-submodules=untracked --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 contains modified content\n+\tSubmodule sm1 17243c9..cfce562:\n+\tdiff --git a/sm1/foo6 b/sm1/foo6\n+\tindex 462398b..e20e2d9 100644\n+\t--- a/sm1/foo6\n+\t+++ b/sm1/foo6\n+\t@@ -1 +1,3 @@\n+\t-foo6\n+\t+new\n+\t+modification\n+\t+modification\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'modified submodule contains untracked and modified content (dirty ignored)' '\n+\techo modification >> sm1/foo6 &&\n+\tgit diff-index -p --ignore-submodules=dirty --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 17243c9..cfce562:\n+\tdiff --git a/sm1/foo6 b/sm1/foo6\n+\tindex 462398b..3e75765 100644\n+\t--- a/sm1/foo6\n+\t+++ b/sm1/foo6\n+\t@@ -1 +1 @@\n+\t-foo6\n+\t+new\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'modified submodule contains untracked and modified content (all ignored)' '\n+\techo modification >> sm1/foo6 &&\n+\tgit diff-index -p --ignore-submodules --submodule=diff HEAD >actual &&\n+\t! test -s actual\n+'\n+\n+# NOT OK\n+test_expect_success 'modified submodule contains modified content' '\n+\trm -f sm1/new-file &&\n+\tgit diff-index -p --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 contains modified content\n+\tSubmodule sm1 17243c9..cfce562:\n+\tdiff --git a/sm1/foo6 b/sm1/foo6\n+\tindex 462398b..ac466ca 100644\n+\t--- a/sm1/foo6\n+\t+++ b/sm1/foo6\n+\t@@ -1 +1,5 @@\n+\t-foo6\n+\t+new\n+\t+modification\n+\t+modification\n+\t+modification\n+\t+modification\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+rm -rf sm1\n+test_expect_success 'deleted submodule' '\n+\tgit diff-index -p --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 17243c9...0000000 (submodule deleted)\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_create_repo sm2 &&\n+head7=$(add_file sm2 foo8 foo9) &&\n+git add sm2\n+\n+test_expect_success 'multiple submodules' '\n+\tgit diff-index -p --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 17243c9...0000000 (submodule deleted)\n+\tSubmodule sm2 0000000...a5a65c9 (new submodule)\n+\tdiff --git a/sm2/foo8 b/sm2/foo8\n+\tnew file mode 100644\n+\tindex 0000000..db9916b\n+\t--- /dev/null\n+\t+++ b/sm2/foo8\n+\t@@ -0,0 +1 @@\n+\t+foo8\n+\tdiff --git a/sm2/foo9 b/sm2/foo9\n+\tnew file mode 100644\n+\tindex 0000000..9c3b4f6\n+\t--- /dev/null\n+\t+++ b/sm2/foo9\n+\t@@ -0,0 +1 @@\n+\t+foo9\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'path filter' '\n+\tgit diff-index -p --submodule=diff HEAD sm2 >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm2 0000000...a5a65c9 (new submodule)\n+\tdiff --git a/sm2/foo8 b/sm2/foo8\n+\tnew file mode 100644\n+\tindex 0000000..db9916b\n+\t--- /dev/null\n+\t+++ b/sm2/foo8\n+\t@@ -0,0 +1 @@\n+\t+foo8\n+\tdiff --git a/sm2/foo9 b/sm2/foo9\n+\tnew file mode 100644\n+\tindex 0000000..9c3b4f6\n+\t--- /dev/null\n+\t+++ b/sm2/foo9\n+\t@@ -0,0 +1 @@\n+\t+foo9\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+commit_file sm2\n+test_expect_success 'given commit' '\n+\tgit diff-index -p --submodule=diff HEAD^ >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 17243c9...0000000 (submodule deleted)\n+\tSubmodule sm2 0000000...a5a65c9 (new submodule)\n+\tdiff --git a/sm2/foo8 b/sm2/foo8\n+\tnew file mode 100644\n+\tindex 0000000..db9916b\n+\t--- /dev/null\n+\t+++ b/sm2/foo8\n+\t@@ -0,0 +1 @@\n+\t+foo8\n+\tdiff --git a/sm2/foo9 b/sm2/foo9\n+\tnew file mode 100644\n+\tindex 0000000..9c3b4f6\n+\t--- /dev/null\n+\t+++ b/sm2/foo9\n+\t@@ -0,0 +1 @@\n+\t+foo9\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'setup .git file for sm2' '\n+\t(cd sm2 &&\n+\t REAL=\"$(pwd)/../.real\" &&\n+\t mv .git \"$REAL\"\n+\t echo \"gitdir: $REAL\" >.git)\n+'\n+\n+test_expect_success 'diff --submodule=diff with .git file' '\n+\tgit diff --submodule=diff HEAD^ >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 17243c9...0000000 (submodule deleted)\n+\tSubmodule sm2 0000000...a5a65c9 (new submodule)\n+\tdiff --git a/sm2/foo8 b/sm2/foo8\n+\tnew file mode 100644\n+\tindex 0000000..db9916b\n+\t--- /dev/null\n+\t+++ b/sm2/foo8\n+\t@@ -0,0 +1 @@\n+\t+foo8\n+\tdiff --git a/sm2/foo9 b/sm2/foo9\n+\tnew file mode 100644\n+\tindex 0000000..9c3b4f6\n+\t--- /dev/null\n+\t+++ b/sm2/foo9\n+\t@@ -0,0 +1 @@\n+\t+foo9\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_done\n-- \n2.10.0.rc0.259.g83512d9\n\n"},{"id":"299887","messageId":"20160822234344.22797-9-jacob.e.keller@intel.com","threadId":"43907","inReplyTo":"20160822234344.22797-1-jacob.e.keller@intel.com","subject":"[PATCH v10 8/9] submodule: refactor show_submodule_summary with helper function","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2016-08-22T23:43:43Z","receivedAt":"2016-08-22T23:44:46Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nA future patch is going to add a new submodule diff format which\ndisplays an inline diff of the submodule changes. To make this easier,\nand to ensure that both submodule diff formats use the same initial\nheader, factor out show_submodule_header() function which will print the\ncurrent submodule header line, and then leave the show_submodule_summary\nfunction to lookup and print the submodule log format.\n\nThis does create one format change in that \"(revision walker failed)\"\nwill now be displayed on its own line rather than as part of the message\nbecause we no longer perform this step directly in the header display\nflow. However, this is a rare case as most causes of the failure will be\ndue to a missing commit which we already check for and avoid previously.\nflow. However, this is a rare case and shouldn't impact much.\n\nSigned-off-by: Jacob Keller \n---\n submodule.c | 115 +++++++++++++++++++++++++++++++++++++++++++-----------------\n 1 file changed, 82 insertions(+), 33 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 422353ccf6cc..1fb753af3066 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -278,9 +278,9 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,\n \n static int prepare_submodule_summary(struct rev_info *rev, const char *path,\n \t\tstruct commit *left, struct commit *right,\n-\t\tint *fast_forward, int *fast_backward)\n+\t\tstruct commit_list *merge_bases)\n {\n-\tstruct commit_list *merge_bases, *list;\n+\tstruct commit_list *list;\n \n \tinit_revisions(rev, NULL);\n \tsetup_revisions(0, NULL, rev, NULL);\n@@ -289,13 +289,6 @@ static int prepare_submodule_summary(struct rev_info *rev, const char *path,\n \tleft->object.flags |= SYMMETRIC_LEFT;\n \tadd_pending_object(rev, &left->object, path);\n \tadd_pending_object(rev, &right->object, path);\n-\tmerge_bases = get_merge_bases(left, right);\n-\tif (merge_bases) {\n-\t\tif (merge_bases->item == left)\n-\t\t\t*fast_forward = 1;\n-\t\telse if (merge_bases->item == right)\n-\t\t\t*fast_backward = 1;\n-\t}\n \tfor (list = merge_bases; list; list = list->next) {\n \t\tlist->item->object.flags |= UNINTERESTING;\n \t\tadd_pending_object(rev, &list->item->object,\n@@ -333,31 +326,23 @@ static void print_submodule_summary(struct rev_info *rev, FILE *f,\n \tstrbuf_release(&sb);\n }\n \n-void show_submodule_summary(FILE *f, const char *path,\n+/* Helper function to display the submodule header line prior to the full\n+ * summary output. If it can locate the submodule objects directory it will\n+ * attempt to lookup both the left and right commits and put them into the\n+ * left and right pointers.\n+ */\n+static void show_submodule_header(FILE *f, const char *path,\n \t\tconst char *line_prefix,\n \t\tstruct object_id *one, struct object_id *two,\n \t\tunsigned dirty_submodule, const char *meta,\n-\t\tconst char *del, const char *add, const char *reset)\n+\t\tconst char *reset,\n+\t\tstruct commit **left, struct commit **right,\n+\t\tstruct commit_list **merge_bases)\n {\n-\tstruct rev_info rev;\n-\tstruct commit *left = NULL, *right = NULL;\n \tconst char *message = NULL;\n \tstruct strbuf sb = STRBUF_INIT;\n \tint fast_forward = 0, fast_backward = 0;\n \n-\tif (is_null_oid(two))\n-\t\tmessage = \"(submodule deleted)\";\n-\telse if (add_submodule_odb(path))\n-\t\tmessage = \"(not initialized)\";\n-\telse if (is_null_oid(one))\n-\t\tmessage = \"(new submodule)\";\n-\telse if (!(left = lookup_commit_reference(one->hash)) ||\n-\t\t !(right = lookup_commit_reference(two->hash)))\n-\t\tmessage = \"(commits not present)\";\n-\telse if (prepare_submodule_summary(&rev, path, left, right,\n-\t\t\t\t\t &fast_forward, &fast_backward))\n-\t\tmessage = \"(revision walker failed)\";\n-\n \tif (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)\n \t\tfprintf(f, \"%sSubmodule %s contains untracked content\\n\",\n \t\t\tline_prefix, path);\n@@ -365,11 +350,46 @@ void show_submodule_summary(FILE *f, const char *path,\n \t\tfprintf(f, \"%sSubmodule %s contains modified content\\n\",\n \t\t\tline_prefix, path);\n \n+\tif (is_null_oid(one))\n+\t\tmessage = \"(new submodule)\";\n+\telse if (is_null_oid(two))\n+\t\tmessage = \"(submodule deleted)\";\n+\n+\tif (add_submodule_odb(path)) {\n+\t\tif (!message)\n+\t\t\tmessage = \"(not initialized)\";\n+\t\tgoto output_header;\n+\t}\n+\n+\t/*\n+\t * Attempt to lookup the commit references, and determine if this is\n+\t * a fast forward or fast backwards update.\n+\t */\n+\t*left = lookup_commit_reference(one->hash);\n+\t*right = lookup_commit_reference(two->hash);\n+\n+\t/*\n+\t * Warn about missing commits in the submodule project, but only if\n+\t * they aren't null.\n+\t */\n+\tif ((!is_null_oid(one) && !*left) ||\n+\t (!is_null_oid(two) && !*right))\n+\t\tmessage = \"(commits not present)\";\n+\n+\t*merge_bases = get_merge_bases(*left, *right);\n+\tif (*merge_bases) {\n+\t\tif ((*merge_bases)->item == *left)\n+\t\t\tfast_forward = 1;\n+\t\telse if ((*merge_bases)->item == *right)\n+\t\t\tfast_backward = 1;\n+\t}\n+\n \tif (!oidcmp(one, two)) {\n \t\tstrbuf_release(&sb);\n \t\treturn;\n \t}\n \n+output_header:\n \tstrbuf_addf(&sb, \"%s%sSubmodule %s %s..\", line_prefix, meta, path,\n \t\t\tfind_unique_abbrev(one->hash, DEFAULT_ABBREV));\n \tif (!fast_backward && !fast_forward)\n@@ -381,16 +401,45 @@ void show_submodule_summary(FILE *f, const char *path,\n \t\tstrbuf_addf(&sb, \"%s:%s\\n\", fast_backward ? \" (rewind)\" : \"\", reset);\n \tfwrite(sb.buf, sb.len, 1, f);\n \n-\tif (!message) /* only NULL if we succeeded in setting up the walk */\n-\t\tprint_submodule_summary(&rev, f, line_prefix, del, add, reset);\n-\tif (left)\n-\t\tclear_commit_marks(left, ~0);\n-\tif (right)\n-\t\tclear_commit_marks(right, ~0);\n-\n \tstrbuf_release(&sb);\n }\n \n+void show_submodule_summary(FILE *f, const char *path,\n+\t\tconst char *line_prefix,\n+\t\tstruct object_id *one, struct object_id *two,\n+\t\tunsigned dirty_submodule, const char *meta,\n+\t\tconst char *del, const char *add, const char *reset)\n+{\n+\tstruct rev_info rev;\n+\tstruct commit *left = NULL, *right = NULL;\n+\tstruct commit_list *merge_bases = NULL;\n+\n+\tshow_submodule_header(f, path, line_prefix, one, two, dirty_submodule,\n+\t\t\t meta, reset, &left, &right, &merge_bases);\n+\n+\t/*\n+\t * If we don't have both a left and a right pointer, there is no\n+\t * reason to try and display a summary. The header line should contain\n+\t * all the information the user needs.\n+\t */\n+\tif (!left || !right)\n+\t\tgoto out;\n+\n+\t/* Treat revision walker failure the same as missing commits */\n+\tif (prepare_submodule_summary(&rev, path, left, right, merge_bases)) {\n+\t\tfprintf(f, \"%s(revision walker failed)\\n\", line_prefix);\n+\t\tgoto out;\n+\t}\n+\n+\tprint_submodule_summary(&rev, f, line_prefix, del, add, reset);\n+\n+out:\n+\tif (merge_bases)\n+\t\tfree_commit_list(merge_bases);\n+\tclear_commit_marks(left, ~0);\n+\tclear_commit_marks(right, ~0);\n+}\n+\n void set_config_fetch_recurse_submodules(int value)\n {\n \tconfig_fetch_recurse_submodules = value;\n-- \n2.10.0.rc0.259.g83512d9\n\n"},{"id":"299889","messageId":"CAGZ79kbPhsWChrHKkjabZK0ZHZSPVE7VnwiTrxqrMPSzsmjHVg@mail.gmail.com","threadId":"43907","inReplyTo":"20160822234344.22797-2-jacob.e.keller@intel.com","subject":"Re: [PATCH v10 1/9] Git 2.10-rc1","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-23T00:28:23Z","receivedAt":"2016-08-23T00:28:30Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Aug 22, 2016 at 4:43 PM, Jacob Keller wrote:\n\nBad rebase?\n"},{"id":"299891","messageId":"CAGZ79kbKibe2RpQ9QWN1naY07feZLKbhc+dts3BD-4+y-pUOVw@mail.gmail.com","threadId":"43907","inReplyTo":"20160822234344.22797-1-jacob.e.keller@intel.com","subject":"Re: [PATCH v10 0/9] submodule inline diff format","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-23T01:00:56Z","receivedAt":"2016-08-23T01:08:10Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Aug 22, 2016 at 4:43 PM, Jacob Keller wrote:\n> From: Jacob Keller \n>\n> A few suggestions from Stefan in regards to falling back to\n> .git/modules/ being a bad idea. I've chosen I think to avoid using\n> die() as we just stick with the current path if we can't find its name.\n\nWhich makes the existing bug more subtle :(\n\n> I think this should be safe since we already do this today.\n\nIt's a bug today already. Thanks for spotting!\n\n> The new flow\n> only changes if we are able to lookup the submodule, so I don't think\n> it's worth adding a die() call.\n\nWell this series improves the buggy-ness as it is only buggy when the name\nis not found, and we fall back on the path.\n"},{"id":"299896","messageId":"CA+P7+xo6Vtxh9RuKAGh-vaK+rqitt-VMK-2KBTeh=4zoDB2s9Q@mail.gmail.com","threadId":"43907","inReplyTo":"CAGZ79kbPhsWChrHKkjabZK0ZHZSPVE7VnwiTrxqrMPSzsmjHVg@mail.gmail.com","subject":"Re: [PATCH v10 1/9] Git 2.10-rc1","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-08-23T06:21:06Z","receivedAt":"2016-08-23T06:22:05Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Mon, Aug 22, 2016 at 5:28 PM, Stefan Beller wrote:\n> On Mon, Aug 22, 2016 at 4:43 PM, Jacob Keller wrote:\n>\n> Bad rebase?\n\nYa not sure what happened here. Will find out tomorrow.\n\nThanks,\nJake\n"},{"id":"299898","messageId":"CA+P7+xqvwp74EvxGtm_anKTJW3f=JxJvJywrz-59_eRr0x9rVw@mail.gmail.com","threadId":"43907","inReplyTo":"CAGZ79kbKibe2RpQ9QWN1naY07feZLKbhc+dts3BD-4+y-pUOVw@mail.gmail.com","subject":"Re: [PATCH v10 0/9] submodule inline diff format","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-08-23T07:23:23Z","receivedAt":"2016-08-23T07:23:50Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Mon, Aug 22, 2016 at 6:00 PM, Stefan Beller wrote:\n> On Mon, Aug 22, 2016 at 4:43 PM, Jacob Keller wrote:\n>> From: Jacob Keller \n>>\n>> A few suggestions from Stefan in regards to falling back to\n>> .git/modules/ being a bad idea. I've chosen I think to avoid using\n>> die() as we just stick with the current path if we can't find its name.\n>\n> Which makes the existing bug more subtle :(\n>\n>> I think this should be safe since we already do this today.\n>\n> It's a bug today already. Thanks for spotting!\n>\n>> The new flow\n>> only changes if we are able to lookup the submodule, so I don't think\n>> it's worth adding a die() call.\n>\n> Well this series improves the buggy-ness as it is only buggy when the name\n> is not found, and we fall back on the path.\n\nWhich should fail because we already failed to read the file correctly\npreviously to this?\n\nWhat should the correct behavior be?\n\nWe need to support a few things I think:\n\na) checked out and initialized submodule\n\nb) initialized submodule which is no longer checked out\n\nc) repository checked out but not initialized, (ie: a fresh clone that\nis then added via \"git add\" after the fact (?)\n\nAny other scenarios we care about?\n\nWhat about:\n\ncloned, added, then the files removed.. should we actually die() in this case?\n\nAny other things we need to handle?\n\nThanks,\nJake\n"},{"id":"299966","messageId":"xmqqlgznwg6o.fsf@gitster.mtv.corp.google.com","threadId":"43907","inReplyTo":"CAGZ79kbKibe2RpQ9QWN1naY07feZLKbhc+dts3BD-4+y-pUOVw@mail.gmail.com","subject":"Re: [PATCH v10 0/9] submodule inline diff format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-23T17:25:35Z","receivedAt":"2016-08-23T17:25:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On Mon, Aug 22, 2016 at 4:43 PM, Jacob Keller wrote:\n>> From: Jacob Keller \n>>\n>> A few suggestions from Stefan in regards to falling back to\n>> .git/modules/ being a bad idea. I've chosen I think to avoid using\n>> die() as we just stick with the current path if we can't find its name.\n>\n> Which makes the existing bug more subtle :(\n>\n>> I think this should be safe since we already do this today.\n>\n> It's a bug today already. Thanks for spotting!\n>\n>> The new flow\n>> only changes if we are able to lookup the submodule, so I don't think\n>> it's worth adding a die() call.\n>\n> Well this series improves the buggy-ness as it is only buggy when the name\n> is not found, and we fall back on the path.\n\nI am not so sure about that. If there is an existing place that is\nbuggy, shouldn't we fix that, instead of spreading the same bug\n(assuming that it is a bug in the first place, which I do not have a\nstrong opinion on, at least not yet)?\n\nCan there be .git/modules// repository that is pointed at an\nin-tree .git file when there is no \"name\" defined? I thought we\nerrored out in module_name helper function in git-submodule.sh when\nwe need a name and only have path (I just checked in the maint-2.6\ntrack); did we break it recently? submodule--helper.c::module_name()\nseems to error out when submodule_from_path() fails to find one and\nwill segfault if it does not have name, so it is not likely.\n\n\n"},{"id":"299975","messageId":"CAGZ79kbsq52Qh0Jtn5hpPPfHu_9r2CUHvAHOTh6Cgz8ODkzreg@mail.gmail.com","threadId":"43907","inReplyTo":"xmqqlgznwg6o.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v10 0/9] submodule inline diff format","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-23T17:47:45Z","receivedAt":"2016-08-23T17:56:52Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Aug 23, 2016 at 10:25 AM, Junio C Hamano wrote:\n> I am not so sure about that. If there is an existing place that is\n> buggy, shouldn't we fix that, instead of spreading the same bug\n> (assuming that it is a bug in the first place, which I do not have a\n> strong opinion on, at least not yet)?\n>\n> Can there be .git/modules// repository that is pointed at an\n> in-tree .git file when there is no \"name\" defined?\n\nIf you're holding it wrong we can come into that state.\n* Checkout the submodule,\n* then remove .gitmodules as well as relevant config in .git/config.\nResult: Then we have a only a gitlink recorded as well as connected\nworking tree to a gitdir inside a superprojects .git/modules/.\n\n> I thought we\n> errored out in module_name helper function in git-submodule.sh when\n> we need a name and only have path (I just checked in the maint-2.6\n> track); did we break it recently? submodule--helper.c::module_name()\n> seems to error out when submodule_from_path() fails to find one and\n> will segfault if it does not have name, so it is not likely.\n\nThe name is literally the only thing that is not optional in a struct submodule\n(see submodule-config.c:182 In lookup_or_create_by_name, these structs are\nadded to the internal cache.\n\nStepping back a bit, I think we'd want to document this expectation more\nin the man pages\n The name unlike the path of a submodule must not be changed (as the\n name is used internally to point at the submodules git dir)\n"},{"id":"300015","messageId":"xmqqfupvt776.fsf@gitster.mtv.corp.google.com","threadId":"43907","inReplyTo":"20160822234344.22797-9-jacob.e.keller@intel.com","subject":"Re: [PATCH v10 8/9] submodule: refactor show_submodule_summary with helper function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-23T23:07:57Z","receivedAt":"2016-08-23T23:08:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller writes:\n\n> From: Jacob Keller \n>\n> A future patch is going to add a new submodule diff format which\n> displays an inline diff of the submodule changes. To make this easier,\n> and to ensure that both submodule diff formats use the same initial\n> header, factor out show_submodule_header() function which will print the\n> current submodule header line, and then leave the show_submodule_summary\n> function to lookup and print the submodule log format.\n>\n> This does create one format change in that \"(revision walker failed)\"\n> will now be displayed on its own line rather than as part of the message\n> because we no longer perform this step directly in the header display\n> flow. However, this is a rare case as most causes of the failure will be\n> due to a missing commit which we already check for and avoid previously.\n> flow. However, this is a rare case and shouldn't impact much.\n>\n> Signed-off-by: Jacob Keller \n> ---\n\nUp to this step it all looked sensible. I'll take a look at 9/9\nlater.\n\nThanks.\n"},{"id":"300022","messageId":"CA+P7+xoae0vdMNd7zv7L7r68VcZ2zJPBoyvc6nzDDn-z9WUcFg@mail.gmail.com","threadId":"43907","inReplyTo":"xmqqfupvt776.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v10 8/9] submodule: refactor show_submodule_summary with helper function","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-08-24T06:28:25Z","receivedAt":"2016-08-24T06:28:56Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Tue, Aug 23, 2016 at 4:07 PM, Junio C Hamano wrote:\n> Jacob Keller writes:\n>\n>> From: Jacob Keller \n>>\n>> A future patch is going to add a new submodule diff format which\n>> displays an inline diff of the submodule changes. To make this easier,\n>> and to ensure that both submodule diff formats use the same initial\n>> header, factor out show_submodule_header() function which will print the\n>> current submodule header line, and then leave the show_submodule_summary\n>> function to lookup and print the submodule log format.\n>>\n>> This does create one format change in that \"(revision walker failed)\"\n>> will now be displayed on its own line rather than as part of the message\n>> because we no longer perform this step directly in the header display\n>> flow. However, this is a rare case as most causes of the failure will be\n>> due to a missing commit which we already check for and avoid previously.\n>> flow. However, this is a rare case and shouldn't impact much.\n>>\n>> Signed-off-by: Jacob Keller \n>> ---\n>\n> Up to this step it all looked sensible. I'll take a look at 9/9\n> later.\n>\n> Thanks.\n\nThere is still some outstanding comments from Stefan that I am looking\nat, whether to use a die() or not in do_submodule_path.\n\nThanks,\nJake\n"},{"id":"300193","messageId":"CA+P7+xpiHmTBqF1RT8zUd=1g=nKtMiwpH19DyhwLi9ikLQ8t8A@mail.gmail.com","threadId":"43907","inReplyTo":"xmqqlgznwg6o.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v10 0/9] submodule inline diff format","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-08-25T20:35:34Z","receivedAt":"2016-08-25T20:36:42Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Tue, Aug 23, 2016 at 10:25 AM, Junio C Hamano wrote:\n> I am not so sure about that. If there is an existing place that is\n> buggy, shouldn't we fix that, instead of spreading the same bug\n> (assuming that it is a bug in the first place, which I do not have a\n> strong opinion on, at least not yet)?\n>\n\nI was saying that I'm not sure it is a bug so I sided with preserving\nbehavior instead.\n\nIf it is indeed a bug we should fix it, and I'm trying to determine\nwhether it actually is a bug here, and what the best solution is.\n\nIf there is a bug and we should die, should we also introduce a\n\"_gently()\" variant of these functions and thus fail properly if they\ndon't work so that we can report an error in producing a diff instead\nof die()ing?\n\nThanks,\nJake\n"},{"id":"300194","messageId":"CA+P7+xrQy33gt8bnsagLVPAHhZPZx-3s0E_Aj7tOwXnHjOoCBg@mail.gmail.com","threadId":"43907","inReplyTo":"CAGZ79kbsq52Qh0Jtn5hpPPfHu_9r2CUHvAHOTh6Cgz8ODkzreg@mail.gmail.com","subject":"Re: [PATCH v10 0/9] submodule inline diff format","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-08-25T20:39:18Z","receivedAt":"2016-08-25T20:40:19Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Tue, Aug 23, 2016 at 10:47 AM, Stefan Beller wrote:\n> On Tue, Aug 23, 2016 at 10:25 AM, Junio C Hamano wrote:\n>> I am not so sure about that. If there is an existing place that is\n>> buggy, shouldn't we fix that, instead of spreading the same bug\n>> (assuming that it is a bug in the first place, which I do not have a\n>> strong opinion on, at least not yet)?\n>>\n>> Can there be .git/modules// repository that is pointed at an\n>> in-tree .git file when there is no \"name\" defined?\n>\n> If you're holding it wrong we can come into that state.\n> * Checkout the submodule,\n> * then remove .gitmodules as well as relevant config in .git/config.\n> Result: Then we have a only a gitlink recorded as well as connected\n> working tree to a gitdir inside a superprojects .git/modules/.\n>\n\nYea, but I think you're right that we shouldn't support that state.\nWhat I'm worried about is the case where we can get this state doing\nsomething sane/acceptable but I don't think we can.\n\nSo we should support the gitlink to a repository stored at \nwithout stuff inside the .git/modules, and we should support submodule\ngitlinks with a proper .gitmodules setup. I don't think we should\ndie() but we should error properly so I will introduce a _gently()\nvariant of these functions, and die properly in the regular flow.\n\n>> I thought we\n>> errored out in module_name helper function in git-submodule.sh when\n>> we need a name and only have path (I just checked in the maint-2.6\n>> track); did we break it recently? submodule--helper.c::module_name()\n>> seems to error out when submodule_from_path() fails to find one and\n>> will segfault if it does not have name, so it is not likely.\n>\n> The name is literally the only thing that is not optional in a struct submodule\n> (see submodule-config.c:182 In lookup_or_create_by_name, these structs are\n> added to the internal cache.\n>\n> Stepping back a bit, I think we'd want to document this expectation more\n> in the man pages\n> The name unlike the path of a submodule must not be changed (as the\n> name is used internally to point at the submodules git dir)\n\nAgreed, this should be documented. Do you mind doing the documentation\npatch for this?\n\nThanks,\nJake\n"},{"id":"300196","messageId":"CAGZ79kb3tRU5vLDMWzEC=bx9Givd8pqFsAzTBy0unxXfKpNnUw@mail.gmail.com","threadId":"43907","inReplyTo":"CA+P7+xrQy33gt8bnsagLVPAHhZPZx-3s0E_Aj7tOwXnHjOoCBg@mail.gmail.com","subject":"Re: [PATCH v10 0/9] submodule inline diff format","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-25T20:46:00Z","receivedAt":"2016-08-25T20:46:41Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Aug 25, 2016 at 1:39 PM, Jacob Keller wrote:\n> On Tue, Aug 23, 2016 at 10:47 AM, Stefan Beller wrote:\n>> On Tue, Aug 23, 2016 at 10:25 AM, Junio C Hamano wrote:\n>>> I am not so sure about that. If there is an existing place that is\n>>> buggy, shouldn't we fix that, instead of spreading the same bug\n>>> (assuming that it is a bug in the first place, which I do not have a\n>>> strong opinion on, at least not yet)?\n>>>\n>>> Can there be .git/modules// repository that is pointed at an\n>>> in-tree .git file when there is no \"name\" defined?\n>>\n>> If you're holding it wrong we can come into that state.\n>> * Checkout the submodule,\n>> * then remove .gitmodules as well as relevant config in .git/config.\n>> Result: Then we have a only a gitlink recorded as well as connected\n>> working tree to a gitdir inside a superprojects .git/modules/.\n>>\n>\n> Yea, but I think you're right that we shouldn't support that state.\n> What I'm worried about is the case where we can get this state doing\n> something sane/acceptable but I don't think we can.\n>\n> So we should support the gitlink to a repository stored at \n> without stuff inside the .git/modules, and we should support submodule\n> gitlinks with a proper .gitmodules setup. I don't think we should\n> die() but we should error properly so I will introduce a _gently()\n> variant of these functions, and die properly in the regular flow.\n\nok, thanks!\n\n>> Stepping back a bit, I think we'd want to document this expectation more\n>> in the man pages\n>> The name unlike the path of a submodule must not be changed (as the\n>> name is used internally to point at the submodules git dir)\n>\n> Agreed, this should be documented. Do you mind doing the documentation\n> patch for this?\n\nWell that documentation is sort of unrelated to this series, so no pressure.\nI have a revamp of the whole submodule documentation on my wish/TODO list,\nincluding this.\n\nThanks,\nStefan\n\n>\n> Thanks,\n> Jake\n"},{"id":"300204","messageId":"CA+P7+xqDUzuX2c4y3vpPEenBRbQ_mttDhDOviV1=k7H-LQVXmQ@mail.gmail.com","threadId":"43907","inReplyTo":"CAGZ79kb3tRU5vLDMWzEC=bx9Givd8pqFsAzTBy0unxXfKpNnUw@mail.gmail.com","subject":"Re: [PATCH v10 0/9] submodule inline diff format","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-08-25T22:16:06Z","receivedAt":"2016-08-25T22:16:38Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Aug 25, 2016 at 1:46 PM, Stefan Beller wrote:\n> On Thu, Aug 25, 2016 at 1:39 PM, Jacob Keller wrote:\n>> On Tue, Aug 23, 2016 at 10:47 AM, Stefan Beller wrote:\n>>> On Tue, Aug 23, 2016 at 10:25 AM, Junio C Hamano wrote:\n>>>> I am not so sure about that. If there is an existing place that is\n>>>> buggy, shouldn't we fix that, instead of spreading the same bug\n>>>> (assuming that it is a bug in the first place, which I do not have a\n>>>> strong opinion on, at least not yet)?\n>>>>\n>>>> Can there be .git/modules// repository that is pointed at an\n>>>> in-tree .git file when there is no \"name\" defined?\n>>>\n>>> If you're holding it wrong we can come into that state.\n>>> * Checkout the submodule,\n>>> * then remove .gitmodules as well as relevant config in .git/config.\n>>> Result: Then we have a only a gitlink recorded as well as connected\n>>> working tree to a gitdir inside a superprojects .git/modules/.\n>>>\n>>\n>> Yea, but I think you're right that we shouldn't support that state.\n>> What I'm worried about is the case where we can get this state doing\n>> something sane/acceptable but I don't think we can.\n>>\n>> So we should support the gitlink to a repository stored at \n>> without stuff inside the .git/modules, and we should support submodule\n>> gitlinks with a proper .gitmodules setup. I don't think we should\n>> die() but we should error properly so I will introduce a _gently()\n>> variant of these functions, and die properly in the regular flow.\n>\n> ok, thanks!\n\n\nOk so a die() doesn't really work. If you use submodule_from_path here\non a newly checked out repository that has sub modules but you haven't\nyet initialized the repository, then the result is that\nsubmodule_from_path will fail.. Is that expected?\n\nThat causes us to die every time on a newly checked out repository.\n\nIf we go with this behavior, then I have to refactor the whole set of\npath() functions to have a _gently() variant which handles this\ngracefully, and the regular revision code would still end up die()ing\nas well......\n\nThanks,\nJake\n"},{"id":"300207","messageId":"xmqqr39cjwxx.fsf@gitster.mtv.corp.google.com","threadId":"43907","inReplyTo":"CA+P7+xrQy33gt8bnsagLVPAHhZPZx-3s0E_Aj7tOwXnHjOoCBg@mail.gmail.com","subject":"Re: [PATCH v10 0/9] submodule inline diff format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-25T22:38:50Z","receivedAt":"2016-08-25T22:45:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller writes:\n\n> So we should support the gitlink to a repository stored at \n> without stuff inside the .git/modules, and we should support submodule\n> gitlinks with a proper .gitmodules setup. I don't think we should\n> die() but we should error properly so I will introduce a _gently()\n> variant of these functions, and die properly in the regular flow.\n\nBecause \"git diff [--cached] []\" in the top-level is\ndriven by a gitlink in the index, immediately after adding a new\nsubmodule to the index but before describing it in .gitmodules you\nmight not have a name (and you know in that case the path will\nbecome the name when adding it to .gitmodules). Also a gitlink in\nthe index may correspond to a submodule the user of the top-level is\nnot interested in, so there may not be anything in .git/modules/\nthat corresponds to it. In these cases, I suspect that you do not\nwant to die, but you can just tell the user \"I do not have enough\ninformation to tell you a useful story yet\".\n\n"},{"id":"300208","messageId":"CA+P7+xqDRD4akNShqxs4D2dDUBK0E5Eyp2Y3-8xK6wEb5gCU6w@mail.gmail.com","threadId":"43907","inReplyTo":"xmqqr39cjwxx.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v10 0/9] submodule inline diff format","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-08-25T22:46:00Z","receivedAt":"2016-08-25T22:46:26Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Aug 25, 2016 at 3:38 PM, Junio C Hamano wrote:\n> Jacob Keller writes:\n>\n>> So we should support the gitlink to a repository stored at \n>> without stuff inside the .git/modules, and we should support submodule\n>> gitlinks with a proper .gitmodules setup. I don't think we should\n>> die() but we should error properly so I will introduce a _gently()\n>> variant of these functions, and die properly in the regular flow.\n>\n> Because \"git diff [--cached] []\" in the top-level is\n> driven by a gitlink in the index, immediately after adding a new\n> submodule to the index but before describing it in .gitmodules you\n> might not have a name (and you know in that case the path will\n> become the name when adding it to .gitmodules). Also a gitlink in\n> the index may correspond to a submodule the user of the top-level is\n> not interested in, so there may not be anything in .git/modules/\n> that corresponds to it. In these cases, I suspect that you do not\n> want to die, but you can just tell the user \"I do not have enough\n> information to tell you a useful story yet\".\n>\n\nRight. submodule_from_path() fails to find a config. I don't think\ndie() is right here, because there is no easy way to make this into a\ngently() variant.... I can still do it if we think a die() is\nworthwhile otherwise for the other callers of do_submodule_path...\n\nHowever, I think the safest thing is to just:\n\na) read_gitfile on /.git\nb) if read_gitfile succeeds, use it's contents, otherwise use\n/.git for next steps\nc) check if the resulting file is a git directory, we're fine.. we\nfound a gitdir, so stop.\nd) otherwise, empty the buffer, then lookup submodules\ne) when submodules lookup succeeds.. see if we found a name. If so, use that.\nf) if we didn't just exit with an empty buffer.\n\nThat empty buffer *should* trigger revision error codes since it\nwon't point to any valid path and it also triggers the regular error\ncode in add_submodule_odb so it handles that with showing not\ninitizlied.\n\nThis method is less work then re-implementing a _gently() variant for\nall of these functions.\n\nStefan, does this make sense and seem reasonable?\n\nIf we just die when submodule_from_path fails I think it's bad for the\nsubmodule=* formats beside short. I don't know if it causes problems\nfor revision code or not... I think falling back to resetting the\nbuffer as our way of indicating error is reasonable enough...\n\nThanks,\nJake\n"},{"id":"300274","messageId":"CAGZ79kYbHz2E6-0qNS47KfA5Gs=Ew327LxhOyq+i3axwPVHKGQ@mail.gmail.com","threadId":"43907","inReplyTo":"CA+P7+xqDRD4akNShqxs4D2dDUBK0E5Eyp2Y3-8xK6wEb5gCU6w@mail.gmail.com","subject":"Re: [PATCH v10 0/9] submodule inline diff format","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-26T17:35:44Z","receivedAt":"2016-08-26T17:35:52Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Aug 25, 2016 at 3:46 PM, Jacob Keller wrote:\n> On Thu, Aug 25, 2016 at 3:38 PM, Junio C Hamano wrote:\n>> Jacob Keller writes:\n>>\n>>> So we should support the gitlink to a repository stored at \n>>> without stuff inside the .git/modules, and we should support submodule\n>>> gitlinks with a proper .gitmodules setup. I don't think we should\n>>> die() but we should error properly so I will introduce a _gently()\n>>> variant of these functions, and die properly in the regular flow.\n>>\n>> Because \"git diff [--cached] []\" in the top-level is\n>> driven by a gitlink in the index, immediately after adding a new\n>> submodule to the index but before describing it in .gitmodules you\n>> might not have a name (and you know in that case the path will\n>> become the name when adding it to .gitmodules). Also a gitlink in\n>> the index may correspond to a submodule the user of the top-level is\n>> not interested in, so there may not be anything in .git/modules/\n>> that corresponds to it. In these cases, I suspect that you do not\n>> want to die, but you can just tell the user \"I do not have enough\n>> information to tell you a useful story yet\".\n>>\n>\n> Right. submodule_from_path() fails to find a config. I don't think\n> die() is right here, because there is no easy way to make this into a\n> gently() variant.... I can still do it if we think a die() is\n> worthwhile otherwise for the other callers of do_submodule_path...\n>\n> However, I think the safest thing is to just:\n>\n> a) read_gitfile on /.git\n> b) if read_gitfile succeeds, use it's contents, otherwise use\n> /.git for next steps\n> c) check if the resulting file is a git directory, we're fine.. we\n> found a gitdir, so stop.\n> d) otherwise, empty the buffer, then lookup submodules\n> e) when submodules lookup succeeds.. see if we found a name. If so, use that.\n\nWhen the submodules lookup succeeds, we can assert the name exists.\nThere is currently only one way the lookup is populated, and that is\nlookup_or_create_by_name in submodule-config.c:182, which fills in\nthe name all the time.\n\n> f) if we didn't just exit with an empty buffer.\n>\n> That empty buffer *should* trigger revision error codes since it\n> won't point to any valid path and it also triggers the regular error\n> code in add_submodule_odb so it handles that with showing not\n> initizlied.\n>\n> This method is less work then re-implementing a _gently() variant for\n> all of these functions.\n>\n> Stefan, does this make sense and seem reasonable?\n\nSounds reasonable to me.\n\nThanks for working on this!\nStefan\n"},{"id":"300283","messageId":"1472236015.28343.3.camel@intel.com","threadId":"43907","inReplyTo":"CAGZ79kYbHz2E6-0qNS47KfA5Gs=Ew327LxhOyq+i3axwPVHKGQ@mail.gmail.com","subject":"Re: [PATCH v10 0/9] submodule inline diff format","fromName":"Keller, Jacob E","fromEmail":"jacob.e.keller@intel.com","sentAt":"2016-08-26T18:26:55Z","receivedAt":"2016-08-26T18:27:02Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Fri, 2016-08-26 at 10:35 -0700, Stefan Beller wrote:\n> > a) read_gitfile on /.git\n> > b) if read_gitfile succeeds, use it's contents, otherwise use\n> > /.git for next steps\n> > c) check if the resulting file is a git directory, we're fine.. we\n> > found a gitdir, so stop.\n> > d) otherwise,  empty the buffer, then lookup submodules\n> > e) when submodules lookup succeeds.. see if we found a name. If so,\n> > use that.\n> \n> When the submodules lookup succeeds, we can assert the name exists.\n> There is currently only one way the lookup is populated, and that is\n> lookup_or_create_by_name in submodule-config.c:182, which fills in\n> the name all the time.\n\nYes, that was how I was trying to word it, and that's what I've done in\ncode.\n\n> \n> > \n> > f) if we didn't just exit with an empty buffer.\n> > \n> > That empty buffer *should* trigger  revision error codes since it\n> > won't point to any valid path and it also triggers the regular\n> > error\n> > code in add_submodule_odb so it handles that with showing not\n> > initizlied.\n> > \n> > This method is less work then re-implementing a _gently() variant\n> > for\n> > all of these functions.\n> > \n> > Stefan, does this make sense and seem reasonable?\n> \n> Sounds reasonable to me.\n> \n> Thanks for working on this!\n> Stefan\n\nThanks for review!\n\nRegards,\nJake\n"}]}