# [PATCH] RFC: commit: add a commit.all-ignore-submodules config option 5 messages from 2020-01-03 to 2020-01-07. Participants: marcandre.lureau@redhat.com, Jonathan Nieder, Marc-André Lureau. Thread: https://gitlist.dev/t/52556 ## marcandre.lureau@redhat.com, 2020-01-03 12:06 Subject: [PATCH] RFC: commit: add a commit.all-ignore-submodules config option Message-ID: <20200103120613.1063828-1-marcandre.lureau@redhat.com> URL: https://gitlist.dev/e/20200103120613.1063828-1-marcandre.lureau%40redhat.com ``` From: Marc-André Lureau One of my most frequent mistake is to commit undesired submodules changes when doing "commit -a", and I have seen a number of people doing the same mistake in various projects. I wish there would be a config to change this default behaviour. submodule..ignore or diff.ignoreSubmodules have different tradeoffs, as they change the diff or status behaviour. I just wish the default behaviour of "commit -a" to be different, to exclude submodules by default. Signed-off-by: Marc-André Lureau --- builtin/add.c | 6 ++++++ builtin/commit.c | 10 +++++++++- cache.h | 1 + 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/builtin/add.c b/builtin/add.c index 4c38aff419..4023ee2681 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -82,6 +82,12 @@ static void update_callback(struct diff_queue_struct *q, for (i = 0; i < q->nr; i++) { struct diff_filepair *p = q->queue[i]; const char *path = p->one->path; + + if (data->flags & ADD_CACHE_IGNORE_SUBMODULES && + S_ISGITLINK(p->one->mode)) { + continue; + } + switch (fix_unmerged_status(p, data)) { default: die(_("unexpected diff status %c"), p->status); diff --git a/builtin/commit.c b/builtin/commit.c index aa1332308a..ce37e4e6da 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -110,6 +110,7 @@ static int config_commit_verbose = -1; /* unspecified */ static int no_post_rewrite, allow_empty_message, pathspec_file_nul; static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg; static char *sign_commit, *pathspec_from_file; +static int commit_all_ignore_submodules; /* * The default commit message cleanup mode will remove the lines @@ -415,8 +416,10 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix * (B) on failure, rollback the real index. */ if (all || (also && pathspec.nr)) { + int flags = commit_all_ignore_submodules ? ADD_CACHE_IGNORE_SUBMODULES : 0; + hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR); - add_files_to_cache(also ? prefix : NULL, &pathspec, 0); + add_files_to_cache(also ? prefix : NULL, &pathspec, flags); refresh_cache_or_die(refresh_flags); update_main_cache_tree(WRITE_TREE_SILENT); if (write_locked_index(&the_index, &index_lock, 0)) @@ -1475,6 +1478,11 @@ static int git_commit_config(const char *k, const char *v, void *cb) return 0; } + if (!strcmp(k, "commit.all-ignore-submodules")) { + commit_all_ignore_submodules = git_config_bool(k, v); + return 0; + } + status = git_gpg_config(k, v, NULL); if (status) return status; diff --git a/cache.h b/cache.h index 1554488d66..5fb3b18916 100644 --- a/cache.h +++ b/cache.h @@ -816,6 +816,7 @@ int remove_file_from_index(struct index_state *, const char *path); #define ADD_CACHE_IGNORE_ERRORS 4 #define ADD_CACHE_IGNORE_REMOVAL 8 #define ADD_CACHE_INTENT 16 +#define ADD_CACHE_IGNORE_SUBMODULES 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 base-commit: 8679ef24ed64018bb62170c43ce73e0261c0600a -- 2.25.0.rc1.1.gb0343b22ed ``` ## Jonathan Nieder, 2020-01-04 00:45 Subject: Re: [PATCH] RFC: commit: add a commit.all-ignore-submodules config option Message-ID: <20200104004516.GB130883@google.com> URL: https://gitlist.dev/e/20200104004516.GB130883%40google.com In-Reply-To: <20200103120613.1063828-1-marcandre.lureau@redhat.com> ``` Hi, Marc-André Lureau wrote: > One of my most frequent mistake is to commit undesired submodules > changes when doing "commit -a", and I have seen a number of people doing > the same mistake in various projects. I wish there would be a config to > change this default behaviour. Can you say more about the overall workflow this is part of? What causes the submodules to change state in the first place here? [...] > --- a/builtin/commit.c > +++ b/builtin/commit.c [...] > @@ -1475,6 +1478,11 @@ static int git_commit_config(const char *k, const char *v, void *cb) > return 0; > } > > + if (!strcmp(k, "commit.all-ignore-submodules")) { > + commit_all_ignore_submodules = git_config_bool(k, v); > + return 0; > + } nit, less important than the comment above: no other config items use this naming scheme. We'd have to come up with a different name if we want to pursue this. If I want to disable this setting for a particular "git commit" invocation, how do I do that? Typically when adding new settings, we add them first as command-line options and then as a separate followup can introduce configuration to change the defaults. To summarize: I'm interested in hearing more about the overall workflow so we can make the standard behavior without any special configuration work better for it, too. Thanks and hope that helps, Jonathan ``` ## Marc-André Lureau, 2020-01-04 17:24 Subject: Re: [PATCH] RFC: commit: add a commit.all-ignore-submodules config option Message-ID: URL: https://gitlist.dev/e/CAMxuvayT8FtovVnWU4bjQCP26drN37yuPG2%2BG2jAUsm0Ns_AYA%40mail.gmail.com In-Reply-To: <20200104004516.GB130883@google.com> ``` Hi On Sat, Jan 4, 2020 at 4:45 AM Jonathan Nieder wrote: > > Hi, > > Marc-André Lureau wrote: > > > One of my most frequent mistake is to commit undesired submodules > > changes when doing "commit -a", and I have seen a number of people doing > > the same mistake in various projects. I wish there would be a config to > > change this default behaviour. > > Can you say more about the overall workflow this is part of? What > causes the submodules to change state in the first place here? The most common case is, I guess, when you work on different branches that have different (compatible) versions of the submodules. It is easy to go unnoticed then, although I am usually quite careful what I include in my commit, and will usually add changes interactively with add -i instead. I often rely on git commit -a during an interactive rebase. I check the project at various points in history, find a small fix, and git commit -a. At this point, I may have included a submodule change inadvertently that may happen later in the series for example. > > [...] > > --- a/builtin/commit.c > > +++ b/builtin/commit.c > [...] > > @@ -1475,6 +1478,11 @@ static int git_commit_config(const char *k, const char *v, void *cb) > > return 0; > > } > > > > + if (!strcmp(k, "commit.all-ignore-submodules")) { > > + commit_all_ignore_submodules = git_config_bool(k, v); > > + return 0; > > + } > > nit, less important than the comment above: no other config items use > this naming scheme. We'd have to come up with a different name if we > want to pursue this. Sure, I am open to suggestions. > > If I want to disable this setting for a particular "git commit" > invocation, how do I do that? Typically when adding new settings, we > add them first as command-line options and then as a separate followup > can introduce configuration to change the defaults. --all=no-ignore ? > > To summarize: I'm interested in hearing more about the overall > workflow so we can make the standard behavior without any special > configuration work better for it, too. > > Thanks and hope that helps, > Jonathan > thanks for the feeback ``` ## Jonathan Nieder, 2020-01-07 00:05 Subject: Re: [PATCH] RFC: commit: add a commit.all-ignore-submodules config option Message-ID: <20200107000551.GE92456@google.com> URL: https://gitlist.dev/e/20200107000551.GE92456%40google.com In-Reply-To: ``` Marc-André Lureau wrote: > On Sat, Jan 4, 2020 at 4:45 AM Jonathan Nieder wrote: >> Marc-André Lureau wrote: >>> One of my most frequent mistake is to commit undesired submodules >>> changes when doing "commit -a", and I have seen a number of people doing >>> the same mistake in various projects. I wish there would be a config to >>> change this default behaviour. >> >> Can you say more about the overall workflow this is part of? What >> causes the submodules to change state in the first place here? > > The most common case is, I guess, when you work on different branches > that have different (compatible) versions of the submodules. Ah! This is because "git checkout" defaults to --no-recurse-submodules, which is a terrible default. Does "git config submodule.recurse true" help? If so, we can look into which it would take to flip that default. Thanks, Jonathan ``` ## Marc-André Lureau, 2020-01-07 05:15 Subject: Re: [PATCH] RFC: commit: add a commit.all-ignore-submodules config option Message-ID: URL: https://gitlist.dev/e/CAMxuvaxpoH_rLLyPENLtnBqcUq_RrwgP2GM6%3DP8QMA309wqFNg%40mail.gmail.com In-Reply-To: <20200107000551.GE92456@google.com> ``` Hi On Tue, Jan 7, 2020 at 4:05 AM Jonathan Nieder wrote: > > Marc-André Lureau wrote: > > On Sat, Jan 4, 2020 at 4:45 AM Jonathan Nieder wrote: > >> Marc-André Lureau wrote: > > >>> One of my most frequent mistake is to commit undesired submodules > >>> changes when doing "commit -a", and I have seen a number of people doing > >>> the same mistake in various projects. I wish there would be a config to > >>> change this default behaviour. > >> > >> Can you say more about the overall workflow this is part of? What > >> causes the submodules to change state in the first place here? > > > > The most common case is, I guess, when you work on different branches > > that have different (compatible) versions of the submodules. > > Ah! This is because "git checkout" defaults to --no-recurse-submodules, > which is a terrible default. Thanks for the hint, I'll give it a try for a while and let you know. > Does "git config submodule.recurse true" help? If so, we can look > into which it would take to flip that default. > > Thanks, > Jonathan > ```