{"thread":{"id":"52556","subject":"[PATCH] RFC: commit: add a commit.all-ignore-submodules config option","startedAt":"2020-01-03T12:06:27Z","lastAt":"2020-01-07T05:15:24Z","messageCount":5,"participants":["marcandre.lureau@redhat.com","Jonathan Nieder","Marc-André Lureau"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"389191","messageId":"20200103120613.1063828-1-marcandre.lureau@redhat.com","threadId":"52556","inReplyTo":null,"subject":"[PATCH] RFC: commit: add a commit.all-ignore-submodules config option","fromName":"","fromEmail":"marcandre.lureau@redhat.com","sentAt":"2020-01-03T12:06:13Z","receivedAt":"2020-01-03T12:06:27Z","isPatch":true,"sender":{"key":"marcandre.lureau@redhat.com","avatar":null},"body":"From: Marc-André Lureau \n\nOne of my most frequent mistake is to commit undesired submodules\nchanges when doing \"commit -a\", and I have seen a number of people doing\nthe same mistake in various projects. I wish there would be a config to\nchange this default behaviour.\n\nsubmodule..ignore or diff.ignoreSubmodules have different\ntradeoffs, as they change the diff or status behaviour. I just wish the\ndefault behaviour of \"commit -a\" to be different, to exclude submodules\nby default.\n\nSigned-off-by: Marc-André Lureau \n---\n builtin/add.c | 6 ++++++\n builtin/commit.c | 10 +++++++++-\n cache.h | 1 +\n 3 files changed, 16 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 4c38aff419..4023ee2681 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -82,6 +82,12 @@ static void update_callback(struct diff_queue_struct *q,\n \tfor (i = 0; i < q->nr; i++) {\n \t\tstruct diff_filepair *p = q->queue[i];\n \t\tconst char *path = p->one->path;\n+\n+\t\tif (data->flags & ADD_CACHE_IGNORE_SUBMODULES &&\n+\t\t S_ISGITLINK(p->one->mode)) {\n+\t\t continue;\n+\t\t}\n+\n \t\tswitch (fix_unmerged_status(p, data)) {\n \t\tdefault:\n \t\t\tdie(_(\"unexpected diff status %c\"), p->status);\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex aa1332308a..ce37e4e6da 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -110,6 +110,7 @@ static int config_commit_verbose = -1; /* unspecified */\n static int no_post_rewrite, allow_empty_message, pathspec_file_nul;\n static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;\n static char *sign_commit, *pathspec_from_file;\n+static int commit_all_ignore_submodules;\n \n /*\n * The default commit message cleanup mode will remove the lines\n@@ -415,8 +416,10 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix\n \t * (B) on failure, rollback the real index.\n \t */\n \tif (all || (also && pathspec.nr)) {\n+\t\tint flags = commit_all_ignore_submodules ? ADD_CACHE_IGNORE_SUBMODULES : 0;\n+\n \t\thold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);\n-\t\tadd_files_to_cache(also ? prefix : NULL, &pathspec, 0);\n+\t\tadd_files_to_cache(also ? prefix : NULL, &pathspec, flags);\n \t\trefresh_cache_or_die(refresh_flags);\n \t\tupdate_main_cache_tree(WRITE_TREE_SILENT);\n \t\tif (write_locked_index(&the_index, &index_lock, 0))\n@@ -1475,6 +1478,11 @@ static int git_commit_config(const char *k, const char *v, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(k, \"commit.all-ignore-submodules\")) {\n+\t\tcommit_all_ignore_submodules = git_config_bool(k, v);\n+\t\treturn 0;\n+\t}\n+\n \tstatus = git_gpg_config(k, v, NULL);\n \tif (status)\n \t\treturn status;\ndiff --git a/cache.h b/cache.h\nindex 1554488d66..5fb3b18916 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -816,6 +816,7 @@ int remove_file_from_index(struct index_state *, const char *path);\n #define ADD_CACHE_IGNORE_ERRORS\t4\n #define ADD_CACHE_IGNORE_REMOVAL 8\n #define ADD_CACHE_INTENT 16\n+#define ADD_CACHE_IGNORE_SUBMODULES 32\n /*\n * These two are used to add the contents of the file at path\n * to the index, marking the working tree up-to-date by storing\n\nbase-commit: 8679ef24ed64018bb62170c43ce73e0261c0600a\n-- \n2.25.0.rc1.1.gb0343b22ed\n\n"},{"id":"389204","messageId":"20200104004516.GB130883@google.com","threadId":"52556","inReplyTo":"20200103120613.1063828-1-marcandre.lureau@redhat.com","subject":"Re: [PATCH] RFC: commit: add a commit.all-ignore-submodules config option","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2020-01-04T00:45:16Z","receivedAt":"2020-01-04T00:45:20Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nMarc-André Lureau wrote:\n\n> One of my most frequent mistake is to commit undesired submodules\n> changes when doing \"commit -a\", and I have seen a number of people doing\n> the same mistake in various projects. I wish there would be a config to\n> change this default behaviour.\n\nCan you say more about the overall workflow this is part of? What\ncauses the submodules to change state in the first place here?\n\n[...]\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n[...]\n> @@ -1475,6 +1478,11 @@ static int git_commit_config(const char *k, const char *v, void *cb)\n> \t\treturn 0;\n> \t}\n> \n> +\tif (!strcmp(k, \"commit.all-ignore-submodules\")) {\n> +\t\tcommit_all_ignore_submodules = git_config_bool(k, v);\n> +\t\treturn 0;\n> +\t}\n\nnit, less important than the comment above: no other config items use\nthis naming scheme. We'd have to come up with a different name if we\nwant to pursue this.\n\nIf I want to disable this setting for a particular \"git commit\"\ninvocation, how do I do that? Typically when adding new settings, we\nadd them first as command-line options and then as a separate followup\ncan introduce configuration to change the defaults.\n\nTo summarize: I'm interested in hearing more about the overall\nworkflow so we can make the standard behavior without any special\nconfiguration work better for it, too.\n\nThanks and hope that helps,\nJonathan\n"},{"id":"389218","messageId":"CAMxuvayT8FtovVnWU4bjQCP26drN37yuPG2+G2jAUsm0Ns_AYA@mail.gmail.com","threadId":"52556","inReplyTo":"20200104004516.GB130883@google.com","subject":"Re: [PATCH] RFC: commit: add a commit.all-ignore-submodules config option","fromName":"Marc-André Lureau","fromEmail":"marcandre.lureau@redhat.com","sentAt":"2020-01-04T17:24:48Z","receivedAt":"2020-01-04T17:25:16Z","isPatch":true,"sender":{"key":"marcandre.lureau@redhat.com","avatar":null},"body":"Hi\n\nOn Sat, Jan 4, 2020 at 4:45 AM Jonathan Nieder wrote:\n>\n> Hi,\n>\n> Marc-André Lureau wrote:\n>\n> > One of my most frequent mistake is to commit undesired submodules\n> > changes when doing \"commit -a\", and I have seen a number of people doing\n> > the same mistake in various projects. I wish there would be a config to\n> > change this default behaviour.\n>\n> Can you say more about the overall workflow this is part of? What\n> causes the submodules to change state in the first place here?\n\nThe most common case is, I guess, when you work on different branches\nthat have different (compatible) versions of the submodules. It is\neasy to go unnoticed then, although I am usually quite careful what I\ninclude in my commit, and will usually add changes interactively with\nadd -i instead.\n\nI often rely on git commit -a during an interactive rebase. I check\nthe project at various points in history, find a small fix, and git\ncommit -a. At this point, I may have included a submodule change\ninadvertently that may happen later in the series for example.\n\n>\n> [...]\n> > --- a/builtin/commit.c\n> > +++ b/builtin/commit.c\n> [...]\n> > @@ -1475,6 +1478,11 @@ static int git_commit_config(const char *k, const char *v, void *cb)\n> > return 0;\n> > }\n> >\n> > + if (!strcmp(k, \"commit.all-ignore-submodules\")) {\n> > + commit_all_ignore_submodules = git_config_bool(k, v);\n> > + return 0;\n> > + }\n>\n> nit, less important than the comment above: no other config items use\n> this naming scheme. We'd have to come up with a different name if we\n> want to pursue this.\n\nSure, I am open to suggestions.\n\n>\n> If I want to disable this setting for a particular \"git commit\"\n> invocation, how do I do that? Typically when adding new settings, we\n> add them first as command-line options and then as a separate followup\n> can introduce configuration to change the defaults.\n\n--all=no-ignore ?\n\n>\n> To summarize: I'm interested in hearing more about the overall\n> workflow so we can make the standard behavior without any special\n> configuration work better for it, too.\n>\n> Thanks and hope that helps,\n> Jonathan\n>\n\nthanks for the feeback\n\n"},{"id":"389305","messageId":"20200107000551.GE92456@google.com","threadId":"52556","inReplyTo":"CAMxuvayT8FtovVnWU4bjQCP26drN37yuPG2+G2jAUsm0Ns_AYA@mail.gmail.com","subject":"Re: [PATCH] RFC: commit: add a commit.all-ignore-submodules config option","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2020-01-07T00:05:51Z","receivedAt":"2020-01-07T00:05:56Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Marc-André Lureau wrote:\n> On Sat, Jan 4, 2020 at 4:45 AM Jonathan Nieder wrote:\n>> Marc-André Lureau wrote:\n\n>>> One of my most frequent mistake is to commit undesired submodules\n>>> changes when doing \"commit -a\", and I have seen a number of people doing\n>>> the same mistake in various projects. I wish there would be a config to\n>>> change this default behaviour.\n>>\n>> Can you say more about the overall workflow this is part of? What\n>> causes the submodules to change state in the first place here?\n>\n> The most common case is, I guess, when you work on different branches\n> that have different (compatible) versions of the submodules.\n\nAh! This is because \"git checkout\" defaults to --no-recurse-submodules,\nwhich is a terrible default.\n\nDoes \"git config submodule.recurse true\" help? If so, we can look\ninto which it would take to flip that default.\n\nThanks,\nJonathan\n"},{"id":"389339","messageId":"CAMxuvaxpoH_rLLyPENLtnBqcUq_RrwgP2GM6=P8QMA309wqFNg@mail.gmail.com","threadId":"52556","inReplyTo":"20200107000551.GE92456@google.com","subject":"Re: [PATCH] RFC: commit: add a commit.all-ignore-submodules config option","fromName":"Marc-André Lureau","fromEmail":"marcandre.lureau@redhat.com","sentAt":"2020-01-07T05:15:03Z","receivedAt":"2020-01-07T05:15:24Z","isPatch":true,"sender":{"key":"marcandre.lureau@redhat.com","avatar":null},"body":"Hi\n\nOn Tue, Jan 7, 2020 at 4:05 AM Jonathan Nieder wrote:\n>\n> Marc-André Lureau wrote:\n> > On Sat, Jan 4, 2020 at 4:45 AM Jonathan Nieder wrote:\n> >> Marc-André Lureau wrote:\n>\n> >>> One of my most frequent mistake is to commit undesired submodules\n> >>> changes when doing \"commit -a\", and I have seen a number of people doing\n> >>> the same mistake in various projects. I wish there would be a config to\n> >>> change this default behaviour.\n> >>\n> >> Can you say more about the overall workflow this is part of? What\n> >> causes the submodules to change state in the first place here?\n> >\n> > The most common case is, I guess, when you work on different branches\n> > that have different (compatible) versions of the submodules.\n>\n> Ah! This is because \"git checkout\" defaults to --no-recurse-submodules,\n> which is a terrible default.\n\nThanks for the hint, I'll give it a try for a while and let you know.\n\n> Does \"git config submodule.recurse true\" help? If so, we can look\n> into which it would take to flip that default.\n>\n> Thanks,\n> Jonathan\n>\n\n"}]}