{"thread":{"id":"38253","subject":"[PATCH 0/2] Fix issue with format-patch and diff.submodule","startedAt":"2014-12-26T23:11:44Z","lastAt":"2015-01-07T20:32:12Z","messageCount":18,"participants":["Doug Kelly","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"254105","messageId":"1419635506-5045-1-git-send-email-dougk.ff7@gmail.com","threadId":"38253","inReplyTo":null,"subject":"[PATCH 0/2] Fix issue with format-patch and diff.submodule","fromName":"Doug Kelly","fromEmail":"dougk.ff7@gmail.com","sentAt":"2014-12-26T23:11:44Z","receivedAt":"2014-12-26T23:11:44Z","isPatch":true,"sender":{"key":"dougk.ff7@gmail.com","avatar":"https://avatars.githubusercontent.com/u/93357?v=4"},"body":"A colleague found an issue that when using diff.submodule=log in his\n.gitconfig, format-patch would use the log format for submodule changes,\nwhich would be ignored or error out when processed by git-am.\nformat-patch now ignores the diff.submodule option and a testcase for\nthis specific issue now exists.\n\nSince this seems like a bug in current versions, I have based and\ntested this on the \"maint\" branch, but there's no reason it shouldn't\napply cleanly to master as well.\n\nApologies for any rawness to the first round of this change.\n\"Long time listener; first time caller.\" Any feedback is appreciated.\n\nDoug Kelly (2):\n  t4255: test am submodule with diff.submodule\n  format-patch: ignore diff.submodule setting\n\n builtin/log.c           |  2 +-\n t/t4255-am-submodule.sh | 83 +++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 84 insertions(+), 1 deletion(-)\n\n-- \n2.0.5\n"},{"id":"254106","messageId":"1419635506-5045-2-git-send-email-dougk.ff7@gmail.com","threadId":"38253","inReplyTo":"1419635506-5045-1-git-send-email-dougk.ff7@gmail.com","subject":"[PATCH 1/2] t4255: test am submodule with diff.submodule","fromName":"Doug Kelly","fromEmail":"dougk.ff7@gmail.com","sentAt":"2014-12-26T23:11:45Z","receivedAt":"2014-12-26T23:11:45Z","isPatch":true,"sender":{"key":"dougk.ff7@gmail.com","avatar":"https://avatars.githubusercontent.com/u/93357?v=4"},"body":"git am will break when using diff.submodule=log; add some test cases\nto illustrate this breakage as simply as possible.  There are\ncurrently two ways this can fail:\n\n* With errors (\"unrecognized input\"), if only change\n* Silently (no submodule change), if other files change\n\nTest for both conditions and ensure without diff.submodule this works.\n\nSigned-off-by: Doug Kelly <dougk.ff7@gmail.com>\n---\n t/t4255-am-submodule.sh | 83 +++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 83 insertions(+)\n\ndiff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh\nindex 8bde7db..d9a1d79 100755\n--- a/t/t4255-am-submodule.sh\n+++ b/t/t4255-am-submodule.sh\n@@ -18,4 +18,87 @@ am_3way () {\n KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n test_submodule_switch \"am_3way\"\n \n+test_expect_success 'setup diff.submodule' '\n+\techo one >one &&\n+\tgit add one &&\n+\ttest_tick &&\n+\tgit commit -m initial &&\n+\tgit rev-parse HEAD >initial &&\n+\n+\tgit init submodule &&\n+\t(cd submodule &&\n+\t\techo two >two &&\n+\t\tgit add two &&\n+\t\ttest_tick &&\n+\t\tgit commit -m \"initial submodule\" &&\n+\t\tgit rev-parse HEAD >../initial-submodule) &&\n+\tgit submodule add ./submodule &&\n+\ttest_tick &&\n+\tgit commit -m first &&\n+\tgit rev-parse HEAD >first &&\n+\n+\t(cd submodule &&\n+\t\techo three >three &&\n+\t\tgit add three &&\n+\t\ttest_tick &&\n+\t\tgit commit -m \"first submodule\" &&\n+\t\tgit rev-parse HEAD >../first-submodule) &&\n+\tgit add submodule &&\n+\ttest_tick &&\n+\tgit commit -m second &&\n+\tgit rev-parse HEAD >second &&\n+\n+\t(cd submodule &&\n+\t\tgit mv two four &&\n+\t\ttest_tick &&\n+\t\tgit commit -m \"second submodule\" &&\n+\t\tgit rev-parse HEAD >../second-submodule) &&\n+\tgit add submodule &&\n+\techo four >four &&\n+\tgit add four &&\n+\ttest_tick &&\n+\tgit commit -m third &&\n+\tgit rev-parse HEAD >third &&\n+\tgit submodule update --init\n+'\n+\n+INITIAL=$(cat initial)\n+SECOND=$(cat second)\n+THIRD=$(cat third)\n+\n+run_test() {\n+\tSTART_COMMIT=$1\n+\tEXPECT=$2\n+\t(git am --abort || true) &&\n+\tgit reset --hard $START_COMMIT &&\n+\trm -f *.patch &&\n+\tgit format-patch -1 &&\n+\tgit reset --hard $START_COMMIT^ &&\n+\tgit submodule update &&\n+\tgit am *.patch &&\n+\tgit submodule update &&\n+\t(cd submodule && git rev-parse HEAD >../actual) &&\n+\ttest_cmp $EXPECT actual\n+}\n+\n+test_expect_success 'diff.submodule unset' '\n+\t(git config --unset diff.submodule || true) &&\n+\trun_test $SECOND 'first-submodule'\n+'\n+\n+test_expect_success 'diff.submodule unset with extra file' '\n+\t(git config --unset diff.submodule || true) &&\n+\trun_test $THIRD 'second-submodule'\n+'\n+\n+test_expect_success 'diff.submodule=log' '\n+\tgit config diff.submodule log &&\n+\trun_test $SECOND 'first-submodule'\n+'\n+\n+test_expect_success 'diff.submodule=log with extra file' '\n+\tgit config diff.submodule log &&\n+\trun_test $THIRD 'second-submodule'\n+'\n+\n test_done\n-- \n2.0.5\n"},{"id":"254107","messageId":"1419635506-5045-3-git-send-email-dougk.ff7@gmail.com","threadId":"38253","inReplyTo":"1419635506-5045-1-git-send-email-dougk.ff7@gmail.com","subject":"[PATCH 2/2] format-patch: ignore diff.submodule setting","fromName":"Doug Kelly","fromEmail":"dougk.ff7@gmail.com","sentAt":"2014-12-26T23:11:46Z","receivedAt":"2014-12-26T23:11:46Z","isPatch":true,"sender":{"key":"dougk.ff7@gmail.com","avatar":"https://avatars.githubusercontent.com/u/93357?v=4"},"body":"diff.submodule when set to log produces output which git-am cannot\nhandle. Ignore this setting when generating patch output.\n\nSigned-off-by: Doug Kelly <dougk.ff7@gmail.com>\n---\n builtin/log.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 734aab3..cb14db4 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -705,7 +705,7 @@ static int git_format_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \tif (!strcmp(var, \"diff.color\") || !strcmp(var, \"color.diff\") ||\n-\t    !strcmp(var, \"color.ui\")) {\n+\t    !strcmp(var, \"color.ui\") || !strcmp(var, \"diff.submodule\")) {\n \t\treturn 0;\n \t}\n \tif (!strcmp(var, \"format.numbered\")) {\n-- \n2.0.5\n"},{"id":"254123","messageId":"CAPig+cT3gA2YpiT2Vr=F5-hB+Zy4ask-kz8DtpL3eFvz9PJb5Q@mail.gmail.com","threadId":"38253","inReplyTo":"1419635506-5045-2-git-send-email-dougk.ff7@gmail.com","subject":"Re: [PATCH 1/2] t4255: test am submodule with diff.submodule","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-12-28T00:37:00Z","receivedAt":"2014-12-28T00:37:00Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Dec 26, 2014 at 6:11 PM, Doug Kelly <dougk.ff7@gmail.com> wrote:\n> git am will break when using diff.submodule=log; add some test cases\n> to illustrate this breakage as simply as possible.  There are\n> currently two ways this can fail:\n>\n> * With errors (\"unrecognized input\"), if only change\n> * Silently (no submodule change), if other files change\n>\n> Test for both conditions and ensure without diff.submodule this works.\n>\n> Signed-off-by: Doug Kelly <dougk.ff7@gmail.com>\n> ---\n> diff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh\n> index 8bde7db..d9a1d79 100755\n> --- a/t/t4255-am-submodule.sh\n> +++ b/t/t4255-am-submodule.sh\n> @@ -18,4 +18,87 @@ am_3way () {\n>  KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n>  test_submodule_switch \"am_3way\"\n>\n> +test_expect_success 'setup diff.submodule' '\n\nSince the tests are actually expected to fail at this point (before\nyou've fixed the problem), use test_expect_failure. The follow-up\npatch, which fixes the problem, should flip them to\ntest_expect_success.\n\n> +       echo one >one &&\n> +       git add one &&\n> +       test_tick &&\n> +       git commit -m initial &&\n\nRather than performing these steps manually (here and below), perhaps\ntest_commit would be suitable and more succinct.\n\n> +       git rev-parse HEAD >initial &&\n\nOther scripts in the test suite don't bother with this indirection.\nInstead, they assign the variable here, then reference it in\nsubsequent tests (and no need to redirect to a file).\n\n    INITIAL=$(git rev-parse HEAD) &&\n\n> +\n> +       git init submodule &&\n> +       (cd submodule &&\n> +               echo two >two &&\n> +               git add two &&\n> +               test_tick &&\n> +               git commit -m \"initial submodule\" &&\n> +               git rev-parse HEAD >../initial-submodule) &&\n\nStyle: Format the subshell like this:\n\n    (\n        ...commands...\n    ) &&\n\n> +       git submodule add ./submodule &&\n> +       test_tick &&\n> +       git commit -m first &&\n> +       git rev-parse HEAD >first &&\n\nIs file 'first' ever used anywhere?\n\n> +       (cd submodule &&\n> +               echo three >three &&\n> +               git add three &&\n> +               test_tick &&\n> +               git commit -m \"first submodule\" &&\n> +               git rev-parse HEAD >../first-submodule) &&\n> +       git add submodule &&\n> +       test_tick &&\n> +       git commit -m second &&\n> +       git rev-parse HEAD >second &&\n> +\n> +       (cd submodule &&\n> +               git mv two four &&\n> +               test_tick &&\n> +               git commit -m \"second submodule\" &&\n> +               git rev-parse HEAD >../second-submodule) &&\n> +       git add submodule &&\n> +       echo four >four &&\n> +       git add four &&\n> +       test_tick &&\n> +       git commit -m third &&\n> +       git rev-parse HEAD >third &&\n> +       git submodule update --init\n> +'\n> +\n> +INITIAL=$(cat initial)\n> +SECOND=$(cat second)\n> +THIRD=$(cat third)\n\nNo need for this extra level of indirection. See above.\n\n> +run_test() {\n> +       START_COMMIT=$1\n> +       EXPECT=$2\n\nAlthough it's not specifically wrong here, someone adding code above\nthese two lines later on may not notice the broken &&-chain, so it\nwould be a good idea to keep the &&-chain intact.\n\n> +       (git am --abort || true) &&\n> +       git reset --hard $START_COMMIT &&\n> +       rm -f *.patch &&\n> +       git format-patch -1 &&\n> +       git reset --hard $START_COMMIT^ &&\n> +       git submodule update &&\n> +       git am *.patch &&\n> +       git submodule update &&\n> +       (cd submodule && git rev-parse HEAD >../actual) &&\n> +       test_cmp $EXPECT actual\n> +}\n> +\n> +test_expect_success 'diff.submodule unset' '\n> +       (git config --unset diff.submodule || true) &&\n> +       run_test $SECOND 'first-submodule'\n\nNote that you're already inside a single-quoted string here, so\n'first-submodule' is not quite doing what you expect. Double quotes\nwould be more appropriate. Or, better, drop the quoting of\nfirst-submodule altogether since it's unnecessary.\n\n> +'\n> +\n> +test_expect_success 'diff.submodule unset with extra file' '\n> +       (git config --unset diff.submodule || true) &&\n> +       run_test $THIRD 'second-submodule'\n> +'\n> +\n> +test_expect_success 'diff.submodule=log' '\n> +       git config diff.submodule log &&\n> +       run_test $SECOND 'first-submodule'\n> +'\n> +\n> +test_expect_success 'diff.submodule=log with extra file' '\n> +       git config diff.submodule log &&\n> +       run_test $THIRD 'second-submodule'\n> +'\n> +\n>  test_done\n> --\n> 2.0.5\n"},{"id":"254124","messageId":"CAEtYS8S4JKihvC4XZC00jv6HX8t6StqGCqCArTFk0RT--hgdSg@mail.gmail.com","threadId":"38253","inReplyTo":"CAPig+cT3gA2YpiT2Vr=F5-hB+Zy4ask-kz8DtpL3eFvz9PJb5Q@mail.gmail.com","subject":"Re: [PATCH 1/2] t4255: test am submodule with diff.submodule","fromName":"Doug Kelly","fromEmail":"dougk.ff7@gmail.com","sentAt":"2014-12-28T01:00:21Z","receivedAt":"2014-12-28T01:00:21Z","isPatch":true,"sender":{"key":"dougk.ff7@gmail.com","avatar":"https://avatars.githubusercontent.com/u/93357?v=4"},"body":"On Sat, Dec 27, 2014 at 6:37 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n>\n> On Fri, Dec 26, 2014 at 6:11 PM, Doug Kelly <dougk.ff7@gmail.com> wrote:\n> > git am will break when using diff.submodule=log; add some test cases\n> > to illustrate this breakage as simply as possible.  There are\n> > currently two ways this can fail:\n> >\n> > * With errors (\"unrecognized input\"), if only change\n> > * Silently (no submodule change), if other files change\n> >\n> > Test for both conditions and ensure without diff.submodule this works.\n> >\n> > Signed-off-by: Doug Kelly <dougk.ff7@gmail.com>\n> > ---\n> > diff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh\n> > index 8bde7db..d9a1d79 100755\n> > --- a/t/t4255-am-submodule.sh\n> > +++ b/t/t4255-am-submodule.sh\n> > @@ -18,4 +18,87 @@ am_3way () {\n> >  KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n> >  test_submodule_switch \"am_3way\"\n> >\n> > +test_expect_success 'setup diff.submodule' '\n>\n> Since the tests are actually expected to fail at this point (before\n> you've fixed the problem), use test_expect_failure. The follow-up\n> patch, which fixes the problem, should flip them to\n> test_expect_success.\n\nOkay, that's easy enough to do.\n\n>\n> > +       echo one >one &&\n> > +       git add one &&\n> > +       test_tick &&\n> > +       git commit -m initial &&\n>\n> Rather than performing these steps manually (here and below), perhaps\n> test_commit would be suitable and more succinct.\n>\n> > +       git rev-parse HEAD >initial &&\n>\n> Other scripts in the test suite don't bother with this indirection.\n> Instead, they assign the variable here, then reference it in\n> subsequent tests (and no need to redirect to a file).\n>\n>     INITIAL=$(git rev-parse HEAD) &&\n>\n\nBoth good comments; wasn't sure if that'd work.  But easy to do!\n\n> > +\n> > +       git init submodule &&\n> > +       (cd submodule &&\n> > +               echo two >two &&\n> > +               git add two &&\n> > +               test_tick &&\n> > +               git commit -m \"initial submodule\" &&\n> > +               git rev-parse HEAD >../initial-submodule) &&\n>\n> Style: Format the subshell like this:\n>\n>     (\n>         ...commands...\n>     ) &&\n\nYeah, I was unsure on this style (docs were unclear), but that's easy to follow.\n\n>\n> > +       git submodule add ./submodule &&\n> > +       test_tick &&\n> > +       git commit -m first &&\n> > +       git rev-parse HEAD >first &&\n>\n> Is file 'first' ever used anywhere?\n\nNope; somewhat vestigial code (that could probably be cleaned out --\nand should).\nOriginally, I was testing the first commit, and then I realized that\nthe second commit\nupdating the submodule introduces the failure I was looking for.\n\n>\n> > +       (cd submodule &&\n> > +               echo three >three &&\n> > +               git add three &&\n> > +               test_tick &&\n> > +               git commit -m \"first submodule\" &&\n> > +               git rev-parse HEAD >../first-submodule) &&\n> > +       git add submodule &&\n> > +       test_tick &&\n> > +       git commit -m second &&\n> > +       git rev-parse HEAD >second &&\n> > +\n> > +       (cd submodule &&\n> > +               git mv two four &&\n> > +               test_tick &&\n> > +               git commit -m \"second submodule\" &&\n> > +               git rev-parse HEAD >../second-submodule) &&\n> > +       git add submodule &&\n> > +       echo four >four &&\n> > +       git add four &&\n> > +       test_tick &&\n> > +       git commit -m third &&\n> > +       git rev-parse HEAD >third &&\n> > +       git submodule update --init\n> > +'\n> > +\n> > +INITIAL=$(cat initial)\n> > +SECOND=$(cat second)\n> > +THIRD=$(cat third)\n>\n> No need for this extra level of indirection. See above.\n>\n> > +run_test() {\n> > +       START_COMMIT=$1\n> > +       EXPECT=$2\n>\n> Although it's not specifically wrong here, someone adding code above\n> these two lines later on may not notice the broken &&-chain, so it\n> would be a good idea to keep the &&-chain intact.\n\nOK, easy enough.\n\n>\n> > +       (git am --abort || true) &&\n> > +       git reset --hard $START_COMMIT &&\n> > +       rm -f *.patch &&\n> > +       git format-patch -1 &&\n> > +       git reset --hard $START_COMMIT^ &&\n> > +       git submodule update &&\n> > +       git am *.patch &&\n> > +       git submodule update &&\n> > +       (cd submodule && git rev-parse HEAD >../actual) &&\n> > +       test_cmp $EXPECT actual\n> > +}\n> > +\n> > +test_expect_success 'diff.submodule unset' '\n> > +       (git config --unset diff.submodule || true) &&\n> > +       run_test $SECOND 'first-submodule'\n>\n> Note that you're already inside a single-quoted string here, so\n> 'first-submodule' is not quite doing what you expect. Double quotes\n> would be more appropriate. Or, better, drop the quoting of\n> first-submodule altogether since it's unnecessary.\n\nYep. Good note.\n\n>\n> > +'\n> > +\n> > +test_expect_success 'diff.submodule unset with extra file' '\n> > +       (git config --unset diff.submodule || true) &&\n> > +       run_test $THIRD 'second-submodule'\n> > +'\n> > +\n> > +test_expect_success 'diff.submodule=log' '\n> > +       git config diff.submodule log &&\n> > +       run_test $SECOND 'first-submodule'\n> > +'\n> > +\n> > +test_expect_success 'diff.submodule=log with extra file' '\n> > +       git config diff.submodule log &&\n> > +       run_test $THIRD 'second-submodule'\n> > +'\n> > +\n> >  test_done\n> > --\n> > 2.0.5\n\nOne other note that might simplify this extra test case that I thought about\nwhile driving home yesterday evening was changing lib-submodule-update to\nset diff.submodule=log inside prolog().  This wouldn't provide very\nclear failure causes, but it would perhaps reduce duplicated code and simplify\nthe test case.\n\nThanks for the feedback, though, I'll send out a new version momentarily.\n"},{"id":"254125","messageId":"1419728664-18627-1-git-send-email-dougk.ff7@gmail.com","threadId":"38253","inReplyTo":"1419635506-5045-1-git-send-email-dougk.ff7@gmail.com","subject":"[PATCH v2 1/2] t4255: test am submodule with diff.submodule","fromName":"Doug Kelly","fromEmail":"dougk.ff7@gmail.com","sentAt":"2014-12-28T01:04:23Z","receivedAt":"2014-12-28T01:04:23Z","isPatch":true,"sender":{"key":"dougk.ff7@gmail.com","avatar":"https://avatars.githubusercontent.com/u/93357?v=4"},"body":"git am will break when using diff.submodule=log; add some test cases\nto illustrate this breakage as simply as possible.  There are\ncurrently two ways this can fail:\n\n* With errors (\"unrecognized input\"), if only change\n* Silently (no submodule change), if other files change\n\nTest for both conditions and ensure without diff.submodule this works.\n\nSigned-off-by: Doug Kelly <dougk.ff7@gmail.com>\n---\n t/t4255-am-submodule.sh | 84 +++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 84 insertions(+)\n\ndiff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh\nindex 8bde7db..a2dc083 100755\n--- a/t/t4255-am-submodule.sh\n+++ b/t/t4255-am-submodule.sh\n@@ -18,4 +18,88 @@ am_3way () {\n KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n test_submodule_switch \"am_3way\"\n \n+test_expect_success 'setup diff.submodule' '\n+\techo one >one &&\n+\tgit add one &&\n+\ttest_tick &&\n+\tgit commit -m initial &&\n+\tINITIAL=$(git rev-parse HEAD) &&\n+\n+\tgit init submodule &&\n+\t(\n+\t\tcd submodule &&\n+\t\techo two >two &&\n+\t\tgit add two &&\n+\t\ttest_tick &&\n+\t\tgit commit -m \"initial submodule\" &&\n+\t\tgit rev-parse HEAD >../initial-submodule\n+\t) &&\n+\tgit submodule add ./submodule &&\n+\ttest_tick &&\n+\tgit commit -m first &&\n+\n+\t(\n+\t\tcd submodule &&\n+\t\techo three >three &&\n+\t\tgit add three &&\n+\t\ttest_tick &&\n+\t\tgit commit -m \"first submodule\" &&\n+\t\tgit rev-parse HEAD >../first-submodule\n+\t) &&\n+\tgit add submodule &&\n+\ttest_tick &&\n+\tgit commit -m second &&\n+\tSECOND=$(git rev-parse HEAD) &&\n+\n+\t(\n+\t\tcd submodule &&\n+\t\tgit mv two four &&\n+\t\ttest_tick &&\n+\t\tgit commit -m \"second submodule\" &&\n+\t\tgit rev-parse HEAD >../second-submodule\n+\t) &&\n+\tgit add submodule &&\n+\techo four >four &&\n+\tgit add four &&\n+\ttest_tick &&\n+\tgit commit -m third &&\n+\tTHIRD=$(git rev-parse HEAD) &&\n+\tgit submodule update --init\n+'\n+\n+run_test() {\n+\tSTART_COMMIT=$1 &&\n+\tEXPECT=$2 &&\n+\t(git am --abort || true) &&\n+\tgit reset --hard $START_COMMIT &&\n+\trm -f *.patch &&\n+\tgit format-patch -1 &&\n+\tgit reset --hard $START_COMMIT^ &&\n+\tgit submodule update &&\n+\tgit am *.patch &&\n+\tgit submodule update &&\n+\t(cd submodule && git rev-parse HEAD >../actual) &&\n+\ttest_cmp $EXPECT actual\n+}\n+\n+test_expect_success 'diff.submodule unset' '\n+\t(git config --unset diff.submodule || true) &&\n+\trun_test $SECOND first-submodule\n+'\n+\n+test_expect_success 'diff.submodule unset with extra file' '\n+\t(git config --unset diff.submodule || true) &&\n+\trun_test $THIRD second-submodule\n+'\n+\n+test_expect_failure 'diff.submodule=log' '\n+\tgit config diff.submodule log &&\n+\trun_test $SECOND first-submodule\n+'\n+\n+test_expect_failure 'diff.submodule=log with extra file' '\n+\tgit config diff.submodule log &&\n+\trun_test $THIRD second-submodule\n+'\n+\n test_done\n-- \n2.0.5\n"},{"id":"254126","messageId":"1419728664-18627-2-git-send-email-dougk.ff7@gmail.com","threadId":"38253","inReplyTo":"1419728664-18627-1-git-send-email-dougk.ff7@gmail.com","subject":"[PATCH v2 2/2] format-patch: ignore diff.submodule setting","fromName":"Doug Kelly","fromEmail":"dougk.ff7@gmail.com","sentAt":"2014-12-28T01:04:24Z","receivedAt":"2014-12-28T01:04:24Z","isPatch":true,"sender":{"key":"dougk.ff7@gmail.com","avatar":"https://avatars.githubusercontent.com/u/93357?v=4"},"body":"diff.submodule when set to log produces output which git-am cannot\nhandle. Ignore this setting when generating patch output.\n\nSigned-off-by: Doug Kelly <dougk.ff7@gmail.com>\n---\n builtin/log.c           | 2 +-\n t/t4255-am-submodule.sh | 4 ++--\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 734aab3..cb14db4 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -705,7 +705,7 @@ static int git_format_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \tif (!strcmp(var, \"diff.color\") || !strcmp(var, \"color.diff\") ||\n-\t    !strcmp(var, \"color.ui\")) {\n+\t    !strcmp(var, \"color.ui\") || !strcmp(var, \"diff.submodule\")) {\n \t\treturn 0;\n \t}\n \tif (!strcmp(var, \"format.numbered\")) {\ndiff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh\nindex a2dc083..b7ec0f1 100755\n--- a/t/t4255-am-submodule.sh\n+++ b/t/t4255-am-submodule.sh\n@@ -92,12 +92,12 @@ test_expect_success 'diff.submodule unset with extra file' '\n \trun_test $THIRD second-submodule\n '\n \n-test_expect_failure 'diff.submodule=log' '\n+test_expect_success 'diff.submodule=log' '\n \tgit config diff.submodule log &&\n \trun_test $SECOND first-submodule\n '\n \n-test_expect_failure 'diff.submodule=log with extra file' '\n+test_expect_success 'diff.submodule=log with extra file' '\n \tgit config diff.submodule log &&\n \trun_test $THIRD second-submodule\n '\n-- \n2.0.5\n"},{"id":"254127","messageId":"20141228022417.GA2256@flurp.local","threadId":"38253","inReplyTo":"1419728664-18627-1-git-send-email-dougk.ff7@gmail.com","subject":"Re: [PATCH v2 1/2] t4255: test am submodule with diff.submodule","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-12-28T02:24:17Z","receivedAt":"2014-12-28T02:24:17Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Dec 27, 2014 at 07:04:23PM -0600, Doug Kelly wrote:\n> git am will break when using diff.submodule=log; add some test cases\n> to illustrate this breakage as simply as possible.  There are\n> currently two ways this can fail:\n> \n> * With errors (\"unrecognized input\"), if only change\n> * Silently (no submodule change), if other files change\n> \n> Test for both conditions and ensure without diff.submodule this works.\n> \n> Signed-off-by: Doug Kelly <dougk.ff7@gmail.com>\n> ---\n\nHere below the \"---\" line is a good place to explain what changed\nsince the last version of the patch (or do so in the cover letter of\nthe new patch series). It's also helpful to reviewers to provide a\nlink to the previous round, like this[1].\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/261830\n\nMore below.\n\n>  t/t4255-am-submodule.sh | 84 +++++++++++++++++++++++++++++++++++++++++++++++++\n>  1 file changed, 84 insertions(+)\n> \n> diff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh\n> index 8bde7db..a2dc083 100755\n> --- a/t/t4255-am-submodule.sh\n> +++ b/t/t4255-am-submodule.sh\n> @@ -18,4 +18,88 @@ am_3way () {\n>  KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n>  test_submodule_switch \"am_3way\"\n>  \n> +test_expect_success 'setup diff.submodule' '\n> +\techo one >one &&\n> +\tgit add one &&\n> +\ttest_tick &&\n> +\tgit commit -m initial &&\n\nPerhaps squash in the following to improve succinctness?\n\n-- >8 --\ndiff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh\nindex b7ec0f1..b58e776 100755\n--- a/t/t4255-am-submodule.sh\n+++ b/t/t4255-am-submodule.sh\n@@ -19,50 +19,37 @@ KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n test_submodule_switch \"am_3way\"\n \n test_expect_success 'setup diff.submodule' '\n-\techo one >one &&\n-\tgit add one &&\n-\ttest_tick &&\n-\tgit commit -m initial &&\n+\ttest_commit one &&\n \tINITIAL=$(git rev-parse HEAD) &&\n \n \tgit init submodule &&\n \t(\n \t\tcd submodule &&\n-\t\techo two >two &&\n-\t\tgit add two &&\n-\t\ttest_tick &&\n-\t\tgit commit -m \"initial submodule\" &&\n+\t\ttest_commit two &&\n \t\tgit rev-parse HEAD >../initial-submodule\n \t) &&\n \tgit submodule add ./submodule &&\n-\ttest_tick &&\n \tgit commit -m first &&\n \n \t(\n \t\tcd submodule &&\n-\t\techo three >three &&\n-\t\tgit add three &&\n-\t\ttest_tick &&\n-\t\tgit commit -m \"first submodule\" &&\n+\t\ttest_commit three &&\n \t\tgit rev-parse HEAD >../first-submodule\n \t) &&\n \tgit add submodule &&\n-\ttest_tick &&\n \tgit commit -m second &&\n \tSECOND=$(git rev-parse HEAD) &&\n \n \t(\n \t\tcd submodule &&\n-\t\tgit mv two four &&\n+\t\tgit mv two.t four.t &&\n \t\ttest_tick &&\n \t\tgit commit -m \"second submodule\" &&\n \t\tgit rev-parse HEAD >../second-submodule\n \t) &&\n+\ttest_commit four &&\n \tgit add submodule &&\n-\techo four >four &&\n-\tgit add four &&\n-\ttest_tick &&\n-\tgit commit -m third &&\n+\tgit commit --amend --no-edit &&\n \tTHIRD=$(git rev-parse HEAD) &&\n \tgit submodule update --init\n '\n-- \n2.2.1.302.gdfcd89f\n"},{"id":"254137","messageId":"xmqqiogu1n06.fsf@gitster.dls.corp.google.com","threadId":"38253","inReplyTo":"CAPig+cT3gA2YpiT2Vr=F5-hB+Zy4ask-kz8DtpL3eFvz9PJb5Q@mail.gmail.com","subject":"Re: [PATCH 1/2] t4255: test am submodule with diff.submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-29T15:42:01Z","receivedAt":"2014-12-29T15:42:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n>> +       (git am --abort || true) &&\n\nWhy (x || y)?  Is 'x' so unreliable that we do not know how should exit?\nShould this be \"test_must_fail git am --abort\"?\n\n>> +       (cd submodule && git rev-parse HEAD >../actual) &&\n\n\"git -C submodule rev-parse HEAD >actual\" perhaps?\n\n>> +test_expect_success 'diff.submodule unset' '\n>> +       (git config --unset diff.submodule || true) &&\n\nI think test_config and test_unconfig were invented for things like\nthis (same for all the other use of \"git config\").\n"},{"id":"254413","messageId":"1420659105-26546-1-git-send-email-dougk.ff7@gmail.com","threadId":"38253","inReplyTo":"1419635506-5045-1-git-send-email-dougk.ff7@gmail.com","subject":"[PATCH v3 1/2] t4255: test am submodule with diff.submodule","fromName":"Doug Kelly","fromEmail":"dougk.ff7@gmail.com","sentAt":"2015-01-07T19:31:44Z","receivedAt":"2015-01-07T19:31:44Z","isPatch":true,"sender":{"key":"dougk.ff7@gmail.com","avatar":"https://avatars.githubusercontent.com/u/93357?v=4"},"body":"git am will break when using diff.submodule=log; add some test cases\nto illustrate this breakage as simply as possible.  There are\ncurrently two ways this can fail:\n\n* With errors (\"unrecognized input\"), if only change\n* Silently (no submodule change), if other files change\n\nTest for both conditions and ensure without diff.submodule this works.\n\nSigned-off-by: Doug Kelly <dougk.ff7@gmail.com>\nThanks-to: Eric Sunshine <sunshine@sunshineco.com>\nThanks-to: Junio C Hamano <gitster@pobox.com>\n---\nUpdated with Eric Sunshine's comments and changes to reduce complexity,\nand also changed to include Junio's suggestions for using test_config,\ntest_unconfig, and test_might_fail (since we don't know if a previous\nam failed or not -- we always want to clean up first).\n\n t/t4255-am-submodule.sh | 72 +++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 72 insertions(+)\n\ndiff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh\nindex 8bde7db..523accf 100755\n--- a/t/t4255-am-submodule.sh\n+++ b/t/t4255-am-submodule.sh\n@@ -18,4 +18,76 @@ am_3way () {\n KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n test_submodule_switch \"am_3way\"\n \n+test_expect_success 'setup diff.submodule' '\n+\ttest_commit one &&\n+\tINITIAL=$(git rev-parse HEAD) &&\n+\n+\tgit init submodule &&\n+\t(\n+\t\tcd submodule &&\n+\t\ttest_commit two &&\n+\t\tgit rev-parse HEAD >../initial-submodule\n+\t) &&\n+\tgit submodule add ./submodule &&\n+\tgit commit -m first &&\n+\n+\t(\n+\t\tcd submodule &&\n+\t\ttest_commit three &&\n+\t\tgit rev-parse HEAD >../first-submodule\n+\t) &&\n+\tgit add submodule &&\n+\ttest_tick &&\n+\tgit commit -m second &&\n+\tSECOND=$(git rev-parse HEAD) &&\n+\n+\t(\n+\t\tcd submodule &&\n+\t\tgit mv two.t four.t &&\n+\t\ttest_tick &&\n+\t\tgit commit -m \"second submodule\" &&\n+\t\tgit rev-parse HEAD >../second-submodule\n+\t) &&\n+\ttest_commit four &&\n+\tgit add submodule &&\n+\tgit commit --amend --no-edit &&\n+\tTHIRD=$(git rev-parse HEAD) &&\n+\tgit submodule update --init\n+'\n+\n+run_test() {\n+\tSTART_COMMIT=$1 &&\n+\tEXPECT=$2 &&\n+\ttest_might_fail git am --abort &&\n+\tgit reset --hard $START_COMMIT &&\n+\trm -f *.patch &&\n+\tgit format-patch -1 &&\n+\tgit reset --hard $START_COMMIT^ &&\n+\tgit submodule update &&\n+\tgit am *.patch &&\n+\tgit submodule update &&\n+\tgit -C submodule rev-parse HEAD >actual &&\n+\ttest_cmp $EXPECT actual\n+}\n+\n+test_expect_success 'diff.submodule unset' '\n+\ttest_unconfig diff.submodule &&\n+\trun_test $SECOND first-submodule\n+'\n+\n+test_expect_success 'diff.submodule unset with extra file' '\n+\ttest_unconfig diff.submodule &&\n+\trun_test $THIRD second-submodule\n+'\n+\n+test_expect_failure 'diff.submodule=log' '\n+\ttest_config diff.submodule log &&\n+\trun_test $SECOND first-submodule\n+'\n+\n+test_expect_failure 'diff.submodule=log with extra file' '\n+\ttest_config diff.submodule log &&\n+\trun_test $THIRD second-submodule\n+'\n+\n test_done\n-- \n2.0.5\n"},{"id":"254414","messageId":"1420659105-26546-2-git-send-email-dougk.ff7@gmail.com","threadId":"38253","inReplyTo":"1420659105-26546-1-git-send-email-dougk.ff7@gmail.com","subject":"[PATCH v3 2/2] format-patch: ignore diff.submodule setting","fromName":"Doug Kelly","fromEmail":"dougk.ff7@gmail.com","sentAt":"2015-01-07T19:31:45Z","receivedAt":"2015-01-07T19:31:45Z","isPatch":true,"sender":{"key":"dougk.ff7@gmail.com","avatar":"https://avatars.githubusercontent.com/u/93357?v=4"},"body":"diff.submodule when set to log produces output which git-am cannot\nhandle. Ignore this setting when generating patch output.\n\nSigned-off-by: Doug Kelly <dougk.ff7@gmail.com>\n---\n builtin/log.c           | 2 +-\n t/t4255-am-submodule.sh | 4 ++--\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 734aab3..cb14db4 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -705,7 +705,7 @@ static int git_format_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \tif (!strcmp(var, \"diff.color\") || !strcmp(var, \"color.diff\") ||\n-\t    !strcmp(var, \"color.ui\")) {\n+\t    !strcmp(var, \"color.ui\") || !strcmp(var, \"diff.submodule\")) {\n \t\treturn 0;\n \t}\n \tif (!strcmp(var, \"format.numbered\")) {\ndiff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh\nindex 523accf..31cbdba 100755\n--- a/t/t4255-am-submodule.sh\n+++ b/t/t4255-am-submodule.sh\n@@ -80,12 +80,12 @@ test_expect_success 'diff.submodule unset with extra file' '\n \trun_test $THIRD second-submodule\n '\n \n-test_expect_failure 'diff.submodule=log' '\n+test_expect_success 'diff.submodule=log' '\n \ttest_config diff.submodule log &&\n \trun_test $SECOND first-submodule\n '\n \n-test_expect_failure 'diff.submodule=log with extra file' '\n+test_expect_success 'diff.submodule=log with extra file' '\n \ttest_config diff.submodule log &&\n \trun_test $THIRD second-submodule\n '\n-- \n2.0.5\n"},{"id":"254415","messageId":"CAEtYS8SiP8bU=82H+XxXZqa47hQ7hOAsZChCr94DwgPNft9L=g@mail.gmail.com","threadId":"38253","inReplyTo":"xmqqiogu1n06.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/2] t4255: test am submodule with diff.submodule","fromName":"Doug Kelly","fromEmail":"dougk.ff7@gmail.com","sentAt":"2015-01-07T19:34:23Z","receivedAt":"2015-01-07T19:34:23Z","isPatch":true,"sender":{"key":"dougk.ff7@gmail.com","avatar":"https://avatars.githubusercontent.com/u/93357?v=4"},"body":"On Mon, Dec 29, 2014 at 9:42 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Eric Sunshine <sunshine@sunshineco.com> writes:\n>\n>>> +       (git am --abort || true) &&\n>\n> Why (x || y)?  Is 'x' so unreliable that we do not know how should exit?\n> Should this be \"test_must_fail git am --abort\"?\n>\nUpdated to test_might_fail -- we don't know if a merge is in progress or not.\nWe still need to clean up, but disregard failure if a merge isn't in progress.\n\n>>> +       (cd submodule && git rev-parse HEAD >../actual) &&\n>\n> \"git -C submodule rev-parse HEAD >actual\" perhaps?\n>\nSeems sane to me.\n\n>>> +test_expect_success 'diff.submodule unset' '\n>>> +       (git config --unset diff.submodule || true) &&\n>\n> I think test_config and test_unconfig were invented for things like\n> this (same for all the other use of \"git config\").\nYep, much nicer. :) Also updated to test_commit as suggested by Eric.\n\nThanks!\n\n--Doug\n"},{"id":"254416","messageId":"CAPig+cQUUoTFY41-++Po=LTPWYVH=CWpT7PUKGPyvACjJoPXxQ@mail.gmail.com","threadId":"38253","inReplyTo":"1420659105-26546-1-git-send-email-dougk.ff7@gmail.com","subject":"Re: [PATCH v3 1/2] t4255: test am submodule with diff.submodule","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-01-07T20:06:47Z","receivedAt":"2015-01-07T20:06:47Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Jan 7, 2015 at 2:31 PM, Doug Kelly <dougk.ff7@gmail.com> wrote:\n> git am will break when using diff.submodule=log; add some test cases\n> to illustrate this breakage as simply as possible.  There are\n> currently two ways this can fail:\n>\n> * With errors (\"unrecognized input\"), if only change\n> * Silently (no submodule change), if other files change\n>\n> Test for both conditions and ensure without diff.submodule this works.\n>\n> Signed-off-by: Doug Kelly <dougk.ff7@gmail.com>\n> Thanks-to: Eric Sunshine <sunshine@sunshineco.com>\n> Thanks-to: Junio C Hamano <gitster@pobox.com>\n\nOn this project, it's customary to say \"Helped-by:\" rather than\n\"Thanks-to:\". Also, place your sign-off last.\n\n> ---\n> Updated with Eric Sunshine's comments and changes to reduce complexity,\n> and also changed to include Junio's suggestions for using test_config,\n> test_unconfig, and test_might_fail (since we don't know if a previous\n> am failed or not -- we always want to clean up first).\n\nLooking much better. Thanks. A couple minor comments below...\n\n> diff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh\n> index 8bde7db..523accf 100755\n> --- a/t/t4255-am-submodule.sh\n> +++ b/t/t4255-am-submodule.sh\n> @@ -18,4 +18,76 @@ am_3way () {\n>  KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n>  test_submodule_switch \"am_3way\"\n>\n> +test_expect_success 'setup diff.submodule' '\n> +       test_commit one &&\n> +       INITIAL=$(git rev-parse HEAD) &&\n> +\n> +       git init submodule &&\n> +       (\n> +               cd submodule &&\n> +               test_commit two &&\n> +               git rev-parse HEAD >../initial-submodule\n> +       ) &&\n> +       git submodule add ./submodule &&\n> +       git commit -m first &&\n> +\n> +       (\n> +               cd submodule &&\n> +               test_commit three &&\n> +               git rev-parse HEAD >../first-submodule\n> +       ) &&\n> +       git add submodule &&\n> +       test_tick &&\n\nYou can drop this test_tick (as I did in my \"squash\"[1]).\n\n> +       git commit -m second &&\n> +       SECOND=$(git rev-parse HEAD) &&\n> +\n> +       (\n> +               cd submodule &&\n> +               git mv two.t four.t &&\n> +               test_tick &&\n\nAnd this one (which I overlooked in [1]).\n\nThe reason I suggest dropping the test_tick invocations is that they\ndo not impact these tests at all, yet their presence misleads the\nreader into thinking that they are somehow significant.\n\n> +               git commit -m \"second submodule\" &&\n> +               git rev-parse HEAD >../second-submodule\n> +       ) &&\n> +       test_commit four &&\n> +       git add submodule &&\n> +       git commit --amend --no-edit &&\n> +       THIRD=$(git rev-parse HEAD) &&\n> +       git submodule update --init\n> +'\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/261852\n"},{"id":"254418","messageId":"1420661623-30692-1-git-send-email-dougk.ff7@gmail.com","threadId":"38253","inReplyTo":"1419635506-5045-1-git-send-email-dougk.ff7@gmail.com","subject":"[PATCH v4 1/2] t4255: test am submodule with diff.submodule","fromName":"Doug Kelly","fromEmail":"dougk.ff7@gmail.com","sentAt":"2015-01-07T20:13:42Z","receivedAt":"2015-01-07T20:13:42Z","isPatch":true,"sender":{"key":"dougk.ff7@gmail.com","avatar":"https://avatars.githubusercontent.com/u/93357?v=4"},"body":"git am will break when using diff.submodule=log; add some test cases\nto illustrate this breakage as simply as possible.  There are\ncurrently two ways this can fail:\n\n* With errors (\"unrecognized input\"), if only change\n* Silently (no submodule change), if other files change\n\nTest for both conditions and ensure without diff.submodule this works.\n\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Doug Kelly <dougk.ff7@gmail.com>\n---\nUpdated to remove test_ticks and clean the commit message.\n\n t/t4255-am-submodule.sh | 70 +++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 70 insertions(+)\n\ndiff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh\nindex 8bde7db..a38c305 100755\n--- a/t/t4255-am-submodule.sh\n+++ b/t/t4255-am-submodule.sh\n@@ -18,4 +18,74 @@ am_3way () {\n KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n test_submodule_switch \"am_3way\"\n \n+test_expect_success 'setup diff.submodule' '\n+\ttest_commit one &&\n+\tINITIAL=$(git rev-parse HEAD) &&\n+\n+\tgit init submodule &&\n+\t(\n+\t\tcd submodule &&\n+\t\ttest_commit two &&\n+\t\tgit rev-parse HEAD >../initial-submodule\n+\t) &&\n+\tgit submodule add ./submodule &&\n+\tgit commit -m first &&\n+\n+\t(\n+\t\tcd submodule &&\n+\t\ttest_commit three &&\n+\t\tgit rev-parse HEAD >../first-submodule\n+\t) &&\n+\tgit add submodule &&\n+\tgit commit -m second &&\n+\tSECOND=$(git rev-parse HEAD) &&\n+\n+\t(\n+\t\tcd submodule &&\n+\t\tgit mv two.t four.t &&\n+\t\tgit commit -m \"second submodule\" &&\n+\t\tgit rev-parse HEAD >../second-submodule\n+\t) &&\n+\ttest_commit four &&\n+\tgit add submodule &&\n+\tgit commit --amend --no-edit &&\n+\tTHIRD=$(git rev-parse HEAD) &&\n+\tgit submodule update --init\n+'\n+\n+run_test() {\n+\tSTART_COMMIT=$1 &&\n+\tEXPECT=$2 &&\n+\ttest_might_fail git am --abort &&\n+\tgit reset --hard $START_COMMIT &&\n+\trm -f *.patch &&\n+\tgit format-patch -1 &&\n+\tgit reset --hard $START_COMMIT^ &&\n+\tgit submodule update &&\n+\tgit am *.patch &&\n+\tgit submodule update &&\n+\tgit -C submodule rev-parse HEAD >actual &&\n+\ttest_cmp $EXPECT actual\n+}\n+\n+test_expect_success 'diff.submodule unset' '\n+\ttest_unconfig diff.submodule &&\n+\trun_test $SECOND first-submodule\n+'\n+\n+test_expect_success 'diff.submodule unset with extra file' '\n+\ttest_unconfig diff.submodule &&\n+\trun_test $THIRD second-submodule\n+'\n+\n+test_expect_failure 'diff.submodule=log' '\n+\ttest_config diff.submodule log &&\n+\trun_test $SECOND first-submodule\n+'\n+\n+test_expect_failure 'diff.submodule=log with extra file' '\n+\ttest_config diff.submodule log &&\n+\trun_test $THIRD second-submodule\n+'\n+\n test_done\n-- \n2.0.5\n"},{"id":"254417","messageId":"1420661623-30692-2-git-send-email-dougk.ff7@gmail.com","threadId":"38253","inReplyTo":"1420661623-30692-1-git-send-email-dougk.ff7@gmail.com","subject":"[PATCH v4 2/2] format-patch: ignore diff.submodule setting","fromName":"Doug Kelly","fromEmail":"dougk.ff7@gmail.com","sentAt":"2015-01-07T20:13:43Z","receivedAt":"2015-01-07T20:13:43Z","isPatch":true,"sender":{"key":"dougk.ff7@gmail.com","avatar":"https://avatars.githubusercontent.com/u/93357?v=4"},"body":"diff.submodule when set to log produces output which git-am cannot\nhandle. Ignore this setting when generating patch output.\n\nSigned-off-by: Doug Kelly <dougk.ff7@gmail.com>\n---\n builtin/log.c           | 2 +-\n t/t4255-am-submodule.sh | 4 ++--\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 734aab3..cb14db4 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -705,7 +705,7 @@ static int git_format_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \tif (!strcmp(var, \"diff.color\") || !strcmp(var, \"color.diff\") ||\n-\t    !strcmp(var, \"color.ui\")) {\n+\t    !strcmp(var, \"color.ui\") || !strcmp(var, \"diff.submodule\")) {\n \t\treturn 0;\n \t}\n \tif (!strcmp(var, \"format.numbered\")) {\ndiff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh\nindex a38c305..27ea698 100755\n--- a/t/t4255-am-submodule.sh\n+++ b/t/t4255-am-submodule.sh\n@@ -78,12 +78,12 @@ test_expect_success 'diff.submodule unset with extra file' '\n \trun_test $THIRD second-submodule\n '\n \n-test_expect_failure 'diff.submodule=log' '\n+test_expect_success 'diff.submodule=log' '\n \ttest_config diff.submodule log &&\n \trun_test $SECOND first-submodule\n '\n \n-test_expect_failure 'diff.submodule=log with extra file' '\n+test_expect_success 'diff.submodule=log with extra file' '\n \ttest_config diff.submodule log &&\n \trun_test $THIRD second-submodule\n '\n-- \n2.0.5\n"},{"id":"254419","messageId":"xmqq387mjqbe.fsf@gitster.dls.corp.google.com","threadId":"38253","inReplyTo":"CAEtYS8SiP8bU=82H+XxXZqa47hQ7hOAsZChCr94DwgPNft9L=g@mail.gmail.com","subject":"Re: [PATCH 1/2] t4255: test am submodule with diff.submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-01-07T20:20:21Z","receivedAt":"2015-01-07T20:20:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Doug Kelly <dougk.ff7@gmail.com> writes:\n\n> On Mon, Dec 29, 2014 at 9:42 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Eric Sunshine <sunshine@sunshineco.com> writes:\n>>\n>>>> +       (git am --abort || true) &&\n>>\n>> Why (x || y)?  Is 'x' so unreliable that we do not know how should exit?\n>> Should this be \"test_must_fail git am --abort\"?\n>>\n> Updated to test_might_fail -- we don't know if a merge is in progress or not.\n> We still need to clean up, but disregard failure if a merge isn't in progress.\n\nAh, OK.  But even with \"test_might_fail\", it may not be clear why it\nmight fail, so it would be easier to maintain if we can read \"we\ndon't know if a merge is in progress\" next to the \"test_might_fail\".\n\nFor now we can add a comment, but in the longer term it might not be\na bad idea to change test_might_fail to require two args, one is a\ncommand to run and the other is a text that explains why the outcome\nis unknown.\n\nThanks for clarifying.\n"},{"id":"254426","messageId":"1420662732-11972-1-git-send-email-dougk.ff7@gmail.com","threadId":"38253","inReplyTo":"1419635506-5045-1-git-send-email-dougk.ff7@gmail.com","subject":"[PATCH v5 1/2] t4255: test am submodule with diff.submodule","fromName":"Doug Kelly","fromEmail":"dougk.ff7@gmail.com","sentAt":"2015-01-07T20:32:11Z","receivedAt":"2015-01-07T20:32:11Z","isPatch":true,"sender":{"key":"dougk.ff7@gmail.com","avatar":"https://avatars.githubusercontent.com/u/93357?v=4"},"body":"git am will break when using diff.submodule=log; add some test cases\nto illustrate this breakage as simply as possible.  There are\ncurrently two ways this can fail:\n\n* With errors (\"unrecognized input\"), if only change\n* Silently (no submodule change), if other files change\n\nTest for both conditions and ensure without diff.submodule this works.\n\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Doug Kelly <dougk.ff7@gmail.com>\n---\nAdded a comment for why test_might_fail is used to abort merges\nin progress.\n\n t/t4255-am-submodule.sh | 72 +++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 72 insertions(+)\n\ndiff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh\nindex 8bde7db..450d261 100755\n--- a/t/t4255-am-submodule.sh\n+++ b/t/t4255-am-submodule.sh\n@@ -18,4 +18,76 @@ am_3way () {\n KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n test_submodule_switch \"am_3way\"\n \n+test_expect_success 'setup diff.submodule' '\n+\ttest_commit one &&\n+\tINITIAL=$(git rev-parse HEAD) &&\n+\n+\tgit init submodule &&\n+\t(\n+\t\tcd submodule &&\n+\t\ttest_commit two &&\n+\t\tgit rev-parse HEAD >../initial-submodule\n+\t) &&\n+\tgit submodule add ./submodule &&\n+\tgit commit -m first &&\n+\n+\t(\n+\t\tcd submodule &&\n+\t\ttest_commit three &&\n+\t\tgit rev-parse HEAD >../first-submodule\n+\t) &&\n+\tgit add submodule &&\n+\tgit commit -m second &&\n+\tSECOND=$(git rev-parse HEAD) &&\n+\n+\t(\n+\t\tcd submodule &&\n+\t\tgit mv two.t four.t &&\n+\t\tgit commit -m \"second submodule\" &&\n+\t\tgit rev-parse HEAD >../second-submodule\n+\t) &&\n+\ttest_commit four &&\n+\tgit add submodule &&\n+\tgit commit --amend --no-edit &&\n+\tTHIRD=$(git rev-parse HEAD) &&\n+\tgit submodule update --init\n+'\n+\n+run_test() {\n+\tSTART_COMMIT=$1 &&\n+\tEXPECT=$2 &&\n+\t# Abort any merges in progress: the previous\n+\t# test may have failed, and we should clean up.\n+\ttest_might_fail git am --abort &&\n+\tgit reset --hard $START_COMMIT &&\n+\trm -f *.patch &&\n+\tgit format-patch -1 &&\n+\tgit reset --hard $START_COMMIT^ &&\n+\tgit submodule update &&\n+\tgit am *.patch &&\n+\tgit submodule update &&\n+\tgit -C submodule rev-parse HEAD >actual &&\n+\ttest_cmp $EXPECT actual\n+}\n+\n+test_expect_success 'diff.submodule unset' '\n+\ttest_unconfig diff.submodule &&\n+\trun_test $SECOND first-submodule\n+'\n+\n+test_expect_success 'diff.submodule unset with extra file' '\n+\ttest_unconfig diff.submodule &&\n+\trun_test $THIRD second-submodule\n+'\n+\n+test_expect_failure 'diff.submodule=log' '\n+\ttest_config diff.submodule log &&\n+\trun_test $SECOND first-submodule\n+'\n+\n+test_expect_failure 'diff.submodule=log with extra file' '\n+\ttest_config diff.submodule log &&\n+\trun_test $THIRD second-submodule\n+'\n+\n test_done\n-- \n2.0.5\n"},{"id":"254425","messageId":"1420662732-11972-2-git-send-email-dougk.ff7@gmail.com","threadId":"38253","inReplyTo":"1420662732-11972-1-git-send-email-dougk.ff7@gmail.com","subject":"[PATCH v5 2/2] format-patch: ignore diff.submodule setting","fromName":"Doug Kelly","fromEmail":"dougk.ff7@gmail.com","sentAt":"2015-01-07T20:32:12Z","receivedAt":"2015-01-07T20:32:12Z","isPatch":true,"sender":{"key":"dougk.ff7@gmail.com","avatar":"https://avatars.githubusercontent.com/u/93357?v=4"},"body":"diff.submodule when set to log produces output which git-am cannot\nhandle. Ignore this setting when generating patch output.\n\nSigned-off-by: Doug Kelly <dougk.ff7@gmail.com>\n---\n builtin/log.c           | 2 +-\n t/t4255-am-submodule.sh | 4 ++--\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 734aab3..cb14db4 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -705,7 +705,7 @@ static int git_format_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \tif (!strcmp(var, \"diff.color\") || !strcmp(var, \"color.diff\") ||\n-\t    !strcmp(var, \"color.ui\")) {\n+\t    !strcmp(var, \"color.ui\") || !strcmp(var, \"diff.submodule\")) {\n \t\treturn 0;\n \t}\n \tif (!strcmp(var, \"format.numbered\")) {\ndiff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh\nindex 450d261..0ba8194 100755\n--- a/t/t4255-am-submodule.sh\n+++ b/t/t4255-am-submodule.sh\n@@ -80,12 +80,12 @@ test_expect_success 'diff.submodule unset with extra file' '\n \trun_test $THIRD second-submodule\n '\n \n-test_expect_failure 'diff.submodule=log' '\n+test_expect_success 'diff.submodule=log' '\n \ttest_config diff.submodule log &&\n \trun_test $SECOND first-submodule\n '\n \n-test_expect_failure 'diff.submodule=log with extra file' '\n+test_expect_success 'diff.submodule=log with extra file' '\n \ttest_config diff.submodule log &&\n \trun_test $THIRD second-submodule\n '\n-- \n2.0.5\n"}]}