git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[RFC/WIP PATCH v2] disable complete ignorance of submodules for index <-> HEAD diff

From
Heiko Voigt <hvoigt@hvoigt.net>
Date
Nov 29, 2013, 23:11 UTC
Message-ID
<20131129231125.GC31636@sandbox-ub>
In-Reply-To
<20131128071001.GA1057@book.hvoigt.net>

If the value of ignore for submodules is set to "all" we would not show whats actually committed during status or diff. This can result in the user committing unexpected submodule references. Lets be nicer and always show whats in the index.

Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
On Thu, Nov 28, 2013 at 08:10:01AM +0100, Heiko Voigt wrote:
Show 11 quoted lines
> On Mon, Nov 25, 2013 at 03:01:34PM +0600, Sergey Sharybin wrote:
> > Tested the patch. `git status` now shows the changes to the
> > submodules, which is nice :)
> > 
> > However, is it possible to make it so `git commit` lists submodules in
> > "changes to be committed" section, so you'll see what's gonna to be in
> > the commit while typing the commit message as well?
> 
> Yes, of course that should be shown. Will add in the next iteration.
> Which will hopefully be a much simpler implementation. Possibly getting
> rid of this new flag.

Here is an updated version of this patch. The code is a little bit more simplified and I changed the existing tests to account for the new behavior we are discussing. If everyone agrees that this a desired change in behavior I would continue adding more tests so commit, status and so on are more explicitly tested.

Cheers Heiko

P.S.: This is still work in progress, the complete series should contain both my patches from this thread.

 builtin/diff.c            |  2 ++
 diff-lib.c                |  3 +++
 diff.h                    |  2 +-
 submodule.c               | 16 ++++++++++++++--
 submodule.h               |  1 +
 t/t4027-diff-submodule.sh | 12 +++++++++---
 t/t7508-status.sh         |  6 +++++-
 7 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/builtin/diff.c b/builtin/diff.c
index adb93a9..c47614d 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -162,6 +162,8 @@ static int builtin_diff_tree(struct rev_info *revs,
 	if (argc > 1)
 		usage(builtin_diff_usage);
 
+	enforce_no_complete_ignore_submodule(&revs->diffopt);
+
 	/*
 	 * We saw two trees, ent0 and ent1.  If ent1 is uninteresting,
 	 * swap them.
diff --git a/diff-lib.c b/diff-lib.c
index 346cac6..c5219cb 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -483,6 +483,9 @@ int run_diff_index(struct rev_info *revs, int cached)
 {
 	struct object_array_entry *ent;
 
+	if (cached)
+		enforce_no_complete_ignore_submodule(&revs->diffopt);
+
 	ent = revs->pending.objects;
 	if (diff_cache(revs, ent->item->sha1, ent->name, cached))
 		exit(128);
diff --git a/diff.h b/diff.h
index e342325..81561b3 100644
--- a/diff.h
+++ b/diff.h
@@ -64,7 +64,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
 #define DIFF_OPT_FIND_COPIES_HARDER  (1 <<  6)
 #define DIFF_OPT_FOLLOW_RENAMES      (1 <<  7)
 #define DIFF_OPT_RENAME_EMPTY        (1 <<  8)
-/* (1 <<  9) unused */
+#define DIFF_OPT_NO_IGNORE_SUBMODULE (1 <<  9)
 #define DIFF_OPT_HAS_CHANGES         (1 << 10)
 #define DIFF_OPT_QUICK               (1 << 11)
 #define DIFF_OPT_NO_INDEX            (1 << 12)
diff --git a/submodule.c b/submodule.c
index 1905d75..e0719b6 100644
--- a/submodule.c
+++ b/submodule.c
@@ -294,6 +294,16 @@ int parse_submodule_config_option(const char *var, const char *value)
 	return 0;
 }
 
