[PATCH 0/5] git-add : Respect submodule ignore=all and only add changes with --force

[PATCH 0/5] git-add : Respect submodule ignore=all and only add changes with --force

[Claus Schneider via GitGitGadget]

The feature of configuring a submodule to "ignore=all" is nicely respected
in commands "status" and "diff". However the "add" command does not respect
the configuration the same way. The behavior is problematic for the logic
between status/diff and add. Secondly it makes it problematic to track
branches in the submodule configuration as developers unintentionally keeps
add submodule updates and get conflicts for no intentional reason. Both adds
unnecessary friction to the usage of submodules.

The patches implement the same logical behavior for ignore=all submodules as
regular ignored files. The status now does not show any diff - nor will the
add command update the reference submodule reference. If you add the
submodule path which is ignore=all then you are presented with a message
that you need to use the --force option. The branch=, ignore=all (and
update=none) now works great with update --remote, but developers does not
have to consider changes in the updates of the submodule sha1. The
implementation removes a friction of working with submodules and can be used
like the repo tool with branches configured. The submodule status report
could be used for build/release documentation for reproduction of a setup.

A few tests used the adding of submodules without --force, hence they have
been updated to use the --force option.

Claus Schneider(Eficode) (5):
  read-cache: update add_files_to_cache to take param
    ignored_too(--force)
  read-cache: let read-cache respect submodule ignore=all and --force
  tests: add new t2206-add-submodule-ignored.sh to test ignore=all
    scenario
  tests: fix existing tests when add an ignore=all submodule
  Documentation: update add --force and submodule ignore=all config

 Documentation/git-add.adoc       |   4 +-
 Documentation/gitmodules.adoc    |   5 +-
 builtin/add.c                    |   2 +-
 builtin/checkout.c               |   2 +-
 builtin/commit.c                 |   2 +-
 read-cache-ll.h                  |   2 +-
 read-cache.c                     |  54 ++++++++++++-
 t/lib-submodule-update.sh        |   6 +-
 t/meson.build                    |   1 +
 t/t2206-add-submodule-ignored.sh | 134 +++++++++++++++++++++++++++++++
 t/t7508-status.sh                |   2 +-
 11 files changed, 202 insertions(+), 12 deletions(-)
 create mode 100755 t/t2206-add-submodule-ignored.sh


base-commit: c44beea485f0f2feaf460e2ac87fdd5608d63cf0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1987%2FPraqma%2Frespect-submodule-ignore-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1987/Praqma/respect-submodule-ignore-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1987
-- 
gitgitgadget

[PATCH 1/5] read-cache: update add_files_to_cache to take param ignored_too(--force)

[Claus Schneider(Eficode) via GitGitGadget]

From: "Claus Schneider(Eficode)" <claus.schneider@eficode.com>

The ignored_too parameter is added to the function add_files_to_cache for
usage of explicit updating the index for the updated submodule using the
explicit patchspec to the submodule.

Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
---
 builtin/add.c      | 2 +-
 builtin/checkout.c | 2 +-
 builtin/commit.c   | 2 +-
 read-cache-ll.h    | 2 +-
 read-cache.c       | 8 +++++++-
 5 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index 0235854f80..cd1116e70a 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -582,7 +582,7 @@ int cmd_add(int argc,
 	else
 		exit_status |= add_files_to_cache(repo, prefix,
 						  &pathspec, ps_matched,
-						  include_sparse, flags);
+						  include_sparse, flags, ignored_too);
 
 	if (take_worktree_changes && !add_renormalize && !ignore_add_errors &&
 	    report_path_error(ps_matched, &pathspec))
diff --git a/builtin/checkout.c b/builtin/checkout.c
index f9453473fe..b2a404051d 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -899,7 +899,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
 			 */
 
 			add_files_to_cache(the_repository, NULL, NULL, NULL, 0,
-					   0);
+					   0, 0 );
 			init_ui_merge_options(&o, the_repository);
 			o.verbosity = 0;
 			work = write_in_core_index_as_tree(the_repository);
diff --git a/builtin/commit.c b/builtin/commit.c
index b5b9608813..5bf7ae5fc1 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -455,7 +455,7 @@ static const char *prepare_index(const char **argv, const char *prefix,
 		repo_hold_locked_index(the_repository, &index_lock,
 				       LOCK_DIE_ON_ERROR);
 		add_files_to_cache(the_repository, also ? prefix : NULL,
-				   &pathspec, ps_matched, 0, 0);
+				   &pathspec, ps_matched, 0, 0, 0 );
 		if (!all && report_path_error(ps_matched, &pathspec))
 			exit(128);
 
diff --git a/read-cache-ll.h b/read-cache-ll.h
index 71b49d9af4..2c8b4b21b1 100644
--- a/read-cache-ll.h
+++ b/read-cache-ll.h
@@ -481,7 +481,7 @@ int cmp_cache_name_compare(const void *a_, const void *b_);
 
 int add_files_to_cache(struct repository *repo, const char *prefix,
 		       const struct pathspec *pathspec, char *ps_matched,
-		       int include_sparse, int flags);
+		       int include_sparse, int flags, int ignored_too );
 
 void overlay_tree_on_index(struct index_state *istate,
 			   const char *tree_name, const char *prefix);
