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

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
[off-list ref] 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 [off-list ref] wrote:

quoted

Hi Claus

[I've adjusted the CC list slightly]

On 18/10/2025 21:07, Claus Schneider via GitGitGadget wrote:

quoted

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

quoted

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