+void enforce_no_complete_ignore_submodule(struct diff_options *diffopt)
+{
+	DIFF_OPT_SET(diffopt, NO_IGNORE_SUBMODULE);
+	if (DIFF_OPT_TST(diffopt, OVERRIDE_SUBMODULE_CONFIG) &&
+	    DIFF_OPT_TST(diffopt, IGNORE_SUBMODULES)) {
+		DIFF_OPT_CLR(diffopt, IGNORE_SUBMODULES);
+		DIFF_OPT_SET(diffopt, IGNORE_DIRTY_SUBMODULES);
+	}
+}
+
 void handle_ignore_submodules_arg(struct diff_options *diffopt,
 				  const char *arg)
 {
@@ -301,9 +311,11 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,
 	DIFF_OPT_CLR(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
 	DIFF_OPT_CLR(diffopt, IGNORE_DIRTY_SUBMODULES);
 
-	if (!strcmp(arg, "all"))
+	if (!strcmp(arg, "all")) {
+		if (DIFF_OPT_TST(diffopt, NO_IGNORE_SUBMODULE))
+			return;
 		DIFF_OPT_SET(diffopt, IGNORE_SUBMODULES);
-	else if (!strcmp(arg, "untracked"))
+	} else if (!strcmp(arg, "untracked"))
 		DIFF_OPT_SET(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
 	else if (!strcmp(arg, "dirty"))
 		DIFF_OPT_SET(diffopt, IGNORE_DIRTY_SUBMODULES);
diff --git a/submodule.h b/submodule.h
index 7beec48..2c8087e 100644
--- a/submodule.h
+++ b/submodule.h
@@ -20,6 +20,7 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
 int submodule_config(const char *var, const char *value, void *cb);
 void gitmodules_config(void);
 int parse_submodule_config_option(const char *var, const char *value);
+void enforce_no_complete_ignore_submodule(struct diff_options *diffopt);
 void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
 int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
 void show_submodule_summary(FILE *f, const char *path,
diff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh
index 518bf95..bd84ea7 100755
--- a/t/t4027-diff-submodule.sh
+++ b/t/t4027-diff-submodule.sh
@@ -258,7 +258,9 @@ test_expect_success 'git diff between submodule commits' '
 	expect_from_to >expect.body $subtip $subprev &&
 	test_cmp expect.body actual.body &&
 	git diff --ignore-submodules HEAD^..HEAD >actual &&
-	! test -s actual
+	sed -e "1,/^@@/d" actual >actual.body &&
+	expect_from_to >expect.body $subtip $subprev &&
+	test_cmp expect.body actual.body
 '
 
 test_expect_success 'git diff between submodule commits [.git/config]' '
@@ -274,7 +276,9 @@ test_expect_success 'git diff between submodule commits [.git/config]' '
 	test_cmp expect.body actual.body &&
 	git config submodule.subname.ignore all &&
 	git diff HEAD^..HEAD >actual &&
-	! test -s actual &&
+	sed -e "1,/^@@/d" actual >actual.body &&
+	expect_from_to >expect.body $subtip $subprev &&
+	test_cmp expect.body actual.body &&
 	git diff --ignore-submodules=dirty HEAD^..HEAD >actual &&
 	sed -e "1,/^@@/d" actual >actual.body &&
 	expect_from_to >expect.body $subtip $subprev &&
@@ -294,7 +298,9 @@ test_expect_success 'git diff between submodule commits [.gitmodules]' '
 	test_cmp expect.body actual.body &&
 	git config -f .gitmodules submodule.subname.ignore all &&
 	git diff HEAD^..HEAD >actual &&
-	! test -s actual &&
+	sed -e "1,/^@@/d" actual >actual.body &&
+	expect_from_to >expect.body $subtip $subprev &&
+	test_cmp expect.body actual.body &&
 	git config submodule.subname.ignore dirty &&
 	git config submodule.subname.path sub &&
 	git diff  HEAD^..HEAD >actual &&
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index c987b5e..977295f 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1357,6 +1357,11 @@ test_expect_success "status (core.commentchar with two chars with submodule summ
 test_expect_success "--ignore-submodules=all suppresses 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)
@@ -1374,7 +1379,6 @@ Untracked files:
 	output
 	untracked
 
-no changes added to commit (use "git add" and/or "git commit -a")
 EOF
 	git status --ignore-submodules=all > output &&
 	test_i18ncmp expect output

-- 
1.8.5.rc3.1.g223caec
Previous: Heiko VoigtNext: Ramkumar Ramachandra
Message 43 of 51 in “Git issues with submodules”
  1. Sergey SharybinNov 22, 2013
  2. Ramkumar RamachandraNov 22, 2013
  3. Sergey SharybinNov 22, 2013
  4. Ramkumar RamachandraNov 22, 2013
  5. Jeff KingNov 22, 2013
  6. Sergey SharybinNov 22, 2013
  7. Ramkumar RamachandraNov 22, 2013
  8. Sergey SharybinNov 22, 2013
  9. Sergey SharybinNov 22, 2013
  10. Ramkumar RamachandraNov 22, 2013
  11. Jens LehmannNov 22, 2013
  12. Sergey SharybinNov 22, 2013
  13. Heiko VoigtNov 22, 2013
  14. Jonathan NiederNov 22, 2013
  15. Jens LehmannNov 23, 2013
  16. Heiko VoigtNov 24, 2013
  17. Jens LehmannNov 24, 2013
  18. Sergey SharybinNov 25, 2013
  19. Heiko VoigtNov 25, 2013
  20. Sergey SharybinNov 25, 2013
  21. Heiko VoigtNov 25, 2013
  22. 0/4 less ignorance of submodules for ignore=allHeiko Voigt, Dec 4, 2013
  23. 1/4 disable complete ignorance of submodules for index <-> HEAD diffHeiko Voigt, Dec 4, 2013
  24. 2/4 fix 'git add' to skip submodules configured as ignoredHeiko Voigt, Dec 4, 2013
  25. 3/4 teach add -f option for ignored submodulesHeiko Voigt, Dec 4, 2013
  26. Junio C HamanoDec 6, 2013
  27. Heiko VoigtDec 9, 2013
  28. 4/4 always show committed submodules in summary after commitHeiko Voigt, Dec 4, 2013
  29. Heiko VoigtDec 4, 2013
  30. Junio C HamanoDec 4, 2013
  31. Heiko VoigtDec 4, 2013
  32. Jens LehmannDec 5, 2013
  33. Heiko VoigtDec 9, 2013
  34. Junio C HamanoDec 9, 2013
  35. Junio C HamanoNov 25, 2013
  36. Jens LehmannNov 26, 2013
  37. Junio C HamanoNov 26, 2013
  38. Jonathan NiederNov 26, 2013
  39. Junio C HamanoNov 26, 2013
  40. disable complete ignorance of submodules for index <-> HEAD diffHeiko Voigt, Nov 23, 2013
  41. Sergey SharybinNov 25, 2013
  42. Heiko VoigtNov 28, 2013
  43. disable complete ignorance of submodules for index <-> HEAD diffHeiko Voigt, Nov 29, 2013
  44. Ramkumar RamachandraNov 23, 2013
  45. Jens LehmannNov 23, 2013
  46. Heiko VoigtNov 24, 2013
  47. Junio C HamanoNov 25, 2013
  48. Heiko VoigtNov 29, 2013
  49. Ramkumar RamachandraNov 23, 2013
  50. Ramkumar RamachandraNov 22, 2013
  51. Jens LehmannNov 22, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.