{"thread":{"id":"36350","subject":"[PATCH 0/2] status/commit: do not ignore staged submodules","startedAt":"2014-04-05T16:57:59Z","lastAt":"2014-04-05T16:59:36Z","messageCount":3,"participants":["Jens Lehmann"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"238390","messageId":"53403617.7050506@web.de","threadId":"36350","inReplyTo":null,"subject":"[PATCH 0/2] status/commit: do not ignore staged submodules","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2014-04-05T16:57:59Z","receivedAt":"2014-04-05T16:57:59Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"This series fixes the problem that ignored but staged submodules do not show\nup in status and commit. Even though we do change default behavior here, I\nbelieve this is the Right Thing to do (and remember all interested parties in\nthe discussion that raised this issue agreed on that [1]).\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/238173\n\nJens Lehmann (2):\n  status/commit: show staged submodules regardless of ignore config\n  commit -m: commit staged submodules regardless of ignore config\n\n Documentation/config.txt     |  8 +++--\n Documentation/gitmodules.txt |  4 ++-\n builtin/commit.c             | 18 +++++++++--\n t/t7508-status.sh            | 74 ++++++++++++++++++++++++++++++++++++++++++--\n wt-status.c                  | 12 ++++++-\n 5 files changed, 108 insertions(+), 8 deletions(-)\n\n-- \n1.9.1.476.g510abc7\n"},{"id":"238391","messageId":"53403657.1020806@web.de","threadId":"36350","inReplyTo":"53403617.7050506@web.de","subject":"[PATCH 1/2] status/commit: show staged submodules regardless of ignore config","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2014-04-05T16:59:03Z","receivedAt":"2014-04-05T16:59:03Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Currently setting submodule.<name>.ignore and/or diff.ignoreSubmodules to\n\"all\" suppresses all output of submodule changes for the diff family,\nstatus and commit. For status and commit this is really confusing, as it\neven when the user chooses to record a new commit for an ignored submodule\nby adding it manually this change won't show up under the to-be-committed\nchanges. To add insult to injury, a later \"git commit\" will error out with\n\"nothing to commit\" when only ignored submodules are staged.\n\nFix that by making wt_status always print staged submodule changes, no\nmatter what ignore settings are configured. The only exception is when the\nuser explicitly uses the \"--ignore-submodules=all\" command line option, in\nthat case the submodule output is still suppressed. This also makes \"git\ncommit\" work again when only modifications of ignored submodules are\nstaged, as that command uses the \"commitable\" member of the wt_status\nstruct to determine if staged changes are present. But this only happens\nwhen the commit command uses the wt_status* functions to produce status\noutput for human consumption (when forking an editor or with --dry-run),\nin all other cases (e.g. when run in a script with '-m') another code path\nis taken which uses index_differs_from() to determine if any changes are\nstaged which still ignores submodules according to their configuration.\nThis will be fixed in a follow-up commit.\n\nChange t7508 to reflect this new behavior and add three new tests to show\nthat a single staged submodule configured to be ignored will be committed\nwhen the status output is generated and won't be if not. Also update the\ndocumentation of the ignore config options accordingly.\n\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n Documentation/config.txt     |  8 +++--\n Documentation/gitmodules.txt |  4 ++-\n t/t7508-status.sh            | 74 ++++++++++++++++++++++++++++++++++++++++++--\n wt-status.c                  | 12 ++++++-\n 4 files changed, 92 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 84c7e3f..171a98e 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2289,7 +2289,9 @@ status.submodulesummary::\n \t--summary-limit option of linkgit:git-submodule[1]). Please note\n \tthat the summary output command will be suppressed for all\n \tsubmodules when `diff.ignoreSubmodules` is set to 'all' or only\n-\tfor those submodules where `submodule.<name>.ignore=all`. To\n+\tfor those submodules where `submodule.<name>.ignore=all`. The only\n+\texception to that rule is that status and commit will show staged\n+\tsubmodule changes. To\n \talso view the summary for ignored submodules you can either use\n \tthe --ignore-submodules=dirty command line option or the 'git\n \tsubmodule summary' command, which shows a similar output but does\n@@ -2320,7 +2322,9 @@ submodule.<name>.fetchRecurseSubmodules::\n submodule.<name>.ignore::\n \tDefines under what circumstances \"git status\" and the diff family show\n \ta submodule as modified. When set to \"all\", it will never be considered\n-\tmodified, \"dirty\" will ignore all changes to the submodules work tree and\n+\tmodified (but it will nonetheless show up in the output of status and\n+\tcommit when it has been staged), \"dirty\" will ignore all changes\n+\tto the submodules work tree and\n \ttakes only differences between the HEAD of the submodule and the commit\n \trecorded in the superproject into account. \"untracked\" will additionally\n \tlet submodules with modified tracked files in their work tree show up.\ndiff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt\nindex 347a9f7..f6c0dfd 100644\n--- a/Documentation/gitmodules.txt\n+++ b/Documentation/gitmodules.txt\n@@ -67,7 +67,9 @@ submodule.<name>.fetchRecurseSubmodules::\n submodule.<name>.ignore::\n \tDefines under what circumstances \"git status\" and the diff family show\n \ta submodule as modified. When set to \"all\", it will never be considered\n-\tmodified, \"dirty\" will ignore all changes to the submodules work tree and\n+\tmodified (but will nonetheless show up in the output of status and\n+\tcommit when it has been staged), \"dirty\" will ignore all changes\n+\tto the submodules work tree and\n \ttakes only differences between the HEAD of the submodule and the commit\n \trecorded in the superproject into account. \"untracked\" will additionally\n \tlet submodules with modified tracked files in their work tree show up.\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex c987b5e..e6483fc 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -1380,7 +1380,32 @@ EOF\n \ttest_i18ncmp expect output\n '\n\n-test_expect_success '.gitmodules ignore=all suppresses submodule summary' '\n+test_expect_success '.gitmodules ignore=all suppresses unstaged submodule summary' '\n+\tcat > expect << EOF &&\n+On branch master\n+Changes to be committed:\n+  (use \"git reset HEAD <file>...\" to unstage)\n+\n+\tmodified:   sm\n+\n+Changes not staged for commit:\n+  (use \"git add <file>...\" to update what will be committed)\n+  (use \"git checkout -- <file>...\" to discard changes in working directory)\n+\n+\tmodified:   dir1/modified\n+\n+Untracked files:\n+  (use \"git add <file>...\" to include in what will be committed)\n+\n+\t.gitmodules\n+\tdir1/untracked\n+\tdir2/modified\n+\tdir2/untracked\n+\texpect\n+\toutput\n+\tuntracked\n+\n+EOF\n \tgit config --add -f .gitmodules submodule.subname.ignore all &&\n \tgit config --add -f .gitmodules submodule.subname.path sm &&\n \tgit status > output &&\n@@ -1388,7 +1413,7 @@ test_expect_success '.gitmodules ignore=all suppresses submodule summary' '\n \tgit config -f .gitmodules  --remove-section submodule.subname\n '\n\n-test_expect_success '.git/config ignore=all suppresses submodule summary' '\n+test_expect_success '.git/config ignore=all suppresses unstaged submodule summary' '\n \tgit config --add -f .gitmodules submodule.subname.ignore none &&\n \tgit config --add -f .gitmodules submodule.subname.path sm &&\n \tgit config --add submodule.subname.ignore all &&\n@@ -1461,4 +1486,49 @@ test_expect_success 'Restore default test environment' '\n \tgit config --unset status.showUntrackedFiles\n '\n\n+test_expect_success 'git commit will commit a staged but ignored submodule' '\n+\tgit config --add -f .gitmodules submodule.subname.ignore all &&\n+\tgit config --add -f .gitmodules submodule.subname.path sm &&\n+\tgit config --add submodule.subname.ignore all &&\n+\tgit status -s --ignore-submodules=dirty >output &&\n+\ttest_i18ngrep \"^M. sm\" output &&\n+\tGIT_EDITOR=\"echo hello >>\\\"\\$1\\\"\" &&\n+\texport GIT_EDITOR &&\n+\tgit commit -uno &&\n+\tgit status -s --ignore-submodules=dirty >output &&\n+\ttest_i18ngrep ! \"^M. sm\" output\n+'\n+\n+test_expect_success 'git commit --dry-run will show a staged but ignored submodule' '\n+\tgit reset HEAD^ &&\n+\tgit add sm &&\n+\tcat >expect << EOF &&\n+On branch master\n+Changes to be committed:\n+  (use \"git reset HEAD <file>...\" to unstage)\n+\n+\tmodified:   sm\n+\n+Changes not staged for commit:\n+  (use \"git add <file>...\" to update what will be committed)\n+  (use \"git checkout -- <file>...\" to discard changes in working directory)\n+\n+\tmodified:   dir1/modified\n+\n+Untracked files not listed (use -u option to show untracked files)\n+EOF\n+\tgit commit -uno --dry-run >output &&\n+\ttest_i18ncmp expect output &&\n+\tgit status -s --ignore-submodules=dirty >output &&\n+\ttest_i18ngrep \"^M. sm\" output\n+'\n+\n+test_expect_failure 'git commit -m will commit a staged but ignored submodule' '\n+\tgit commit -uno -m message &&\n+\tgit status -s --ignore-submodules=dirty >output &&\n+\t test_i18ngrep ! \"^M. sm\" output &&\n+\tgit config --remove-section submodule.subname &&\n+\tgit config -f .gitmodules  --remove-section submodule.subname\n+'\n+\n test_done\ndiff --git a/wt-status.c b/wt-status.c\nindex ec7344e..86fec89 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -519,9 +519,19 @@ static void wt_status_collect_changes_index(struct wt_status *s)\n \topt.def = s->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference;\n \tsetup_revisions(0, NULL, &rev, &opt);\n\n+\tDIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);\n \tif (s->ignore_submodule_arg) {\n-\t\tDIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);\n \t\thandle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);\n+\t} else {\n+\t\t/*\n+\t\t * Unless the user did explicitly request a submodule ignore\n+\t\t * mode by passing a command line option we do not ignore any\n+\t\t * changed submodule SHA-1s when comparing index and HEAD, no\n+\t\t * matter what is configured. Otherwise the user won't be\n+\t\t * shown any submodules she manually added (and which are\n+\t\t * staged to be committed), which would be really confusing.\n+\t\t */\n+\t\thandle_ignore_submodules_arg(&rev.diffopt, \"dirty\");\n \t}\n\n \trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n-- \n1.9.1.476.g510abc7\n"},{"id":"238392","messageId":"53403678.3060708@web.de","threadId":"36350","inReplyTo":"53403617.7050506@web.de","subject":"[PATCH 2/2] commit -m: commit staged submodules regardless of ignore config","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2014-04-05T16:59:36Z","receivedAt":"2014-04-05T16:59:36Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"The previous commit fixed the problem that the staged but that ignored\nsubmodules did not show up in the status output of the commit command and\nweren't committed afterwards either. But when commit doesn't generate the\nstatus output (e.g. when used in a script with '-m') the ignored submodule\nwill still not be committed. This is because in that case a different code\npath is taken which calls index_differs_from() instead of calling the\nwt_status functions.\n\nFix that by calling index_differs_from() from builtin/commit.c with a\ndiff_options argument value that tells it not ignore any submodule changes\nunless the '--ignore-submodules' option is used. Even though this option\nisn't yet implemented for cmd_commit() but only for cmd_status() this\nprepares cmd_commit() to correctly handle the '--ignore-submodules' option\nlater. As status and commit share the same ignore_submodule_arg variable\nthis makes the code more robust against accidental breakage and documents\nhow to correctly call index_differs_from().\n\nChange the expected result of the test documenting this problem from\nfailure to success.\n\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n builtin/commit.c  | 18 ++++++++++++++++--\n t/t7508-status.sh |  2 +-\n 2 files changed, 17 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex d9550c5..a456a60 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -833,8 +833,22 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n\n \t\tif (get_sha1(parent, sha1))\n \t\t\tcommitable = !!active_nr;\n-\t\telse\n-\t\t\tcommitable = index_differs_from(parent, 0);\n+\t\telse {\n+\t\t\t/*\n+\t\t\t * Unless the user did explicitly request a submodule\n+\t\t\t * ignore mode by passing a command line option we do\n+\t\t\t * not ignore any changed submodule SHA-1s when\n+\t\t\t * comparing index and parent, no matter what is\n+\t\t\t * configured. Otherwise we won't commit any\n+\t\t\t * submodules which were manually staged, which would\n+\t\t\t * be really confusing.\n+\t\t\t */\n+\t\t\tint diff_flags = DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;\n+\t\t\tif (ignore_submodule_arg &&\n+\t\t\t    !strcmp(ignore_submodule_arg, \"all\"))\n+\t\t\t\tdiff_flags |= DIFF_OPT_IGNORE_SUBMODULES;\n+\t\t\tcommitable = index_differs_from(parent, diff_flags);\n+\t\t}\n \t}\n \tstrbuf_release(&committer_ident);\n\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex e6483fc..d480069 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -1523,7 +1523,7 @@ EOF\n \ttest_i18ngrep \"^M. sm\" output\n '\n\n-test_expect_failure 'git commit -m will commit a staged but ignored submodule' '\n+test_expect_success 'git commit -m will commit a staged but ignored submodule' '\n \tgit commit -uno -m message &&\n \tgit status -s --ignore-submodules=dirty >output &&\n \t test_i18ngrep ! \"^M. sm\" output &&\n-- \n1.9.1.476.g510abc7\n"}]}