diff --git a/read-cache.c b/read-cache.c
index 06ad74db22..56a3ef424c 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -3880,9 +3880,12 @@ void overlay_tree_on_index(struct index_state *istate,
 
 struct update_callback_data {
 	struct index_state *index;
+	struct repository *repo;
+	struct pathspec *pathspec;
 	int include_sparse;
 	int flags;
 	int add_errors;
+	int ignored_too;
 };
 
 static int fix_unmerged_status(struct diff_filepair *p,
@@ -3945,7 +3948,7 @@ static void update_callback(struct diff_queue_struct *q,
 
 int add_files_to_cache(struct repository *repo, const char *prefix,
 		       const struct pathspec *pathspec, char *ps_matched,
-		       int include_sparse, int flags)
+		       int include_sparse, int flags, int ignored_too )
 {
 	struct update_callback_data data;
 	struct rev_info rev;
@@ -3954,6 +3957,9 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
 	data.index = repo->index;
 	data.include_sparse = include_sparse;
 	data.flags = flags;
+	data.repo = repo;
+	data.ignored_too = ignored_too;
+	data.pathspec = (struct pathspec *)pathspec;
 
 	repo_init_revisions(repo, &rev, prefix);
 	setup_revisions(0, NULL, &rev, NULL);
-- 
gitgitgadget

[PATCH 2/5] read-cache: let read-cache respect submodule ignore=all and --force

[Claus Schneider(Eficode) via GitGitGadget]

From: "Claus Schneider(Eficode)" <claus.schneider@eficode.com>

Given the submdule configuration is ignore=all then only update the
submdule if the --force option is given and the submodule is explicit
given in the pathspec.

A message is printed (like ignored files) guiding the user to use the
--force flag if the user has explicitely want to update the submodule
reference.

The reason for the change is support submodule branch tracking or
similar and git status state nothing and git add should not add either.
The workflow is more logic and similar to regular ignored files even
the submodule is already tracked.

The change opens up a lot of possibilities for submodules to be used
more freely and a like the repo tool. A submodule can be added for many
more reason and loosely coupled dependencies to the super repo which often
gives the friction of handle the explicit commits and updates without
the need for tracking the submodule sha1 by sha1.

Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
---
 read-cache.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/read-cache.c b/read-cache.c
index 56a3ef424c..3e87d03518 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -48,6 +48,8 @@
 #include "csum-file.h"
 #include "promisor-remote.h"
 #include "hook.h"
+#include "submodule.h"
+#include "submodule-config.h"
 
 /* Mask for the name length in ce_flags in the on-disk index */
 
@@ -3927,13 +3929,55 @@ static void update_callback(struct diff_queue_struct *q,
 		default:
 			die(_("unexpected diff status %c"), p->status);
 		case DIFF_STATUS_MODIFIED:
-		case DIFF_STATUS_TYPE_CHANGED:
+		case DIFF_STATUS_TYPE_CHANGED: {
+			struct stat st;
+			if (!lstat(path, &st) && S_ISDIR(st.st_mode)) { // only consider submodule if it is a directory
+				const struct submodule *sub = submodule_from_path(data->repo, null_oid(the_hash_algo), path);
+				if (sub && sub->name && sub->ignore && !strcmp(sub->ignore, "all")) {
+					int pathspec_matches = 0;
+					char *norm_pathspec = NULL;
+					int ps_i;
+					trace_printf("ignore=all %s\n", path);
+					trace_printf("pathspec %s\n",
+							(data->pathspec && data->pathspec->nr) ? "has pathspec" : "no pathspec");
+					/* Safely scan all pathspec items (q->nr may exceed pathspec->nr). */
+					if (data->pathspec) {
+						for (ps_i = 0; ps_i < data->pathspec->nr; ps_i++) {
+							const char *m = data->pathspec->items[ps_i].match;
+							if (!m)
+								continue;
+							norm_pathspec = xstrdup(m);
+							strip_dir_trailing_slashes(norm_pathspec);
+							if (!strcmp(path, norm_pathspec)) {
+								pathspec_matches = 1;
+								FREE_AND_NULL(norm_pathspec);
+								break;
+							}
+							FREE_AND_NULL(norm_pathspec);
+						}
+					}
+					if (pathspec_matches) {
+						if (data->ignored_too && data->ignored_too > 0) {
+							trace_printf("Forcing add of submodule ignored=all due to --force: %s\n", path);
+						} else {
+							printf(_("Skipping submodule due to ignore=all: %s"), path);
+							printf(_("Use -f if you really want to add them.") );
+							continue;
+						}
+					} else {
+						/* No explicit pathspec match -> skip silently (or with trace). */
+						trace_printf("pathspec does not match %s\n", path);
+						continue;
+					}
+				}
+			}
 			if (add_file_to_index(data->index, path, data->flags)) {
 				if (!(data->flags & ADD_CACHE_IGNORE_ERRORS))
 					die(_("updating files failed"));
 				data->add_errors++;
 			}
 			break;
+		}
 		case DIFF_STATUS_DELETED:
 			if (data->flags & ADD_CACHE_IGNORE_REMOVAL)
 				break;
-- 
gitgitgadget

[PATCH 3/5] tests: add new t2206-add-submodule-ignored.sh to test ignore=all scenario

[Claus Schneider(Eficode) via GitGitGadget]

From: "Claus Schneider(Eficode)" <claus.schneider@eficode.com>

The tests verify that the submodule behavior is intact and updating the
config with ignore=all also behaves as intended with configuration in
.gitmodules and configuration given on the command line.

Testfile is added to meson.build for execution.

Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
---
 t/meson.build                    |   1 +
 t/t2206-add-submodule-ignored.sh | 134 +++++++++++++++++++++++++++++++
 2 files changed, 135 insertions(+)
 create mode 100755 t/t2206-add-submodule-ignored.sh

diff --git a/t/meson.build b/t/meson.build
index bbeba1a8d5..1a6fb7edb2 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -281,6 +281,7 @@ integration_tests = [
   't2203-add-intent.sh',
   't2204-add-ignored.sh',
   't2205-add-worktree-config.sh',
+  't2206-add-submodule-ignored.sh',
   't2300-cd-to-toplevel.sh',
   't2400-worktree-add.sh',
   't2401-worktree-prune.sh',
diff --git a/t/t2206-add-submodule-ignored.sh b/t/t2206-add-submodule-ignored.sh
new file mode 100755
index 0000000000..77f7b0ebbc
--- /dev/null
+++ b/t/t2206-add-submodule-ignored.sh
@@ -0,0 +1,134 @@
+#!/bin/sh
+# shellcheck disable=SC2016
+
+# shellcheck disable=SC2034
+test_description='git add respects submodule ignore=all and explicit pathspec'
+
+# This test covers the behavior of "git add", "git status" and "git log" when
+# dealing with submodules that have the ignore=all setting in
+# .gitmodules. It ensures that changes in such submodules are
+# ignored by default, but can be staged with "git add --force".
+
+# shellcheck disable=SC1091
+. ./test-lib.sh
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+base_path=$(pwd -P)
+
+#1
+test_expect_success 'setup: create origin repos'  '
+	cd "${base_path}" &&
+	git config --global protocol.file.allow always &&
+	git init sub &&
+		pwd &&
+		cd sub &&
+		test_commit sub_file1 &&
+		git tag v1.0 &&
+		test_commit sub_file2 &&
+		git tag v2.0 &&
+		test_commit sub_file3 &&
+		git tag v3.0 &&
+	cd "${base_path}" &&
+	git init main &&
+		cd main &&
+		test_commit first &&
+	cd "${base_path}"
+'
+#2
+# add submodule with default config (ignore=none) and
+# check log that is contains a path entry for the submodule 'sub'
+# change the commit in the submodule and check that 'git status' shows it as modified
+test_expect_success 'main: add submodule with default config'  '
+	cd "${base_path}" &&
+	cd main &&
+	git submodule add ../sub &&
+	git commit -m "add submodule" &&
+	git log --oneline --name-only | grep "^sub$" &&
+	git -C sub reset --hard v2.0 &&
+	git status --porcelain | grep "^ M sub$" &&
+	echo
+'
+#3
+# change the submodule config to ignore=all and check that status and log do not show changes
+test_expect_success 'main: submodule config ignore=all'  '
+	cd "${base_path}" &&
+	cd main &&
+	git config -f .gitmodules submodule.sub.ignore all &&
+	GIT_TRACE=1 git add . &&
+	git commit -m "update submodule config sub.ignore all" &&
+	! git status --porcelain | grep "^.*$" &&
+	! git log --oneline --name-only | grep "^sub$" &&
+	echo
+'
+#4
+# change the commit in the submodule and check that 'git status' does not show it as modified
+# but 'git status --ignore-submodules=none' does show it as modified
+test_expect_success 'sub: change to different sha1 and check status in main'  '
+	cd "${base_path}" &&
+	cd main &&
+	git -C sub reset --hard v1.0 &&
+	! git status --porcelain | grep "^ M sub$" &&
+	git status --ignore-submodules=none --porcelain | grep "^ M sub$" &&
+	echo
+'
+
+#5
+# check that normal 'git add' does not stage the change in the submodule
+test_expect_success 'main: check normal add and status'  '
+	cd "${base_path}" &&
+	cd main &&
+	GIT_TRACE=1 git add . &&
+	! git status --porcelain | grep "^ M sub$" &&
+	echo
+'
+
+#6
+# check that 'git add --force .' does not stage the change in the submodule
+# and that 'git status' does not show it as modified
+test_expect_success 'main: check --force add . and status'  '
+	cd "${base_path}" &&
+	cd main &&
+	GIT_TRACE=1 git add --force . &&
+	! git status --porcelain | grep "^M  sub$" &&
+	echo
+'
+
+#7
+# check that 'git add .' does not stage the change in the submodule
+# and that 'git status' does not show it as modified
+test_expect_success 'main: check _add sub_ and status'  '
+	cd "${base_path}" &&
+	cd main &&
+	GIT_TRACE=1 git add sub | grep "Skipping submodule due to ignore=all: sub" &&
+	! git status --porcelain | grep "^M  sub$" &&
+	echo
+'
+
+#8
+# check that 'git add --force sub' does stage the change in the submodule
+# check that 'git add --force ./sub/' does stage the change in the submodule
+# and that 'git status --porcelain' does show it as modified
+# commit it..
+# check that 'git log --ignore-submodules=none' shows the submodule change
+# in the log
+test_expect_success 'main: check force add sub and ./sub/ and status'  '
+	cd "${base_path}" &&
+	cd main &&
+	echo "Adding with --force should work: git add --force sub" &&
+	GIT_TRACE=1 git add --force sub &&
+	git status --porcelain | grep "^M  sub$" &&
+	git restore --staged sub &&
+	! git status --porcelain | grep "^M  sub$" &&
+	echo "Adding with --force should work: git add --force ./sub/" &&
+	GIT_TRACE=1 git add --force ./sub/ &&
+	git status --porcelain | grep "^M  sub$" &&
+	git commit -m "update submodule pointer" &&
+	! git status --porcelain | grep "^ M sub$" &&
+	git log --ignore-submodules=none --name-only --oneline | grep "^sub$" &&
+	echo
+'
+
+test_done
+exit 0
-- 
gitgitgadget

[PATCH 4/5] tests: fix existing tests when add an ignore=all submodule

[Claus Schneider(Eficode) via GitGitGadget]

From: "Claus Schneider(Eficode)" <claus.schneider@eficode.com>

There are tests that rely on "git add <submodule>" also adds it. A --force
is needed with this enhancement hence they are added accordingly in these
tests.

Updated tests:
- t1013-read-tree-submodule.sh ( fixed in: t/lib-submodule-update.sh )
- t7406-submodule-update.sh
- t7508-status.sh

Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
---
 t/lib-submodule-update.sh | 6 +++---
 t/t7508-status.sh         | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 36f767cb74..f591de6120 100644
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -95,14 +95,14 @@ create_lib_submodule_repo () {
 			git commit -m "modified file2 and added file3" &&
 			git push origin modifications
 		) &&
-		git add sub1 &&
+		git add --force sub1 &&
 		git commit -m "Modify sub1" &&
 
 		git checkout -b add_nested_sub modify_sub1 &&
 		git -C sub1 checkout -b "add_nested_sub" &&
 		git -C sub1 submodule add --branch no_submodule ../submodule_update_sub2 sub2 &&
 		git -C sub1 commit -a -m "add a nested submodule" &&
-		git add sub1 &&
+		git add --force sub1 &&
 		git commit -a -m "update submodule, that updates a nested submodule" &&
 		git checkout -b modify_sub1_recursively &&
 		git -C sub1 checkout -b modify_sub1_recursively &&
@@ -112,7 +112,7 @@ create_lib_submodule_repo () {
 		git -C sub1/sub2 commit -m "make a change in nested sub" &&
 		git -C sub1 add sub2 &&
 		git -C sub1 commit -m "update nested sub" &&
-		git add sub1 &&
+		git add --force sub1 &&
 		git commit -m "update sub1, that updates nested sub" &&
 		git -C sub1 push origin modify_sub1_recursively &&
 		git -C sub1/sub2 push origin modify_sub1_recursively &&
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index cdc1d6fcc7..7617be14e8 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1565,7 +1565,7 @@ test_expect_success 'git commit will commit a staged but ignored submodule' '
 
 test_expect_success 'git commit --dry-run will show a staged but ignored submodule' '
 	git reset HEAD^ &&
-	git add sm &&
+	git add --force sm &&
 	cat >expect << EOF &&
 On branch main
 Your branch and '\''upstream'\'' have diverged,
-- 
gitgitgadget

[PATCH 5/5] Documentation: update add --force and submodule ignore=all config

[Claus Schneider(Eficode) via GitGitGadget]

From: "Claus Schneider(Eficode)" <claus.schneider@eficode.com>

- The add --force doc: append additionel info about the submodule
functionality.
- The submodule config ignore=all now need --force in order to update
  the index.

Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
---
 Documentation/git-add.adoc    | 4 +++-
 Documentation/gitmodules.adoc | 5 ++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-add.adoc b/Documentation/git-add.adoc
index b7a735824d..2234b1e846 100644
--- a/Documentation/git-add.adoc
+++ b/Documentation/git-add.adoc
@@ -77,7 +77,9 @@ in linkgit:gitglossary[7].
 
 `-f`::
 `--force`::
-	Allow adding otherwise ignored files.
+	Allow adding otherwise ignored files. The option is also used
+	when updating submodules and `submodule.<name>.ignore=all`
+	is set. The `path` to the submodule must be explicitly specified.
 
 `--sparse`::
 	Allow updating index entries outside of the sparse-checkout cone.
diff --git a/Documentation/gitmodules.adoc b/Documentation/gitmodules.adoc
index d9bec8b187..3aa71170ee 100644
--- a/Documentation/gitmodules.adoc
+++ b/Documentation/gitmodules.adoc
@@ -70,7 +70,10 @@ submodule.<name>.ignore::
 --
 	all;; The submodule will never be considered modified (but will
 	    nonetheless show up in the output of status and commit when it has
-	    been staged).
+	    been staged). Add `(new commits)` can be overruled using the
+	    `git add --force <submodule.path>` similar to ignored files. The setting has effect
+	    for `status`, `update-index`, `diff` and `log`(due to underlaying
+	    `diff`)
 
 	dirty;; All changes to the submodule's work tree will be ignored, only
 	    committed differences between the `HEAD` of the submodule and its
-- 
gitgitgadget

Re: [PATCH 0/5] git-add : Respect submodule ignore=all and only add changes with --force

[Phillip Wood]

Hi Claus

[I've adjusted the CC list slightly]

On 18/10/2025 21:07, Claus Schneider via GitGitGadget wrote:
> The feature of configuring a submodule to "ignore=all" is nicely respected
> in commands "status" and "diff". However the "add" command does not respect
> the configuration the same way.

I was curious why, when "git add" uses the same machinery as "git diff" 
to figure out which paths need updating, it behaves differently. It 
turns out that add_files_to_cache() contains

	rev.diffopt.flags.override_submodule_config = 1;

which makes "git add" ignore "submodule.<name>.ignore". Tracing the 
history of this line, it originates from 5556808690e (add, reset: ensure 
submodules can be added or reset, 2017-07-25) which made a deliberate 
choice for both "git add" and "git reset" not to behave like "git diff". 
If we're going to change the behavior then it would be helpful to 
explain how this patch series ameliorates the concerns that lead to that 
commit and why it is sensible to change the behavior of "git add" but 
not "git reset". It also suggests that a much simpler way of 
implementing the change would be to delete that line.

I'm not convinced that the approach of using "--force" is a good idea as 
it conflates ignoring changes to tracked paths (which is what 
submodule.<name>.ignore" does) with ignoring untracked paths (which is 
what ".gitignore" does). If we're happy to break existing uses that rely 
on the current behavior then having a new option to override 
submodule.<name>.ignore strikes me as a better way forward. I don't have 
much experience of using submodules so I can't comment on whether 
changing the behavior is a good idea or not.

Thanks

Phillip


  The behavior is problematic for the logic
> between status/diff and add. Secondly it makes it problematic to track
> branches in the submodule configuration as developers unintentionally keeps
> add submodule updates and get conflicts for no intentional reason. Both adds
> unnecessary friction to the usage of submodules.
> 
> The patches implement the same logical behavior for ignore=all submodules as
> regular ignored files. The status now does not show any diff - nor will the
> add command update the reference submodule reference. If you add the
> submodule path which is ignore=all then you are presented with a message
> that you need to use the --force option. The branch=, ignore=all (and
> update=none) now works great with update --remote, but developers does not
> have to consider changes in the updates of the submodule sha1. The
> implementation removes a friction of working with submodules and can be used
> like the repo tool with branches configured. The submodule status report
> could be used for build/release documentation for reproduction of a setup.
> 
> A few tests used the adding of submodules without --force, hence they have
> been updated to use the --force option.
> 
> Claus Schneider(Eficode) (5):
>    read-cache: update add_files_to_cache to take param
>      ignored_too(--force)
>    read-cache: let read-cache respect submodule ignore=all and --force
>    tests: add new t2206-add-submodule-ignored.sh to test ignore=all
>      scenario
>    tests: fix existing tests when add an ignore=all submodule
>    Documentation: update add --force and submodule ignore=all config
> 
>   Documentation/git-add.adoc       |   4 +-
>   Documentation/gitmodules.adoc    |   5 +-
>   builtin/add.c                    |   2 +-
>   builtin/checkout.c               |   2 +-
>   builtin/commit.c                 |   2 +-
>   read-cache-ll.h                  |   2 +-
>   read-cache.c                     |  54 ++++++++++++-
>   t/lib-submodule-update.sh        |   6 +-
>   t/meson.build                    |   1 +
>   t/t2206-add-submodule-ignored.sh | 134 +++++++++++++++++++++++++++++++
>   t/t7508-status.sh                |   2 +-
>   11 files changed, 202 insertions(+), 12 deletions(-)
>   create mode 100755 t/t2206-add-submodule-ignored.sh
> 
> 
> base-commit: c44beea485f0f2feaf460e2ac87fdd5608d63cf0
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1987%2FPraqma%2Frespect-submodule-ignore-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1987/Praqma/respect-submodule-ignore-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1987

Re: [PATCH 0/5] git-add : Respect submodule ignore=all and only add changes with --force

[Claus Schneider]

Hi again ..

I have read the documentation for submodule and reset. See from a
non-git-developer it is actually pretty hard to understand what
happens with regards to the "reset" command.

The only hint I can find which does not mean a lot to an ordinary user:
- "Defines under what circumstances "git status" and the diff family
show a submodule as modified".
- GPT-5 does not say anything regarding "reset", if asked a few levels
of "/explains" regarding "diff family". I find the documentation
inconclusive as an end-user.
- Looking at the "reset" --help it does only talk about
--[no-]recurse-submodules but not really what the ignore=all
- I can find the override_submodule_config=1 in reset, commit, status,
add-interactive, read-cache.
.. so I am a bit confused.

From an end-user perspective I would expect this of "all":
- (pre-action): "status" and "diff-of-workspace/staging". It should
respect ignore=all unless overridden or explicit specified
- (action): "add" : It should respect ignore=all unless overridden or
explicit specified(--force)
- (action): "reset", "restore" ..<etc>: It should respect ignore=all
unless overridden or explicit specified and leave it to submodules
--init / --remote
- (action): "clean": It could be argued of the -x option should also
act on submodules - but fair to leave it to submodules --init /
--remote
- (post-action) "diff-of-history", "log" (aka <ref1> <ref2> ) : It
should always tell what happened in history (git-graph) despite what
the current .gitmodules states regards to ignore=all

It could be a new option value should be added like "always" or
"logical" respectively the above and then leave the "all" to the
current behavior.

Best regards
Claus



On Sun, Oct 19, 2025 at 11:55 PM Claus Schneider
<claus.schneider@eficode.com> wrote:
>
> Hi Philip ..
>
> Thank you for your feedback and for investigating this. I was not aware of the setting that causes `add` and `reset` to override submodule configuration, and I will need to look into `reset` further.
>
> I understand the problematic aspect of not being able to add an update of a submodule reference, which likely led to the overwrite setting. From a Git developer's perspective, always adding it might have seemed like the simplest approach.
>
> However, from an end-user perspective, it's not logical for `status` to show nothing while `add` has an effect. A more intuitive workflow would align with how ignored files are handled even though it is already tracked.
>
> My patch implements what I believe should have been in the first place. My implementation still needs the `overwrite=1` set in order to get the diff files list so I can 'operate' on it and make the `--force` logic like the ignore files.
>
> Best regards
> Claus
>
> On Sun, Oct 19, 2025, 17:34 Phillip Wood <phillip.wood123@gmail.com> wrote:
>>
>> Hi Claus
>>
>> [I've adjusted the CC list slightly]
>>
>> On 18/10/2025 21:07, Claus Schneider via GitGitGadget wrote:
>> > The feature of configuring a submodule to "ignore=all" is nicely respected
>> > in commands "status" and "diff". However the "add" command does not respect
>> > the configuration the same way.
>>
>> I was curious why, when "git add" uses the same machinery as "git diff"
>> to figure out which paths need updating, it behaves differently. It
>> turns out that add_files_to_cache() contains
>>
>>         rev.diffopt.flags.override_submodule_config = 1;
>>
>> which makes "git add" ignore "submodule.<name>.ignore". Tracing the
>> history of this line, it originates from 5556808690e (add, reset: ensure
>> submodules can be added or reset, 2017-07-25) which made a deliberate
>> choice for both "git add" and "git reset" not to behave like "git diff".
>> If we're going to change the behavior then it would be helpful to
>> explain how this patch series ameliorates the concerns that lead to that
>> commit and why it is sensible to change the behavior of "git add" but
>> not "git reset". It also suggests that a much simpler way of
>> implementing the change would be to delete that line.
>>
>> I'm not convinced that the approach of using "--force" is a good idea as
>> it conflates ignoring changes to tracked paths (which is what
>> submodule.<name>.ignore" does) with ignoring untracked paths (which is
>> what ".gitignore" does). If we're happy to break existing uses that rely
>> on the current behavior then having a new option to override
>> submodule.<name>.ignore strikes me as a better way forward. I don't have
>> much experience of using submodules so I can't comment on whether
>> changing the behavior is a good idea or not.
>>
>> Thanks
>>
>> Phillip
>>
>>
>>   The behavior is problematic for the logic
>> > between status/diff and add. Secondly it makes it problematic to track
>> > branches in the submodule configuration as developers unintentionally keeps
>> > add submodule updates and get conflicts for no intentional reason. Both adds
>> > unnecessary friction to the usage of submodules.
>> >
>> > The patches implement the same logical behavior for ignore=all submodules as
>> > regular ignored files. The status now does not show any diff - nor will the
>> > add command update the reference submodule reference. If you add the
>> > submodule path which is ignore=all then you are presented with a message
>> > that you need to use the --force option. The branch=, ignore=all (and
>> > update=none) now works great with update --remote, but developers does not
>> > have to consider changes in the updates of the submodule sha1. The
>> > implementation removes a friction of working with submodules and can be used
>> > like the repo tool with branches configured. The submodule status report
>> > could be used for build/release documentation for reproduction of a setup.
>> >
>> > A few tests used the adding of submodules without --force, hence they have
>> > been updated to use the --force option.
>> >
>> > Claus Schneider(Eficode) (5):
>> >    read-cache: update add_files_to_cache to take param
>> >      ignored_too(--force)
>> >    read-cache: let read-cache respect submodule ignore=all and --force
>> >    tests: add new t2206-add-submodule-ignored.sh to test ignore=all
>> >      scenario
>> >    tests: fix existing tests when add an ignore=all submodule
>> >    Documentation: update add --force and submodule ignore=all config
>> >
>> >   Documentation/git-add.adoc       |   4 +-
>> >   Documentation/gitmodules.adoc    |   5 +-
>> >   builtin/add.c                    |   2 +-
>> >   builtin/checkout.c               |   2 +-
>> >   builtin/commit.c                 |   2 +-
>> >   read-cache-ll.h                  |   2 +-
>> >   read-cache.c                     |  54 ++++++++++++-
>> >   t/lib-submodule-update.sh        |   6 +-
>> >   t/meson.build                    |   1 +
>> >   t/t2206-add-submodule-ignored.sh | 134 +++++++++++++++++++++++++++++++
>> >   t/t7508-status.sh                |   2 +-
>> >   11 files changed, 202 insertions(+), 12 deletions(-)
>> >   create mode 100755 t/t2206-add-submodule-ignored.sh
>> >
>> >
>> > base-commit: c44beea485f0f2feaf460e2ac87fdd5608d63cf0
>> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1987%2FPraqma%2Frespect-submodule-ignore-v1
>> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1987/Praqma/respect-submodule-ignore-v1
>> > Pull-Request: https://github.com/gitgitgadget/git/pull/1987
>>

Re: [PATCH 0/5] git-add : Respect submodule ignore=all and only add changes with --force

[Phillip Wood]

Hi Claus

On 19/10/2025 22:39, Claus Schneider wrote:
> On Sun, Oct 19, 2025, 17:34 Phillip Wood <phillip.wood123@gmail.com 
> <mailto:phillip.wood123@gmail.com>> wrote:
> 
>> I was curious why, when "git add" uses the same machinery as "git diff"
>> to figure out which paths need updating, it behaves differently. It
>> turns out that add_files_to_cache() contains
>> 
>>           rev.diffopt.flags.override_submodule_config = 1;
>> 
>> which makes "git add" ignore "submodule.<name>.ignore". Tracing the
>> history of this line, it originates from 5556808690e (add, reset: ensure
>> submodules can be added or reset, 2017-07-25) which made a deliberate
>> choice for both "git add" and "git reset" not to behave like "git diff".

> Thank you for your feedback and for investigating this. I was not aware > of the setting that causes `add` and `reset` to override submodule 
> configuration, and I will need to look into `reset` further.

You should mention the commit that added the current behavior and the
reason it was added in the commit message where you change the behavior.

> I understand the problematic aspect of not being able to add an update 
> of a submodule reference, which likely led to the overwrite setting. 
> From a Git developer's perspective, always adding it might have seemed 
> like the simplest approach.
> 
> However, from an end-user perspective, it's not logical for `status` to 
> show nothing while `add` has an effect. 

I'm quite sympathetic to this view, I think you should explain this in the
commit message where you change the behavior and see what others think.

> A more intuitive workflow would 
> align with how ignored files are handled even though it is already tracked.

As I said before do not think conflating ignoring changes to tracked files
with ignoring files is a good idea. The two are fundamentally different
because ignored files are not tracked. I would suggest adding a new option
such as "--include-ignored-submodules" instead of piggybacking on "--force".
Using a different option also means that user's will not accidentally stage
ignored files when they're trying to stage a submodule whose changes are
normally ignored.

> My patch implements what I believe should have been in the first place. 
> My implementation still needs the `overwrite=1` set in order to get the 
> diff files list so I can 'operate' on it and make the `--force` logic 
> like the ignore files.

Oh, you're right, if we want to print a warning we will need to keep
overriding the submodule config.

Hopefully someone with more experience of submodules will be able to review
the code soon.

Thanks

Phillip

Re: [PATCH 0/5] git-add : Respect submodule ignore=all and only add changes with --force

[Claus Schneider]

Thanks for the feedback and suggestions.

I have now implemented the --include-ignored-submodules option instead
of --force.
I have also extended the commit messages with more reasoning.

? Is it best to have a single commit with everything or a set of
commit for each part of the solution

? How do I update the patches with the modifications accordingly (
from GitGadget PR with a new /submit ) ?

Thanks in advance
Claus Schneider

On Fri, Oct 24, 2025 at 3:55 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Claus
>
> On 19/10/2025 22:39, Claus Schneider wrote:
> > On Sun, Oct 19, 2025, 17:34 Phillip Wood <phillip.wood123@gmail.com
> > <mailto:phillip.wood123@gmail.com>> wrote:
> >
> >> I was curious why, when "git add" uses the same machinery as "git diff"
> >> to figure out which paths need updating, it behaves differently. It
> >> turns out that add_files_to_cache() contains
> >>
> >>           rev.diffopt.flags.override_submodule_config = 1;
> >>
> >> which makes "git add" ignore "submodule.<name>.ignore". Tracing the
> >> history of this line, it originates from 5556808690e (add, reset: ensure
> >> submodules can be added or reset, 2017-07-25) which made a deliberate
> >> choice for both "git add" and "git reset" not to behave like "git diff".
>
> > Thank you for your feedback and for investigating this. I was not aware > of the setting that causes `add` and `reset` to override submodule
> > configuration, and I will need to look into `reset` further.
>
> You should mention the commit that added the current behavior and the
> reason it was added in the commit message where you change the behavior.
>
> > I understand the problematic aspect of not being able to add an update
> > of a submodule reference, which likely led to the overwrite setting.
> > From a Git developer's perspective, always adding it might have seemed
> > like the simplest approach.
> >
> > However, from an end-user perspective, it's not logical for `status` to
> > show nothing while `add` has an effect.
>
> I'm quite sympathetic to this view, I think you should explain this in the
> commit message where you change the behavior and see what others think.
>
> > A more intuitive workflow would
> > align with how ignored files are handled even though it is already tracked.
>
> As I said before do not think conflating ignoring changes to tracked files
> with ignoring files is a good idea. The two are fundamentally different
> because ignored files are not tracked. I would suggest adding a new option
> such as "--include-ignored-submodules" instead of piggybacking on "--force".
> Using a different option also means that user's will not accidentally stage
> ignored files when they're trying to stage a submodule whose changes are
> normally ignored.
>
> > My patch implements what I believe should have been in the first place.
> > My implementation still needs the `overwrite=1` set in order to get the
> > diff files list so I can 'operate' on it and make the `--force` logic
> > like the ignore files.
>
> Oh, you're right, if we want to print a warning we will need to keep
> overriding the submodule config.
>
> Hopefully someone with more experience of submodules will be able to review
> the code soon.
>
> Thanks
>
> Phillip

[PATCH v2 0/5] git-add : Respect submodule ignore=all and only add changes with --force

[Claus Schneider via GitGitGadget]

The feature of configuring a submodule to "ignore=all" is nicely respected
in commands "status" and "diff". However the "add" command does not respect
the configuration the same way. The behavior is problematic for the logic
between status/diff and add. Secondly it makes it problematic to track
branches in the submodule configuration as developers unintentionally keeps
add submodule updates and get conflicts for no intentional reason. Both adds
unnecessary friction to the usage of submodules.

The patches implement the same logical behavior for ignore=all submodules as
regular ignored files. The status now does not show any diff - nor will the
add command update the reference submodule reference. If you add the
submodule path which is ignore=all then you are presented with a message
that you need to use the --force option. The branch=, ignore=all (and
update=none) now works great with update --remote, but developers does not
have to consider changes in the updates of the submodule sha1. The
implementation removes a friction of working with submodules and can be used
like the repo tool with branches configured. The submodule status report
could be used for build/release documentation for reproduction of a setup.

A few tests used the adding of submodules without --force, hence they have
been updated to use the --force option.

Claus Schneider(Eficode) (5):
  read-cache: update add_files_to_cache take param
    include_ignored_submodules
  read-cache: add/read-cache respect submodule ignore=all
  tests: add new t2206-add-submodule-ignored.sh to test ignore=all
    scenario
  tests: fix existing tests when add an ignore=all submodule
  Documentation: add --include_ignored_submodules + ignore=all config

 .devcontainer/Dockerfile            |  70 +++++++++++++++
 .devcontainer/Dockerfile.standalone |  76 ++++++++++++++++
 .devcontainer/devcontainer.json     |  25 ++++++
 Documentation/config/submodule.adoc |  13 +--
 Documentation/git-add.adoc          |   5 ++
 Documentation/gitmodules.adoc       |   5 +-
 builtin/add.c                       |   4 +-
 builtin/checkout.c                  |   2 +-
 builtin/commit.c                    |   2 +-
 read-cache-ll.h                     |   2 +-
 read-cache.c                        |  54 ++++++++++-
 t/lib-submodule-update.sh           |   6 +-
 t/meson.build                       |   1 +
 t/t2206-add-submodule-ignored.sh    | 134 ++++++++++++++++++++++++++++
 t/t7508-status.sh                   |   2 +-
 15 files changed, 384 insertions(+), 17 deletions(-)
 create mode 100644 .devcontainer/Dockerfile
 create mode 100644 .devcontainer/Dockerfile.standalone
 create mode 100644 .devcontainer/devcontainer.json
 create mode 100755 t/t2206-add-submodule-ignored.sh


base-commit: 81f86aacc4eb74cdb9c2c8082d36d2070c666045
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1987%2FPraqma%2Frespect-submodule-ignore-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1987/Praqma/respect-submodule-ignore-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1987

Range-diff vs v1:

 1:  d98cca698d ! 1:  5796009122 read-cache: update add_files_to_cache to take param ignored_too(--force)
     @@ Metadata
      Author: Claus Schneider(Eficode) <claus.schneider@eficode.com>
      
       ## Commit message ##
     -    read-cache: update add_files_to_cache to take param ignored_too(--force)
     +    read-cache: update add_files_to_cache take param include_ignored_submodules
      
     -    The ignored_too parameter is added to the function add_files_to_cache for
     -    usage of explicit updating the index for the updated submodule using the
     -    explicit patchspec to the submodule.
     +    The include_ignored_submodules parameter is added to the function
     +    add_files_to_cache for usage of explicit updating the index for the updated
     +    submodule using the explicit patchspec to the submodule.
      
          Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
      
       ## builtin/add.c ##
     +@@ builtin/add.c: N_("The following paths are ignored by one of your .gitignore files:\n");
     + static int verbose, show_only, ignored_too, refresh_only;
     + static int ignore_add_errors, intent_to_add, ignore_missing;
     + static int warn_on_embedded_repo = 1;
     ++static int include_ignored_submodules;
     + 
     + #define ADDREMOVE_DEFAULT 1
     + static int addremove = ADDREMOVE_DEFAULT;
     +@@ builtin/add.c: static struct option builtin_add_options[] = {
     + 	OPT_BOOL( 0 , "ignore-errors", &ignore_add_errors, N_("just skip files which cannot be added because of errors")),
     + 	OPT_BOOL( 0 , "ignore-missing", &ignore_missing, N_("check if - even missing - files are ignored in dry run")),
     + 	OPT_BOOL(0, "sparse", &include_sparse, N_("allow updating entries outside of the sparse-checkout cone")),
     ++    OPT_BOOL(0, "include-ignored-submodules", &include_ignored_submodules, N_("add submodules even if they has configuration ignore=all")),
     + 	OPT_STRING(0, "chmod", &chmod_arg, "(+|-)x",
     + 		   N_("override the executable bit of the listed files")),
     + 	OPT_HIDDEN_BOOL(0, "warn-embedded-repo", &warn_on_embedded_repo,
      @@ builtin/add.c: int cmd_add(int argc,
       	else
       		exit_status |= add_files_to_cache(repo, prefix,
       						  &pathspec, ps_matched,
      -						  include_sparse, flags);
     -+						  include_sparse, flags, ignored_too);
     ++						  include_sparse, flags, include_ignored_submodules);
       
       	if (take_worktree_changes && !add_renormalize && !ignore_add_errors &&
       	    report_path_error(ps_matched, &pathspec))
     @@ read-cache.c: void overlay_tree_on_index(struct index_state *istate,
       	int include_sparse;
       	int flags;
       	int add_errors;
     -+	int ignored_too;
     ++	int include_ignored_submodules;
       };
       
       static int fix_unmerged_status(struct diff_filepair *p,
      @@ read-cache.c: static void update_callback(struct diff_queue_struct *q,
     + 		default:
     + 			die(_("unexpected diff status %c"), p->status);
     + 		case DIFF_STATUS_MODIFIED:
     +-		case DIFF_STATUS_TYPE_CHANGED:
     ++		case DIFF_STATUS_TYPE_CHANGED: {
     ++			struct stat st;
     ++			if (!lstat(path, &st) && S_ISDIR(st.st_mode)) { // only consider submodule if it is a directory
     ++				const struct submodule *sub = submodule_from_path(data->repo, null_oid(the_hash_algo), path);
     ++				if (sub && sub->name && sub->ignore && !strcmp(sub->ignore, "all")) {
     ++					int pathspec_matches = 0;
     ++					char *norm_pathspec = NULL;
     ++					int ps_i;
     ++					trace_printf("ignore=all %s\n", path);
     ++					trace_printf("pathspec %s\n",
     ++							(data->pathspec && data->pathspec->nr) ? "has pathspec" : "no pathspec");
     ++					/* Safely scan all pathspec items (q->nr may exceed pathspec->nr). */
     ++					if (data->pathspec) {
     ++						for (ps_i = 0; ps_i < data->pathspec->nr; ps_i++) {
     ++							const char *m = data->pathspec->items[ps_i].match;
     ++							if (!m)
     ++								continue;
     ++							norm_pathspec = xstrdup(m);
     ++							strip_dir_trailing_slashes(norm_pathspec);
     ++							if (!strcmp(path, norm_pathspec)) {
     ++								pathspec_matches = 1;
     ++								FREE_AND_NULL(norm_pathspec);
     ++								break;
     ++							}
     ++							FREE_AND_NULL(norm_pathspec);
     ++						}
     ++					}
     ++					if (pathspec_matches) {
     ++						if (data->include_ignored_submodules && data->include_ignored_submodules > 0) {
     ++							trace_printf("Add ignored=all submodule due to --include_ignored_submodules: %s\n", path);
     ++						} else {
     ++							printf(_("Skipping submodule due to ignore=all: %s"), path);
     ++							printf(_("Use --include_ignored_submodules, if you really want to add them.") );
     ++							continue;
     ++						}
     ++					} else {
     ++						/* No explicit pathspec match -> skip silently (or with trace). */
     ++						trace_printf("pathspec does not match %s\n", path);
     ++						continue;
     ++					}
     ++				}
     ++			}
     + 			if (add_file_to_index(data->index, path, data->flags)) {
     + 				if (!(data->flags & ADD_CACHE_IGNORE_ERRORS))
     + 					die(_("updating files failed"));
     +@@ read-cache.c: static void update_callback(struct diff_queue_struct *q,
       
       int add_files_to_cache(struct repository *repo, const char *prefix,
       		       const struct pathspec *pathspec, char *ps_matched,
      -		       int include_sparse, int flags)
     -+		       int include_sparse, int flags, int ignored_too )
     ++		       int include_sparse, int flags, int include_ignored_submodules )
       {
       	struct update_callback_data data;
       	struct rev_info rev;
     @@ read-cache.c: int add_files_to_cache(struct repository *repo, const char *prefix
       	data.include_sparse = include_sparse;
       	data.flags = flags;
      +	data.repo = repo;
     -+	data.ignored_too = ignored_too;
     ++	data.include_ignored_submodules = include_ignored_submodules;
      +	data.pathspec = (struct pathspec *)pathspec;
       
       	repo_init_revisions(repo, &rev, prefix);
 2:  d1b02617e6 ! 2:  9ec79b9a11 read-cache: let read-cache respect submodule ignore=all and --force
     @@ Metadata
      Author: Claus Schneider(Eficode) <claus.schneider@eficode.com>
      
       ## Commit message ##
     -    read-cache: let read-cache respect submodule ignore=all and --force
     +    read-cache: add/read-cache respect submodule ignore=all
      
     -    Given the submdule configuration is ignore=all then only update the
     -    submdule if the --force option is given and the submodule is explicit
     -    given in the pathspec.
     +    Submodules configured with ignore=all are now skipped during add operations
     +    unless overridden by --include-ignored-submodules and the submodule path is
     +    explicitly specified.
      
          A message is printed (like ignored files) guiding the user to use the
     -    --force flag if the user has explicitely want to update the submodule
     -    reference.
     +    --include-ignored-submodules flag if the user has explicitely want to update
     +    the submodule reference.
      
          The reason for the change is support submodule branch tracking or
          similar and git status state nothing and git add should not add either.
     @@ Commit message
          the submodule is already tracked.
      
          The change opens up a lot of possibilities for submodules to be used
     -    more freely and a like the repo tool. A submodule can be added for many
     +    more freely and simular to the repo tool. A submodule can be added for many
          more reason and loosely coupled dependencies to the super repo which often
          gives the friction of handle the explicit commits and updates without
          the need for tracking the submodule sha1 by sha1.
     @@ read-cache.c
       /* Mask for the name length in ce_flags in the on-disk index */
       
      @@ read-cache.c: static void update_callback(struct diff_queue_struct *q,
     - 		default:
     - 			die(_("unexpected diff status %c"), p->status);
     - 		case DIFF_STATUS_MODIFIED:
     --		case DIFF_STATUS_TYPE_CHANGED:
     -+		case DIFF_STATUS_TYPE_CHANGED: {
     -+			struct stat st;
     -+			if (!lstat(path, &st) && S_ISDIR(st.st_mode)) { // only consider submodule if it is a directory
     -+				const struct submodule *sub = submodule_from_path(data->repo, null_oid(the_hash_algo), path);
     -+				if (sub && sub->name && sub->ignore && !strcmp(sub->ignore, "all")) {
     -+					int pathspec_matches = 0;
     -+					char *norm_pathspec = NULL;
     -+					int ps_i;
     -+					trace_printf("ignore=all %s\n", path);
     -+					trace_printf("pathspec %s\n",
     -+							(data->pathspec && data->pathspec->nr) ? "has pathspec" : "no pathspec");
     -+					/* Safely scan all pathspec items (q->nr may exceed pathspec->nr). */
     -+					if (data->pathspec) {
     -+						for (ps_i = 0; ps_i < data->pathspec->nr; ps_i++) {
     -+							const char *m = data->pathspec->items[ps_i].match;
     -+							if (!m)
     -+								continue;
     -+							norm_pathspec = xstrdup(m);
     -+							strip_dir_trailing_slashes(norm_pathspec);
     -+							if (!strcmp(path, norm_pathspec)) {
     -+								pathspec_matches = 1;
     -+								FREE_AND_NULL(norm_pathspec);
     -+								break;
     -+							}
     -+							FREE_AND_NULL(norm_pathspec);
     -+						}
     -+					}
     -+					if (pathspec_matches) {
     -+						if (data->ignored_too && data->ignored_too > 0) {
     -+							trace_printf("Forcing add of submodule ignored=all due to --force: %s\n", path);
     -+						} else {
     -+							printf(_("Skipping submodule due to ignore=all: %s"), path);
     -+							printf(_("Use -f if you really want to add them.") );
     -+							continue;
     -+						}
     -+					} else {
     -+						/* No explicit pathspec match -> skip silently (or with trace). */
     -+						trace_printf("pathspec does not match %s\n", path);
     -+						continue;
     -+					}
     -+				}
     -+			}
     - 			if (add_file_to_index(data->index, path, data->flags)) {
     - 				if (!(data->flags & ADD_CACHE_IGNORE_ERRORS))
     - 					die(_("updating files failed"));
     + 					}
     + 					if (pathspec_matches) {
     + 						if (data->include_ignored_submodules && data->include_ignored_submodules > 0) {
     +-							trace_printf("Add ignored=all submodule due to --include_ignored_submodules: %s\n", path);
     ++							trace_printf("Add submodule due to --include_ignored_submodules: %s\n", path);
     + 						} else {
     + 							printf(_("Skipping submodule due to ignore=all: %s"), path);
     + 							printf(_("Use --include_ignored_submodules, if you really want to add them.") );
     +@@ read-cache.c: static void update_callback(struct diff_queue_struct *q,
     + 						}
     + 					} else {
     + 						/* No explicit pathspec match -> skip silently (or with trace). */
     +-						trace_printf("pathspec does not match %s\n", path);
     ++						trace_printf("Pathspec to submodule does not match explicitly: %s\n", path);
     + 						continue;
     + 					}
     + 				}
     +@@ read-cache.c: static void update_callback(struct diff_queue_struct *q,
       				data->add_errors++;
       			}
       			break;
 3:  8f3d5f7ec1 ! 3:  399a153b95 tests: add new t2206-add-submodule-ignored.sh to test ignore=all scenario
     @@ Commit message
          config with ignore=all also behaves as intended with configuration in
          .gitmodules and configuration given on the command line.
      
     -    Testfile is added to meson.build for execution.
     +    The usage of --include_ignored_submodules is showcased and tested in the
     +    test suite.
     +
     +    The test file is added to meson.build for execution.
      
          Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
      
     @@ t/t2206-add-submodule-ignored.sh (new)
      +# This test covers the behavior of "git add", "git status" and "git log" when
      +# dealing with submodules that have the ignore=all setting in
      +# .gitmodules. It ensures that changes in such submodules are
     -+# ignored by default, but can be staged with "git add --force".
     ++# ignored by default, but can be staged with "git add --include-ignored-submodules".
      +
      +# shellcheck disable=SC1091
      +. ./test-lib.sh
     @@ t/t2206-add-submodule-ignored.sh (new)
      +'
      +
      +#6
     -+# check that 'git add --force .' does not stage the change in the submodule
     ++# check that 'git add --include-ignored-submodules .' does not stage the change in the submodule
      +# and that 'git status' does not show it as modified
     -+test_expect_success 'main: check --force add . and status'  '
     ++test_expect_success 'main: check --include-ignored-submodules add . and status'  '
      +	cd "${base_path}" &&
      +	cd main &&
     -+	GIT_TRACE=1 git add --force . &&
     ++	GIT_TRACE=1 git add --include-ignored-submodules . &&
      +	! git status --porcelain | grep "^M  sub$" &&
      +	echo
      +'
     @@ t/t2206-add-submodule-ignored.sh (new)
      +'
      +
      +#8
     -+# check that 'git add --force sub' does stage the change in the submodule
     -+# check that 'git add --force ./sub/' does stage the change in the submodule
     ++# check that 'git add --include-ignored-submodules sub' does stage the change in the submodule
     ++# check that 'git add --include-ignored-submodules ./sub/' does stage the change in the submodule
      +# and that 'git status --porcelain' does show it as modified
      +# commit it..
      +# check that 'git log --ignore-submodules=none' shows the submodule change
     @@ t/t2206-add-submodule-ignored.sh (new)
      +test_expect_success 'main: check force add sub and ./sub/ and status'  '
      +	cd "${base_path}" &&
      +	cd main &&
     -+	echo "Adding with --force should work: git add --force sub" &&
     -+	GIT_TRACE=1 git add --force sub &&
     ++	echo "Adding with --include-ignored-submodules should work: git add --include-ignored-submodules sub" &&
     ++	GIT_TRACE=1 git add --include-ignored-submodules sub &&
      +	git status --porcelain | grep "^M  sub$" &&
      +	git restore --staged sub &&
      +	! git status --porcelain | grep "^M  sub$" &&
     -+	echo "Adding with --force should work: git add --force ./sub/" &&
     -+	GIT_TRACE=1 git add --force ./sub/ &&
     ++	echo "Adding with --include-ignored-submodules should work: git add --include-ignored-submodules ./sub/" &&
     ++	GIT_TRACE=1 git add --include-ignored-submodules ./sub/ &&
      +	git status --porcelain | grep "^M  sub$" &&
      +	git commit -m "update submodule pointer" &&
      +	! git status --porcelain | grep "^ M sub$" &&
 4:  58563a7b90 ! 4:  93c95954f1 tests: fix existing tests when add an ignore=all submodule
     @@ Metadata
       ## Commit message ##
          tests: fix existing tests when add an ignore=all submodule
      
     -    There are tests that rely on "git add <submodule>" also adds it. A --force
     -    is needed with this enhancement hence they are added accordingly in these
     -    tests.
     +    There are tests that rely on "git add <submodule>" to add updates in the
     +    parent repository. A new option --include-ignored-submodules is introduced
     +    as it is now needed with this enhancement.
      
          Updated tests:
          - t1013-read-tree-submodule.sh ( fixed in: t/lib-submodule-update.sh )
     +    - t2013-checkout-submodule.sh ( fixed in: t/lib-submodule-update.sh )
          - t7406-submodule-update.sh
          - t7508-status.sh
      
     @@ t/lib-submodule-update.sh: create_lib_submodule_repo () {
       			git push origin modifications
       		) &&
      -		git add sub1 &&
     -+		git add --force sub1 &&
     ++		git add --include-ignored-submodules sub1 &&
       		git commit -m "Modify sub1" &&
       
       		git checkout -b add_nested_sub modify_sub1 &&
     @@ t/lib-submodule-update.sh: create_lib_submodule_repo () {
       		git -C sub1 submodule add --branch no_submodule ../submodule_update_sub2 sub2 &&
       		git -C sub1 commit -a -m "add a nested submodule" &&
      -		git add sub1 &&
     -+		git add --force sub1 &&
     ++		git add --include-ignored-submodules sub1 &&
       		git commit -a -m "update submodule, that updates a nested submodule" &&
       		git checkout -b modify_sub1_recursively &&
       		git -C sub1 checkout -b modify_sub1_recursively &&
     @@ t/lib-submodule-update.sh: create_lib_submodule_repo () {
       		git -C sub1 add sub2 &&
       		git -C sub1 commit -m "update nested sub" &&
      -		git add sub1 &&
     -+		git add --force sub1 &&
     ++		git add --include-ignored-submodules sub1 &&
       		git commit -m "update sub1, that updates nested sub" &&
       		git -C sub1 push origin modify_sub1_recursively &&
       		git -C sub1/sub2 push origin modify_sub1_recursively &&
     @@ t/t7508-status.sh: test_expect_success 'git commit will commit a staged but igno
       test_expect_success 'git commit --dry-run will show a staged but ignored submodule' '
       	git reset HEAD^ &&
      -	git add sm &&
     -+	git add --force sm &&
     ++	git add --include-ignored-submodules sm &&
       	cat >expect << EOF &&
       On branch main
       Your branch and '\''upstream'\'' have diverged,
 5:  416695f439 < -:  ---------- Documentation: update add --force and submodule ignore=all config
 -:  ---------- > 5:  ee84190cd8 Documentation: add --include_ignored_submodules + ignore=all config

-- 
gitgitgadget

[PATCH v2 1/5] read-cache: update add_files_to_cache take param include_ignored_submodules

[Claus Schneider(Eficode) via GitGitGadget]

From: "Claus Schneider(Eficode)" <claus.schneider@eficode.com>

The include_ignored_submodules parameter is added to the function
add_files_to_cache for usage of explicit updating the index for the updated
submodule using the explicit patchspec to the submodule.

Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
---
 builtin/add.c      |  4 +++-
 builtin/checkout.c |  2 +-
 builtin/commit.c   |  2 +-
 read-cache-ll.h    |  2 +-
 read-cache.c       | 51 ++++++++++++++++++++++++++++++++++++++++++++--
 5 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index 0235854f80..6d11382f33 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -233,6 +233,7 @@ N_("The following paths are ignored by one of your .gitignore files:\n");
 static int verbose, show_only, ignored_too, refresh_only;
 static int ignore_add_errors, intent_to_add, ignore_missing;
 static int warn_on_embedded_repo = 1;
+static int include_ignored_submodules;
 
 #define ADDREMOVE_DEFAULT 1
 static int addremove = ADDREMOVE_DEFAULT;
@@ -271,6 +272,7 @@ static struct option builtin_add_options[] = {
 	OPT_BOOL( 0 , "ignore-errors", &ignore_add_errors, N_("just skip files which cannot be added because of errors")),
 	OPT_BOOL( 0 , "ignore-missing", &ignore_missing, N_("check if - even missing - files are ignored in dry run")),
 	OPT_BOOL(0, "sparse", &include_sparse, N_("allow updating entries outside of the sparse-checkout cone")),
+    OPT_BOOL(0, "include-ignored-submodules", &include_ignored_submodules, N_("add submodules even if they has configuration ignore=all")),
 	OPT_STRING(0, "chmod", &chmod_arg, "(+|-)x",
 		   N_("override the executable bit of the listed files")),
 	OPT_HIDDEN_BOOL(0, "warn-embedded-repo", &warn_on_embedded_repo,
@@ -582,7 +584,7 @@ int cmd_add(int argc,
 	else
 		exit_status |= add_files_to_cache(repo, prefix,
 						  &pathspec, ps_matched,
-						  include_sparse, flags);
+						  include_sparse, flags, include_ignored_submodules);
 
 	if (take_worktree_changes && !add_renormalize && !ignore_add_errors &&
 	    report_path_error(ps_matched, &pathspec))
diff --git a/builtin/checkout.c b/builtin/checkout.c
index f9453473fe..b2a404051d 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -899,7 +899,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
 			 */
 
 			add_files_to_cache(the_repository, NULL, NULL, NULL, 0,
-					   0);
+					   0, 0 );
 			init_ui_merge_options(&o, the_repository);
 			o.verbosity = 0;
 			work = write_in_core_index_as_tree(the_repository);
diff --git a/builtin/commit.c b/builtin/commit.c
index b5b9608813..5bf7ae5fc1 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -455,7 +455,7 @@ static const char *prepare_index(const char **argv, const char *prefix,
 		repo_hold_locked_index(the_repository, &index_lock,
 				       LOCK_DIE_ON_ERROR);
 		add_files_to_cache(the_repository, also ? prefix : NULL,
-				   &pathspec, ps_matched, 0, 0);
+				   &pathspec, ps_matched, 0, 0, 0 );
 		if (!all && report_path_error(ps_matched, &pathspec))
 			exit(128);
 
diff --git a/read-cache-ll.h b/read-cache-ll.h
index 71b49d9af4..2c8b4b21b1 100644
--- a/read-cache-ll.h
+++ b/read-cache-ll.h
@@ -481,7 +481,7 @@ int cmp_cache_name_compare(const void *a_, const void *b_);
 
 int add_files_to_cache(struct repository *repo, const char *prefix,
 		       const struct pathspec *pathspec, char *ps_matched,
-		       int include_sparse, int flags);
+		       int include_sparse, int flags, int ignored_too );
 
 void overlay_tree_on_index(struct index_state *istate,
 			   const char *tree_name, const char *prefix);
diff --git a/read-cache.c b/read-cache.c
index 06ad74db22..32f32bdb4c 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -3880,9 +3880,12 @@ void overlay_tree_on_index(struct index_state *istate,
 
 struct update_callback_data {
 	struct index_state *index;
+	struct repository *repo;
+	struct pathspec *pathspec;
 	int include_sparse;
 	int flags;
 	int add_errors;
+	int include_ignored_submodules;
 };
 
 static int fix_unmerged_status(struct diff_filepair *p,
@@ -3924,7 +3927,48 @@ static void update_callback(struct diff_queue_struct *q,
 		default:
 			die(_("unexpected diff status %c"), p->status);
 		case DIFF_STATUS_MODIFIED:
-		case DIFF_STATUS_TYPE_CHANGED:
+		case DIFF_STATUS_TYPE_CHANGED: {
+			struct stat st;
+			if (!lstat(path, &st) && S_ISDIR(st.st_mode)) { // only consider submodule if it is a directory
+				const struct submodule *sub = submodule_from_path(data->repo, null_oid(the_hash_algo), path);
+				if (sub && sub->name && sub->ignore && !strcmp(sub->ignore, "all")) {
+					int pathspec_matches = 0;
+					char *norm_pathspec = NULL;
+					int ps_i;
+					trace_printf("ignore=all %s\n", path);
+					trace_printf("pathspec %s\n",
+							(data->pathspec && data->pathspec->nr) ? "has pathspec" : "no pathspec");
+					/* Safely scan all pathspec items (q->nr may exceed pathspec->nr). */
+					if (data->pathspec) {
+						for (ps_i = 0; ps_i < data->pathspec->nr; ps_i++) {
+							const char *m = data->pathspec->items[ps_i].match;
+							if (!m)
+								continue;
+							norm_pathspec = xstrdup(m);
+							strip_dir_trailing_slashes(norm_pathspec);
+							if (!strcmp(path, norm_pathspec)) {
+								pathspec_matches = 1;
+								FREE_AND_NULL(norm_pathspec);
+								break;
+							}
+							FREE_AND_NULL(norm_pathspec);
+						}
+					}
+					if (pathspec_matches) {
+						if (data->include_ignored_submodules && data->include_ignored_submodules > 0) {
+							trace_printf("Add ignored=all submodule due to --include_ignored_submodules: %s\n", path);
+						} else {
+							printf(_("Skipping submodule due to ignore=all: %s"), path);
+							printf(_("Use --include_ignored_submodules, if you really want to add them.") );
+							continue;
+						}
+					} else {
+						/* No explicit pathspec match -> skip silently (or with trace). */
+						trace_printf("pathspec does not match %s\n", path);
+						continue;
+					}
+				}
+			}
 			if (add_file_to_index(data->index, path, data->flags)) {
 				if (!(data->flags & ADD_CACHE_IGNORE_ERRORS))
 					die(_("updating files failed"));
@@ -3945,7 +3989,7 @@ static void update_callback(struct diff_queue_struct *q,
 
 int add_files_to_cache(struct repository *repo, const char *prefix,
 		       const struct pathspec *pathspec, char *ps_matched,
-		       int include_sparse, int flags)
+		       int include_sparse, int flags, int include_ignored_submodules )
 {
 	struct update_callback_data data;
 	struct rev_info rev;
@@ -3954,6 +3998,9 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
 	data.index = repo->index;
 	data.include_sparse = include_sparse;
 	data.flags = flags;
+	data.repo = repo;
+	data.include_ignored_submodules = include_ignored_submodules;
+	data.pathspec = (struct pathspec *)pathspec;
 
 	repo_init_revisions(repo, &rev, prefix);
 	setup_revisions(0, NULL, &rev, NULL);
-- 
gitgitgadget

[PATCH v2 2/5] read-cache: add/read-cache respect submodule ignore=all

[Claus Schneider(Eficode) via GitGitGadget]

From: "Claus Schneider(Eficode)" <claus.schneider@eficode.com>

Submodules configured with ignore=all are now skipped during add operations
unless overridden by --include-ignored-submodules and the submodule path is
explicitly specified.

A message is printed (like ignored files) guiding the user to use the
--include-ignored-submodules flag if the user has explicitely want to update
the submodule reference.

The reason for the change is support submodule branch tracking or
similar and git status state nothing and git add should not add either.
The workflow is more logic and similar to regular ignored files even
the submodule is already tracked.

The change opens up a lot of possibilities for submodules to be used
more freely and simular to the repo tool. A submodule can be added for many
more reason and loosely coupled dependencies to the super repo which often
gives the friction of handle the explicit commits and updates without
the need for tracking the submodule sha1 by sha1.

Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
---
 read-cache.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 32f32bdb4c..7b6d1b2914 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -48,6 +48,8 @@
 #include "csum-file.h"
 #include "promisor-remote.h"
 #include "hook.h"
+#include "submodule.h"
+#include "submodule-config.h"
 
 /* Mask for the name length in ce_flags in the on-disk index */
 
@@ -3956,7 +3958,7 @@ static void update_callback(struct diff_queue_struct *q,
 					}
 					if (pathspec_matches) {
 						if (data->include_ignored_submodules && data->include_ignored_submodules > 0) {
-							trace_printf("Add ignored=all submodule due to --include_ignored_submodules: %s\n", path);
+							trace_printf("Add submodule due to --include_ignored_submodules: %s\n", path);
 						} else {
 							printf(_("Skipping submodule due to ignore=all: %s"), path);
 							printf(_("Use --include_ignored_submodules, if you really want to add them.") );
@@ -3964,7 +3966,7 @@ static void update_callback(struct diff_queue_struct *q,
 						}
 					} else {
 						/* No explicit pathspec match -> skip silently (or with trace). */
-						trace_printf("pathspec does not match %s\n", path);
+						trace_printf("Pathspec to submodule does not match explicitly: %s\n", path);
 						continue;
 					}
 				}
@@ -3975,6 +3977,7 @@ static void update_callback(struct diff_queue_struct *q,
 				data->add_errors++;
 			}
 			break;
+		}
 		case DIFF_STATUS_DELETED:
 			if (data->flags & ADD_CACHE_IGNORE_REMOVAL)
 				break;
-- 
gitgitgadget

[PATCH v2 3/5] tests: add new t2206-add-submodule-ignored.sh to test ignore=all scenario

[Claus Schneider(Eficode) via GitGitGadget]

From: "Claus Schneider(Eficode)" <claus.schneider@eficode.com>

The tests verify that the submodule behavior is intact and updating the
config with ignore=all also behaves as intended with configuration in
.gitmodules and configuration given on the command line.

The usage of --include_ignored_submodules is showcased and tested in the
test suite.

The test file is added to meson.build for execution.

Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
---
 t/meson.build                    |   1 +
 t/t2206-add-submodule-ignored.sh | 134 +++++++++++++++++++++++++++++++
 2 files changed, 135 insertions(+)
 create mode 100755 t/t2206-add-submodule-ignored.sh

diff --git a/t/meson.build b/t/meson.build
index 983245501c..49e29ae82f 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -282,6 +282,7 @@ integration_tests = [
   't2203-add-intent.sh',
   't2204-add-ignored.sh',
   't2205-add-worktree-config.sh',
+  't2206-add-submodule-ignored.sh',
   't2300-cd-to-toplevel.sh',
   't2400-worktree-add.sh',
   't2401-worktree-prune.sh',
diff --git a/t/t2206-add-submodule-ignored.sh b/t/t2206-add-submodule-ignored.sh
new file mode 100755
index 0000000000..2c8a523641
--- /dev/null
+++ b/t/t2206-add-submodule-ignored.sh
@@ -0,0 +1,134 @@
+#!/bin/sh
+# shellcheck disable=SC2016
+
+# shellcheck disable=SC2034
+test_description='git add respects submodule ignore=all and explicit pathspec'
+
+# This test covers the behavior of "git add", "git status" and "git log" when
+# dealing with submodules that have the ignore=all setting in
+# .gitmodules. It ensures that changes in such submodules are
+# ignored by default, but can be staged with "git add --include-ignored-submodules".
+
+# shellcheck disable=SC1091
+. ./test-lib.sh
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+base_path=$(pwd -P)
+
+#1
+test_expect_success 'setup: create origin repos'  '
+	cd "${base_path}" &&
+	git config --global protocol.file.allow always &&
+	git init sub &&
+		pwd &&
+		cd sub &&
+		test_commit sub_file1 &&
+		git tag v1.0 &&
+		test_commit sub_file2 &&
+		git tag v2.0 &&
+		test_commit sub_file3 &&
+		git tag v3.0 &&
+	cd "${base_path}" &&
+	git init main &&
+		cd main &&
+		test_commit first &&
+	cd "${base_path}"
+'
+#2
+# add submodule with default config (ignore=none) and
+# check log that is contains a path entry for the submodule 'sub'
+# change the commit in the submodule and check that 'git status' shows it as modified
+test_expect_success 'main: add submodule with default config'  '
+	cd "${base_path}" &&
+	cd main &&
+	git submodule add ../sub &&
+	git commit -m "add submodule" &&
+	git log --oneline --name-only | grep "^sub$" &&
+	git -C sub reset --hard v2.0 &&
+	git status --porcelain | grep "^ M sub$" &&
+	echo
+'
+#3
+# change the submodule config to ignore=all and check that status and log do not show changes
+test_expect_success 'main: submodule config ignore=all'  '
+	cd "${base_path}" &&
+	cd main &&
+	git config -f .gitmodules submodule.sub.ignore all &&
+	GIT_TRACE=1 git add . &&
+	git commit -m "update submodule config sub.ignore all" &&
+	! git status --porcelain | grep "^.*$" &&
+	! git log --oneline --name-only | grep "^sub$" &&
+	echo
+'
+#4
+# change the commit in the submodule and check that 'git status' does not show it as modified
+# but 'git status --ignore-submodules=none' does show it as modified
+test_expect_success 'sub: change to different sha1 and check status in main'  '
+	cd "${base_path}" &&
+	cd main &&
+	git -C sub reset --hard v1.0 &&
+	! git status --porcelain | grep "^ M sub$" &&
+	git status --ignore-submodules=none --porcelain | grep "^ M sub$" &&
+	echo
+'
+
+#5
+# check that normal 'git add' does not stage the change in the submodule
+test_expect_success 'main: check normal add and status'  '
+	cd "${base_path}" &&
+	cd main &&
+	GIT_TRACE=1 git add . &&
+	! git status --porcelain | grep "^ M sub$" &&
+	echo
+'
+
+#6
+# check that 'git add --include-ignored-submodules .' does not stage the change in the submodule
+# and that 'git status' does not show it as modified
+test_expect_success 'main: check --include-ignored-submodules add . and status'  '
+	cd "${base_path}" &&
+	cd main &&
+	GIT_TRACE=1 git add --include-ignored-submodules . &&
+	! git status --porcelain | grep "^M  sub$" &&
+	echo
+'
+
+#7
+# check that 'git add .' does not stage the change in the submodule
+# and that 'git status' does not show it as modified
+test_expect_success 'main: check _add sub_ and status'  '
+	cd "${base_path}" &&
+	cd main &&
+	GIT_TRACE=1 git add sub | grep "Skipping submodule due to ignore=all: sub" &&
+	! git status --porcelain | grep "^M  sub$" &&
+	echo
+'
+
+#8
+# check that 'git add --include-ignored-submodules sub' does stage the change in the submodule
+# check that 'git add --include-ignored-submodules ./sub/' does stage the change in the submodule
+# and that 'git status --porcelain' does show it as modified
+# commit it..
+# check that 'git log --ignore-submodules=none' shows the submodule change
+# in the log
+test_expect_success 'main: check force add sub and ./sub/ and status'  '
+	cd "${base_path}" &&
+	cd main &&
+	echo "Adding with --include-ignored-submodules should work: git add --include-ignored-submodules sub" &&
+	GIT_TRACE=1 git add --include-ignored-submodules sub &&
+	git status --porcelain | grep "^M  sub$" &&
+	git restore --staged sub &&
+	! git status --porcelain | grep "^M  sub$" &&
+	echo "Adding with --include-ignored-submodules should work: git add --include-ignored-submodules ./sub/" &&
+	GIT_TRACE=1 git add --include-ignored-submodules ./sub/ &&
+	git status --porcelain | grep "^M  sub$" &&
+	git commit -m "update submodule pointer" &&
+	! git status --porcelain | grep "^ M sub$" &&
+	git log --ignore-submodules=none --name-only --oneline | grep "^sub$" &&
+	echo
+'
+
+test_done
+exit 0
-- 
gitgitgadget

[PATCH v2 4/5] tests: fix existing tests when add an ignore=all submodule

[Claus Schneider(Eficode) via GitGitGadget]

From: "Claus Schneider(Eficode)" <claus.schneider@eficode.com>

There are tests that rely on "git add <submodule>" to add updates in the
parent repository. A new option --include-ignored-submodules is introduced
as it is now needed with this enhancement.

Updated tests:
- t1013-read-tree-submodule.sh ( fixed in: t/lib-submodule-update.sh )
- t2013-checkout-submodule.sh ( fixed in: t/lib-submodule-update.sh )
- t7406-submodule-update.sh
- t7508-status.sh

Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
---
 t/lib-submodule-update.sh | 6 +++---
 t/t7508-status.sh         | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 36f767cb74..fde5dbee02 100644
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -95,14 +95,14 @@ create_lib_submodule_repo () {
 			git commit -m "modified file2 and added file3" &&
 			git push origin modifications
 		) &&
-		git add sub1 &&
+		git add --include-ignored-submodules sub1 &&
 		git commit -m "Modify sub1" &&
 
 		git checkout -b add_nested_sub modify_sub1 &&
 		git -C sub1 checkout -b "add_nested_sub" &&
 		git -C sub1 submodule add --branch no_submodule ../submodule_update_sub2 sub2 &&
 		git -C sub1 commit -a -m "add a nested submodule" &&
-		git add sub1 &&
+		git add --include-ignored-submodules sub1 &&
 		git commit -a -m "update submodule, that updates a nested submodule" &&
 		git checkout -b modify_sub1_recursively &&
 		git -C sub1 checkout -b modify_sub1_recursively &&
@@ -112,7 +112,7 @@ create_lib_submodule_repo () {
 		git -C sub1/sub2 commit -m "make a change in nested sub" &&
 		git -C sub1 add sub2 &&
 		git -C sub1 commit -m "update nested sub" &&
-		git add sub1 &&
+		git add --include-ignored-submodules sub1 &&
 		git commit -m "update sub1, that updates nested sub" &&
 		git -C sub1 push origin modify_sub1_recursively &&
 		git -C sub1/sub2 push origin modify_sub1_recursively &&
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index cdc1d6fcc7..5c8ffed21b 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1565,7 +1565,7 @@ test_expect_success 'git commit will commit a staged but ignored submodule' '
 
 test_expect_success 'git commit --dry-run will show a staged but ignored submodule' '
 	git reset HEAD^ &&
-	git add sm &&
+	git add --include-ignored-submodules sm &&
 	cat >expect << EOF &&
 On branch main
 Your branch and '\''upstream'\'' have diverged,
-- 
gitgitgadget

[PATCH v2 5/5] Documentation: add --include_ignored_submodules + ignore=all config

[Claus Schneider(Eficode) via GitGitGadget]

From: "Claus Schneider(Eficode)" <claus.schneider@eficode.com>

- git-add.adoc: Add new documentation for --include_ignored_submodules
  option to supress ignore=all and new submodule functionality of not
  adding a ignore=all submodule by default.
- gitmodules.adoc and config/submodule.adoc: The submodule config
  ignore=all now need --include_ignored_submodules in order to update
  the index.

Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
---
 .devcontainer/Dockerfile            | 70 ++++++++++++++++++++++++++
 .devcontainer/Dockerfile.standalone | 76 +++++++++++++++++++++++++++++
 .devcontainer/devcontainer.json     | 25 ++++++++++
 Documentation/config/submodule.adoc | 13 ++---
 Documentation/git-add.adoc          |  5 ++
 Documentation/gitmodules.adoc       |  5 +-
 6 files changed, 187 insertions(+), 7 deletions(-)
 create mode 100644 .devcontainer/Dockerfile
 create mode 100644 .devcontainer/Dockerfile.standalone
 create mode 100644 .devcontainer/devcontainer.json

diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile
new file mode 100644
index 0000000000..680ca5f3ad
--- /dev/null
+++ b/.devcontainer/Dockerfile
@@ -0,0 +1,70 @@
+FROM ubuntu:latest
+
+ARG USER_ID
+ARG GROUP_ID
+
+ENV DEBIAN_FRONTEND=noninteractive
+ENV TZ=Europe/Copenhagen
+
+RUN apt-get update \
+    && \
+    apt-get install -y \
+        sudo \
+        build-essential \
+        libcurl4-gnutls-dev \
+        libexpat1-dev \
+        gettext \
+        libz-dev \
+        libssl-dev \
+        asciidoc \
+        xmlto \
+        docbook-xsl \
+        \
+        tzdata \
+        git \
+        coccinelle \
+    && \
+    ln -fs /usr/share/zoneinfo/$TZ /etc/localtime \
+    && \
+    dpkg-reconfigure --frontend noninteractive tzdata
+
+RUN  apt-get update && apt-get install -y autoconf
+
+RUN addgroup -gid 1001 gituser
+RUN adduser --disabled-password -u 1001 -gid 1001 gituser
+RUN usermod -aG sudo gituser
+RUN echo 'gituser ALL=(ALL) NOPASSWD:ALL' > /etc/sudoers.d/010-gituser
+RUN chmod 0440 /etc/sudoers.d/010-gituser
+
+USER 1001
+
+#WORKDIR /home/gituser/git-src/
+
+#RUN  make configure
+#RUN ./configure --prefix=${HOME}/.local/
+
+#RUN make -j$(nproc) gitweb || make gitweb 
+#RUN make install-gitweb
+
+#RUN make -j$(nproc) || make
+#RUN make -j$(nproc) gitweb || make gitweb 
+#RUN make install-gitweb
+#RUN make -j$(nproc) NO_PERL=YesPlease install || make NO_PERL=YesPlease install
+#RUN make install
+
+
+RUN mkdir -p  ${HOME}/.local/
+#RUN ls -la ${HOME}/.local/bin
+#ENV PATH="/home/gituser/.local/bin:${PATH}"
+#RUN ls -l $HOME/.local/bin
+#RUN echo $PATH $HOME 
+
+#RUN git --version
+#RUN which git 
+
+RUN git config --global user.email "gituser@example.com"
+RUN git config --global user.name "Git User"
+
+#WORKDIR /home/gituser/git-test/
+
+#CMD [ "git" ]
diff --git a/.devcontainer/Dockerfile.standalone b/.devcontainer/Dockerfile.standalone
new file mode 100644
index 0000000000..39bda42c0e
--- /dev/null
+++ b/.devcontainer/Dockerfile.standalone
@@ -0,0 +1,76 @@
+FROM ubuntu:latest
+
+ARG USER_ID
+ARG GROUP_ID
+
+ENV DEBIAN_FRONTEND=noninteractive
+ENV TZ=Europe/Copenhagen
+
+RUN apt-get update \
+    && \
+    apt-get install -y \
+        build-essential \
+        libcurl4-gnutls-dev \
+        libexpat1-dev \
+        gettext \
+        libz-dev \
+        libssl-dev \
+        asciidoc \
+        xmlto \
+        docbook-xsl \
+        \
+        tzdata \
+        \
+        nano \
+        vim \
+    && \
+    ln -fs /usr/share/zoneinfo/$TZ /etc/localtime \
+    && \
+    dpkg-reconfigure --frontend noninteractive tzdata
+
+RUN  apt-get update && apt-get install -y autoconf
+
+RUN addgroup -gid ${GROUP_ID} gituser
+RUN adduser --disabled-password -u ${USER_ID} -gid ${GROUP_ID} gituser
+
+
+COPY --chown=${USER_ID}:${GROUP_ID} ./git /home/gituser/git-src/
+
+USER ${USER_ID}
+
+WORKDIR /home/gituser/git-src/
+
+RUN  make configure
+RUN ./configure --prefix=${HOME}/.local/
+
+#RUN make -j$(nproc) gitweb || make gitweb 
+#RUN make install-gitweb
+
+#RUN make -j$(nproc) || make
+#RUN make -j$(nproc) gitweb || make gitweb 
+#RUN make install-gitweb
+RUN make -j$(nproc) NO_PERL=YesPlease install || make NO_PERL=YesPlease install
+RUN make install
+
+WORKDIR /home/gituser/git-src/t
+RUN ./t2206-add-submodule-ignored.sh -v
+
+RUN ls -la ${HOME}/.local/
+RUN ls -la ${HOME}/.local/bin
+ENV PATH="/home/gituser/.local/bin:${PATH}"
+RUN ls -l $HOME/.local/bin
+RUN echo $PATH $HOME 
+
+RUN git --version
+RUN which git 
+
+RUN git config --global user.email "gituser@example.com"
+RUN git config --global user.name "Git User"
+
+WORKDIR /home/gituser/git-src/t
+RUN pwd && ls -la 
+RUN ./t2206-add-submodule-ignored.sh -v
+
+WORKDIR /home/gituser/git-test/
+
+CMD [ "git" ]
diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json
new file mode 100644
index 0000000000..2bc13902d8
--- /dev/null
+++ b/.devcontainer/devcontainer.json
@@ -0,0 +1,25 @@
+{
+  "name": "Git Dev Container",
+  "build": {
+    "dockerfile": "Dockerfile",
+    "context": "",
+    "args": {
+      "user_id": "1001",
+      "group_id": "1001"
+    }
+  },
+  "customizations": {
+    "vscode": {
+      "settings": {
+        "terminal.integrated.shell.linux": "/bin/bash"
+      }
+    }
+  },
+  "remoteUser": "gituser",
+  "features": {},
+  "mounts": [
+    "source=${localWorkspaceFolder}/,target=/home/gitusers/git-src,type=bind,consistency=cached",
+    "source=${localWorkspaceFolder}/,target=/home/gitusers/git-test,type=bind,consistency=cached"
+  ],
+  "postCreateCommand": "echo"
+}
diff --git a/Documentation/config/submodule.adoc b/Documentation/config/submodule.adoc
index 0672d99117..0753adbab5 100644
--- a/Documentation/config/submodule.adoc
+++ b/Documentation/config/submodule.adoc
@@ -32,15 +32,16 @@ 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 (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 submodule's work tree and
+	a submodule as modified.
+	Set to "all" will never considered the submodule modified. It can
+	nevertheless be staged using the option --include_ignored_submodules and
+	it will then show up in the output of status.
+	Set to "dirty" will ignore all changes to the submodule's 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.
-	Using "none" (the default when this option is not set) also shows
-	submodules that have untracked files in their work tree as changed.
+	Set to "none"(default) It is also shows submodules that have untracked
+	files in their work tree as changed.
 	This setting overrides any setting made in .gitmodules for this submodule,
 	both settings can be overridden on the command line by using the
 	"--ignore-submodules" option. The 'git submodule' commands are not
diff --git a/Documentation/git-add.adoc b/Documentation/git-add.adoc
index b7a735824d..ab72fad651 100644
--- a/Documentation/git-add.adoc
+++ b/Documentation/git-add.adoc
@@ -79,6 +79,11 @@ in linkgit:gitglossary[7].
 `--force`::
 	Allow adding otherwise ignored files.
 
+`--include_ignored_submodules`::
+	The option is also used when `submodule.<name>.ignore=all`
+	is set, but you want to stage an update of the submodule. The
+	`path` to the submodule must be explicitly specified.
+
 `--sparse`::
 	Allow updating index entries outside of the sparse-checkout cone.
 	Normally, `git add` refuses to update index entries whose paths do
diff --git a/Documentation/gitmodules.adoc b/Documentation/gitmodules.adoc
index d9bec8b187..ea1fd377e6 100644
--- a/Documentation/gitmodules.adoc
+++ b/Documentation/gitmodules.adoc
@@ -70,7 +70,10 @@ submodule.<name>.ignore::
 --
 	all;; The submodule will never be considered modified (but will
 	    nonetheless show up in the output of status and commit when it has
-	    been staged).
+	    been staged). Add `(new commits)` can be overruled using the
+	    `git add --include_ignored_submodules <submodule.path>` 
+		The setting affects `status`, `update-index`, `diff` and `log`(due 
+		to underlaying `diff`).
 
 	dirty;; All changes to the submodule's work tree will be ignored, only
 	    committed differences between the `HEAD` of the submodule and its
-- 
gitgitgadget

Re: [PATCH v2 0/5] git-add : Respect submodule ignore=all and only add changes with --force

[Junio C Hamano]

"Claus Schneider via GitGitGadget" <gitgitgadget@gmail.com> writes:

> The feature of configuring a submodule to "ignore=all" is nicely respected
> in commands "status" and "diff".

"nicely respected" is not very informative for those who do not know
what the setting does.  Saying something like

    "git status" and "git diff" will not report modified submodules
    with submodule.<name>.ignore set to "all".

would not waste significantly more bytes than what you wrote, and is
more helpful.

> However the "add" command does not respect
> the configuration the same way.

Again, "does not respect" and then what?  Running "git add" on a
submodule with submodule.<name>.ignore set to "all" does what?
Complains that it has changes but because .ignore is set it won't
add?  Adds it silently?  Something else?

> The behavior is problematic for the logic
> between status/diff and add.

After this sentence, "because ..." is missing.  Please help readers
understand the issue you perceive as problematic more easily.

I am guessing that you are assuming that an "add", after "diff" or
"status" said there is no change, is expected to be a no-op, but I
cannot be sure if that is what you are referring to here with the
reason left unsaid like the above.

> Secondly it makes it problematic to track
> branches in the submodule configuration as developers unintentionally keeps
> add submodule updates and get conflicts for no intentional reason. Both adds
> unnecessary friction to the usage of submodules.
>
> The patches implement the same logical behavior for ignore=all submodules as
> regular ignored files. The status now does not show any diff - nor will the
> add command update the reference submodule reference. If you add the
> submodule path which is ignore=all then you are presented with a message
> that you need to use the --force option.

I vaguely recall that an earlier discussion between you and Phillip
were concluding against "--force"?  I personally feel it is in line
with "git add foo.o" (when '*.o' is in .gitignore) gets rejected and
"git add -f foo.o" is a way to override it, but in the list of
patches below, I see --include-ignored-submodules (no, our command
line option names do not use underscore for inter-word-gaps), so I
suspect the description in the cover letter around here is stale?


> The branch=, ignore=all (and
> update=none) now works great with update --remote,

Again, "great" is not very informative, and as bad as "nicely
respected".  Avoid using these adjectives loaded with unnecessary
value judgements, and instead trust your readers.  They are
intelligent to judge if the updated behaviour is great or not for
themselves.  Try to use the same bytes on helping readers understand
what actually happens.

> but developers does not

"do not".

> have to consider changes in the updates of the submodule sha1. The
> implementation removes a friction of working with submodules and can be used
> like the repo tool with branches configured. The submodule status report
> could be used for build/release documentation for reproduction of a setup.
>
> A few tests used the adding of submodules without --force, hence they have
> been updated to use the --force option.
>
> Claus Schneider(Eficode) (5):
>   read-cache: update add_files_to_cache take param
>     include_ignored_submodules
>   read-cache: add/read-cache respect submodule ignore=all
>   tests: add new t2206-add-submodule-ignored.sh to test ignore=all
>     scenario
>   tests: fix existing tests when add an ignore=all submodule
>   Documentation: add --include_ignored_submodules + ignore=all config
>
>  .devcontainer/Dockerfile            |  70 +++++++++++++++
>  .devcontainer/Dockerfile.standalone |  76 ++++++++++++++++
>  .devcontainer/devcontainer.json     |  25 ++++++
>  Documentation/config/submodule.adoc |  13 +--
>  Documentation/git-add.adoc          |   5 ++
>  Documentation/gitmodules.adoc       |   5 +-
>  builtin/add.c                       |   4 +-
>  builtin/checkout.c                  |   2 +-
>  builtin/commit.c                    |   2 +-
>  read-cache-ll.h                     |   2 +-
>  read-cache.c                        |  54 ++++++++++-
>  t/lib-submodule-update.sh           |   6 +-
>  t/meson.build                       |   1 +
>  t/t2206-add-submodule-ignored.sh    | 134 ++++++++++++++++++++++++++++
>  t/t7508-status.sh                   |   2 +-
>  15 files changed, 384 insertions(+), 17 deletions(-)
>  create mode 100644 .devcontainer/Dockerfile
>  create mode 100644 .devcontainer/Dockerfile.standalone
>  create mode 100644 .devcontainer/devcontainer.json
>  create mode 100755 t/t2206-add-submodule-ignored.sh
>
>
> base-commit: 81f86aacc4eb74cdb9c2c8082d36d2070c666045
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1987%2FPraqma%2Frespect-submodule-ignore-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1987/Praqma/respect-submodule-ignore-v2
> Pull-Request: https://github.com/gitgitgadget/git/pull/1987
>
> Range-diff vs v1:
>
>  1:  d98cca698d ! 1:  5796009122 read-cache: update add_files_to_cache to take param ignored_too(--force)
>      @@ Metadata
>       Author: Claus Schneider(Eficode) <claus.schneider@eficode.com>
>       
>        ## Commit message ##
>      -    read-cache: update add_files_to_cache to take param ignored_too(--force)
>      +    read-cache: update add_files_to_cache take param include_ignored_submodules
>       
>      -    The ignored_too parameter is added to the function add_files_to_cache for
>      -    usage of explicit updating the index for the updated submodule using the
>      -    explicit patchspec to the submodule.
>      +    The include_ignored_submodules parameter is added to the function
>      +    add_files_to_cache for usage of explicit updating the index for the updated
>      +    submodule using the explicit patchspec to the submodule.
>       
>           Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
>       
>        ## builtin/add.c ##
>      +@@ builtin/add.c: N_("The following paths are ignored by one of your .gitignore files:\n");
>      + static int verbose, show_only, ignored_too, refresh_only;
>      + static int ignore_add_errors, intent_to_add, ignore_missing;
>      + static int warn_on_embedded_repo = 1;
>      ++static int include_ignored_submodules;
>      + 
>      + #define ADDREMOVE_DEFAULT 1
>      + static int addremove = ADDREMOVE_DEFAULT;
>      +@@ builtin/add.c: static struct option builtin_add_options[] = {
>      + 	OPT_BOOL( 0 , "ignore-errors", &ignore_add_errors, N_("just skip files which cannot be added because of errors")),
>      + 	OPT_BOOL( 0 , "ignore-missing", &ignore_missing, N_("check if - even missing - files are ignored in dry run")),
>      + 	OPT_BOOL(0, "sparse", &include_sparse, N_("allow updating entries outside of the sparse-checkout cone")),
>      ++    OPT_BOOL(0, "include-ignored-submodules", &include_ignored_submodules, N_("add submodules even if they has configuration ignore=all")),
>      + 	OPT_STRING(0, "chmod", &chmod_arg, "(+|-)x",
>      + 		   N_("override the executable bit of the listed files")),
>      + 	OPT_HIDDEN_BOOL(0, "warn-embedded-repo", &warn_on_embedded_repo,
>       @@ builtin/add.c: int cmd_add(int argc,
>        	else
>        		exit_status |= add_files_to_cache(repo, prefix,
>        						  &pathspec, ps_matched,
>       -						  include_sparse, flags);
>      -+						  include_sparse, flags, ignored_too);
>      ++						  include_sparse, flags, include_ignored_submodules);
>        
>        	if (take_worktree_changes && !add_renormalize && !ignore_add_errors &&
>        	    report_path_error(ps_matched, &pathspec))
>      @@ read-cache.c: void overlay_tree_on_index(struct index_state *istate,
>        	int include_sparse;
>        	int flags;
>        	int add_errors;
>      -+	int ignored_too;
>      ++	int include_ignored_submodules;
>        };
>        
>        static int fix_unmerged_status(struct diff_filepair *p,
>       @@ read-cache.c: static void update_callback(struct diff_queue_struct *q,
>      + 		default:
>      + 			die(_("unexpected diff status %c"), p->status);
>      + 		case DIFF_STATUS_MODIFIED:
>      +-		case DIFF_STATUS_TYPE_CHANGED:
>      ++		case DIFF_STATUS_TYPE_CHANGED: {
>      ++			struct stat st;
>      ++			if (!lstat(path, &st) && S_ISDIR(st.st_mode)) { // only consider submodule if it is a directory
>      ++				const struct submodule *sub = submodule_from_path(data->repo, null_oid(the_hash_algo), path);
>      ++				if (sub && sub->name && sub->ignore && !strcmp(sub->ignore, "all")) {
>      ++					int pathspec_matches = 0;
>      ++					char *norm_pathspec = NULL;
>      ++					int ps_i;
>      ++					trace_printf("ignore=all %s\n", path);
>      ++					trace_printf("pathspec %s\n",
>      ++							(data->pathspec && data->pathspec->nr) ? "has pathspec" : "no pathspec");
>      ++					/* Safely scan all pathspec items (q->nr may exceed pathspec->nr). */
>      ++					if (data->pathspec) {
>      ++						for (ps_i = 0; ps_i < data->pathspec->nr; ps_i++) {
>      ++							const char *m = data->pathspec->items[ps_i].match;
>      ++							if (!m)
>      ++								continue;
>      ++							norm_pathspec = xstrdup(m);
>      ++							strip_dir_trailing_slashes(norm_pathspec);
>      ++							if (!strcmp(path, norm_pathspec)) {
>      ++								pathspec_matches = 1;
>      ++								FREE_AND_NULL(norm_pathspec);
>      ++								break;
>      ++							}
>      ++							FREE_AND_NULL(norm_pathspec);
>      ++						}
>      ++					}
>      ++					if (pathspec_matches) {
>      ++						if (data->include_ignored_submodules && data->include_ignored_submodules > 0) {
>      ++							trace_printf("Add ignored=all submodule due to --include_ignored_submodules: %s\n", path);
>      ++						} else {
>      ++							printf(_("Skipping submodule due to ignore=all: %s"), path);
>      ++							printf(_("Use --include_ignored_submodules, if you really want to add them.") );
>      ++							continue;
>      ++						}
>      ++					} else {
>      ++						/* No explicit pathspec match -> skip silently (or with trace). */
>      ++						trace_printf("pathspec does not match %s\n", path);
>      ++						continue;
>      ++					}
>      ++				}
>      ++			}
>      + 			if (add_file_to_index(data->index, path, data->flags)) {
>      + 				if (!(data->flags & ADD_CACHE_IGNORE_ERRORS))
>      + 					die(_("updating files failed"));
>      +@@ read-cache.c: static void update_callback(struct diff_queue_struct *q,
>        
>        int add_files_to_cache(struct repository *repo, const char *prefix,
>        		       const struct pathspec *pathspec, char *ps_matched,
>       -		       int include_sparse, int flags)
>      -+		       int include_sparse, int flags, int ignored_too )
>      ++		       int include_sparse, int flags, int include_ignored_submodules )
>        {
>        	struct update_callback_data data;
>        	struct rev_info rev;
>      @@ read-cache.c: int add_files_to_cache(struct repository *repo, const char *prefix
>        	data.include_sparse = include_sparse;
>        	data.flags = flags;
>       +	data.repo = repo;
>      -+	data.ignored_too = ignored_too;
>      ++	data.include_ignored_submodules = include_ignored_submodules;
>       +	data.pathspec = (struct pathspec *)pathspec;
>        
>        	repo_init_revisions(repo, &rev, prefix);
>  2:  d1b02617e6 ! 2:  9ec79b9a11 read-cache: let read-cache respect submodule ignore=all and --force
>      @@ Metadata
>       Author: Claus Schneider(Eficode) <claus.schneider@eficode.com>
>       
>        ## Commit message ##
>      -    read-cache: let read-cache respect submodule ignore=all and --force
>      +    read-cache: add/read-cache respect submodule ignore=all
>       
>      -    Given the submdule configuration is ignore=all then only update the
>      -    submdule if the --force option is given and the submodule is explicit
>      -    given in the pathspec.
>      +    Submodules configured with ignore=all are now skipped during add operations
>      +    unless overridden by --include-ignored-submodules and the submodule path is
>      +    explicitly specified.
>       
>           A message is printed (like ignored files) guiding the user to use the
>      -    --force flag if the user has explicitely want to update the submodule
>      -    reference.
>      +    --include-ignored-submodules flag if the user has explicitely want to update
>      +    the submodule reference.
>       
>           The reason for the change is support submodule branch tracking or
>           similar and git status state nothing and git add should not add either.
>      @@ Commit message
>           the submodule is already tracked.
>       
>           The change opens up a lot of possibilities for submodules to be used
>      -    more freely and a like the repo tool. A submodule can be added for many
>      +    more freely and simular to the repo tool. A submodule can be added for many
>           more reason and loosely coupled dependencies to the super repo which often
>           gives the friction of handle the explicit commits and updates without
>           the need for tracking the submodule sha1 by sha1.
>      @@ read-cache.c
>        /* Mask for the name length in ce_flags in the on-disk index */
>        
>       @@ read-cache.c: static void update_callback(struct diff_queue_struct *q,
>      - 		default:
>      - 			die(_("unexpected diff status %c"), p->status);
>      - 		case DIFF_STATUS_MODIFIED:
>      --		case DIFF_STATUS_TYPE_CHANGED:
>      -+		case DIFF_STATUS_TYPE_CHANGED: {
>      -+			struct stat st;
>      -+			if (!lstat(path, &st) && S_ISDIR(st.st_mode)) { // only consider submodule if it is a directory
>      -+				const struct submodule *sub = submodule_from_path(data->repo, null_oid(the_hash_algo), path);
>      -+				if (sub && sub->name && sub->ignore && !strcmp(sub->ignore, "all")) {
>      -+					int pathspec_matches = 0;
>      -+					char *norm_pathspec = NULL;
>      -+					int ps_i;
>      -+					trace_printf("ignore=all %s\n", path);
>      -+					trace_printf("pathspec %s\n",
>      -+							(data->pathspec && data->pathspec->nr) ? "has pathspec" : "no pathspec");
>      -+					/* Safely scan all pathspec items (q->nr may exceed pathspec->nr). */
>      -+					if (data->pathspec) {
>      -+						for (ps_i = 0; ps_i < data->pathspec->nr; ps_i++) {
>      -+							const char *m = data->pathspec->items[ps_i].match;
>      -+							if (!m)
>      -+								continue;
>      -+							norm_pathspec = xstrdup(m);
>      -+							strip_dir_trailing_slashes(norm_pathspec);
>      -+							if (!strcmp(path, norm_pathspec)) {
>      -+								pathspec_matches = 1;
>      -+								FREE_AND_NULL(norm_pathspec);
>      -+								break;
>      -+							}
>      -+							FREE_AND_NULL(norm_pathspec);
>      -+						}
>      -+					}
>      -+					if (pathspec_matches) {
>      -+						if (data->ignored_too && data->ignored_too > 0) {
>      -+							trace_printf("Forcing add of submodule ignored=all due to --force: %s\n", path);
>      -+						} else {
>      -+							printf(_("Skipping submodule due to ignore=all: %s"), path);
>      -+							printf(_("Use -f if you really want to add them.") );
>      -+							continue;
>      -+						}
>      -+					} else {
>      -+						/* No explicit pathspec match -> skip silently (or with trace). */
>      -+						trace_printf("pathspec does not match %s\n", path);
>      -+						continue;
>      -+					}
>      -+				}
>      -+			}
>      - 			if (add_file_to_index(data->index, path, data->flags)) {
>      - 				if (!(data->flags & ADD_CACHE_IGNORE_ERRORS))
>      - 					die(_("updating files failed"));
>      + 					}
>      + 					if (pathspec_matches) {
>      + 						if (data->include_ignored_submodules && data->include_ignored_submodules > 0) {
>      +-							trace_printf("Add ignored=all submodule due to --include_ignored_submodules: %s\n", path);
>      ++							trace_printf("Add submodule due to --include_ignored_submodules: %s\n", path);
>      + 						} else {
>      + 							printf(_("Skipping submodule due to ignore=all: %s"), path);
>      + 							printf(_("Use --include_ignored_submodules, if you really want to add them.") );
>      +@@ read-cache.c: static void update_callback(struct diff_queue_struct *q,
>      + 						}
>      + 					} else {
>      + 						/* No explicit pathspec match -> skip silently (or with trace). */
>      +-						trace_printf("pathspec does not match %s\n", path);
>      ++						trace_printf("Pathspec to submodule does not match explicitly: %s\n", path);
>      + 						continue;
>      + 					}
>      + 				}
>      +@@ read-cache.c: static void update_callback(struct diff_queue_struct *q,
>        				data->add_errors++;
>        			}
>        			break;
>  3:  8f3d5f7ec1 ! 3:  399a153b95 tests: add new t2206-add-submodule-ignored.sh to test ignore=all scenario
>      @@ Commit message
>           config with ignore=all also behaves as intended with configuration in
>           .gitmodules and configuration given on the command line.
>       
>      -    Testfile is added to meson.build for execution.
>      +    The usage of --include_ignored_submodules is showcased and tested in the
>      +    test suite.
>      +
>      +    The test file is added to meson.build for execution.
>       
>           Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
>       
>      @@ t/t2206-add-submodule-ignored.sh (new)
>       +# This test covers the behavior of "git add", "git status" and "git log" when
>       +# dealing with submodules that have the ignore=all setting in
>       +# .gitmodules. It ensures that changes in such submodules are
>      -+# ignored by default, but can be staged with "git add --force".
>      ++# ignored by default, but can be staged with "git add --include-ignored-submodules".
>       +
>       +# shellcheck disable=SC1091
>       +. ./test-lib.sh
>      @@ t/t2206-add-submodule-ignored.sh (new)
>       +'
>       +
>       +#6
>      -+# check that 'git add --force .' does not stage the change in the submodule
>      ++# check that 'git add --include-ignored-submodules .' does not stage the change in the submodule
>       +# and that 'git status' does not show it as modified
>      -+test_expect_success 'main: check --force add . and status'  '
>      ++test_expect_success 'main: check --include-ignored-submodules add . and status'  '
>       +	cd "${base_path}" &&
>       +	cd main &&
>      -+	GIT_TRACE=1 git add --force . &&
>      ++	GIT_TRACE=1 git add --include-ignored-submodules . &&
>       +	! git status --porcelain | grep "^M  sub$" &&
>       +	echo
>       +'
>      @@ t/t2206-add-submodule-ignored.sh (new)
>       +'
>       +
>       +#8
>      -+# check that 'git add --force sub' does stage the change in the submodule
>      -+# check that 'git add --force ./sub/' does stage the change in the submodule
>      ++# check that 'git add --include-ignored-submodules sub' does stage the change in the submodule
>      ++# check that 'git add --include-ignored-submodules ./sub/' does stage the change in the submodule
>       +# and that 'git status --porcelain' does show it as modified
>       +# commit it..
>       +# check that 'git log --ignore-submodules=none' shows the submodule change
>      @@ t/t2206-add-submodule-ignored.sh (new)
>       +test_expect_success 'main: check force add sub and ./sub/ and status'  '
>       +	cd "${base_path}" &&
>       +	cd main &&
>      -+	echo "Adding with --force should work: git add --force sub" &&
>      -+	GIT_TRACE=1 git add --force sub &&
>      ++	echo "Adding with --include-ignored-submodules should work: git add --include-ignored-submodules sub" &&
>      ++	GIT_TRACE=1 git add --include-ignored-submodules sub &&
>       +	git status --porcelain | grep "^M  sub$" &&
>       +	git restore --staged sub &&
>       +	! git status --porcelain | grep "^M  sub$" &&
>      -+	echo "Adding with --force should work: git add --force ./sub/" &&
>      -+	GIT_TRACE=1 git add --force ./sub/ &&
>      ++	echo "Adding with --include-ignored-submodules should work: git add --include-ignored-submodules ./sub/" &&
>      ++	GIT_TRACE=1 git add --include-ignored-submodules ./sub/ &&
>       +	git status --porcelain | grep "^M  sub$" &&
>       +	git commit -m "update submodule pointer" &&
>       +	! git status --porcelain | grep "^ M sub$" &&
>  4:  58563a7b90 ! 4:  93c95954f1 tests: fix existing tests when add an ignore=all submodule
>      @@ Metadata
>        ## Commit message ##
>           tests: fix existing tests when add an ignore=all submodule
>       
>      -    There are tests that rely on "git add <submodule>" also adds it. A --force
>      -    is needed with this enhancement hence they are added accordingly in these
>      -    tests.
>      +    There are tests that rely on "git add <submodule>" to add updates in the
>      +    parent repository. A new option --include-ignored-submodules is introduced
>      +    as it is now needed with this enhancement.
>       
>           Updated tests:
>           - t1013-read-tree-submodule.sh ( fixed in: t/lib-submodule-update.sh )
>      +    - t2013-checkout-submodule.sh ( fixed in: t/lib-submodule-update.sh )
>           - t7406-submodule-update.sh
>           - t7508-status.sh
>       
>      @@ t/lib-submodule-update.sh: create_lib_submodule_repo () {
>        			git push origin modifications
>        		) &&
>       -		git add sub1 &&
>      -+		git add --force sub1 &&
>      ++		git add --include-ignored-submodules sub1 &&
>        		git commit -m "Modify sub1" &&
>        
>        		git checkout -b add_nested_sub modify_sub1 &&
>      @@ t/lib-submodule-update.sh: create_lib_submodule_repo () {
>        		git -C sub1 submodule add --branch no_submodule ../submodule_update_sub2 sub2 &&
>        		git -C sub1 commit -a -m "add a nested submodule" &&
>       -		git add sub1 &&
>      -+		git add --force sub1 &&
>      ++		git add --include-ignored-submodules sub1 &&
>        		git commit -a -m "update submodule, that updates a nested submodule" &&
>        		git checkout -b modify_sub1_recursively &&
>        		git -C sub1 checkout -b modify_sub1_recursively &&
>      @@ t/lib-submodule-update.sh: create_lib_submodule_repo () {
>        		git -C sub1 add sub2 &&
>        		git -C sub1 commit -m "update nested sub" &&
>       -		git add sub1 &&
>      -+		git add --force sub1 &&
>      ++		git add --include-ignored-submodules sub1 &&
>        		git commit -m "update sub1, that updates nested sub" &&
>        		git -C sub1 push origin modify_sub1_recursively &&
>        		git -C sub1/sub2 push origin modify_sub1_recursively &&
>      @@ t/t7508-status.sh: test_expect_success 'git commit will commit a staged but igno
>        test_expect_success 'git commit --dry-run will show a staged but ignored submodule' '
>        	git reset HEAD^ &&
>       -	git add sm &&
>      -+	git add --force sm &&
>      ++	git add --include-ignored-submodules sm &&
>        	cat >expect << EOF &&
>        On branch main
>        Your branch and '\''upstream'\'' have diverged,
>  5:  416695f439 < -:  ---------- Documentation: update add --force and submodule ignore=all config
>  -:  ---------- > 5:  ee84190cd8 Documentation: add --include_ignored_submodules + ignore=all config

Re: [PATCH v2 1/5] read-cache: update add_files_to_cache take param include_ignored_submodules

[Junio C Hamano]

"Claus Schneider(Eficode) via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: "Claus Schneider(Eficode)" <claus.schneider@eficode.com>
>
> The include_ignored_submodules parameter is added to the function
> add_files_to_cache for usage of explicit updating the index for the updated
> submodule using the explicit patchspec to the submodule.
>
> Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
> ---

I still do not know enough if the overall idea of this topic is a
good idea, but let me regiew the code change at the mechanical level
(i.e., what needs fixing if these changes are good things to have).

> diff --git a/builtin/add.c b/builtin/add.c
> index 0235854f80..6d11382f33 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -233,6 +233,7 @@ N_("The following paths are ignored by one of your .gitignore files:\n");
>  static int verbose, show_only, ignored_too, refresh_only;
>  static int ignore_add_errors, intent_to_add, ignore_missing;
>  static int warn_on_embedded_repo = 1;
> +static int include_ignored_submodules;
>  
>  #define ADDREMOVE_DEFAULT 1
>  static int addremove = ADDREMOVE_DEFAULT;
> @@ -271,6 +272,7 @@ static struct option builtin_add_options[] = {
>  	OPT_BOOL( 0 , "ignore-errors", &ignore_add_errors, N_("just skip files which cannot be added because of errors")),
>  	OPT_BOOL( 0 , "ignore-missing", &ignore_missing, N_("check if - even missing - files are ignored in dry run")),
>  	OPT_BOOL(0, "sparse", &include_sparse, N_("allow updating entries outside of the sparse-checkout cone")),
> +    OPT_BOOL(0, "include-ignored-submodules", &include_ignored_submodules, N_("add submodules even if they has configuration ignore=all")),

Wrong indentation.  Also, perhaps wrap this new line that is overly
long (*WITHOUT* rewrapping existing overly long lines)?

>  			add_files_to_cache(the_repository, NULL, NULL, NULL, 0,
> -					   0);
> +					   0, 0 );

Our coding style does not allow an extra space after "(" or before ")".

> diff --git a/read-cache-ll.h b/read-cache-ll.h
> index 71b49d9af4..2c8b4b21b1 100644
> --- a/read-cache-ll.h
> +++ b/read-cache-ll.h
> @@ -481,7 +481,7 @@ int cmp_cache_name_compare(const void *a_, const void *b_);
>  
>  int add_files_to_cache(struct repository *repo, const char *prefix,
>  		       const struct pathspec *pathspec, char *ps_matched,
> -		       int include_sparse, int flags);
> +		       int include_sparse, int flags, int ignored_too );

I am not sure if adding an extra parameter randomly like this is a
good idea.  Existing include_sparse is already a single bit that is
stuffed in an int, so it might make sense to change it into a set of
bits (i.e., "unsigned int") in one preliminary patch, and then your
change can borrow another bit in the same flag word.

See the attached patch (not even compile tested, though) for an
illustration of what such a "preliminary clean-up" step to get rid
of the extra include_sparse parameter may look like.

> +++ b/read-cache.c
> @@ -3880,9 +3880,12 @@ void overlay_tree_on_index(struct index_state *istate,
>  
>  struct update_callback_data {
>  	struct index_state *index;
> +	struct repository *repo;
> +	struct pathspec *pathspec;
>  	int include_sparse;
>  	int flags;
>  	int add_errors;
> +	int include_ignored_submodules;
>  };
>  
>  static int fix_unmerged_status(struct diff_filepair *p,
> @@ -3924,7 +3927,48 @@ static void update_callback(struct diff_queue_struct *q,
>  		default:
>  			die(_("unexpected diff status %c"), p->status);
>  		case DIFF_STATUS_MODIFIED:
> -		case DIFF_STATUS_TYPE_CHANGED:
> +		case DIFF_STATUS_TYPE_CHANGED: {
> +			struct stat st;
> +			if (!lstat(path, &st) && S_ISDIR(st.st_mode)) { // only consider submodule if it is a directory

/* Our single liner comment should look like this, not with double-slashes */

The fact that this block has to be so deeply indented strongly
indicates that it should probably become a separate helper function.

I'll stop here for now.



 builtin/add.c      | 8 ++++----
 builtin/checkout.c | 3 +--
 builtin/commit.c   | 2 +-
 read-cache-ll.h    | 6 +++++-
 read-cache.c       | 8 +++-----
 5 files changed, 14 insertions(+), 13 deletions(-)

diff --git c/builtin/add.c w/builtin/add.c
index 32709794b3..49d0199c0b 100644
--- c/builtin/add.c
+++ w/builtin/add.c
@@ -384,7 +384,7 @@ int cmd_add(int argc,
 	int exit_status = 0;
 	struct pathspec pathspec;
 	struct dir_struct dir = DIR_INIT;
-	int flags;
+	unsigned flags;
 	int add_new_files;
 	int require_pathspec;
 	char *seen = NULL;
@@ -485,7 +485,8 @@ int cmd_add(int argc,
 		 (intent_to_add ? ADD_CACHE_INTENT : 0) |
 		 (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |
 		 (!(addremove || take_worktree_changes)
-		  ? ADD_CACHE_IGNORE_REMOVAL : 0));
+		  ? ADD_CACHE_IGNORE_REMOVAL : 0) |
+		 (include_sparse ? ADD_CACHE_INCLUDE_SPARSE : 0));
 
 	if (repo_read_index_preload(repo, &pathspec, 0) < 0)
 		die(_("index file corrupt"));
@@ -583,8 +584,7 @@ int cmd_add(int argc,
 		exit_status |= renormalize_tracked_files(repo, &pathspec, flags);
 	else
 		exit_status |= add_files_to_cache(repo, prefix,
-						  &pathspec, ps_matched,
-						  include_sparse, flags);
+						  &pathspec, ps_matched, flags);
 
 	if (take_worktree_changes && !add_renormalize && !ignore_add_errors &&
 	    report_path_error(ps_matched, &pathspec))
diff --git c/builtin/checkout.c w/builtin/checkout.c
index f9453473fe..766a8e3b66 100644
--- c/builtin/checkout.c
+++ w/builtin/checkout.c
@@ -898,8 +898,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
 			 * entries in the index.
 			 */
 
-			add_files_to_cache(the_repository, NULL, NULL, NULL, 0,
-					   0);
+			add_files_to_cache(the_repository, NULL, NULL, NULL, 0);
 			init_ui_merge_options(&o, the_repository);
 			o.verbosity = 0;
 			work = write_in_core_index_as_tree(the_repository);
diff --git c/builtin/commit.c w/builtin/commit.c
index 0243f17d53..6e42534977 100644
--- c/builtin/commit.c
+++ w/builtin/commit.c
@@ -455,7 +455,7 @@ static const char *prepare_index(const char **argv, const char *prefix,
 		repo_hold_locked_index(the_repository, &index_lock,
 				       LOCK_DIE_ON_ERROR);
 		add_files_to_cache(the_repository, also ? prefix : NULL,
-				   &pathspec, ps_matched, 0, 0);
+				   &pathspec, ps_matched, 0);
 		if (!all && report_path_error(ps_matched, &pathspec))
 			exit(128);
 
diff --git c/read-cache-ll.h w/read-cache-ll.h
index 71b49d9af4..c9311f845b 100644
--- c/read-cache-ll.h
+++ w/read-cache-ll.h
@@ -390,11 +390,15 @@ int remove_index_entry_at(struct index_state *, int pos);
 
 void remove_marked_cache_entries(struct index_state *istate, int invalidate);
 int remove_file_from_index(struct index_state *, const char *path);
+
+/* add_files_to_cache() flags */
 #define ADD_CACHE_VERBOSE 1
 #define ADD_CACHE_PRETEND 2
 #define ADD_CACHE_IGNORE_ERRORS	4
 #define ADD_CACHE_IGNORE_REMOVAL 8
 #define ADD_CACHE_INTENT 16
+#define ADD_CACHE_INCLUDE_SPARSE 32
+
 /*
  * These two are used to add the contents of the file at path
  * to the index, marking the working tree up-to-date by storing
@@ -481,7 +485,7 @@ int cmp_cache_name_compare(const void *a_, const void *b_);
 
 int add_files_to_cache(struct repository *repo, const char *prefix,
 		       const struct pathspec *pathspec, char *ps_matched,
-		       int include_sparse, int flags);
+		       unsigned flags);
 
 void overlay_tree_on_index(struct index_state *istate,
 			   const char *tree_name, const char *prefix);
diff --git c/read-cache.c w/read-cache.c
index 032480d0c7..f32b642446 100644
--- c/read-cache.c
+++ w/read-cache.c
@@ -3881,8 +3881,7 @@ void overlay_tree_on_index(struct index_state *istate,
 
 struct update_callback_data {
 	struct index_state *index;
-	int include_sparse;
-	int flags;
+	unsigned flags;
 	int add_errors;
 };
 
@@ -3917,7 +3916,7 @@ static void update_callback(struct diff_queue_struct *q,
 		struct diff_filepair *p = q->queue[i];
 		const char *path = p->one->path;
 
-		if (!data->include_sparse &&
+		if (!(data->flags & ADD_CACHE_INCLUDE_SPARSE) &&
 		    !path_in_sparse_checkout(path, data->index))
 			continue;
 
@@ -3946,7 +3945,7 @@ static void update_callback(struct diff_queue_struct *q,
 
 int add_files_to_cache(struct repository *repo, const char *prefix,
 		       const struct pathspec *pathspec, char *ps_matched,
-		       int include_sparse, int flags)
+		       unsigned flags)
 {
 	struct odb_transaction *transaction;
 	struct update_callback_data data;
@@ -3954,7 +3953,6 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
 
 	memset(&data, 0, sizeof(data));
 	data.index = repo->index;
-	data.include_sparse = include_sparse;
 	data.flags = flags;
 
 	repo_init_revisions(repo, &rev, prefix);

Re: [PATCH v2 0/5] git-add : Respect submodule ignore=all and only add changes with --force

[Claus Schneider]

Thanks Junio - well received and noted. I have updated the PR
description accordingly, but I have not changed the "--force" in the
description even though I have implemented
''--include-ignored-submodules' as Philip had the input not to use
"--force". He suggested using a new option. Philips comment:

> I'm not convinced that the approach of using "--force" is a good idea as
> it conflates ignoring changes to tracked paths (which is what
> submodule.<name>.ignore" does) with ignoring untracked paths (which is
> what ".gitignore" does). If we're happy to break existing uses that rely
> on the current behavior then having a new option to override
> submodule.<name>.ignore strikes me as a better way forward. I don't have
> much experience of using submodules so I can't comment on whether
> changing the behavior is a good idea or not.

I think it will be more simple to use the '--force' option though and
keep the amount of options lower and less to remember. Given your
comments about more usages of bytes for option also becomes obsolete
if we stick to "--force". I am happy to do so.

I am investigating your other comments on the patches in the meantime.

On Thu, Nov 13, 2025 at 8:58 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Claus Schneider via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > The feature of configuring a submodule to "ignore=all" is nicely respected
> > in commands "status" and "diff".
>
> "nicely respected" is not very informative for those who do not know
> what the setting does.  Saying something like
>
>     "git status" and "git diff" will not report modified submodules
>     with submodule.<name>.ignore set to "all".
>
> would not waste significantly more bytes than what you wrote, and is
> more helpful.
>
> > However the "add" command does not respect
> > the configuration the same way.
>
> Again, "does not respect" and then what?  Running "git add" on a
> submodule with submodule.<name>.ignore set to "all" does what?
> Complains that it has changes but because .ignore is set it won't
> add?  Adds it silently?  Something else?
>
> > The behavior is problematic for the logic
> > between status/diff and add.
>
> After this sentence, "because ..." is missing.  Please help readers
> understand the issue you perceive as problematic more easily.
>
> I am guessing that you are assuming that an "add", after "diff" or
> "status" said there is no change, is expected to be a no-op, but I
> cannot be sure if that is what you are referring to here with the
> reason left unsaid like the above.
>
> > Secondly it makes it problematic to track
> > branches in the submodule configuration as developers unintentionally keeps
> > add submodule updates and get conflicts for no intentional reason. Both adds
> > unnecessary friction to the usage of submodules.
> >
> > The patches implement the same logical behavior for ignore=all submodules as
> > regular ignored files. The status now does not show any diff - nor will the
> > add command update the reference submodule reference. If you add the
> > submodule path which is ignore=all then you are presented with a message
> > that you need to use the --force option.
>
> I vaguely recall that an earlier discussion between you and Phillip
> were concluding against "--force"?  I personally feel it is in line
> with "git add foo.o" (when '*.o' is in .gitignore) gets rejected and
> "git add -f foo.o" is a way to override it, but in the list of
> patches below, I see --include-ignored-submodules (no, our command
> line option names do not use underscore for inter-word-gaps), so I
> suspect the description in the cover letter around here is stale?
>
>
> > The branch=, ignore=all (and
> > update=none) now works great with update --remote,
>
> Again, "great" is not very informative, and as bad as "nicely
> respected".  Avoid using these adjectives loaded with unnecessary
> value judgements, and instead trust your readers.  They are
> intelligent to judge if the updated behaviour is great or not for
> themselves.  Try to use the same bytes on helping readers understand
> what actually happens.
>
> > but developers does not
>
> "do not".
>
> > have to consider changes in the updates of the submodule sha1. The
> > implementation removes a friction of working with submodules and can be used
> > like the repo tool with branches configured. The submodule status report
> > could be used for build/release documentation for reproduction of a setup.
> >
> > A few tests used the adding of submodules without --force, hence they have
> > been updated to use the --force option.
> >
> > Claus Schneider(Eficode) (5):
> >   read-cache: update add_files_to_cache take param
> >     include_ignored_submodules
> >   read-cache: add/read-cache respect submodule ignore=all
> >   tests: add new t2206-add-submodule-ignored.sh to test ignore=all
> >     scenario
> >   tests: fix existing tests when add an ignore=all submodule
> >   Documentation: add --include_ignored_submodules + ignore=all config
> >
> >  .devcontainer/Dockerfile            |  70 +++++++++++++++
> >  .devcontainer/Dockerfile.standalone |  76 ++++++++++++++++
> >  .devcontainer/devcontainer.json     |  25 ++++++
> >  Documentation/config/submodule.adoc |  13 +--
> >  Documentation/git-add.adoc          |   5 ++
> >  Documentation/gitmodules.adoc       |   5 +-
> >  builtin/add.c                       |   4 +-
> >  builtin/checkout.c                  |   2 +-
> >  builtin/commit.c                    |   2 +-
> >  read-cache-ll.h                     |   2 +-
> >  read-cache.c                        |  54 ++++++++++-
> >  t/lib-submodule-update.sh           |   6 +-
> >  t/meson.build                       |   1 +
> >  t/t2206-add-submodule-ignored.sh    | 134 ++++++++++++++++++++++++++++
> >  t/t7508-status.sh                   |   2 +-
> >  15 files changed, 384 insertions(+), 17 deletions(-)
> >  create mode 100644 .devcontainer/Dockerfile
> >  create mode 100644 .devcontainer/Dockerfile.standalone
> >  create mode 100644 .devcontainer/devcontainer.json
> >  create mode 100755 t/t2206-add-submodule-ignored.sh
> >
> >
> > base-commit: 81f86aacc4eb74cdb9c2c8082d36d2070c666045
> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1987%2FPraqma%2Frespect-submodule-ignore-v2
> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1987/Praqma/respect-submodule-ignore-v2
> > Pull-Request: https://github.com/gitgitgadget/git/pull/1987
> >
> > Range-diff vs v1:
> >
> >  1:  d98cca698d ! 1:  5796009122 read-cache: update add_files_to_cache to take param ignored_too(--force)
> >      @@ Metadata
> >       Author: Claus Schneider(Eficode) <claus.schneider@eficode.com>
> >
> >        ## Commit message ##
> >      -    read-cache: update add_files_to_cache to take param ignored_too(--force)
> >      +    read-cache: update add_files_to_cache take param include_ignored_submodules
> >
> >      -    The ignored_too parameter is added to the function add_files_to_cache for
> >      -    usage of explicit updating the index for the updated submodule using the
> >      -    explicit patchspec to the submodule.
> >      +    The include_ignored_submodules parameter is added to the function
> >      +    add_files_to_cache for usage of explicit updating the index for the updated
> >      +    submodule using the explicit patchspec to the submodule.
> >
> >           Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
> >
> >        ## builtin/add.c ##
> >      +@@ builtin/add.c: N_("The following paths are ignored by one of your .gitignore files:\n");
> >      + static int verbose, show_only, ignored_too, refresh_only;
> >      + static int ignore_add_errors, intent_to_add, ignore_missing;
> >      + static int warn_on_embedded_repo = 1;
> >      ++static int include_ignored_submodules;
> >      +
> >      + #define ADDREMOVE_DEFAULT 1
> >      + static int addremove = ADDREMOVE_DEFAULT;
> >      +@@ builtin/add.c: static struct option builtin_add_options[] = {
> >      +        OPT_BOOL( 0 , "ignore-errors", &ignore_add_errors, N_("just skip files which cannot be added because of errors")),
> >      +        OPT_BOOL( 0 , "ignore-missing", &ignore_missing, N_("check if - even missing - files are ignored in dry run")),
> >      +        OPT_BOOL(0, "sparse", &include_sparse, N_("allow updating entries outside of the sparse-checkout cone")),
> >      ++    OPT_BOOL(0, "include-ignored-submodules", &include_ignored_submodules, N_("add submodules even if they has configuration ignore=all")),
> >      +        OPT_STRING(0, "chmod", &chmod_arg, "(+|-)x",
> >      +                   N_("override the executable bit of the listed files")),
> >      +        OPT_HIDDEN_BOOL(0, "warn-embedded-repo", &warn_on_embedded_repo,
> >       @@ builtin/add.c: int cmd_add(int argc,
> >               else
> >                       exit_status |= add_files_to_cache(repo, prefix,
> >                                                         &pathspec, ps_matched,
> >       -                                                 include_sparse, flags);
> >      -+                                                 include_sparse, flags, ignored_too);
> >      ++                                                 include_sparse, flags, include_ignored_submodules);
> >
> >               if (take_worktree_changes && !add_renormalize && !ignore_add_errors &&
> >                   report_path_error(ps_matched, &pathspec))
> >      @@ read-cache.c: void overlay_tree_on_index(struct index_state *istate,
> >               int include_sparse;
> >               int flags;
> >               int add_errors;
> >      -+       int ignored_too;
> >      ++       int include_ignored_submodules;
> >        };
> >
> >        static int fix_unmerged_status(struct diff_filepair *p,
> >       @@ read-cache.c: static void update_callback(struct diff_queue_struct *q,
> >      +                default:
> >      +                        die(_("unexpected diff status %c"), p->status);
> >      +                case DIFF_STATUS_MODIFIED:
> >      +-               case DIFF_STATUS_TYPE_CHANGED:
> >      ++               case DIFF_STATUS_TYPE_CHANGED: {
> >      ++                       struct stat st;
> >      ++                       if (!lstat(path, &st) && S_ISDIR(st.st_mode)) { // only consider submodule if it is a directory
> >      ++                               const struct submodule *sub = submodule_from_path(data->repo, null_oid(the_hash_algo), path);
> >      ++                               if (sub && sub->name && sub->ignore && !strcmp(sub->ignore, "all")) {
> >      ++                                       int pathspec_matches = 0;
> >      ++                                       char *norm_pathspec = NULL;
> >      ++                                       int ps_i;
> >      ++                                       trace_printf("ignore=all %s\n", path);
> >      ++                                       trace_printf("pathspec %s\n",
> >      ++                                                       (data->pathspec && data->pathspec->nr) ? "has pathspec" : "no pathspec");
> >      ++                                       /* Safely scan all pathspec items (q->nr may exceed pathspec->nr). */
> >      ++                                       if (data->pathspec) {
> >      ++                                               for (ps_i = 0; ps_i < data->pathspec->nr; ps_i++) {
> >      ++                                                       const char *m = data->pathspec->items[ps_i].match;
> >      ++                                                       if (!m)
> >      ++                                                               continue;
> >      ++                                                       norm_pathspec = xstrdup(m);
> >      ++                                                       strip_dir_trailing_slashes(norm_pathspec);
> >      ++                                                       if (!strcmp(path, norm_pathspec)) {
> >      ++                                                               pathspec_matches = 1;
> >      ++                                                               FREE_AND_NULL(norm_pathspec);
> >      ++                                                               break;
> >      ++                                                       }
> >      ++                                                       FREE_AND_NULL(norm_pathspec);
> >      ++                                               }
> >      ++                                       }
> >      ++                                       if (pathspec_matches) {
> >      ++                                               if (data->include_ignored_submodules && data->include_ignored_submodules > 0) {
> >      ++                                                       trace_printf("Add ignored=all submodule due to --include_ignored_submodules: %s\n", path);
> >      ++                                               } else {
> >      ++                                                       printf(_("Skipping submodule due to ignore=all: %s"), path);
> >      ++                                                       printf(_("Use --include_ignored_submodules, if you really want to add them.") );
> >      ++                                                       continue;
> >      ++                                               }
> >      ++                                       } else {
> >      ++                                               /* No explicit pathspec match -> skip silently (or with trace). */
> >      ++                                               trace_printf("pathspec does not match %s\n", path);
> >      ++                                               continue;
> >      ++                                       }
> >      ++                               }
> >      ++                       }
> >      +                        if (add_file_to_index(data->index, path, data->flags)) {
> >      +                                if (!(data->flags & ADD_CACHE_IGNORE_ERRORS))
> >      +                                        die(_("updating files failed"));
> >      +@@ read-cache.c: static void update_callback(struct diff_queue_struct *q,
> >
> >        int add_files_to_cache(struct repository *repo, const char *prefix,
> >                              const struct pathspec *pathspec, char *ps_matched,
> >       -                      int include_sparse, int flags)
> >      -+                      int include_sparse, int flags, int ignored_too )
> >      ++                      int include_sparse, int flags, int include_ignored_submodules )
> >        {
> >               struct update_callback_data data;
> >               struct rev_info rev;
> >      @@ read-cache.c: int add_files_to_cache(struct repository *repo, const char *prefix
> >               data.include_sparse = include_sparse;
> >               data.flags = flags;
> >       +       data.repo = repo;
> >      -+       data.ignored_too = ignored_too;
> >      ++       data.include_ignored_submodules = include_ignored_submodules;
> >       +       data.pathspec = (struct pathspec *)pathspec;
> >
> >               repo_init_revisions(repo, &rev, prefix);
> >  2:  d1b02617e6 ! 2:  9ec79b9a11 read-cache: let read-cache respect submodule ignore=all and --force
> >      @@ Metadata
> >       Author: Claus Schneider(Eficode) <claus.schneider@eficode.com>
> >
> >        ## Commit message ##
> >      -    read-cache: let read-cache respect submodule ignore=all and --force
> >      +    read-cache: add/read-cache respect submodule ignore=all
> >
> >      -    Given the submdule configuration is ignore=all then only update the
> >      -    submdule if the --force option is given and the submodule is explicit
> >      -    given in the pathspec.
> >      +    Submodules configured with ignore=all are now skipped during add operations
> >      +    unless overridden by --include-ignored-submodules and the submodule path is
> >      +    explicitly specified.
> >
> >           A message is printed (like ignored files) guiding the user to use the
> >      -    --force flag if the user has explicitely want to update the submodule
> >      -    reference.
> >      +    --include-ignored-submodules flag if the user has explicitely want to update
> >      +    the submodule reference.
> >
> >           The reason for the change is support submodule branch tracking or
> >           similar and git status state nothing and git add should not add either.
> >      @@ Commit message
> >           the submodule is already tracked.
> >
> >           The change opens up a lot of possibilities for submodules to be used
> >      -    more freely and a like the repo tool. A submodule can be added for many
> >      +    more freely and simular to the repo tool. A submodule can be added for many
> >           more reason and loosely coupled dependencies to the super repo which often
> >           gives the friction of handle the explicit commits and updates without
> >           the need for tracking the submodule sha1 by sha1.
> >      @@ read-cache.c
> >        /* Mask for the name length in ce_flags in the on-disk index */
> >
> >       @@ read-cache.c: static void update_callback(struct diff_queue_struct *q,
> >      -                default:
> >      -                        die(_("unexpected diff status %c"), p->status);
> >      -                case DIFF_STATUS_MODIFIED:
> >      --               case DIFF_STATUS_TYPE_CHANGED:
> >      -+               case DIFF_STATUS_TYPE_CHANGED: {
> >      -+                       struct stat st;
> >      -+                       if (!lstat(path, &st) && S_ISDIR(st.st_mode)) { // only consider submodule if it is a directory
> >      -+                               const struct submodule *sub = submodule_from_path(data->repo, null_oid(the_hash_algo), path);
> >      -+                               if (sub && sub->name && sub->ignore && !strcmp(sub->ignore, "all")) {
> >      -+                                       int pathspec_matches = 0;
> >      -+                                       char *norm_pathspec = NULL;
> >      -+                                       int ps_i;
> >      -+                                       trace_printf("ignore=all %s\n", path);
> >      -+                                       trace_printf("pathspec %s\n",
> >      -+                                                       (data->pathspec && data->pathspec->nr) ? "has pathspec" : "no pathspec");
> >      -+                                       /* Safely scan all pathspec items (q->nr may exceed pathspec->nr). */
> >      -+                                       if (data->pathspec) {
> >      -+                                               for (ps_i = 0; ps_i < data->pathspec->nr; ps_i++) {
> >      -+                                                       const char *m = data->pathspec->items[ps_i].match;
> >      -+                                                       if (!m)
> >      -+                                                               continue;
> >      -+                                                       norm_pathspec = xstrdup(m);
> >      -+                                                       strip_dir_trailing_slashes(norm_pathspec);
> >      -+                                                       if (!strcmp(path, norm_pathspec)) {
> >      -+                                                               pathspec_matches = 1;
> >      -+                                                               FREE_AND_NULL(norm_pathspec);
> >      -+                                                               break;
> >      -+                                                       }
> >      -+                                                       FREE_AND_NULL(norm_pathspec);
> >      -+                                               }
> >      -+                                       }
> >      -+                                       if (pathspec_matches) {
> >      -+                                               if (data->ignored_too && data->ignored_too > 0) {
> >      -+                                                       trace_printf("Forcing add of submodule ignored=all due to --force: %s\n", path);
> >      -+                                               } else {
> >      -+                                                       printf(_("Skipping submodule due to ignore=all: %s"), path);
> >      -+                                                       printf(_("Use -f if you really want to add them.") );
> >      -+                                                       continue;
> >      -+                                               }
> >      -+                                       } else {
> >      -+                                               /* No explicit pathspec match -> skip silently (or with trace). */
> >      -+                                               trace_printf("pathspec does not match %s\n", path);
> >      -+                                               continue;
> >      -+                                       }
> >      -+                               }
> >      -+                       }
> >      -                        if (add_file_to_index(data->index, path, data->flags)) {
> >      -                                if (!(data->flags & ADD_CACHE_IGNORE_ERRORS))
> >      -                                        die(_("updating files failed"));
> >      +                                        }
> >      +                                        if (pathspec_matches) {
> >      +                                                if (data->include_ignored_submodules && data->include_ignored_submodules > 0) {
> >      +-                                                       trace_printf("Add ignored=all submodule due to --include_ignored_submodules: %s\n", path);
> >      ++                                                       trace_printf("Add submodule due to --include_ignored_submodules: %s\n", path);
> >      +                                                } else {
> >      +                                                        printf(_("Skipping submodule due to ignore=all: %s"), path);
> >      +                                                        printf(_("Use --include_ignored_submodules, if you really want to add them.") );
> >      +@@ read-cache.c: static void update_callback(struct diff_queue_struct *q,
> >      +                                                }
> >      +                                        } else {
> >      +                                                /* No explicit pathspec match -> skip silently (or with trace). */
> >      +-                                               trace_printf("pathspec does not match %s\n", path);
> >      ++                                               trace_printf("Pathspec to submodule does not match explicitly: %s\n", path);
> >      +                                                continue;
> >      +                                        }
> >      +                                }
> >      +@@ read-cache.c: static void update_callback(struct diff_queue_struct *q,
> >                                       data->add_errors++;
> >                               }
> >                               break;
> >  3:  8f3d5f7ec1 ! 3:  399a153b95 tests: add new t2206-add-submodule-ignored.sh to test ignore=all scenario
> >      @@ Commit message
> >           config with ignore=all also behaves as intended with configuration in
> >           .gitmodules and configuration given on the command line.
> >
> >      -    Testfile is added to meson.build for execution.
> >      +    The usage of --include_ignored_submodules is showcased and tested in the
> >      +    test suite.
> >      +
> >      +    The test file is added to meson.build for execution.
> >
> >           Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
> >
> >      @@ t/t2206-add-submodule-ignored.sh (new)
> >       +# This test covers the behavior of "git add", "git status" and "git log" when
> >       +# dealing with submodules that have the ignore=all setting in
> >       +# .gitmodules. It ensures that changes in such submodules are
> >      -+# ignored by default, but can be staged with "git add --force".
> >      ++# ignored by default, but can be staged with "git add --include-ignored-submodules".
> >       +
> >       +# shellcheck disable=SC1091
> >       +. ./test-lib.sh
> >      @@ t/t2206-add-submodule-ignored.sh (new)
> >       +'
> >       +
> >       +#6
> >      -+# check that 'git add --force .' does not stage the change in the submodule
> >      ++# check that 'git add --include-ignored-submodules .' does not stage the change in the submodule
> >       +# and that 'git status' does not show it as modified
> >      -+test_expect_success 'main: check --force add . and status'  '
> >      ++test_expect_success 'main: check --include-ignored-submodules add . and status'  '
> >       +       cd "${base_path}" &&
> >       +       cd main &&
> >      -+       GIT_TRACE=1 git add --force . &&
> >      ++       GIT_TRACE=1 git add --include-ignored-submodules . &&
> >       +       ! git status --porcelain | grep "^M  sub$" &&
> >       +       echo
> >       +'
> >      @@ t/t2206-add-submodule-ignored.sh (new)
> >       +'
> >       +
> >       +#8
> >      -+# check that 'git add --force sub' does stage the change in the submodule
> >      -+# check that 'git add --force ./sub/' does stage the change in the submodule
> >      ++# check that 'git add --include-ignored-submodules sub' does stage the change in the submodule
> >      ++# check that 'git add --include-ignored-submodules ./sub/' does stage the change in the submodule
> >       +# and that 'git status --porcelain' does show it as modified
> >       +# commit it..
> >       +# check that 'git log --ignore-submodules=none' shows the submodule change
> >      @@ t/t2206-add-submodule-ignored.sh (new)
> >       +test_expect_success 'main: check force add sub and ./sub/ and status'  '
> >       +       cd "${base_path}" &&
> >       +       cd main &&
> >      -+       echo "Adding with --force should work: git add --force sub" &&
> >      -+       GIT_TRACE=1 git add --force sub &&
> >      ++       echo "Adding with --include-ignored-submodules should work: git add --include-ignored-submodules sub" &&
> >      ++       GIT_TRACE=1 git add --include-ignored-submodules sub &&
> >       +       git status --porcelain | grep "^M  sub$" &&
> >       +       git restore --staged sub &&
> >       +       ! git status --porcelain | grep "^M  sub$" &&
> >      -+       echo "Adding with --force should work: git add --force ./sub/" &&
> >      -+       GIT_TRACE=1 git add --force ./sub/ &&
> >      ++       echo "Adding with --include-ignored-submodules should work: git add --include-ignored-submodules ./sub/" &&
> >      ++       GIT_TRACE=1 git add --include-ignored-submodules ./sub/ &&
> >       +       git status --porcelain | grep "^M  sub$" &&
> >       +       git commit -m "update submodule pointer" &&
> >       +       ! git status --porcelain | grep "^ M sub$" &&
> >  4:  58563a7b90 ! 4:  93c95954f1 tests: fix existing tests when add an ignore=all submodule
> >      @@ Metadata
> >        ## Commit message ##
> >           tests: fix existing tests when add an ignore=all submodule
> >
> >      -    There are tests that rely on "git add <submodule>" also adds it. A --force
> >      -    is needed with this enhancement hence they are added accordingly in these
> >      -    tests.
> >      +    There are tests that rely on "git add <submodule>" to add updates in the
> >      +    parent repository. A new option --include-ignored-submodules is introduced
> >      +    as it is now needed with this enhancement.
> >
> >           Updated tests:
> >           - t1013-read-tree-submodule.sh ( fixed in: t/lib-submodule-update.sh )
> >      +    - t2013-checkout-submodule.sh ( fixed in: t/lib-submodule-update.sh )
> >           - t7406-submodule-update.sh
> >           - t7508-status.sh
> >
> >      @@ t/lib-submodule-update.sh: create_lib_submodule_repo () {
> >                               git push origin modifications
> >                       ) &&
> >       -               git add sub1 &&
> >      -+               git add --force sub1 &&
> >      ++               git add --include-ignored-submodules sub1 &&
> >                       git commit -m "Modify sub1" &&
> >
> >                       git checkout -b add_nested_sub modify_sub1 &&
> >      @@ t/lib-submodule-update.sh: create_lib_submodule_repo () {
> >                       git -C sub1 submodule add --branch no_submodule ../submodule_update_sub2 sub2 &&
> >                       git -C sub1 commit -a -m "add a nested submodule" &&
> >       -               git add sub1 &&
> >      -+               git add --force sub1 &&
> >      ++               git add --include-ignored-submodules sub1 &&
> >                       git commit -a -m "update submodule, that updates a nested submodule" &&
> >                       git checkout -b modify_sub1_recursively &&
> >                       git -C sub1 checkout -b modify_sub1_recursively &&
> >      @@ t/lib-submodule-update.sh: create_lib_submodule_repo () {
> >                       git -C sub1 add sub2 &&
> >                       git -C sub1 commit -m "update nested sub" &&
> >       -               git add sub1 &&
> >      -+               git add --force sub1 &&
> >      ++               git add --include-ignored-submodules sub1 &&
> >                       git commit -m "update sub1, that updates nested sub" &&
> >                       git -C sub1 push origin modify_sub1_recursively &&
> >                       git -C sub1/sub2 push origin modify_sub1_recursively &&
> >      @@ t/t7508-status.sh: test_expect_success 'git commit will commit a staged but igno
> >        test_expect_success 'git commit --dry-run will show a staged but ignored submodule' '
> >               git reset HEAD^ &&
> >       -       git add sm &&
> >      -+       git add --force sm &&
> >      ++       git add --include-ignored-submodules sm &&
> >               cat >expect << EOF &&
> >        On branch main
> >        Your branch and '\''upstream'\'' have diverged,
> >  5:  416695f439 < -:  ---------- Documentation: update add --force and submodule ignore=all config
> >  -:  ---------- > 5:  ee84190cd8 Documentation: add --include_ignored_submodules + ignore=all config