{"thread":{"id":"56268","subject":"[PATCH v3 1/3] Remove unused var","startedAt":"2021-08-12T00:13:53Z","lastAt":"2021-08-30T13:46:23Z","messageCount":4,"participants":["David Turner"],"isPatch":true,"patchVersion":3,"patchTotal":3},"messages":[{"id":"432547","messageId":"20210812001332.715876-1-dturner@twosigma.com","threadId":"56268","inReplyTo":null,"subject":"[PATCH v3 1/3] Remove unused var","fromName":"David Turner","fromEmail":"dturner@twosigma.com","sentAt":"2021-08-12T00:13:30Z","receivedAt":"2021-08-12T00:13:53Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Signed-off-by: David Turner <dturner@twosigma.com>\n---\n t/t4060-diff-submodule-option-diff-format.sh | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/t/t4060-diff-submodule-option-diff-format.sh b/t/t4060-diff-submodule-option-diff-format.sh\nindex dc7b242697..69b9946931 100755\n--- a/t/t4060-diff-submodule-option-diff-format.sh\n+++ b/t/t4060-diff-submodule-option-diff-format.sh\n@@ -361,7 +361,6 @@ test_expect_success 'typechanged submodule(submodule->blob)' '\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-- \n2.11.GIT\n\n"},{"id":"432548","messageId":"20210812001332.715876-3-dturner@twosigma.com","threadId":"56268","inReplyTo":"20210812001332.715876-1-dturner@twosigma.com","subject":"[PATCH v3 3/3] diff --submodule=diff: Don't print failure message twice","fromName":"David Turner","fromEmail":"dturner@twosigma.com","sentAt":"2021-08-12T00:13:32Z","receivedAt":"2021-08-12T00:13:55Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"When we fail to start a diff command inside a submodule, immediately\nexit the routine rather than trying to finish the command and printing\na second message.\n\nSigned-off-by: David Turner <dturner@twosigma.com>\n---\n submodule.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex d13d103975..2e98e840af 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -720,8 +720,10 @@ void show_submodule_inline_diff(struct diff_options *o, const char *path,\n \t\tstrvec_push(&cp.env_array, GIT_WORK_TREE_ENVIRONMENT \"=.\");\n \t}\n \n-\tif (start_command(&cp))\n+\tif (start_command(&cp)) {\n \t\tdiff_emit_submodule_error(o, \"(diff failed)\\n\");\n+\t\tgoto done;\n+\t}\n \n \twhile (strbuf_getwholeline_fd(&sb, cp.out, '\\n') != EOF)\n \t\tdiff_emit_submodule_pipethrough(o, sb.buf, sb.len);\n-- \n2.11.GIT\n\n"},{"id":"432549","messageId":"20210812001332.715876-2-dturner@twosigma.com","threadId":"56268","inReplyTo":"20210812001332.715876-1-dturner@twosigma.com","subject":"[PATCH v3 2/3] diff --submodule=diff: do not fail on ever-initialied deleted submodules","fromName":"David Turner","fromEmail":"dturner@twosigma.com","sentAt":"2021-08-12T00:13:31Z","receivedAt":"2021-08-12T00:22:18Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"If you have ever initialized a submodule, open_submodule will open it.\nIf you then delete the submodule's worktree directory (but don't\nremove it from .gitmodules), git diff --submodule=diff would error out\nas it attempted to chdir into the now-deleted working tree directory.\n\nInstead, we chdir into the submodule's git directory and run the diff\nfrom there.\n\nSigned-off-by: David Turner <dturner@twosigma.com>\n---\n submodule.c                                  |  10 ++\n t/t4060-diff-submodule-option-diff-format.sh | 158 +++++++++++++++++++++++++--\n 2 files changed, 161 insertions(+), 7 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 0b1d9c1dde..d13d103975 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -710,6 +710,16 @@ void show_submodule_inline_diff(struct diff_options *o, const char *path,\n \t\tstrvec_push(&cp.args, oid_to_hex(new_oid));\n \n \tprepare_submodule_repo_env(&cp.env_array);\n+\n+\tif (!is_directory(path)) {\n+\t\t// fall back to absorbed git dir, if any\n+\t\tif (!sub)\n+\t\t\tgoto done;\n+\t\tcp.dir = sub->gitdir;\n+\t\tstrvec_push(&cp.env_array, GIT_DIR_ENVIRONMENT \"=.\");\n+\t\tstrvec_push(&cp.env_array, GIT_WORK_TREE_ENVIRONMENT \"=.\");\n+\t}\n+\n \tif (start_command(&cp))\n \t\tdiff_emit_submodule_error(o, \"(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 69b9946931..d86e38abd8 100755\n--- a/t/t4060-diff-submodule-option-diff-format.sh\n+++ b/t/t4060-diff-submodule-option-diff-format.sh\n@@ -703,10 +703,26 @@ test_expect_success 'path filter' '\n \tdiff_cmp expected actual\n '\n \n-commit_file sm2\n+cat >.gitmodules <<-EOF\n+[submodule \"sm2\"]\n+\tpath = sm2\n+\turl = bogus_url\n+EOF\n+git add .gitmodules\n+commit_file sm2 .gitmodules\n+\n test_expect_success 'given commit' '\n \tgit diff-index -p --submodule=diff HEAD^ >actual &&\n \tcat >expected <<-EOF &&\n+\tdiff --git a/.gitmodules b/.gitmodules\n+\tnew file mode 100644\n+\tindex 1234567..89abcde\n+\t--- /dev/null\n+\t+++ b/.gitmodules\n+\t@@ -0,0 +1,3 @@\n+\t+[submodule \"sm2\"]\n+\t+path = sm2\n+\t+url = bogus_url\n \tSubmodule sm1 $head7...0000000 (submodule deleted)\n \tSubmodule sm2 0000000...$head9 (new submodule)\n \tdiff --git a/sm2/foo8 b/sm2/foo8\n@@ -728,15 +744,21 @@ test_expect_success 'given commit' '\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+\tgit submodule absorbgitdirs sm2\n '\n \n test_expect_success 'diff --submodule=diff with .git file' '\n \tgit diff --submodule=diff HEAD^ >actual &&\n \tcat >expected <<-EOF &&\n+\tdiff --git a/.gitmodules b/.gitmodules\n+\tnew file mode 100644\n+\tindex 1234567..89abcde\n+\t--- /dev/null\n+\t+++ b/.gitmodules\n+\t@@ -0,0 +1,3 @@\n+\t+[submodule \"sm2\"]\n+\t+path = sm2\n+\t+url = bogus_url\n \tSubmodule sm1 $head7...0000000 (submodule deleted)\n \tSubmodule sm2 0000000...$head9 (new submodule)\n \tdiff --git a/sm2/foo8 b/sm2/foo8\n@@ -757,9 +779,67 @@ test_expect_success 'diff --submodule=diff with .git file' '\n \tdiff_cmp expected actual\n '\n \n+mv sm2 sm2-bak\n+\n+test_expect_success 'deleted submodule with .git file' '\n+\tgit diff-index -p --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 $head7...0000000 (submodule deleted)\n+\tSubmodule sm2 $head9...0000000 (submodule deleted)\n+\tdiff --git a/sm2/foo8 b/sm2/foo8\n+\tdeleted file mode 100644\n+\tindex 1234567..89abcde\n+\t--- a/sm2/foo8\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-foo8\n+\tdiff --git a/sm2/foo9 b/sm2/foo9\n+\tdeleted file mode 100644\n+\tindex 1234567..89abcde\n+\t--- a/sm2/foo9\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-foo9\n+\tEOF\n+\tdiff_cmp expected actual\n+'\n+\n+echo submodule-to-blob>sm2\n+\n+test_expect_success 'typechanged(submodule->blob) submodule with .git file' '\n+\tgit diff-index -p --submodule=diff HEAD >actual &&\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 $head7...0000000 (submodule deleted)\n+\tSubmodule sm2 $head9...0000000 (submodule deleted)\n+\tdiff --git a/sm2/foo8 b/sm2/foo8\n+\tdeleted file mode 100644\n+\tindex 1234567..89abcde\n+\t--- a/sm2/foo8\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-foo8\n+\tdiff --git a/sm2/foo9 b/sm2/foo9\n+\tdeleted file mode 100644\n+\tindex 1234567..89abcde\n+\t--- a/sm2/foo9\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-foo9\n+\tdiff --git a/sm2 b/sm2\n+\tnew file mode 100644\n+\tindex 1234567..89abcde\n+\t--- /dev/null\n+\t+++ b/sm2\n+\t@@ -0,0 +1 @@\n+\t+submodule-to-blob\n+\tEOF\n+\tdiff_cmp expected actual\n+'\n+\n+rm sm2\n+mv sm2-bak sm2\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 \thead10=$(git -C sm2 rev-parse --short --verify HEAD)\n@@ -790,6 +870,7 @@ test_expect_success 'diff --submodule=diff with moved nested submodule HEAD' '\n \n test_expect_success 'diff --submodule=diff recurses into nested submodules' '\n \tcat >expected <<-EOF &&\n+\tSubmodule sm1 $head7...0000000 (submodule deleted)\n \tSubmodule sm2 contains modified content\n \tSubmodule sm2 $head9..$head10:\n \tdiff --git a/sm2/.gitmodules b/sm2/.gitmodules\n@@ -829,4 +910,67 @@ test_expect_success 'diff --submodule=diff recurses into nested submodules' '\n \tdiff_cmp expected actual\n '\n \n+(cd sm2; commit_file nested)\n+commit_file sm2\n+head12=$(cd sm2; git rev-parse --short --verify HEAD)\n+\n+mv sm2 sm2-bak\n+\n+test_expect_success 'diff --submodule=diff recurses into deleted nested submodules' '\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm1 $head7...0000000 (submodule deleted)\n+\tSubmodule sm2 $head12...0000000 (submodule deleted)\n+\tdiff --git a/sm2/.gitmodules b/sm2/.gitmodules\n+\tdeleted file mode 100644\n+\tindex 3a816b8..0000000\n+\t--- a/sm2/.gitmodules\n+\t+++ /dev/null\n+\t@@ -1,3 +0,0 @@\n+\t-[submodule \"nested\"]\n+\t-\tpath = nested\n+\t-\turl = ../sm2\n+\tdiff --git a/sm2/foo8 b/sm2/foo8\n+\tdeleted file mode 100644\n+\tindex db9916b..0000000\n+\t--- a/sm2/foo8\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-foo8\n+\tdiff --git a/sm2/foo9 b/sm2/foo9\n+\tdeleted file mode 100644\n+\tindex 9c3b4f6..0000000\n+\t--- a/sm2/foo9\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-foo9\n+\tSubmodule nested $head11...0000000 (submodule deleted)\n+\tdiff --git a/sm2/nested/file b/sm2/nested/file\n+\tdeleted file mode 100644\n+\tindex ca281f5..0000000\n+\t--- a/sm2/nested/file\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-nested content\n+\tdiff --git a/sm2/nested/foo8 b/sm2/nested/foo8\n+\tdeleted file mode 100644\n+\tindex db9916b..0000000\n+\t--- a/sm2/nested/foo8\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-foo8\n+\tdiff --git a/sm2/nested/foo9 b/sm2/nested/foo9\n+\tdeleted file mode 100644\n+\tindex 9c3b4f6..0000000\n+\t--- a/sm2/nested/foo9\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-foo9\n+\tEOF\n+\tgit diff --submodule=diff >actual 2>err &&\n+\ttest_must_be_empty err &&\n+\tdiff_cmp expected actual\n+'\n+\n+mv sm2-bak sm2\n+\n test_done\n-- \n2.11.GIT\n\n"},{"id":"434077","messageId":"d14d2c180f59b9115754318e4d3567a404769d06.camel@novalis.org","threadId":"56268","inReplyTo":"20210812001332.715876-3-dturner@twosigma.com","subject":"Re: [PATCH v3 3/3] diff --submodule=diff: Don't print failure message twice","fromName":"David Turner","fromEmail":"novalis@novalis.org","sentAt":"2021-08-30T13:45:56Z","receivedAt":"2021-08-30T13:46:23Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"I keep seeing these as \"will merge to next?\" in the \"what's cooking\"\nemails.  But I don't see any direct replies, and they don't seem to be\nmerged.  Is there something I need to do to get these merged?\n\nThanks.\n\nOn Wed, 2021-08-11 at 20:13 -0400, David Turner wrote:\n> When we fail to start a diff command inside a submodule, immediately\n> exit the routine rather than trying to finish the command and\n> printing\n> a second message.\n> \n> Signed-off-by: David Turner <dturner@twosigma.com>\n> ---\n>  submodule.c | 4 +++-\n>  1 file changed, 3 insertions(+), 1 deletion(-)\n> \n> diff --git a/submodule.c b/submodule.c\n> index d13d103975..2e98e840af 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -720,8 +720,10 @@ void show_submodule_inline_diff(struct\n> diff_options *o, const char *path,\n>                 strvec_push(&cp.env_array, GIT_WORK_TREE_ENVIRONMENT\n> \"=.\");\n>         }\n>  \n> -       if (start_command(&cp))\n> +       if (start_command(&cp)) {\n>                 diff_emit_submodule_error(o, \"(diff failed)\\n\");\n> +               goto done;\n> +       }\n>  \n>         while (strbuf_getwholeline_fd(&sb, cp.out, '\\n') != EOF)\n>                 diff_emit_submodule_pipethrough(o, sb.buf, sb.len);\n\n\n"}]}