* [PATCH 1/2] status/commit: show staged submodules regardless of ignore config
2014-04-05 16:57 [PATCH 0/2] status/commit: do not ignore staged submodules Jens Lehmann
@ 2014-04-05 16:59 ` Jens Lehmann
2014-04-05 16:59 ` [PATCH 2/2] commit -m: commit " Jens Lehmann
1 sibling, 0 replies; 3+ messages in thread
From: Jens Lehmann @ 2014-04-05 16:59 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Heiko Voigt, Ronald Weiss, Sergey Sharybin
Currently setting submodule.<name>.ignore and/or diff.ignoreSubmodules to
"all" suppresses all output of submodule changes for the diff family,
status and commit. For status and commit this is really confusing, as it
even when the user chooses to record a new commit for an ignored submodule
by adding it manually this change won't show up under the to-be-committed
changes. To add insult to injury, a later "git commit" will error out with
"nothing to commit" when only ignored submodules are staged.
Fix that by making wt_status always print staged submodule changes, no
matter what ignore settings are configured. The only exception is when the
user explicitly uses the "--ignore-submodules=all" command line option, in
that case the submodule output is still suppressed. This also makes "git
commit" work again when only modifications of ignored submodules are
staged, as that command uses the "commitable" member of the wt_status
struct to determine if staged changes are present. But this only happens
when the commit command uses the wt_status* functions to produce status
output for human consumption (when forking an editor or with --dry-run),
in all other cases (e.g. when run in a script with '-m') another code path
is taken which uses index_differs_from() to determine if any changes are
staged which still ignores submodules according to their configuration.
This will be fixed in a follow-up commit.
Change t7508 to reflect this new behavior and add three new tests to show
that a single staged submodule configured to be ignored will be committed
when the status output is generated and won't be if not. Also update the
documentation of the ignore config options accordingly.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
Documentation/config.txt | 8 +++--
Documentation/gitmodules.txt | 4 ++-
t/t7508-status.sh | 74 ++++++++++++++++++++++++++++++++++++++++++--
wt-status.c | 12 ++++++-
4 files changed, 92 insertions(+), 6 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 84c7e3f..171a98e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2289,7 +2289,9 @@ status.submodulesummary::
--summary-limit option of linkgit:git-submodule[1]). Please note
that the summary output command will be suppressed for all
submodules when `diff.ignoreSubmodules` is set to 'all' or only
- for those submodules where `submodule.<name>.ignore=all`. To
+ for those submodules where `submodule.<name>.ignore=all`. The only
+ exception to that rule is that status and commit will show staged
+ submodule changes. To
also view the summary for ignored submodules you can either use
the --ignore-submodules=dirty command line option or the 'git
submodule summary' command, which shows a similar output but does
@@ -2320,7 +2322,9 @@ submodule.<name>.fetchRecurseSubmodules::
submodule.<name>.ignore::
Defines under what circumstances "git status" and the diff family show
a submodule as modified. When set to "all", it will never be considered
- modified, "dirty" will ignore all changes to the submodules work tree and
+ modified (but it will nonetheless show up in the output of status and
+ commit when it has been staged), "dirty" will ignore all changes
+ to the submodules work tree and
takes only differences between the HEAD of the submodule and the commit
recorded in the superproject into account. "untracked" will additionally
let submodules with modified tracked files in their work tree show up.
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index 347a9f7..f6c0dfd 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -67,7 +67,9 @@ submodule.<name>.fetchRecurseSubmodules::
submodule.<name>.ignore::
Defines under what circumstances "git status" and the diff family show
a submodule as modified. When set to "all", it will never be considered
- modified, "dirty" will ignore all changes to the submodules work tree and
+ modified (but will nonetheless show up in the output of status and
+ commit when it has been staged), "dirty" will ignore all changes
+ to the submodules work tree and
takes only differences between the HEAD of the submodule and the commit
recorded in the superproject into account. "untracked" will additionally
let submodules with modified tracked files in their work tree show up.
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index c987b5e..e6483fc 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1380,7 +1380,32 @@ EOF
test_i18ncmp expect output
'
-test_expect_success '.gitmodules ignore=all suppresses submodule summary' '
+test_expect_success '.gitmodules ignore=all suppresses unstaged submodule summary' '
+ cat > expect << EOF &&
+On branch master
+Changes to be committed:
+ (use "git reset HEAD <file>..." to unstage)
+
+ modified: sm
+
+Changes not staged for commit:
+ (use "git add <file>..." to update what will be committed)
+ (use "git checkout -- <file>..." to discard changes in working directory)
+
+ modified: dir1/modified
+
+Untracked files:
+ (use "git add <file>..." to include in what will be committed)
+
+ .gitmodules
+ dir1/untracked
+ dir2/modified
+ dir2/untracked
+ expect
+ output
+ untracked
+
+EOF
git config --add -f .gitmodules submodule.subname.ignore all &&
git config --add -f .gitmodules submodule.subname.path sm &&
git status > output &&
@@ -1388,7 +1413,7 @@ test_expect_success '.gitmodules ignore=all suppresses submodule summary' '
git config -f .gitmodules --remove-section submodule.subname
'
-test_expect_success '.git/config ignore=all suppresses submodule summary' '
+test_expect_success '.git/config ignore=all suppresses unstaged submodule summary' '
git config --add -f .gitmodules submodule.subname.ignore none &&
git config --add -f .gitmodules submodule.subname.path sm &&
git config --add submodule.subname.ignore all &&
@@ -1461,4 +1486,49 @@ test_expect_success 'Restore default test environment' '
git config --unset status.showUntrackedFiles
'
+test_expect_success 'git commit will commit a staged but ignored submodule' '
+ git config --add -f .gitmodules submodule.subname.ignore all &&
+ git config --add -f .gitmodules submodule.subname.path sm &&
+ git config --add submodule.subname.ignore all &&
+ git status -s --ignore-submodules=dirty >output &&
+ test_i18ngrep "^M. sm" output &&
+ GIT_EDITOR="echo hello >>\"\$1\"" &&
+ export GIT_EDITOR &&
+ git commit -uno &&
+ git status -s --ignore-submodules=dirty >output &&
+ test_i18ngrep ! "^M. sm" output
+'
+
+test_expect_success 'git commit --dry-run will show a staged but ignored submodule' '
+ git reset HEAD^ &&
+ git add sm &&
+ cat >expect << EOF &&
+On branch master
+Changes to be committed:
+ (use "git reset HEAD <file>..." to unstage)
+
+ modified: sm
+
+Changes not staged for commit:
+ (use "git add <file>..." to update what will be committed)
+ (use "git checkout -- <file>..." to discard changes in working directory)
+
+ modified: dir1/modified
+
+Untracked files not listed (use -u option to show untracked files)
+EOF
+ git commit -uno --dry-run >output &&
+ test_i18ncmp expect output &&
+ git status -s --ignore-submodules=dirty >output &&
+ test_i18ngrep "^M. sm" output
+'
+
+test_expect_failure 'git commit -m will commit a staged but ignored submodule' '
+ git commit -uno -m message &&
+ git status -s --ignore-submodules=dirty >output &&
+ test_i18ngrep ! "^M. sm" output &&
+ git config --remove-section submodule.subname &&
+ git config -f .gitmodules --remove-section submodule.subname
+'
+
test_done
diff --git a/wt-status.c b/wt-status.c
index ec7344e..86fec89 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -519,9 +519,19 @@ static void wt_status_collect_changes_index(struct wt_status *s)
opt.def = s->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference;
setup_revisions(0, NULL, &rev, &opt);
+ DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
if (s->ignore_submodule_arg) {
- DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
handle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);
+ } else {
+ /*
+ * Unless the user did explicitly request a submodule ignore
+ * mode by passing a command line option we do not ignore any
+ * changed submodule SHA-1s when comparing index and HEAD, no
+ * matter what is configured. Otherwise the user won't be
+ * shown any submodules she manually added (and which are
+ * staged to be committed), which would be really confusing.
+ */
+ handle_ignore_submodules_arg(&rev.diffopt, "dirty");
}
rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
--
1.9.1.476.g510abc7
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH 2/2] commit -m: commit staged submodules regardless of ignore config
2014-04-05 16:57 [PATCH 0/2] status/commit: do not ignore staged submodules Jens Lehmann
2014-04-05 16:59 ` [PATCH 1/2] status/commit: show staged submodules regardless of ignore config Jens Lehmann
@ 2014-04-05 16:59 ` Jens Lehmann
1 sibling, 0 replies; 3+ messages in thread
From: Jens Lehmann @ 2014-04-05 16:59 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Heiko Voigt, Ronald Weiss, Sergey Sharybin
The previous commit fixed the problem that the staged but that ignored
submodules did not show up in the status output of the commit command and
weren't committed afterwards either. But when commit doesn't generate the
status output (e.g. when used in a script with '-m') the ignored submodule
will still not be committed. This is because in that case a different code
path is taken which calls index_differs_from() instead of calling the
wt_status functions.
Fix that by calling index_differs_from() from builtin/commit.c with a
diff_options argument value that tells it not ignore any submodule changes
unless the '--ignore-submodules' option is used. Even though this option
isn't yet implemented for cmd_commit() but only for cmd_status() this
prepares cmd_commit() to correctly handle the '--ignore-submodules' option
later. As status and commit share the same ignore_submodule_arg variable
this makes the code more robust against accidental breakage and documents
how to correctly call index_differs_from().
Change the expected result of the test documenting this problem from
failure to success.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
builtin/commit.c | 18 ++++++++++++++++--
t/t7508-status.sh | 2 +-
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index d9550c5..a456a60 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -833,8 +833,22 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
if (get_sha1(parent, sha1))
commitable = !!active_nr;
- else
- commitable = index_differs_from(parent, 0);
+ else {
+ /*
+ * Unless the user did explicitly request a submodule
+ * ignore mode by passing a command line option we do
+ * not ignore any changed submodule SHA-1s when
+ * comparing index and parent, no matter what is
+ * configured. Otherwise we won't commit any
+ * submodules which were manually staged, which would
+ * be really confusing.
+ */
+ int diff_flags = DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
+ if (ignore_submodule_arg &&
+ !strcmp(ignore_submodule_arg, "all"))
+ diff_flags |= DIFF_OPT_IGNORE_SUBMODULES;
+ commitable = index_differs_from(parent, diff_flags);
+ }
}
strbuf_release(&committer_ident);
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index e6483fc..d480069 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1523,7 +1523,7 @@ EOF
test_i18ngrep "^M. sm" output
'
-test_expect_failure 'git commit -m will commit a staged but ignored submodule' '
+test_expect_success 'git commit -m will commit a staged but ignored submodule' '
git commit -uno -m message &&
git status -s --ignore-submodules=dirty >output &&
test_i18ngrep ! "^M. sm" output &&
--
1.9.1.476.g510abc7
^ permalink raw reply related [flat|nested] 3+ messages in thread