{"thread":{"id":"45575","subject":"[PATCH 0/2] Re: git diff --submodule=diff fails with submodules in a submodule","startedAt":"2017-03-31T23:17:45Z","lastAt":"2017-03-31T23:17:50Z","messageCount":3,"participants":["Stefan Beller"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"315959","messageId":"20170331231733.11123-1-sbeller@google.com","threadId":"45575","inReplyTo":null,"subject":"[PATCH 0/2] Re: git diff --submodule=diff fails with submodules in a submodule","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-03-31T23:17:31Z","receivedAt":"2017-03-31T23:17:45Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"I came up with a patch that fixes the issue locally.\n(It is the first patch, such that we can track the first patch down to maint)\n\nWhile doing so, I noticed 2 issues:\n* when having nested submodules, we probably want to have --submodule=diff\n  to recurse into the nested submodules, so pass on the option.\n  (2nd patch)\n* the tests in t4060 hardcode hashes literally. I do not have a patch for that\n  yet, but I will send patches later. (c.f. \"sanitize_output\" in t4202)\n  \nThanks,\nStefan\n\nStefan Beller (2):\n  diff: submodule inline diff to initialize env array.\n  diff: recurse into nested submodules for inline diff\n\n submodule.c                                  |  4 +-\n t/t4060-diff-submodule-option-diff-format.sh | 70 ++++++++++++++++++++++++++++\n 2 files changed, 73 insertions(+), 1 deletion(-)\n\n-- \n2.12.2.576.g7be6e4ba40.dirty\n\n"},{"id":"315960","messageId":"20170331231733.11123-2-sbeller@google.com","threadId":"45575","inReplyTo":"20170331231733.11123-1-sbeller@google.com","subject":"[PATCH 1/2] diff: submodule inline diff to initialize env array.","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-03-31T23:17:32Z","receivedAt":"2017-03-31T23:17:49Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"David reported:\n> When I try to run `git diff --submodule=diff` in a submodule which has\n> it's own submodules that have changes I get the error: fatal: bad\n> object.\n\nThis happens, because we do not properly initialize the environment\nin which the diff is run in the submodule. That means we inherit the\nenvironment from the main process, which sets environment variables.\n(Apparently we do set environment variables which we do not set\nwhen not in a submodules, i.e. the .git directory is linked)\n\nThis commit, just like fd47ae6a5b (diff: teach diff to display\nsubmodule difference with an inline diff, 2016-08-31) introduces bad\ntest code (i.e. hard coded hash values), which will be cleanup up in\na later patch.\n\nReported-by: David Parrish <daveparrish@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n submodule.c                                  |  1 +\n t/t4060-diff-submodule-option-diff-format.sh | 29 ++++++++++++++++++++++++++++\n 2 files changed, 30 insertions(+)\n\ndiff --git a/submodule.c b/submodule.c\nindex c52d6634c5..d84109908f 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -576,6 +576,7 @@ void show_submodule_inline_diff(FILE *f, const char *path,\n \tif (!(dirty_submodule & DIRTY_SUBMODULE_MODIFIED))\n \t\targv_array_push(&cp.args, oid_to_hex(new));\n \n+\tprepare_submodule_repo_env(&cp.env_array);\n \tif (run_command(&cp))\n \t\tfprintf(f, \"(diff failed)\\n\");\n \ndiff --git a/t/t4060-diff-submodule-option-diff-format.sh b/t/t4060-diff-submodule-option-diff-format.sh\nindex 7e23b55ea4..d4a3ffa69c 100755\n--- a/t/t4060-diff-submodule-option-diff-format.sh\n+++ b/t/t4060-diff-submodule-option-diff-format.sh\n@@ -746,4 +746,33 @@ test_expect_success 'diff --submodule=diff with .git file' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'setup nested submodule' '\n+\tgit submodule add -f ./sm2 &&\n+\tgit commit -a -m \"add sm2\" &&\n+\tgit -C sm2 submodule add ../sm2 nested &&\n+\tgit -C sm2 commit -a -m \"nested sub\"\n+'\n+\n+test_expect_success 'move nested submodule HEAD' '\n+\techo \"nested content\" >sm2/nested/file &&\n+\tgit -C sm2/nested add file &&\n+\tgit -C sm2/nested commit --allow-empty -m \"new HEAD\"\n+'\n+\n+test_expect_success 'diff --submodule=diff with moved nested submodule HEAD' '\n+\tcat >expected <<-EOF &&\n+\tSubmodule nested a5a65c9..b55928c:\n+\tdiff --git a/nested/file b/nested/file\n+\tnew file mode 100644\n+\tindex 0000000..ca281f5\n+\t--- /dev/null\n+\t+++ b/nested/file\n+\t@@ -0,0 +1 @@\n+\t+nested content\n+\tEOF\n+\tgit -C sm2 diff --submodule=diff >actual 2>err &&\n+\ttest_must_be_empty err &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.12.2.576.g7be6e4ba40.dirty\n\n"},{"id":"315961","messageId":"20170331231733.11123-3-sbeller@google.com","threadId":"45575","inReplyTo":"20170331231733.11123-1-sbeller@google.com","subject":"[PATCH 2/2] diff: recurse into nested submodules for inline diff","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-03-31T23:17:33Z","receivedAt":"2017-03-31T23:17:50Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When fd47ae6a5b (diff: teach diff to display submodule difference with an\ninline diff, 2016-08-31) was introduced, we did not think of recursing\ninto nested submodules.\n\nWhen showing the inline diff for submodules, automatically recurse\ninto nested submodules as well with inline submodule diffs.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n submodule.c                                  |  3 +-\n t/t4060-diff-submodule-option-diff-format.sh | 41 ++++++++++++++++++++++++++++\n 2 files changed, 43 insertions(+), 1 deletion(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex d84109908f..471ca9ce7d 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -553,7 +553,8 @@ void show_submodule_inline_diff(FILE *f, const char *path,\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_pushl(&cp.args, \"diff\", \"--submodule=diff\", NULL);\n+\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/\",\ndiff --git a/t/t4060-diff-submodule-option-diff-format.sh b/t/t4060-diff-submodule-option-diff-format.sh\nindex d4a3ffa69c..33ec26d755 100755\n--- a/t/t4060-diff-submodule-option-diff-format.sh\n+++ b/t/t4060-diff-submodule-option-diff-format.sh\n@@ -775,4 +775,45 @@ test_expect_success 'diff --submodule=diff with moved nested submodule HEAD' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'diff --submodule=diff recurses into nested submodules' '\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm2 contains modified content\n+\tSubmodule sm2 a5a65c9..280969a:\n+\tdiff --git a/sm2/.gitmodules b/sm2/.gitmodules\n+\tnew file mode 100644\n+\tindex 0000000..3a816b8\n+\t--- /dev/null\n+\t+++ b/sm2/.gitmodules\n+\t@@ -0,0 +1,3 @@\n+\t+[submodule \"nested\"]\n+\t+\tpath = nested\n+\t+\turl = ../sm2\n+\tSubmodule nested 0000000...b55928c (new submodule)\n+\tdiff --git a/sm2/nested/file b/sm2/nested/file\n+\tnew file mode 100644\n+\tindex 0000000..ca281f5\n+\t--- /dev/null\n+\t+++ b/sm2/nested/file\n+\t@@ -0,0 +1 @@\n+\t+nested content\n+\tdiff --git a/sm2/nested/foo8 b/sm2/nested/foo8\n+\tnew file mode 100644\n+\tindex 0000000..db9916b\n+\t--- /dev/null\n+\t+++ b/sm2/nested/foo8\n+\t@@ -0,0 +1 @@\n+\t+foo8\n+\tdiff --git a/sm2/nested/foo9 b/sm2/nested/foo9\n+\tnew file mode 100644\n+\tindex 0000000..9c3b4f6\n+\t--- /dev/null\n+\t+++ b/sm2/nested/foo9\n+\t@@ -0,0 +1 @@\n+\t+foo9\n+\tEOF\n+\tgit diff --submodule=diff >actual 2>err &&\n+\ttest_must_be_empty err &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.12.2.576.g7be6e4ba40.dirty\n\n"}]}