git-add : Respect submodule ignore=all and only add changes with --force <path>

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.

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

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))

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);

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);
 

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);

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 */
 
@@ -3880,9 +3882,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,
@@ -3924,13 +3929,57 @@ 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(norm_pathspec);
+								norm_pathspec = NULL;
+								break;
+							}
+							free(norm_pathspec);
+							norm_pathspec = NULL;
+						}
+					}
+					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;
@@ -3945,7 +3994,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 +4003,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);

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 &&

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',

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

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,