{"thread":{"id":"43040","subject":"[PATCH RFC] diff: add SUBMODULE_DIFF format to display submodule diff","startedAt":"2016-08-09T22:32:26Z","lastAt":"2016-08-10T19:44:29Z","messageCount":7,"participants":["Jacob Keller","Junio C Hamano","Stefan Beller","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"293545","messageId":"20160809223219.17982-1-jacob.e.keller@intel.com","threadId":"43040","inReplyTo":null,"subject":"[PATCH RFC] diff: add SUBMODULE_DIFF format to display submodule diff","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2016-08-09T22:32:19Z","receivedAt":"2016-08-09T22:32:26Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nFor projects which have frequent updates to submodules it is often\nuseful to be able to see a submodule update commit as a difference.\nTeach diff's --submodule= a new \"diff\" format which will execute a diff\nfor the submodule between the old and new commit, and display it as\na standard diff. This allows users to easily see what changed in\na submodule without having to dig into the submodule themselves.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\n---\nThere are no tests yet. Stefan suggested the use of child_process, but I\nreally believe there has to be some way of getting the diff without\nhaving to run a sub process (as this means we also can't do the diff\nwithout a checked out submodule). It doesn't properly handle options\neither, so a better solution would be much appreciated.\n\nI also would prefer if the diff actually prefixed the files with\n\"path/to/submodule\" so that it was obvious where the file was located in\nthe directory.\n\nSuggestions welcome.\n\n Documentation/diff-config.txt  |  3 +-\n Documentation/diff-options.txt |  6 ++--\n diff.c                         | 21 +++++++++++--\n diff.h                         |  2 +-\n submodule.c                    | 67 ++++++++++++++++++++++++++++++++++++++++++\n submodule.h                    |  5 ++++\n 6 files changed, 97 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt\nindex d5a5b17d5088..f5d693afad6c 100644\n--- a/Documentation/diff-config.txt\n+++ b/Documentation/diff-config.txt\n@@ -123,7 +123,8 @@ diff.suppressBlankEmpty::\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+\tlinkgit:git-submodule[1] `summary` does.  The \"diff\" format shows the\n+\tdiff between the beginning and end of the range. 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 \ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 705a87394200..b17348407805 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -215,8 +215,10 @@ any of those replacements occurred.\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+\tat the beginning and end of the range. When `--submodule=diff` is\n+\tgiven, the 'diff' format is used. This format shows the diff between\n+\tthe old and new submodule commmit from the perspective of the\n+\tsubmodule.  Can be tweaked via the `diff.submodule` configuration variable.\n \n --color[=<when>]::\n \tShow colored diff.\ndiff --git a/diff.c b/diff.c\nindex b43d3dd2ecb7..74d43931c8df 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -130,12 +130,18 @@ static int parse_dirstat_params(struct diff_options *options, const char *params\n \n static int parse_submodule_params(struct diff_options *options, const char *value)\n {\n-\tif (!strcmp(value, \"log\"))\n+\tif (!strcmp(value, \"log\")) {\n \t\tDIFF_OPT_SET(options, SUBMODULE_LOG);\n-\telse if (!strcmp(value, \"short\"))\n+\t\tDIFF_OPT_CLR(options, SUBMODULE_DIFF);\n+\telse if (!strcmp(value, \"diff\")) {\n+\t\tDIFF_OPT_SET(options, SUBMODULE_DIFF);\n \t\tDIFF_OPT_CLR(options, SUBMODULE_LOG);\n-\telse\n+\t} else if (!strcmp(value, \"short\")) {\n+\t\tDIFF_OPT_CLR(options, SUBMODULE_DIFF);\n+\t\tDIFF_OPT_CLR(options, SUBMODULE_LOG);\n+\t} else {\n \t\treturn -1;\n+\t}\n \treturn 0;\n }\n \n@@ -2310,6 +2316,15 @@ 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 (DIFF_OPT_TST(o, SUBMODULE_DIFF) &&\n+\t\t\t(!one->mode || S_ISGITLINK(one->mode)) &&\n+\t\t\t(!two->mode || S_ISGITLINK(two->mode))) {\n+\t\tshow_submodule_diff(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\ttwo->dirty_submodule,\n+\t\t\t\tmeta, reset);\n+\t\treturn;\n \t}\n \n \tif (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) {\ndiff --git a/diff.h b/diff.h\nindex 125447be09eb..a80f3e5bafe9 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -69,7 +69,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n #define DIFF_OPT_FIND_COPIES_HARDER  (1 <<  6)\n #define DIFF_OPT_FOLLOW_RENAMES      (1 <<  7)\n #define DIFF_OPT_RENAME_EMPTY        (1 <<  8)\n-/* (1 <<  9) unused */\n+#define DIFF_OPT_SUBMODULE_DIFF      (1 <<  9)\n #define DIFF_OPT_HAS_CHANGES         (1 << 10)\n #define DIFF_OPT_QUICK               (1 << 11)\n #define DIFF_OPT_NO_INDEX            (1 << 12)\ndiff --git a/submodule.c b/submodule.c\nindex 1b5cdfb7e784..4a322513d27c 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -333,6 +333,73 @@ static void print_submodule_summary(struct rev_info *rev, FILE *f,\n \tstrbuf_release(&sb);\n }\n \n+static int prepare_submodule_diff(struct strbuf *buf, const char *path,\n+\t\tunsigned char one[20], unsigned char two[20])\n+{\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tcp.git_cmd = 1;\n+\tcp.dir = path;\n+\tcp.out = -1;\n+\tcp.no_stdin = 1;\n+\targv_array_push(&cp.args, \"diff\");\n+\targv_array_push(&cp.args, sha1_to_hex(one));\n+\targv_array_push(&cp.args, sha1_to_hex(two));\n+\n+\tif (start_command(&cp))\n+\t\treturn -1;\n+\n+\tif (strbuf_read(buf, cp.out, 0) < 0)\n+\t\treturn -1;\n+\n+\tif (finish_command(&cp))\n+\t\treturn -1;\n+\n+\treturn 0;\n+}\n+\n+void show_submodule_diff(FILE *f, const char *path,\n+\t\tconst char *line_prefix,\n+\t\tunsigned char one[20], unsigned char two[20],\n+\t\tunsigned dirty_submodule, const char *meta,\n+\t\tconst char *reset)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tconst char *message = NULL;\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+\tif (dirty_submodule & DIRTY_SUBMODULE_MODIFIED)\n+\t\tfprintf(f, \"%sSubmodule %s contains modified content\\n\",\n+\t\t\tline_prefix, path);\n+\n+\tif (!hashcmp(one, two)) {\n+\t\tstrbuf_release(&sb);\n+\t\treturn;\n+\t}\n+\n+\tif (add_submodule_odb(path))\n+\t\tmessage = \"(not checked out)\";\n+\telse if (prepare_submodule_diff(&buf, path, one, two))\n+\t\tmessage = \"(diff failed)\";\n+\n+\tstrbuf_addf(&sb, \"%s%sSubmodule %s %s..\", line_prefix, meta, path,\n+\t\t\tfind_unique_abbrev(one, DEFAULT_ABBREV));\n+\tstrbuf_addf(&sb, \"%s\", find_unique_abbrev(two, DEFAULT_ABBREV));\n+\tif (message)\n+\t\tstrbuf_addf(&sb, \" %s%s\\n\", message, reset);\n+\telse\n+\t\tstrbuf_addf(&sb, \":%s\\n\", reset);\n+\tfwrite(sb.buf, sb.len, 1, f);\n+\n+\tif (!message) /* only NULL if we succeeded in obtaining a diff */\n+\t\tfwrite(buf.buf, buf.len, 1, f);\n+\n+\tstrbuf_release(&buf);\n+\tstrbuf_release(&sb);\n+}\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],\ndiff --git a/submodule.h b/submodule.h\nindex 2af939099819..f9180712ae2c 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -41,6 +41,11 @@ int parse_submodule_update_strategy(const char *value,\n \t\tstruct submodule_update_strategy *dst);\n const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);\n void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);\n+void show_submodule_diff(FILE *f, const char *path,\n+\t\tconst char *line_prefix,\n+\t\tunsigned char one[20], unsigned char two[20],\n+\t\tunsigned dirty_submodule, const char *meta,\n+\t\tconst char *reset);\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-- \n2.9.2.1004.gdad50a3\n\n"},{"id":"293546","messageId":"xmqq37md8t2a.fsf@gitster.mtv.corp.google.com","threadId":"43040","inReplyTo":"20160809223219.17982-1-jacob.e.keller@intel.com","subject":"Re: [PATCH RFC] diff: add SUBMODULE_DIFF format to display submodule diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-09T22:40:29Z","receivedAt":"2016-08-09T22:40:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller <jacob.e.keller@intel.com> writes:\n\n> +static int prepare_submodule_diff(struct strbuf *buf, const char *path,\n> +\t\tunsigned char one[20], unsigned char two[20])\n> +{\n> +\tstruct child_process cp = CHILD_PROCESS_INIT;\n> +\tcp.git_cmd = 1;\n> +\tcp.dir = path;\n> +\tcp.out = -1;\n> +\tcp.no_stdin = 1;\n> +\targv_array_push(&cp.args, \"diff\");\n> +\targv_array_push(&cp.args, sha1_to_hex(one));\n> +\targv_array_push(&cp.args, sha1_to_hex(two));\n> +\n> +\tif (start_command(&cp))\n> +\t\treturn -1;\n> +\n> +\tif (strbuf_read(buf, cp.out, 0) < 0)\n> +\t\treturn -1;\n> +\n> +\tif (finish_command(&cp))\n> +\t\treturn -1;\n> +\n> +\treturn 0;\n> +}\n\nIt is a good idea to keep the submodule data isolated from the main\nprocess by going through run-command/start-command interface (I\nthink the call to add_submodule_odb() should be rethought and\nremoved if possible).\n\nI however wonder if you want to use src/dst-prefix to make the path\nto the submodule appear there?  That is, if your superproject has a\nsubmodule at \"dir/\" and two versions of the submodule changes a file\n\"doc/README\", wouldn't you rather want to see\n\n\tdiff --git a/dir/doc/README b/dir/doc/README\n\ninstead of comparison between a/doc/README and b/doc/README?\n"},{"id":"293547","messageId":"CAGZ79kbzDhpsfpY8s=7O86_dNLA=VRkyC=8eiCqMambKvUnXmw@mail.gmail.com","threadId":"43040","inReplyTo":"20160809223219.17982-1-jacob.e.keller@intel.com","subject":"Re: [PATCH RFC] diff: add SUBMODULE_DIFF format to display submodule diff","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T22:50:48Z","receivedAt":"2016-08-09T22:50:58Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Aug 9, 2016 at 3:32 PM, Jacob Keller <jacob.e.keller@intel.com> wrote:\n> From: Jacob Keller <jacob.keller@gmail.com>\n>\n> For projects which have frequent updates to submodules it is often\n> useful to be able to see a submodule update commit as a difference.\n> Teach diff's --submodule= a new \"diff\" format which will execute a diff\n> for the submodule between the old and new commit, and display it as\n> a standard diff. This allows users to easily see what changed in\n> a submodule without having to dig into the submodule themselves.\n>\n> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>\n> ---\n> There are no tests yet. Stefan suggested the use of child_process,\n\nWell you said \"I just want this one command but don't know where to put it \"\nso it's natural to suggest using child_process.  ;)\n\n> but I\n> really believe there has to be some way of getting the diff without\n> having to run a sub process (as this means we also can't do the diff\n> without a checked out submodule). It doesn't properly handle options\n> either, so a better solution would be much appreciated.\n\nOh right, we would need to codify all options again (and all future options,\ntoo)\n\n>\n> I also would prefer if the diff actually prefixed the files with\n> \"path/to/submodule\" so that it was obvious where the file was located in\n> the directory.\n>\n> Suggestions welcome.\n>\n> +\n> +       if (start_command(&cp))\n\nI wonder if we want to stay single-execution here,\ni.e. if we rather want to use run_processes_parallel\nfor all the submodules at the same time?\n\nThough then non deterministic ordering may be an issue.\n\n> +               return -1;\n> +\n> +       if (strbuf_read(buf, cp.out, 0) < 0)\n\nSo we keep the whole diff in memory\nI don't know much about the diff machinery, but I thought\nthe rest of the diff machinery just streams it out?\n\n> +\n> +void show_submodule_diff(FILE *f, const char *path,\n> +               const char *line_prefix,\n> +               unsigned char one[20], unsigned char two[20],\n> +               unsigned dirty_submodule, const char *meta,\n> +               const char *reset)\n> +{\n> +       struct strbuf buf = STRBUF_INIT;\n> +       struct strbuf sb = STRBUF_INIT;\n> +       const char *message = NULL;\n> +\n> +       if (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)\n> +               fprintf(f, \"%sSubmodule %s contains untracked content\\n\",\n> +                       line_prefix, path);\n> +       if (dirty_submodule & DIRTY_SUBMODULE_MODIFIED)\n> +               fprintf(f, \"%sSubmodule %s contains modified content\\n\",\n> +                       line_prefix, path);\n> +\n> +       if (!hashcmp(one, two)) {\n> +               strbuf_release(&sb);\n> +               return;\n> +       }\n> +\n> +       if (add_submodule_odb(path))\n> +               message = \"(not checked out)\";\n\nWhen not checked out, we can invoke the diff command\nin .git/modules/<name> as that is the git dir of the submodule,\ni.e. operating diff with a bare repo?\n\nThanks,\nStefan\n"},{"id":"293548","messageId":"CA+P7+xqnALzwkgLXrP57BUtFGaxGudAEG7cmGTakP47ofFEjHQ@mail.gmail.com","threadId":"43040","inReplyTo":"CAGZ79kbzDhpsfpY8s=7O86_dNLA=VRkyC=8eiCqMambKvUnXmw@mail.gmail.com","subject":"Re: [PATCH RFC] diff: add SUBMODULE_DIFF format to display submodule diff","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-08-09T22:56:41Z","receivedAt":"2016-08-09T22:57:06Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Tue, Aug 9, 2016 at 3:50 PM, Stefan Beller <sbeller@google.com> wrote:\n> On Tue, Aug 9, 2016 at 3:32 PM, Jacob Keller <jacob.e.keller@intel.com> wrote:\n>> From: Jacob Keller <jacob.keller@gmail.com>\n>>\n>> For projects which have frequent updates to submodules it is often\n>> useful to be able to see a submodule update commit as a difference.\n>> Teach diff's --submodule= a new \"diff\" format which will execute a diff\n>> for the submodule between the old and new commit, and display it as\n>> a standard diff. This allows users to easily see what changed in\n>> a submodule without having to dig into the submodule themselves.\n>>\n>> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>\n>> ---\n>> There are no tests yet. Stefan suggested the use of child_process,\n>\n> Well you said \"I just want this one command but don't know where to put it \"\n> so it's natural to suggest using child_process.  ;)\n\nRight this is a straight forward way..\n\n>\n>> but I\n>> really believe there has to be some way of getting the diff without\n>> having to run a sub process (as this means we also can't do the diff\n>> without a checked out submodule). It doesn't properly handle options\n>> either, so a better solution would be much appreciated.\n>\n> Oh right, we would need to codify all options again (and all future options,\n> too)\n\n\nThat's why I'd prefer if we had a way to do this via builtins, because\nit would make option passing easier.\n\n>\n>>\n>> I also would prefer if the diff actually prefixed the files with\n>> \"path/to/submodule\" so that it was obvious where the file was located in\n>> the directory.\n>>\n>> Suggestions welcome.\n>>\n>> +\n>> +       if (start_command(&cp))\n>\n> I wonder if we want to stay single-execution here,\n> i.e. if we rather want to use run_processes_parallel\n> for all the submodules at the same time?\n>\n> Though then non deterministic ordering may be an issue.\n\nThere is no easy way to do this, we're given it one module at a time\nand it would be a huge re-write to make it go in parallel. I think\nthat's the wrong way to think about this.\n\n>\n>> +               return -1;\n>> +\n>> +       if (strbuf_read(buf, cp.out, 0) < 0)\n>\n> So we keep the whole diff in memory\n> I don't know much about the diff machinery, but I thought\n> the rest of the diff machinery just streams it out?\n\nYea, but I can't figure out how to do that. Is there an easy way to\nstream chunks from the pipe straight into the file?\n\n>\n>> +\n>> +void show_submodule_diff(FILE *f, const char *path,\n>> +               const char *line_prefix,\n>> +               unsigned char one[20], unsigned char two[20],\n>> +               unsigned dirty_submodule, const char *meta,\n>> +               const char *reset)\n>> +{\n>> +       struct strbuf buf = STRBUF_INIT;\n>> +       struct strbuf sb = STRBUF_INIT;\n>> +       const char *message = NULL;\n>> +\n>> +       if (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)\n>> +               fprintf(f, \"%sSubmodule %s contains untracked content\\n\",\n>> +                       line_prefix, path);\n>> +       if (dirty_submodule & DIRTY_SUBMODULE_MODIFIED)\n>> +               fprintf(f, \"%sSubmodule %s contains modified content\\n\",\n>> +                       line_prefix, path);\n>> +\n>> +       if (!hashcmp(one, two)) {\n>> +               strbuf_release(&sb);\n>> +               return;\n>> +       }\n>> +\n>> +       if (add_submodule_odb(path))\n>> +               message = \"(not checked out)\";\n>\n> When not checked out, we can invoke the diff command\n> in .git/modules/<name> as that is the git dir of the submodule,\n> i.e. operating diff with a bare repo?\n\nWe can actually do this every time. How would you pass that in a\nchild_process? I don't think it's \"dir\" but instead passing\n\"--git-dir\" somehow?\n\nThanks,\nJake\n\n>\n> Thanks,\n> Stefan\n"},{"id":"293550","messageId":"CAGZ79kZbHAsxf9G-8=nz3o_zOMNFPa=kSphdo=4zLq97jycbYg@mail.gmail.com","threadId":"43040","inReplyTo":"CA+P7+xqnALzwkgLXrP57BUtFGaxGudAEG7cmGTakP47ofFEjHQ@mail.gmail.com","subject":"Re: [PATCH RFC] diff: add SUBMODULE_DIFF format to display submodule diff","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T23:10:53Z","receivedAt":"2016-08-09T23:11:58Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Aug 9, 2016 at 3:56 PM, Jacob Keller <jacob.keller@gmail.com> wrote:\n\n>>> +\n>>> +       if (strbuf_read(buf, cp.out, 0) < 0)\n>>\n>> So we keep the whole diff in memory\n>> I don't know much about the diff machinery, but I thought\n>> the rest of the diff machinery just streams it out?\n>\n> Yea, but I can't figure out how to do that. Is there an easy way to\n> stream chunks from the pipe straight into the file?\n\nMaybe, roughly:\n\n    cp.stdout = -1;\n    start_command(&cp);\n    do {\n        int r = xread(cp.stdout, buf, MAX_IO_SIZE);\n    } while (r >=0);\n    finish_command(&cp);\n\nxread does use a poll() for you so it is not active polling,\nbut only reading when data is available.\n\n\n>>\n>> When not checked out, we can invoke the diff command\n>> in .git/modules/<name> as that is the git dir of the submodule,\n>> i.e. operating diff with a bare repo?\n>\n> We can actually do this every time. How would you pass that in a\n> child_process? I don't think it's \"dir\" but instead passing\n> \"--git-dir\" somehow?\n\ngit -C $GIT_DIR diff --relative ${superprojects ce->name}\n\nwith $GIT_DIR = ${SUPERPROJECTS GITDIR + modules/<name>/ with name to be looked\nas submodule_from_path(ce->name) and then taking `name` from the\nsubmodule struct)\n\n\nThanks,\nStefan\n"},{"id":"293551","messageId":"CAGZ79kYkA=jJPHy+jqe5zVz0kM+95hxFUZPTNCKMJzbmJ7gaew@mail.gmail.com","threadId":"43040","inReplyTo":"CAGZ79kZbHAsxf9G-8=nz3o_zOMNFPa=kSphdo=4zLq97jycbYg@mail.gmail.com","subject":"Re: [PATCH RFC] diff: add SUBMODULE_DIFF format to display submodule diff","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T23:15:27Z","receivedAt":"2016-08-09T23:15:33Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Aug 9, 2016 at 4:10 PM, Stefan Beller <sbeller@google.com> wrote:\n\n> xread does use a poll() for you so it is not active polling,\n> but only reading when data is available.\n\ns/active polling/ spinning/\n\n>\n>\n>>>\n>>> When not checked out, we can invoke the diff command\n>>> in .git/modules/<name> as that is the git dir of the submodule,\n>>> i.e. operating diff with a bare repo?\n>>\n>> We can actually do this every time. How would you pass that in a\n>> child_process? I don't think it's \"dir\" but instead passing\n>> \"--git-dir\" somehow?\n\nI think it doesn't matter. You can still use .dir.\n(That would be equivalent to `cd $GIT_DIR && git diff sha1..sha1`\nwhich works just as well, even in the submodule case)\nAlternatively you could do\n    argv_array_pushf(cp.env_array, \"GIT_DIR=%s\", ...)\n\nSo I would drop the -C here and just use the .dir attribute.\n\n> git -C $GIT_DIR diff --relative ${superprojects ce->name}\n"},{"id":"293622","messageId":"b44c853f-2b4b-0eeb-ba1a-df0e650f7868@kdbg.org","threadId":"43040","inReplyTo":"CA+P7+xqnALzwkgLXrP57BUtFGaxGudAEG7cmGTakP47ofFEjHQ@mail.gmail.com","subject":"Re: [PATCH RFC] diff: add SUBMODULE_DIFF format to display submodule diff","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-08-10T19:41:56Z","receivedAt":"2016-08-10T19:44:29Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 10.08.2016 um 00:56 schrieb Jacob Keller:\n> On Tue, Aug 9, 2016 at 3:50 PM, Stefan Beller <sbeller@google.com> wrote:\n>> On Tue, Aug 9, 2016 at 3:32 PM, Jacob Keller <jacob.e.keller@intel.com> wrote:\n>>> +       if (strbuf_read(buf, cp.out, 0) < 0)\n>>\n>> So we keep the whole diff in memory\n>> I don't know much about the diff machinery, but I thought\n>> the rest of the diff machinery just streams it out?\n>\n> Yea, but I can't figure out how to do that. Is there an easy way to\n> stream chunks from the pipe straight into the file?\n\nYou don't stream via a pipe, you let the sub-process write directly to \nthe file:\n\n\tfflush(f);\n\tcp.out = dup(fileno(f));\n\nOf course, you no longer \"prepare_submodule_diff\" anymore (which \ncurrently runs the child process), but you print the \"Submodule\" header \nline, then invoke the child process, and if there was a failure, you \nfprintf(f, \"(diff failed)\").\n\n-- Hannes\n\n"}]}