{"thread":{"id":"55458","subject":"[RFC PATCH 0/2] share a config between submodule and superproject","startedAt":"2021-04-08T23:39:52Z","lastAt":"2021-04-23T12:45:14Z","messageCount":22,"participants":["Emily Shaffer","Philip Oakley","Matheus Tavares Bernardino","Junio C Hamano","Ævar Arnfjörð Bjarmason","Phillip Wood","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"421300","messageId":"20210408233936.533342-1-emilyshaffer@google.com","threadId":"55458","inReplyTo":null,"subject":"[RFC PATCH 0/2] share a config between submodule and superproject","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-04-08T23:39:34Z","receivedAt":"2021-04-08T23:39:52Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Hi folks,\n\nEspecially after es/config-based-hooks makes its way to 'next' and\n'master', we think it would be really useful to have a config that\napplies to a whole \"project\" - where \"project\" means \"superproject and\nits submodules.\" There's a longer defense in patch 2, but that's the\nspeedy summary.\n\nI'm not wild about the implementation of this solution as it calls out\nto 'git rev-parse' and 'git ls-files' - not once, but twice! - and I\nunderstand that is considerably slower on some platforms than others.\nBut I'm open to suggestions - I couldn't find any other examples of a\nrepo figuring out whether it's a submodule or not. (I thought there may\nbe some, as 'repository.submodule_prefix' exists, but it seems like\nthat's only set in some special cases during operations initated from\nthe superproject.)\n\nI'm hoping to work on some other submodule-centric stuff over the coming\nmonths, and it might end up being very useful to be able to tell \"am I a\nsubmodule?\" and \"how do I talk to my superproject?\" more generally - so\nI'm really open to figuring out a better way than this, if folks have\nideas.\n\nPatch 1 is a small refactor that we can take or leave - I found\n\"SCOPE_SUBMODULE\" to be pretty ambiguous, especially since it seems to\nrefer to configs from .gitmodules. Even though I decided that\n\"superproject\" was a better name than \"submodule\" I still wasn't super\nhappy with the ambiguity. But we can drop it if folks don't want to\nrename.\n\nThanks a bunch,\n - Emily\n\nEmily Shaffer (2):\n config: rename \"submodule\" scope to \"gitmodules\"\n config: add 'config.superproject' file\n\n Documentation/git-config.txt | 21 +++++-\n builtin/config.c | 10 ++-\n config.c | 30 +++++++-\n config.h | 5 +-\n submodule-config.c | 2 +-\n submodule.c | 29 ++++++++\n submodule.h | 6 ++\n t/t1311-superproject-config.sh | 124 +++++++++++++++++++++++++++++++++\n 8 files changed, 220 insertions(+), 7 deletions(-)\n create mode 100755 t/t1311-superproject-config.sh\n\n-- \n2.31.1.295.g9ea45b61b8-goog\n\n"},{"id":"421301","messageId":"20210408233936.533342-2-emilyshaffer@google.com","threadId":"55458","inReplyTo":"20210408233936.533342-1-emilyshaffer@google.com","subject":"[RFC PATCH 1/2] config: rename \"submodule\" scope to \"gitmodules\"","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-04-08T23:39:35Z","receivedAt":"2021-04-08T23:39:57Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"To prepare for the addition of a new config scope which only applies to\nsubmodule projects, disambiguate \"CONFIG_SCOPE_SUBMODULES\". This scope\nrefers to configs gathered from the .gitmodules file in the\nsuperproject, so just call it \"gitmodules.\"\n\nSigned-off-by: Emily Shaffer \n---\n config.c | 4 ++--\n config.h | 2 +-\n submodule-config.c | 2 +-\n 3 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex 6428393a41..67d9bf2238 100644\n--- a/config.c\n+++ b/config.c\n@@ -3513,8 +3513,8 @@ const char *config_scope_name(enum config_scope scope)\n \t\treturn \"worktree\";\n \tcase CONFIG_SCOPE_COMMAND:\n \t\treturn \"command\";\n-\tcase CONFIG_SCOPE_SUBMODULE:\n-\t\treturn \"submodule\";\n+\tcase CONFIG_SCOPE_GITMODULES:\n+\t\treturn \"gitmodules\";\n \tdefault:\n \t\treturn \"unknown\";\n \t}\ndiff --git a/config.h b/config.h\nindex 19a9adbaa9..535f5517b8 100644\n--- a/config.h\n+++ b/config.h\n@@ -42,7 +42,7 @@ enum config_scope {\n \tCONFIG_SCOPE_LOCAL,\n \tCONFIG_SCOPE_WORKTREE,\n \tCONFIG_SCOPE_COMMAND,\n-\tCONFIG_SCOPE_SUBMODULE,\n+\tCONFIG_SCOPE_GITMODULES,\n };\n const char *config_scope_name(enum config_scope scope);\n \ndiff --git a/submodule-config.c b/submodule-config.c\nindex f502505566..0e435e6fdd 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -637,7 +637,7 @@ static void config_from_gitmodules(config_fn_t fn, struct repository *repo, void\n {\n \tif (repo->worktree) {\n \t\tstruct git_config_source config_source = {\n-\t\t\t0, .scope = CONFIG_SCOPE_SUBMODULE\n+\t\t\t0, .scope = CONFIG_SCOPE_GITMODULES\n \t\t};\n \t\tconst struct config_options opts = { 0 };\n \t\tstruct object_id oid;\n-- \n2.31.1.295.g9ea45b61b8-goog\n\n"},{"id":"421302","messageId":"20210408233936.533342-3-emilyshaffer@google.com","threadId":"55458","inReplyTo":"20210408233936.533342-1-emilyshaffer@google.com","subject":"[RFC PATCH 2/2] config: add 'config.superproject' file","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-04-08T23:39:36Z","receivedAt":"2021-04-08T23:39:57Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Some configs, such as wrapper directives like gerrit.createChangeId, or\nforthcoming hook configs, should apply to a superproject as well as all\nits submodules. It may not be appropriate to apply them globally - for\nexample, if the user also contributes to many projects which do not use\nthe configs necessary for one project-with-submodules - and it may be\nburdensome to apply them locally to the superproject and each of its\nsubmodules. Even if the user runs 'git submodule foreach \"git config\n--local foo.bar', if a new submodule is added later on, that config is\nnot applied to the new submodule.\n\nIt is also inappropriate to share the entire superproject config, since\nsome items - like remote URLs or partial-clone filters - would not apply\nto a submodule.\n\nTo make life easier for projects with many submodules, then, create a\nnew \"config.superproject\" config scope, which is included in the config\nparse for the superproject as well as for all the submodules of that\nsuperproject.\n\nFor the superproject, this new config file is equally local to the local\nconfig; for the submodule, the new config file is less local than the\nlocal config. So let's include it directly before the local config\nduring the config parse.\n\nSigned-off-by: Emily Shaffer \n---\n Documentation/git-config.txt | 21 +++++-\n builtin/config.c | 10 ++-\n config.c | 26 +++++++\n config.h | 3 +\n submodule.c | 29 ++++++++\n submodule.h | 6 ++\n t/t1311-superproject-config.sh | 124 +++++++++++++++++++++++++++++++++\n 7 files changed, 216 insertions(+), 3 deletions(-)\n create mode 100755 t/t1311-superproject-config.sh\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex 4b4cc5c5e8..a33136fb08 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -48,7 +48,7 @@ unset an existing `--type` specifier with `--no-type`.\n \n When reading, the values are read from the system, global and\n repository local configuration files by default, and options\n-`--system`, `--global`, `--local`, `--worktree` and\n+`--system`, `--global`, `--superproject`, `--local`, `--worktree` and\n `--file ` can be used to tell the command to read from only\n that location (see <>).\n \n@@ -127,6 +127,17 @@ rather than from all available files.\n +\n See also <>.\n \n+--superproject::\n+\tFor writing options: write to the superproject's\n+\t`.git/config.superproject` file, even if run from a submodule of that\n+\tsuperproject.\n++\n+For reading options: read only from the superproject's\n+`.git/config.superproject` file, even if run from a submodule of that\n+superproject, rather than from all available files.\n++\n+See also <>.\n+\n --local::\n \tFor writing options: write to the repository `.git/config` file.\n \tThis is the default behavior.\n@@ -283,7 +294,7 @@ The default is to use a pager.\n FILES\n -----\n \n-If not set explicitly with `--file`, there are four files where\n+If not set explicitly with `--file`, there are five files where\n 'git config' will search for configuration options:\n \n $(prefix)/etc/gitconfig::\n@@ -301,6 +312,12 @@ $XDG_CONFIG_HOME/git/config::\n \tUser-specific configuration file. Also called \"global\"\n \tconfiguration file.\n \n+$GIT_DIR/config.superproject::\n+\tWhen `git config` is run from a project which is a submodule of another\n+\tproject, that superproject's $GIT_DIR will be used. Use this config file\n+\tto set configurations which need to be the same across a superproject\n+\tand all its submodules.\n+\n $GIT_DIR/config::\n \tRepository specific configuration file.\n \ndiff --git a/builtin/config.c b/builtin/config.c\nindex f71fa39b38..f0a57a89ca 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -26,7 +26,7 @@ static char key_delim = ' ';\n static char term = '\\n';\n \n static int use_global_config, use_system_config, use_local_config;\n-static int use_worktree_config;\n+static int use_worktree_config, use_superproject_config;\n static struct git_config_source given_config_source;\n static int actions, type;\n static char *default_value;\n@@ -130,6 +130,8 @@ static struct option builtin_config_options[] = {\n \tOPT_GROUP(N_(\"Config file location\")),\n \tOPT_BOOL(0, \"global\", &use_global_config, N_(\"use global config file\")),\n \tOPT_BOOL(0, \"system\", &use_system_config, N_(\"use system config file\")),\n+\tOPT_BOOL(0, \"superproject\",\n+\t\t &use_superproject_config, N_(\"use superproject config file\")),\n \tOPT_BOOL(0, \"local\", &use_local_config, N_(\"use repository config file\")),\n \tOPT_BOOL(0, \"worktree\", &use_worktree_config, N_(\"use per-worktree config file\")),\n \tOPT_STRING('f', \"file\", &given_config_source.file, N_(\"file\"), N_(\"use given config file\")),\n@@ -697,6 +699,12 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \telse if (use_system_config) {\n \t\tgiven_config_source.file = git_etc_gitconfig();\n \t\tgiven_config_source.scope = CONFIG_SCOPE_SYSTEM;\n+\t} else if (use_superproject_config) {\n+\t\tstruct strbuf superproject_cfg = STRBUF_INIT;\n+\t\tgit_config_superproject(&superproject_cfg, get_git_dir());\n+\t\tgiven_config_source.file = xstrdup(superproject_cfg.buf);\n+\t\tgiven_config_source.scope = CONFIG_SCOPE_SUPERPROJECT;\n+\t\tstrbuf_release(&superproject_cfg);\n \t} else if (use_local_config) {\n \t\tgiven_config_source.file = git_pathdup(\"config\");\n \t\tgiven_config_source.scope = CONFIG_SCOPE_LOCAL;\ndiff --git a/config.c b/config.c\nindex 67d9bf2238..28bb80fd0d 100644\n--- a/config.c\n+++ b/config.c\n@@ -21,6 +21,7 @@\n #include \"dir.h\"\n #include \"color.h\"\n #include \"refs.h\"\n+#include \"submodule.h\"\n \n struct config_source {\n \tstruct config_source *prev;\n@@ -1852,6 +1853,17 @@ const char *git_etc_gitconfig(void)\n \treturn system_wide;\n }\n \n+void git_config_superproject(struct strbuf *sb, const char *gitdir)\n+{\n+\tif (!get_superproject_gitdir(sb)) {\n+\t\t/* not a submodule */\n+\t\tstrbuf_reset(sb);\n+\t\tstrbuf_addstr(sb, gitdir);\n+\t}\n+\n+\tstrbuf_addstr(sb, \"/config.superproject\");\n+}\n+\n /*\n * Parse environment variable 'k' as a boolean (in various\n * possible spellings); if missing, use the default value 'def'.\n@@ -1909,6 +1921,17 @@ static int do_git_config_sequence(const struct config_options *opts,\n \tif (user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK))\n \t\tret += git_config_from_file(fn, user_config, data);\n \n+\tcurrent_parsing_scope = CONFIG_SCOPE_SUPERPROJECT;\n+\tif (opts->git_dir && !opts->ignore_superproject) {\n+\n+\t\tstruct strbuf superproject_gitdir = STRBUF_INIT;\n+\t\tgit_config_superproject(&superproject_gitdir, opts->git_dir);\n+\t\tif (!access_or_die(superproject_gitdir.buf, R_OK, 0))\n+\t\t\tret += git_config_from_file(fn, superproject_gitdir.buf, data);\n+\n+\t\tstrbuf_release(&superproject_gitdir);\n+\t}\n+\n \tcurrent_parsing_scope = CONFIG_SCOPE_LOCAL;\n \tif (!opts->ignore_repo && repo_config &&\n \t !access_or_die(repo_config, R_OK, 0))\n@@ -2027,6 +2050,7 @@ void read_very_early_config(config_fn_t cb, void *data)\n \n \topts.respect_includes = 1;\n \topts.ignore_repo = 1;\n+\topts.ignore_superproject = 1;\n \topts.ignore_worktree = 1;\n \topts.ignore_cmdline = 1;\n \topts.system_gently = 1;\n@@ -3515,6 +3539,8 @@ const char *config_scope_name(enum config_scope scope)\n \t\treturn \"command\";\n \tcase CONFIG_SCOPE_GITMODULES:\n \t\treturn \"gitmodules\";\n+\tcase CONFIG_SCOPE_SUPERPROJECT:\n+\t\treturn \"superproject\";\n \tdefault:\n \t\treturn \"unknown\";\n \t}\ndiff --git a/config.h b/config.h\nindex 535f5517b8..b42e1d13eb 100644\n--- a/config.h\n+++ b/config.h\n@@ -43,6 +43,7 @@ enum config_scope {\n \tCONFIG_SCOPE_WORKTREE,\n \tCONFIG_SCOPE_COMMAND,\n \tCONFIG_SCOPE_GITMODULES,\n+\tCONFIG_SCOPE_SUPERPROJECT,\n };\n const char *config_scope_name(enum config_scope scope);\n \n@@ -84,6 +85,7 @@ typedef int (*config_parser_event_fn_t)(enum config_event_t type,\n struct config_options {\n \tunsigned int respect_includes : 1;\n \tunsigned int ignore_repo : 1;\n+\tunsigned int ignore_superproject : 1;\n \tunsigned int ignore_worktree : 1;\n \tunsigned int ignore_cmdline : 1;\n \tunsigned int system_gently : 1;\n@@ -319,6 +321,7 @@ int git_config_rename_section_in_file(const char *, const char *, const char *);\n int git_config_copy_section(const char *, const char *);\n int git_config_copy_section_in_file(const char *, const char *, const char *);\n const char *git_etc_gitconfig(void);\n+void git_config_superproject(struct strbuf *, const char *);\n int git_env_bool(const char *, int);\n unsigned long git_env_ulong(const char *, unsigned long);\n int git_config_system(void);\ndiff --git a/submodule.c b/submodule.c\nindex 9767ba9893..92b00f8697 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -2178,6 +2178,35 @@ void absorb_git_dir_into_superproject(const char *path,\n \t}\n }\n \n+int get_superproject_gitdir(struct strbuf *buf)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tint rc = 0;\n+\n+\t/* NEEDSWORK: this call also calls out to a subprocess! */\n+\trc = get_superproject_working_tree(&sb);\n+\tstrbuf_release(&sb);\n+\n+\tif (!rc)\n+\t\treturn rc;\n+\n+\tprepare_submodule_repo_env_no_git_dir(&cp.env_array);\n+\n+\tstrvec_pushl(&cp.args, \"-C\", \"..\", \"rev-parse\", \"--absolute-git-dir\", NULL);\n+\tcp.git_cmd = 1;\n+\n+\trc = capture_command(&cp, buf, 0);\n+\tstrbuf_trim_trailing_newline(buf);\n+\n+\t/* leave buf empty if we didn't have a superproject gitdir */\n+\tif (rc)\n+\t\tstrbuf_reset(buf);\n+\n+\t/* rc reflects the exit code of the rev-parse; invert into a bool */\n+\treturn !rc;\n+}\n+\n int get_superproject_working_tree(struct strbuf *buf)\n {\n \tstruct child_process cp = CHILD_PROCESS_INIT;\ndiff --git a/submodule.h b/submodule.h\nindex 4ac6e31cf1..1308d5ae2d 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -149,6 +149,12 @@ void prepare_submodule_repo_env(struct strvec *out);\n void absorb_git_dir_into_superproject(const char *path,\n \t\t\t\t unsigned flags);\n \n+/*\n+ * Return the gitdir of the superproject, which this project is a submodule of.\n+ * If this repository is not a submodule of another repository, return 0.\n+ */\n+int get_superproject_gitdir(struct strbuf *buf);\n+\n /*\n * Return the absolute path of the working tree of the superproject, which this\n * project is a submodule of. If this repository is not a submodule of\ndiff --git a/t/t1311-superproject-config.sh b/t/t1311-superproject-config.sh\nnew file mode 100755\nindex 0000000000..650c4d24c7\n--- /dev/null\n+++ b/t/t1311-superproject-config.sh\n@@ -0,0 +1,124 @@\n+#!/bin/sh\n+\n+test_description='Test git config --superproject in different settings'\n+\n+. ./test-lib.sh\n+\n+# follow t7400's example and use the trash dir repo as a submodule to add\n+submodurl=$(pwd -P)\n+\n+# since only the configs are modified, set up the repo structure only once\n+test_expect_success 'setup repo structure' '\n+\ttest_commit \"base\" &&\n+\tgit submodule add \"${submodurl}\" sub/ &&\n+\tgit commit -m \"add a submodule\"\n+'\n+\n+test_expect_success 'superproject config applies to super and submodule' '\n+\tcat >.git/config.superproject <<-EOF &&\n+\t[foo]\n+\t\tbar = baz\n+\tEOF\n+\n+\tgit config --get foo.bar &&\n+\tgit -C sub config --get foo.bar &&\n+\n+\trm .git/config.superproject\n+'\n+\n+test_expect_success 'can add from super or sub' '\n+\tgit config --superproject apple.species honeycrisp &&\n+\tgit -C sub config --superproject banana.species cavendish &&\n+\n+\tcat >expect <<-EOF &&\n+\tapple.species=honeycrisp\n+\tbanana.species=cavendish\n+\tEOF\n+\n+\tgit config --list >actual &&\n+\tgrep -Ff expect actual &&\n+\n+\tgit -C sub config --list >actual &&\n+\tgrep -Ff expect actual &&\n+\n+\trm .git/config.superproject\n+'\n+\n+test_expect_success 'can --unset from super or sub' '\n+\tgit config --superproject apple.species honeycrisp &&\n+\tgit -C sub config --superproject banana.species cavendish &&\n+\n+\tgit config --unset --superproject banana.species &&\n+\tgit -C sub config --unset --superproject apple.species\n+'\n+\n+test_expect_success 'can --edit superproject config' '\n+\ttest_config core.editor \"echo [foo]bar=baz >\" &&\n+\tgit config --edit --superproject &&\n+\n+\tgit config --get foo.bar &&\n+\n+\trm .git/config.superproject\n+'\n+\n+test_expect_success 'can --show-origin the superproject config' '\n+\tgit config --superproject --add foo.bar baz &&\n+\n+\tgit config --list --show-origin >actual &&\n+\tgrep -F \"config.superproject\" actual &&\n+\n+\trm .git/config.superproject\n+'\n+\n+test_expect_success 'can --show-scope the superproject config' '\n+\tgit config --superproject --add foo.bar baz &&\n+\n+\tgit config --list --show-scope >actual &&\n+\tgrep \"superproject\" actual &&\n+\n+\trm .git/config.superproject\n+'\n+\n+test_expect_success 'pre-existing config applies to new submodule' '\n+\tgit config --superproject --add foo.bar baz &&\n+\n+\tgit submodule add \"${submodurl}\" sub2/ &&\n+\tgit commit -m \"add a second submodule\" &&\n+\n+\tgit -C sub2 config --get foo.bar &&\n+\n+\t# clean up\n+\tgit reset HEAD^ &&\n+\trm -fr sub2 &&\n+\trm .git/config.superproject\n+'\n+\n+# NEEDSWORK: submodule.c:get_superproject_working_tree doesn't support worktree\n+test_expect_failure 'worktrees can still access config.superproject' '\n+\tgit config --superproject --add foo.bar baz &&\n+\n+\tgit worktree add wt &&\n+\t(\n+\t\tcd wt &&\n+\t\tgit config --get foo.bar\n+\t) &&\n+\n+\t# clean up\n+\tgit worktree remove wt &&\n+\trm .git/config.superproject\n+'\n+\n+# This test deletes the submodule! Keep it at the end of the test suite.\n+test_expect_success 'config.submodule works even with no submodules' '\n+\t# get rid of the submodule\n+\tgit reset HEAD^ &&\n+\trm -fr sub &&\n+\n+\tgit config --superproject --add foo.bar baz &&\n+\n+\tgit config --get foo.bar &&\n+\n+\trm .git/config.superproject\n+'\n+\n+test_done\n-- \n2.31.1.295.g9ea45b61b8-goog\n\n"},{"id":"421371","messageId":"e19a250a-c4ca-fc32-83f9-a03aa03cd88a@iee.email","threadId":"55458","inReplyTo":"20210408233936.533342-3-emilyshaffer@google.com","subject":"Re: [RFC PATCH 2/2] config: add 'config.superproject' file","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2021-04-09T11:10:27Z","receivedAt":"2021-04-09T11:10:32Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 09/04/2021 00:39, Emily Shaffer wrote:\n> Some configs, such as wrapper directives like gerrit.createChangeId, or\n> forthcoming hook configs, should apply to a superproject as well as all\n> its submodules. It may not be appropriate to apply them globally - for\n> example, if the user also contributes to many projects which do not use\n> the configs necessary for one project-with-submodules - and it may be\n> burdensome to apply them locally to the superproject and each of its\n> submodules. Even if the user runs 'git submodule foreach \"git config\n> --local foo.bar', if a new submodule is added later on, that config is\n> not applied to the new submodule.\n>\n> It is also inappropriate to share the entire superproject config, since\n> some items - like remote URLs or partial-clone filters - would not apply\n> to a submodule.\n>\n> To make life easier for projects with many submodules, then, create a\n> new \"config.superproject\" config scope, which is included in the config\n> parse for the superproject as well as for all the submodules of that\n> superproject.\n>\n> For the superproject, this new config file is equally local to the local\n> config; for the submodule, the new config file is less local than the\n> local config. So let's include it directly before the local config\n> during the config parse.\n>\n> Signed-off-by: Emily Shaffer \n> ---\n\nDoes this need an update to the `git config --show-origin --show-scope`\ncapability?\n--\nPhilip\n> Documentation/git-config.txt | 21 +++++-\n> builtin/config.c | 10 ++-\n> config.c | 26 +++++++\n> config.h | 3 +\n> submodule.c | 29 ++++++++\n> submodule.h | 6 ++\n> t/t1311-superproject-config.sh | 124 +++++++++++++++++++++++++++++++++\n> 7 files changed, 216 insertions(+), 3 deletions(-)\n> create mode 100755 t/t1311-superproject-config.sh\n>\n> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\n> index 4b4cc5c5e8..a33136fb08 100644\n> --- a/Documentation/git-config.txt\n> +++ b/Documentation/git-config.txt\n> @@ -48,7 +48,7 @@ unset an existing `--type` specifier with `--no-type`.\n> \n> When reading, the values are read from the system, global and\n> repository local configuration files by default, and options\n> -`--system`, `--global`, `--local`, `--worktree` and\n> +`--system`, `--global`, `--superproject`, `--local`, `--worktree` and\n> `--file ` can be used to tell the command to read from only\n> that location (see <>).\n> \n> @@ -127,6 +127,17 @@ rather than from all available files.\n> +\n> See also <>.\n> \n> +--superproject::\n> +\tFor writing options: write to the superproject's\n> +\t`.git/config.superproject` file, even if run from a submodule of that\n> +\tsuperproject.\n> ++\n> +For reading options: read only from the superproject's\n> +`.git/config.superproject` file, even if run from a submodule of that\n> +superproject, rather than from all available files.\n> ++\n> +See also <>.\n> +\n> --local::\n> \tFor writing options: write to the repository `.git/config` file.\n> \tThis is the default behavior.\n> @@ -283,7 +294,7 @@ The default is to use a pager.\n> FILES\n> -----\n> \n> -If not set explicitly with `--file`, there are four files where\n> +If not set explicitly with `--file`, there are five files where\n> 'git config' will search for configuration options:\n> \n> $(prefix)/etc/gitconfig::\n> @@ -301,6 +312,12 @@ $XDG_CONFIG_HOME/git/config::\n> \tUser-specific configuration file. Also called \"global\"\n> \tconfiguration file.\n> \n> +$GIT_DIR/config.superproject::\n> +\tWhen `git config` is run from a project which is a submodule of another\n> +\tproject, that superproject's $GIT_DIR will be used. Use this config file\n> +\tto set configurations which need to be the same across a superproject\n> +\tand all its submodules.\n> +\n> $GIT_DIR/config::\n> \tRepository specific configuration file.\n> \n> diff --git a/builtin/config.c b/builtin/config.c\n> index f71fa39b38..f0a57a89ca 100644\n> --- a/builtin/config.c\n> +++ b/builtin/config.c\n> @@ -26,7 +26,7 @@ static char key_delim = ' ';\n> static char term = '\\n';\n> \n> static int use_global_config, use_system_config, use_local_config;\n> -static int use_worktree_config;\n> +static int use_worktree_config, use_superproject_config;\n> static struct git_config_source given_config_source;\n> static int actions, type;\n> static char *default_value;\n> @@ -130,6 +130,8 @@ static struct option builtin_config_options[] = {\n> \tOPT_GROUP(N_(\"Config file location\")),\n> \tOPT_BOOL(0, \"global\", &use_global_config, N_(\"use global config file\")),\n> \tOPT_BOOL(0, \"system\", &use_system_config, N_(\"use system config file\")),\n> +\tOPT_BOOL(0, \"superproject\",\n> +\t\t &use_superproject_config, N_(\"use superproject config file\")),\n> \tOPT_BOOL(0, \"local\", &use_local_config, N_(\"use repository config file\")),\n> \tOPT_BOOL(0, \"worktree\", &use_worktree_config, N_(\"use per-worktree config file\")),\n> \tOPT_STRING('f', \"file\", &given_config_source.file, N_(\"file\"), N_(\"use given config file\")),\n> @@ -697,6 +699,12 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n> \telse if (use_system_config) {\n> \t\tgiven_config_source.file = git_etc_gitconfig();\n> \t\tgiven_config_source.scope = CONFIG_SCOPE_SYSTEM;\n> +\t} else if (use_superproject_config) {\n> +\t\tstruct strbuf superproject_cfg = STRBUF_INIT;\n> +\t\tgit_config_superproject(&superproject_cfg, get_git_dir());\n> +\t\tgiven_config_source.file = xstrdup(superproject_cfg.buf);\n> +\t\tgiven_config_source.scope = CONFIG_SCOPE_SUPERPROJECT;\n> +\t\tstrbuf_release(&superproject_cfg);\n> \t} else if (use_local_config) {\n> \t\tgiven_config_source.file = git_pathdup(\"config\");\n> \t\tgiven_config_source.scope = CONFIG_SCOPE_LOCAL;\n> diff --git a/config.c b/config.c\n> index 67d9bf2238..28bb80fd0d 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -21,6 +21,7 @@\n> #include \"dir.h\"\n> #include \"color.h\"\n> #include \"refs.h\"\n> +#include \"submodule.h\"\n> \n> struct config_source {\n> \tstruct config_source *prev;\n> @@ -1852,6 +1853,17 @@ const char *git_etc_gitconfig(void)\n> \treturn system_wide;\n> }\n> \n> +void git_config_superproject(struct strbuf *sb, const char *gitdir)\n> +{\n> +\tif (!get_superproject_gitdir(sb)) {\n> +\t\t/* not a submodule */\n> +\t\tstrbuf_reset(sb);\n> +\t\tstrbuf_addstr(sb, gitdir);\n> +\t}\n> +\n> +\tstrbuf_addstr(sb, \"/config.superproject\");\n> +}\n> +\n> /*\n> * Parse environment variable 'k' as a boolean (in various\n> * possible spellings); if missing, use the default value 'def'.\n> @@ -1909,6 +1921,17 @@ static int do_git_config_sequence(const struct config_options *opts,\n> \tif (user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK))\n> \t\tret += git_config_from_file(fn, user_config, data);\n> \n> +\tcurrent_parsing_scope = CONFIG_SCOPE_SUPERPROJECT;\n> +\tif (opts->git_dir && !opts->ignore_superproject) {\n> +\n> +\t\tstruct strbuf superproject_gitdir = STRBUF_INIT;\n> +\t\tgit_config_superproject(&superproject_gitdir, opts->git_dir);\n> +\t\tif (!access_or_die(superproject_gitdir.buf, R_OK, 0))\n> +\t\t\tret += git_config_from_file(fn, superproject_gitdir.buf, data);\n> +\n> +\t\tstrbuf_release(&superproject_gitdir);\n> +\t}\n> +\n> \tcurrent_parsing_scope = CONFIG_SCOPE_LOCAL;\n> \tif (!opts->ignore_repo && repo_config &&\n> \t !access_or_die(repo_config, R_OK, 0))\n> @@ -2027,6 +2050,7 @@ void read_very_early_config(config_fn_t cb, void *data)\n> \n> \topts.respect_includes = 1;\n> \topts.ignore_repo = 1;\n> +\topts.ignore_superproject = 1;\n> \topts.ignore_worktree = 1;\n> \topts.ignore_cmdline = 1;\n> \topts.system_gently = 1;\n> @@ -3515,6 +3539,8 @@ const char *config_scope_name(enum config_scope scope)\n> \t\treturn \"command\";\n> \tcase CONFIG_SCOPE_GITMODULES:\n> \t\treturn \"gitmodules\";\n> +\tcase CONFIG_SCOPE_SUPERPROJECT:\n> +\t\treturn \"superproject\";\n> \tdefault:\n> \t\treturn \"unknown\";\n> \t}\n> diff --git a/config.h b/config.h\n> index 535f5517b8..b42e1d13eb 100644\n> --- a/config.h\n> +++ b/config.h\n> @@ -43,6 +43,7 @@ enum config_scope {\n> \tCONFIG_SCOPE_WORKTREE,\n> \tCONFIG_SCOPE_COMMAND,\n> \tCONFIG_SCOPE_GITMODULES,\n> +\tCONFIG_SCOPE_SUPERPROJECT,\n> };\n> const char *config_scope_name(enum config_scope scope);\n> \n> @@ -84,6 +85,7 @@ typedef int (*config_parser_event_fn_t)(enum config_event_t type,\n> struct config_options {\n> \tunsigned int respect_includes : 1;\n> \tunsigned int ignore_repo : 1;\n> +\tunsigned int ignore_superproject : 1;\n> \tunsigned int ignore_worktree : 1;\n> \tunsigned int ignore_cmdline : 1;\n> \tunsigned int system_gently : 1;\n> @@ -319,6 +321,7 @@ int git_config_rename_section_in_file(const char *, const char *, const char *);\n> int git_config_copy_section(const char *, const char *);\n> int git_config_copy_section_in_file(const char *, const char *, const char *);\n> const char *git_etc_gitconfig(void);\n> +void git_config_superproject(struct strbuf *, const char *);\n> int git_env_bool(const char *, int);\n> unsigned long git_env_ulong(const char *, unsigned long);\n> int git_config_system(void);\n> diff --git a/submodule.c b/submodule.c\n> index 9767ba9893..92b00f8697 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -2178,6 +2178,35 @@ void absorb_git_dir_into_superproject(const char *path,\n> \t}\n> }\n> \n> +int get_superproject_gitdir(struct strbuf *buf)\n> +{\n> +\tstruct strbuf sb = STRBUF_INIT;\n> +\tstruct child_process cp = CHILD_PROCESS_INIT;\n> +\tint rc = 0;\n> +\n> +\t/* NEEDSWORK: this call also calls out to a subprocess! */\n> +\trc = get_superproject_working_tree(&sb);\n> +\tstrbuf_release(&sb);\n> +\n> +\tif (!rc)\n> +\t\treturn rc;\n> +\n> +\tprepare_submodule_repo_env_no_git_dir(&cp.env_array);\n> +\n> +\tstrvec_pushl(&cp.args, \"-C\", \"..\", \"rev-parse\", \"--absolute-git-dir\", NULL);\n> +\tcp.git_cmd = 1;\n> +\n> +\trc = capture_command(&cp, buf, 0);\n> +\tstrbuf_trim_trailing_newline(buf);\n> +\n> +\t/* leave buf empty if we didn't have a superproject gitdir */\n> +\tif (rc)\n> +\t\tstrbuf_reset(buf);\n> +\n> +\t/* rc reflects the exit code of the rev-parse; invert into a bool */\n> +\treturn !rc;\n> +}\n> +\n> int get_superproject_working_tree(struct strbuf *buf)\n> {\n> \tstruct child_process cp = CHILD_PROCESS_INIT;\n> diff --git a/submodule.h b/submodule.h\n> index 4ac6e31cf1..1308d5ae2d 100644\n> --- a/submodule.h\n> +++ b/submodule.h\n> @@ -149,6 +149,12 @@ void prepare_submodule_repo_env(struct strvec *out);\n> void absorb_git_dir_into_superproject(const char *path,\n> \t\t\t\t unsigned flags);\n> \n> +/*\n> + * Return the gitdir of the superproject, which this project is a submodule of.\n> + * If this repository is not a submodule of another repository, return 0.\n> + */\n> +int get_superproject_gitdir(struct strbuf *buf);\n> +\n> /*\n> * Return the absolute path of the working tree of the superproject, which this\n> * project is a submodule of. If this repository is not a submodule of\n> diff --git a/t/t1311-superproject-config.sh b/t/t1311-superproject-config.sh\n> new file mode 100755\n> index 0000000000..650c4d24c7\n> --- /dev/null\n> +++ b/t/t1311-superproject-config.sh\n> @@ -0,0 +1,124 @@\n> +#!/bin/sh\n> +\n> +test_description='Test git config --superproject in different settings'\n> +\n> +. ./test-lib.sh\n> +\n> +# follow t7400's example and use the trash dir repo as a submodule to add\n> +submodurl=$(pwd -P)\n> +\n> +# since only the configs are modified, set up the repo structure only once\n> +test_expect_success 'setup repo structure' '\n> +\ttest_commit \"base\" &&\n> +\tgit submodule add \"${submodurl}\" sub/ &&\n> +\tgit commit -m \"add a submodule\"\n> +'\n> +\n> +test_expect_success 'superproject config applies to super and submodule' '\n> +\tcat >.git/config.superproject <<-EOF &&\n> +\t[foo]\n> +\t\tbar = baz\n> +\tEOF\n> +\n> +\tgit config --get foo.bar &&\n> +\tgit -C sub config --get foo.bar &&\n> +\n> +\trm .git/config.superproject\n> +'\n> +\n> +test_expect_success 'can add from super or sub' '\n> +\tgit config --superproject apple.species honeycrisp &&\n> +\tgit -C sub config --superproject banana.species cavendish &&\n> +\n> +\tcat >expect <<-EOF &&\n> +\tapple.species=honeycrisp\n> +\tbanana.species=cavendish\n> +\tEOF\n> +\n> +\tgit config --list >actual &&\n> +\tgrep -Ff expect actual &&\n> +\n> +\tgit -C sub config --list >actual &&\n> +\tgrep -Ff expect actual &&\n> +\n> +\trm .git/config.superproject\n> +'\n> +\n> +test_expect_success 'can --unset from super or sub' '\n> +\tgit config --superproject apple.species honeycrisp &&\n> +\tgit -C sub config --superproject banana.species cavendish &&\n> +\n> +\tgit config --unset --superproject banana.species &&\n> +\tgit -C sub config --unset --superproject apple.species\n> +'\n> +\n> +test_expect_success 'can --edit superproject config' '\n> +\ttest_config core.editor \"echo [foo]bar=baz >\" &&\n> +\tgit config --edit --superproject &&\n> +\n> +\tgit config --get foo.bar &&\n> +\n> +\trm .git/config.superproject\n> +'\n> +\n> +test_expect_success 'can --show-origin the superproject config' '\n> +\tgit config --superproject --add foo.bar baz &&\n> +\n> +\tgit config --list --show-origin >actual &&\n> +\tgrep -F \"config.superproject\" actual &&\n> +\n> +\trm .git/config.superproject\n> +'\n> +\n> +test_expect_success 'can --show-scope the superproject config' '\n> +\tgit config --superproject --add foo.bar baz &&\n> +\n> +\tgit config --list --show-scope >actual &&\n> +\tgrep \"superproject\" actual &&\n> +\n> +\trm .git/config.superproject\n> +'\n> +\n> +test_expect_success 'pre-existing config applies to new submodule' '\n> +\tgit config --superproject --add foo.bar baz &&\n> +\n> +\tgit submodule add \"${submodurl}\" sub2/ &&\n> +\tgit commit -m \"add a second submodule\" &&\n> +\n> +\tgit -C sub2 config --get foo.bar &&\n> +\n> +\t# clean up\n> +\tgit reset HEAD^ &&\n> +\trm -fr sub2 &&\n> +\trm .git/config.superproject\n> +'\n> +\n> +# NEEDSWORK: submodule.c:get_superproject_working_tree doesn't support worktree\n> +test_expect_failure 'worktrees can still access config.superproject' '\n> +\tgit config --superproject --add foo.bar baz &&\n> +\n> +\tgit worktree add wt &&\n> +\t(\n> +\t\tcd wt &&\n> +\t\tgit config --get foo.bar\n> +\t) &&\n> +\n> +\t# clean up\n> +\tgit worktree remove wt &&\n> +\trm .git/config.superproject\n> +'\n> +\n> +# This test deletes the submodule! Keep it at the end of the test suite.\n> +test_expect_success 'config.submodule works even with no submodules' '\n> +\t# get rid of the submodule\n> +\tgit reset HEAD^ &&\n> +\trm -fr sub &&\n> +\n> +\tgit config --superproject --add foo.bar baz &&\n> +\n> +\tgit config --get foo.bar &&\n> +\n> +\trm .git/config.superproject\n> +'\n> +\n> +test_done\n\n"},{"id":"421397","messageId":"CAHd-oW4VoBbZHc7cLdn0LPM531qNDGOfwPZdKiKoG4BoRFaqdg@mail.gmail.com","threadId":"55458","inReplyTo":"20210408233936.533342-3-emilyshaffer@google.com","subject":"Re: [RFC PATCH 2/2] config: add 'config.superproject' file","fromName":"Matheus Tavares Bernardino","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-04-09T14:35:13Z","receivedAt":"2021-04-09T14:35:31Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"Hi, Emily\n\nI'm not familiar enough with this code to give a full review and I\nimagine you probably want comments more focused on the design level,\nwhile this is an RFC, but here are some small nitpicks I found while\nreading the patch. I Hope it helps :)\n\nOn Thu, Apr 8, 2021 at 8:39 PM Emily Shaffer wrote:\n>\n> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\n> index 4b4cc5c5e8..a33136fb08 100644\n> --- a/Documentation/git-config.txt\n> +++ b/Documentation/git-config.txt\n> @@ -48,7 +48,7 @@ unset an existing `--type` specifier with `--no-type`.\n>\n> When reading, the values are read from the system, global and\n> repository local configuration files by default, and options\n> -`--system`, `--global`, `--local`, `--worktree` and\n> +`--system`, `--global`, `--superproject`, `--local`, `--worktree` and\n> `--file ` can be used to tell the command to read from only\n> that location (see <>).\n>\n> @@ -127,6 +127,17 @@ rather than from all available files.\n> +\n> See also <>.\n>\n> +--superproject::\n> + For writing options: write to the superproject's\n> + `.git/config.superproject` file, even if run from a submodule of that\n> + superproject.\n\nHmm, I wonder what happens if a repo is both a submodule and a\nsuperproject (i.e. in case of nested submodules). Let's see:\n\n# Create repo/sub/sub2\n$ git init repo\n$ cd repo\n$ touch F && git add F && git commit -m F\n$ git submodule add ./ sub\n$ git -C sub submodule add ./sub sub2\n$ git -C sub commit -m sub2\n$ git commit -m sub\n\n# Now test the config\n$ git -C sub/sub2 config --superproject foo.bar 1\n$ git -C sub/sub2 config --get foo.bar\n1\n$ git -C sub config --get foo.bar\n\n$ git config --get foo.bar\n\n\nIt makes sense to me that `foo.bar` is not defined on `repo`, but\nshouldn't it be defined on `repo/sub`? Or am I doing something wrong?\n\n(`git -C sub rev-parse --git-dir` gives `.git/modules/sub/`, where\nindeed there is a config.superproject with `foo.bar` set.)\n\n> diff --git a/builtin/config.c b/builtin/config.c\n> index f71fa39b38..f0a57a89ca 100644\n> --- a/builtin/config.c\n> +++ b/builtin/config.c\n> @@ -26,7 +26,7 @@ static char key_delim = ' ';\n> static char term = '\\n';\n>\n> static int use_global_config, use_system_config, use_local_config;\n> -static int use_worktree_config;\n> +static int use_worktree_config, use_superproject_config;\n> static struct git_config_source given_config_source;\n> static int actions, type;\n> static char *default_value;\n> @@ -130,6 +130,8 @@ static struct option builtin_config_options[] = {\n> OPT_GROUP(N_(\"Config file location\")),\n> OPT_BOOL(0, \"global\", &use_global_config, N_(\"use global config file\")),\n> OPT_BOOL(0, \"system\", &use_system_config, N_(\"use system config file\")),\n> + OPT_BOOL(0, \"superproject\",\n> + &use_superproject_config, N_(\"use superproject config file\")),\n> OPT_BOOL(0, \"local\", &use_local_config, N_(\"use repository config file\")),\n> OPT_BOOL(0, \"worktree\", &use_worktree_config, N_(\"use per-worktree config file\")),\n> OPT_STRING('f', \"file\", &given_config_source.file, N_(\"file\"), N_(\"use given config file\")),\n> @@ -697,6 +699,12 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n> else if (use_system_config) {\n> given_config_source.file = git_etc_gitconfig();\n> given_config_source.scope = CONFIG_SCOPE_SYSTEM;\n> + } else if (use_superproject_config) {\n> + struct strbuf superproject_cfg = STRBUF_INIT;\n> + git_config_superproject(&superproject_cfg, get_git_dir());\n> + given_config_source.file = xstrdup(superproject_cfg.buf);\n> + given_config_source.scope = CONFIG_SCOPE_SUPERPROJECT;\n> + strbuf_release(&superproject_cfg);\n\nNit: maybe it would be a bit cleaner to replace the xstrdup() +\nstrbuf_release() lines with a single:\n\n given_config_source.file = strbuf_detach(superproject_cfg, NULL);\n\n> } else if (use_local_config) {\n> given_config_source.file = git_pathdup(\"config\");\n> given_config_source.scope = CONFIG_SCOPE_LOCAL;\n> diff --git a/config.c b/config.c\n> index 67d9bf2238..28bb80fd0d 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -21,6 +21,7 @@\n> #include \"dir.h\"\n> #include \"color.h\"\n> #include \"refs.h\"\n> +#include \"submodule.h\"\n>\n> struct config_source {\n> struct config_source *prev;\n> @@ -1852,6 +1853,17 @@ const char *git_etc_gitconfig(void)\n> return system_wide;\n> }\n>\n> +void git_config_superproject(struct strbuf *sb, const char *gitdir)\n> +{\n> + if (!get_superproject_gitdir(sb)) {\n> + /* not a submodule */\n> + strbuf_reset(sb);\n\nDo we have to reset `sb` here? It seems that get_superproject_gitdir()\nleaves the buffer empty when we are not inside a submodule.\n\n> diff --git a/submodule.h b/submodule.h\n> index 4ac6e31cf1..1308d5ae2d 100644\n> --- a/submodule.h\n> +++ b/submodule.h\n> @@ -149,6 +149,12 @@ void prepare_submodule_repo_env(struct strvec *out);\n> void absorb_git_dir_into_superproject(const char *path,\n> unsigned flags);\n>\n> +/*\n> + * Return the gitdir of the superproject, which this project is a submodule of.\n> + * If this repository is not a submodule of another repository, return 0.\n\nNit: it might be nice to say what's the state of `buf` on a 0 return.\nPerhaps also be more explicit about the return codes? Maybe something\nlike:\n\n\"If this repository is a submodule of another repository, save the\nsuperproject's gitdir on `buf` and return 1. Otherwise, return 0 and\nleave `buf` empty.\"\n\n> +int get_superproject_gitdir(struct strbuf *buf);\n> +\n> /*\n> * Return the absolute path of the working tree of the superproject, which this\n> * project is a submodule of. If this repository is not a submodule of\n> diff --git a/t/t1311-superproject-config.sh b/t/t1311-superproject-config.sh\n> new file mode 100755\n> index 0000000000..650c4d24c7\n> --- /dev/null\n> +++ b/t/t1311-superproject-config.sh\n> @@ -0,0 +1,124 @@\n[...]\n> +test_expect_success 'superproject config applies to super and submodule' '\n> + cat >.git/config.superproject <<-EOF &&\n> + [foo]\n> + bar = baz\n> + EOF\n> +\n> + git config --get foo.bar &&\n> + git -C sub config --get foo.bar &&\n> +\n> + rm .git/config.superproject\n\nHmm, if this test fails before removing the config.superproject file,\ncouldn't it interfere with other tests (like the 'can --edit\nsuperproject config')? Perhaps this and the other similar cleanup\nremovals could be declared inside a `test_when_finished` clause, to\nensure they are performed even on test failure.\n\n> +test_expect_success 'can --unset from super or sub' '\n> + git config --superproject apple.species honeycrisp &&\n> + git -C sub config --superproject banana.species cavendish &&\n> +\n> + git config --unset --superproject banana.species &&\n> + git -C sub config --unset --superproject apple.species\n> +'\n\nNice \"cross-setting/unsetting\" test :)\n\n[...]\n> +# This test deletes the submodule! Keep it at the end of the test suite.\n> +test_expect_success 'config.submodule works even with no submodules' '\n\ns/config.submodule/config.superproject/ ?\n"},{"id":"421487","messageId":"xmqqk0pbm6qt.fsf@gitster.g","threadId":"55458","inReplyTo":"CAHd-oW4VoBbZHc7cLdn0LPM531qNDGOfwPZdKiKoG4BoRFaqdg@mail.gmail.com","subject":"Re: [RFC PATCH 2/2] config: add 'config.superproject' file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-09T22:29:46Z","receivedAt":"2021-04-09T22:29:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matheus Tavares Bernardino writes:\n\n> Hi, Emily\n>\n> I'm not familiar enough with this code to give a full review and I\n> imagine you probably want comments more focused on the design level,\n> while this is an RFC, but here are some small nitpicks I found while\n> reading the patch. I Hope it helps :)\n>\n> On Thu, Apr 8, 2021 at 8:39 PM Emily Shaffer wrote:\n>>\n>> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\n>> index 4b4cc5c5e8..a33136fb08 100644\n>> --- a/Documentation/git-config.txt\n>> +++ b/Documentation/git-config.txt\n>> @@ -48,7 +48,7 @@ unset an existing `--type` specifier with `--no-type`.\n>>\n>> When reading, the values are read from the system, global and\n>> repository local configuration files by default, and options\n>> -`--system`, `--global`, `--local`, `--worktree` and\n>> +`--system`, `--global`, `--superproject`, `--local`, `--worktree` and\n>> `--file ` can be used to tell the command to read from only\n>> that location (see <>).\n>>\n>> @@ -127,6 +127,17 @@ rather than from all available files.\n>> +\n>> See also <>.\n>>\n>> +--superproject::\n>> + For writing options: write to the superproject's\n>> + `.git/config.superproject` file, even if run from a submodule of that\n>> + superproject.\n>\n> Hmm, I wonder what happens if a repo is both a submodule and a\n> superproject (i.e. in case of nested submodules).\n\nAnother thing I am not sure about the design is that a repository\ncan be shared as a submodule by more than one superprojects. The\nsuperprojects may want their submodule checkouts at different\nsubmodule commits, but that is something doable by having multiple\nworktrees connected to a single submodule repository.\n\nI think our design principle has been that it is perfectly OK for a\nsuperproject to be in total control if its submodules, but\nsubmodules should not even be aware of being used as a submodule by\na superproject, and that allows a submodule repository to be shared\nby multiple superprojects. As \"write to the superproject's X file\"\nrequires a submodule to know who THE superproject of itself is, this\nfeature itself (not the implementation) feels somewhat iffy.\n"},{"id":"421911","messageId":"YHXdTg8cm5yUWt5J@google.com","threadId":"55458","inReplyTo":"e19a250a-c4ca-fc32-83f9-a03aa03cd88a@iee.email","subject":"Re: [RFC PATCH 2/2] config: add 'config.superproject' file","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-04-13T18:05:02Z","receivedAt":"2021-04-13T18:05:09Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Fri, Apr 09, 2021 at 12:10:27PM +0100, Philip Oakley wrote:\n> \n> On 09/04/2021 00:39, Emily Shaffer wrote:\n> > Some configs, such as wrapper directives like gerrit.createChangeId, or\n> > forthcoming hook configs, should apply to a superproject as well as all\n> > its submodules. It may not be appropriate to apply them globally - for\n> > example, if the user also contributes to many projects which do not use\n> > the configs necessary for one project-with-submodules - and it may be\n> > burdensome to apply them locally to the superproject and each of its\n> > submodules. Even if the user runs 'git submodule foreach \"git config\n> > --local foo.bar', if a new submodule is added later on, that config is\n> > not applied to the new submodule.\n> >\n> > It is also inappropriate to share the entire superproject config, since\n> > some items - like remote URLs or partial-clone filters - would not apply\n> > to a submodule.\n> >\n> > To make life easier for projects with many submodules, then, create a\n> > new \"config.superproject\" config scope, which is included in the config\n> > parse for the superproject as well as for all the submodules of that\n> > superproject.\n> >\n> > For the superproject, this new config file is equally local to the local\n> > config; for the submodule, the new config file is less local than the\n> > local config. So let's include it directly before the local config\n> > during the config parse.\n> >\n> > Signed-off-by: Emily Shaffer \n> > ---\n> \n> Does this need an update to the `git config --show-origin --show-scope`\n> capability?\n\nIt's included:\n\n> > --- a/config.c\n> > +++ b/config.c\n> > @@ -3515,6 +3539,8 @@ const char *config_scope_name(enum config_scope scope)\n> > \t\treturn \"command\";\n> > \tcase CONFIG_SCOPE_GITMODULES:\n> > \t\treturn \"gitmodules\";\n> > +\tcase CONFIG_SCOPE_SUPERPROJECT:\n> > +\t\treturn \"superproject\";\n> > \tdefault:\n> > \t\treturn \"unknown\";\n> > \t}\n> > diff --git a/config.h b/config.h\n> > index 535f5517b8..b42e1d13eb 100644\n> > --- a/config.h\n> > +++ b/config.h\n> > @@ -43,6 +43,7 @@ enum config_scope {\n> > \tCONFIG_SCOPE_WORKTREE,\n> > \tCONFIG_SCOPE_COMMAND,\n> > \tCONFIG_SCOPE_GITMODULES,\n> > +\tCONFIG_SCOPE_SUPERPROJECT,\n> > };\n\n> > diff --git a/t/t1311-superproject-config.sh b/t/t1311-superproject-config.sh\n> > new file mode 100755\n> > index 0000000000..650c4d24c7\n> > --- /dev/null\n> > +++ b/t/t1311-superproject-config.sh\n> > +test_expect_success 'can --show-origin the superproject config' '\n> > +\tgit config --superproject --add foo.bar baz &&\n> > +\n> > +\tgit config --list --show-origin >actual &&\n> > +\tgrep -F \"config.superproject\" actual &&\n> > +\n> > +\trm .git/config.superproject\n> > +'\n> > +\n> > +test_expect_success 'can --show-scope the superproject config' '\n> > +\tgit config --superproject --add foo.bar baz &&\n> > +\n> > +\tgit config --list --show-scope >actual &&\n> > +\tgrep \"superproject\" actual &&\n> > +\n> > +\trm .git/config.superproject\n> > +'\n\n - Emily\n"},{"id":"421916","messageId":"YHXnjFS/xPn73VzJ@google.com","threadId":"55458","inReplyTo":"CAHd-oW4VoBbZHc7cLdn0LPM531qNDGOfwPZdKiKoG4BoRFaqdg@mail.gmail.com","subject":"Re: [RFC PATCH 2/2] config: add 'config.superproject' file","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-04-13T18:48:44Z","receivedAt":"2021-04-13T18:48:53Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Fri, Apr 09, 2021 at 11:35:13AM -0300, Matheus Tavares Bernardino wrote:\n> \n> Hi, Emily\n> \n> I'm not familiar enough with this code to give a full review and I\n> imagine you probably want comments more focused on the design level,\n> while this is an RFC, but here are some small nitpicks I found while\n> reading the patch. I Hope it helps :)\n> \n> On Thu, Apr 8, 2021 at 8:39 PM Emily Shaffer wrote:\n> >\n> > diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\n> > index 4b4cc5c5e8..a33136fb08 100644\n> > --- a/Documentation/git-config.txt\n> > +++ b/Documentation/git-config.txt\n> > @@ -48,7 +48,7 @@ unset an existing `--type` specifier with `--no-type`.\n> >\n> > When reading, the values are read from the system, global and\n> > repository local configuration files by default, and options\n> > -`--system`, `--global`, `--local`, `--worktree` and\n> > +`--system`, `--global`, `--superproject`, `--local`, `--worktree` and\n> > `--file ` can be used to tell the command to read from only\n> > that location (see <>).\n> >\n> > @@ -127,6 +127,17 @@ rather than from all available files.\n> > +\n> > See also <>.\n> >\n> > +--superproject::\n> > + For writing options: write to the superproject's\n> > + `.git/config.superproject` file, even if run from a submodule of that\n> > + superproject.\n> \n> Hmm, I wonder what happens if a repo is both a submodule and a\n> superproject (i.e. in case of nested submodules). Let's see:\n> \n> # Create repo/sub/sub2\n> $ git init repo\n> $ cd repo\n> $ touch F && git add F && git commit -m F\n> $ git submodule add ./ sub\n> $ git -C sub submodule add ./sub sub2\n> $ git -C sub commit -m sub2\n> $ git commit -m sub\n> \n> # Now test the config\n> $ git -C sub/sub2 config --superproject foo.bar 1\n> $ git -C sub/sub2 config --get foo.bar\n> 1\n> $ git -C sub config --get foo.bar\n> \n> $ git config --get foo.bar\n> \n> \n> It makes sense to me that `foo.bar` is not defined on `repo`, but\n> shouldn't it be defined on `repo/sub`? Or am I doing something wrong?\n> \n> (`git -C sub rev-parse --git-dir` gives `.git/modules/sub/`, where\n> indeed there is a config.superproject with `foo.bar` set.)\n\nYeah, this isn't very surprising. The code does essentially:\n\n parent_gitdir = git -C ../ rev-parse --git-dir\n config_parse_order += $parent_gitdir/config.superproject\n\nThat is, in the nested submodules case, it's looking at\n.git/modules/sub/config.superproject - but 'sub' is getting its\nsuperproject config from .git/config.superproject and has no such file\nof its own.\n\nOne way I could see to solve it would be to skip the \"find parent\ngitdir\" thing entirely:\n\n git -C ../ config --superproject --list >parent_superproject_cfg\n config_parse_order += parent_superproject_cfg\n config_parse_order += $my_own_gitdir/config.superproject\n\nThe recursion is a little icky, I guess, but submodules are all\nrecursive anyway, right? :) Hmm.\n\n> \n> > diff --git a/builtin/config.c b/builtin/config.c\n> > index f71fa39b38..f0a57a89ca 100644\n> > --- a/builtin/config.c\n> > +++ b/builtin/config.c\n> > @@ -26,7 +26,7 @@ static char key_delim = ' ';\n> > static char term = '\\n';\n> >\n> > static int use_global_config, use_system_config, use_local_config;\n> > -static int use_worktree_config;\n> > +static int use_worktree_config, use_superproject_config;\n> > static struct git_config_source given_config_source;\n> > static int actions, type;\n> > static char *default_value;\n> > @@ -130,6 +130,8 @@ static struct option builtin_config_options[] = {\n> > OPT_GROUP(N_(\"Config file location\")),\n> > OPT_BOOL(0, \"global\", &use_global_config, N_(\"use global config file\")),\n> > OPT_BOOL(0, \"system\", &use_system_config, N_(\"use system config file\")),\n> > + OPT_BOOL(0, \"superproject\",\n> > + &use_superproject_config, N_(\"use superproject config file\")),\n> > OPT_BOOL(0, \"local\", &use_local_config, N_(\"use repository config file\")),\n> > OPT_BOOL(0, \"worktree\", &use_worktree_config, N_(\"use per-worktree config file\")),\n> > OPT_STRING('f', \"file\", &given_config_source.file, N_(\"file\"), N_(\"use given config file\")),\n> > @@ -697,6 +699,12 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n> > else if (use_system_config) {\n> > given_config_source.file = git_etc_gitconfig();\n> > given_config_source.scope = CONFIG_SCOPE_SYSTEM;\n> > + } else if (use_superproject_config) {\n> > + struct strbuf superproject_cfg = STRBUF_INIT;\n> > + git_config_superproject(&superproject_cfg, get_git_dir());\n> > + given_config_source.file = xstrdup(superproject_cfg.buf);\n> > + given_config_source.scope = CONFIG_SCOPE_SUPERPROJECT;\n> > + strbuf_release(&superproject_cfg);\n> \n> Nit: maybe it would be a bit cleaner to replace the xstrdup() +\n> strbuf_release() lines with a single:\n> \n> given_config_source.file = strbuf_detach(superproject_cfg, NULL);\n\nOh nice, thanks!\n\n> \n> > } else if (use_local_config) {\n> > given_config_source.file = git_pathdup(\"config\");\n> > given_config_source.scope = CONFIG_SCOPE_LOCAL;\n> > diff --git a/config.c b/config.c\n> > index 67d9bf2238..28bb80fd0d 100644\n> > --- a/config.c\n> > +++ b/config.c\n> > @@ -21,6 +21,7 @@\n> > #include \"dir.h\"\n> > #include \"color.h\"\n> > #include \"refs.h\"\n> > +#include \"submodule.h\"\n> >\n> > struct config_source {\n> > struct config_source *prev;\n> > @@ -1852,6 +1853,17 @@ const char *git_etc_gitconfig(void)\n> > return system_wide;\n> > }\n> >\n> > +void git_config_superproject(struct strbuf *sb, const char *gitdir)\n> > +{\n> > + if (!get_superproject_gitdir(sb)) {\n> > + /* not a submodule */\n> > + strbuf_reset(sb);\n> \n> Do we have to reset `sb` here? It seems that get_superproject_gitdir()\n> leaves the buffer empty when we are not inside a submodule.\n\nSince I didn't promise it in the documentation I included the reset\nhere, but I see your comment just after this suggesting I should also\npromise it in the documentation ;)\n\n> \n> > diff --git a/submodule.h b/submodule.h\n> > index 4ac6e31cf1..1308d5ae2d 100644\n> > --- a/submodule.h\n> > +++ b/submodule.h\n> > @@ -149,6 +149,12 @@ void prepare_submodule_repo_env(struct strvec *out);\n> > void absorb_git_dir_into_superproject(const char *path,\n> > unsigned flags);\n> >\n> > +/*\n> > + * Return the gitdir of the superproject, which this project is a submodule of.\n> > + * If this repository is not a submodule of another repository, return 0.\n> \n> Nit: it might be nice to say what's the state of `buf` on a 0 return.\n> Perhaps also be more explicit about the return codes? Maybe something\n> like:\n> \n> \"If this repository is a submodule of another repository, save the\n> superproject's gitdir on `buf` and return 1. Otherwise, return 0 and\n> leave `buf` empty.\"\n\nSeems reasonable.\n\n> \n> > +int get_superproject_gitdir(struct strbuf *buf);\n> > +\n> > /*\n> > * Return the absolute path of the working tree of the superproject, which this\n> > * project is a submodule of. If this repository is not a submodule of\n> > diff --git a/t/t1311-superproject-config.sh b/t/t1311-superproject-config.sh\n> > new file mode 100755\n> > index 0000000000..650c4d24c7\n> > --- /dev/null\n> > +++ b/t/t1311-superproject-config.sh\n> > @@ -0,0 +1,124 @@\n> [...]\n> > +test_expect_success 'superproject config applies to super and submodule' '\n> > + cat >.git/config.superproject <<-EOF &&\n> > + [foo]\n> > + bar = baz\n> > + EOF\n> > +\n> > + git config --get foo.bar &&\n> > + git -C sub config --get foo.bar &&\n> > +\n> > + rm .git/config.superproject\n> \n> Hmm, if this test fails before removing the config.superproject file,\n> couldn't it interfere with other tests (like the 'can --edit\n> superproject config')? Perhaps this and the other similar cleanup\n> removals could be declared inside a `test_when_finished` clause, to\n> ensure they are performed even on test failure.\n\nOh sure, thanks.\n\n> \n> > +test_expect_success 'can --unset from super or sub' '\n> > + git config --superproject apple.species honeycrisp &&\n> > + git -C sub config --superproject banana.species cavendish &&\n> > +\n> > + git config --unset --superproject banana.species &&\n> > + git -C sub config --unset --superproject apple.species\n> > +'\n> \n> Nice \"cross-setting/unsetting\" test :)\n> \n> [...]\n> > +# This test deletes the submodule! Keep it at the end of the test suite.\n> > +test_expect_success 'config.submodule works even with no submodules' '\n> \n> s/config.submodule/config.superproject/ ?\nAaauuurgggh :)\n\nThanks for the nits!\n - Emily\n"},{"id":"421922","messageId":"YHX0zwaES/9Nz97U@google.com","threadId":"55458","inReplyTo":"xmqqk0pbm6qt.fsf@gitster.g","subject":"Re: [RFC PATCH 2/2] config: add 'config.superproject' file","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-04-13T19:45:19Z","receivedAt":"2021-04-13T19:45:27Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Fri, Apr 09, 2021 at 03:29:46PM -0700, Junio C Hamano wrote:\n> \n> Matheus Tavares Bernardino writes:\n> \n> > Hi, Emily\n> >\n> > I'm not familiar enough with this code to give a full review and I\n> > imagine you probably want comments more focused on the design level,\n> > while this is an RFC, but here are some small nitpicks I found while\n> > reading the patch. I Hope it helps :)\n> >\n> > On Thu, Apr 8, 2021 at 8:39 PM Emily Shaffer wrote:\n> >>\n> >> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\n> >> index 4b4cc5c5e8..a33136fb08 100644\n> >> --- a/Documentation/git-config.txt\n> >> +++ b/Documentation/git-config.txt\n> >> @@ -48,7 +48,7 @@ unset an existing `--type` specifier with `--no-type`.\n> >>\n> >> When reading, the values are read from the system, global and\n> >> repository local configuration files by default, and options\n> >> -`--system`, `--global`, `--local`, `--worktree` and\n> >> +`--system`, `--global`, `--superproject`, `--local`, `--worktree` and\n> >> `--file ` can be used to tell the command to read from only\n> >> that location (see <>).\n> >>\n> >> @@ -127,6 +127,17 @@ rather than from all available files.\n> >> +\n> >> See also <>.\n> >>\n> >> +--superproject::\n> >> + For writing options: write to the superproject's\n> >> + `.git/config.superproject` file, even if run from a submodule of that\n> >> + superproject.\n> >\n> > Hmm, I wonder what happens if a repo is both a submodule and a\n> > superproject (i.e. in case of nested submodules).\n> \n> Another thing I am not sure about the design is that a repository\n> can be shared as a submodule by more than one superprojects. The\n> superprojects may want their submodule checkouts at different\n> submodule commits, but that is something doable by having multiple\n> worktrees connected to a single submodule repository.\n\nI think the implementation as-written actually handles this\nsharing-via-worktree case you describe gracefully, as it discovers the\ngitdir belonging to the worktree above the worktree where it is being\nrun now:\n\nsuperproject-a\n-> sub-a \nsuperproject-b\n-> sub-b \n\nIn this case running `git config --list --superproject` in sub-a will\nyield superproject-a/.git/config.superproject and running it in sub-b\nwill yield superproject-b/.git/config.superproject; that seems logical\nto me. If I am adding libc as a submodule via worktree to Git as well\nas, say, Wireshark, just to save me on disk space, I wouldn't want my\nWireshark hooks to run in Git project or vice versa.\n\n> I think our design principle has been that it is perfectly OK for a\n> superproject to be in total control if its submodules, but\n> submodules should not even be aware of being used as a submodule by\n> a superproject, and that allows a submodule repository to be shared\n> by multiple superprojects. As \"write to the superproject's X file\"\n> requires a submodule to know who THE superproject of itself is, this\n> feature itself (not the implementation) feels somewhat iffy.\n\nAs for this, I wonder what the reasoning is. I guess to simplify the\ncode, and to make the behavior more predictable (for example, 'git\ncommit' doesn't suddenly make a commit in some project that isn't this\none)?\n\nOne could imagine some really nice quality-of-life improvements if the\nsubmodule is allowed to know it's a submodule (even by a config, for\nexample):\n\n - We could teach 'git status' to indicate the state when the submodule\n index is clean, but the superproject does not contain a commit\n pointing to the submodule's HEAD - which could still be considered a\n dirty state, since the change isn't associated with the larger project\n yet. I could imagine there might be other handy information related\n to submodule/superproject status we may want to display too.\n - We could teach 'git log' to decorate commits which are referred to by\n superproject commits, perhaps?\n - We could teach 'git push' to, by option or config, push the entire\n superproject-and-submodules package at once, to make it easier to\n coordinate changes across the whole superproject\n - One could envision some other niceties like 'git stash\n --whole-superproject' or similar, where a user could operate on the\n entire overall project (that is, the superproject and all its\n submodules) without needing to switch context away\n\nI don't think lacking these things would stop a user from doing\nsomething they want to do, but it does seem like they could make life\nmore comfortable for a user developing in a project made up of many\nsubmodules.\n\n - Emily\n"},{"id":"421962","messageId":"87v98p17im.fsf@evledraar.gmail.com","threadId":"55458","inReplyTo":"20210408233936.533342-1-emilyshaffer@google.com","subject":"Future structure of submodules and .git/, .git/modules/* organization","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-04-14T10:32:17Z","receivedAt":"2021-04-14T10:32:22Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Apr 09 2021, Emily Shaffer wrote:\n\n> I'm hoping to work on some other submodule-centric stuff over the coming\n> months, and it might end up being very useful to be able to tell \"am I a\n> submodule?\" and \"how do I talk to my superproject?\" more generally - so\n> I'm really open to figuring out a better way than this, if folks have\n> ideas.\n>\n> Patch 1 is a small refactor that we can take or leave - I found\n> \"SCOPE_SUBMODULE\" to be pretty ambiguous, especially since it seems to\n> refer to configs from .gitmodules. Even though I decided that\n> \"superproject\" was a better name than \"submodule\" I still wasn't super\n> happy with the ambiguity. But we can drop it if folks don't want to\n> rename.\n\nThis is less on your patch, and more on the larger work you're\nsuggesting, but the two are kind of related. Skip to the paragraph\nstarting with \"But why\" below for the relevance :)\n\nI very much wish that we could eventually make the use of submodules\ntotally transparent, i.e. (taking the example of git.git):\n\n * You clone, and we just get objects from\n https://github.com/cr-marcstevens/sha1collisiondetection.git too\n\n * The fact that we have:\n\n 160000 commit 855827c583bc30645ba427885caa40c5b81764d2 sha1collisiondetection\n\n Would become totally invisible to most users unless they run some\n gutsy ls-tree/files comand.\n\n We used to have a full git dir at sha1collisiondetection/.git and all\n the UX issues that entailed (e.g. switching to an old commit without\n the submodule).\n\n Now it's a stub and the actual repo is at\n .git/modules/sha1collisiondetection/, so we're kind of partially\n there.\n\n * I would think that the next (but big) logical step would be to use\n some combination of delta islands, upcoming sparse indexes etc. to\n actually share the object stores of the parent and submodule.\n\n Things like \"git fsck\" which now just punt on COMMIT would need to\n become smarter, but e.g. we could repack (or not, with islands)\n between parent and submodule.\n\nI would think that this end goal makes more sense than the current\nstatus quo of teaching every command that needs to e.g. grep the tree to\nhave a \"--recurse-submodules\". The distinction would be invisible to the\nlikes of \"git-grep\".\n\nIt would mean more complexity in e.g. \"git commit\", but we can imagine\nif you wanted a cross-submodule commit it could do those commits\nrecursively, update parent COMMIT entries etc. (and even, optionally,\npush out the submodule changes). That particular thing being so ad-hoc\nis a *very* frequent pain point in submodule use.\n\nBut why am I talking about this here when all you're suggesting is\nanother config level?\n\nWell, I think (but have not carefully thought about) that this\nCONFIG_SCOPE_GITMODULES is probably a narrow net improvement now. If you\nset most options in your .git/config to you that's the same logical\nproject, why shouldn't you get your diff setting or whatever because you\ncd'd to a submodule \"in the same project\" (from the view of the user).\n\nBut I think that for a wider \"improve submodules\" effort it's worth\nsomeone (and right now, that sounds like it's you) thinking about where\nwe're going with the feature. Maybe with some technical doc identifying\nthe most common pain points, what we propose (or could envision) doing\nabout them.\n\nSo e.g. in this case, having per-submodule config could be a step\nforward, but it could also be one more step of walking in a circle.\n\nI.e. don't think any user asked for or wanted to stitch together\nmultiple .git directories into one linked pseudo-checkout, that's\nultimately something we're exposing as an implementation detail. If we\nno longer expose that implementation detail, would we be stuck\nsupporting what's ultimately a workaround feature?\n\nNone of that means we shouldn't have that one step forward that solves\nreal problems today.\n\nBut I think we should think about the end goal(s) sooner than\nlater. E.g. in your case, do you *really* want another config level, or\nis it just the easiest way to get what you actually want, which is for a\n\"git config\" in the submodule dir to perhaps consider its .git/config\nand .git/modules/sha1collisiondetection/config as the same file for the\npurposes of config parsing? Sans things like the remote URLs etc.\n"},{"id":"422062","messageId":"YHivLSiJmZ0VtKs3@google.com","threadId":"55458","inReplyTo":"87v98p17im.fsf@evledraar.gmail.com","subject":"Re: Future structure of submodules and .git/, .git/modules/* organization","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-04-15T21:25:01Z","receivedAt":"2021-04-15T21:25:11Z","isPatch":false,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Wed, Apr 14, 2021 at 12:32:17PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> On Fri, Apr 09 2021, Emily Shaffer wrote:\n> \n> > I'm hoping to work on some other submodule-centric stuff over the coming\n> > months, and it might end up being very useful to be able to tell \"am I a\n> > submodule?\" and \"how do I talk to my superproject?\" more generally - so\n> > I'm really open to figuring out a better way than this, if folks have\n> > ideas.\n> >\n> > Patch 1 is a small refactor that we can take or leave - I found\n> > \"SCOPE_SUBMODULE\" to be pretty ambiguous, especially since it seems to\n> > refer to configs from .gitmodules. Even though I decided that\n> > \"superproject\" was a better name than \"submodule\" I still wasn't super\n> > happy with the ambiguity. But we can drop it if folks don't want to\n> > rename.\n> \n> This is less on your patch, and more on the larger work you're\n> suggesting, but the two are kind of related. Skip to the paragraph\n> starting with \"But why\" below for the relevance :)\n> \n> I very much wish that we could eventually make the use of submodules\n> totally transparent, i.e. (taking the example of git.git):\n> \n> * You clone, and we just get objects from\n> https://github.com/cr-marcstevens/sha1collisiondetection.git too\n> \n> * The fact that we have:\n> \n> 160000 commit 855827c583bc30645ba427885caa40c5b81764d2 sha1collisiondetection\n> \n> Would become totally invisible to most users unless they run some\n> gutsy ls-tree/files comand.\n> \n> We used to have a full git dir at sha1collisiondetection/.git and all\n> the UX issues that entailed (e.g. switching to an old commit without\n> the submodule).\n> \n> Now it's a stub and the actual repo is at\n> .git/modules/sha1collisiondetection/, so we're kind of partially\n> there.\n\nSide note: when I was writing the tests for patch 2 in this series I\nnoticed it was still really easy to end up with a full git dir at e.g.\nsha1collisiondetection/.git, if you are trying to create a new repo to\nuse as a submodule (easily could be the case when working on a\n\"greenfield\" project and you're the original author). There is\ndefinitely a reason that I copied the (IMO) hack from the other\nsubmodule test suite using the trash directory as a remote for my new\nsubmodule. ;) I wonder whether I was just doing it wrong, or if we need\nsome established flow (maybe with `git submodule` subcommand) to create\na brand new submodule, not cloned from somewhere, and put its gitdir\ninside of .git/modules?\n\n> * I would think that the next (but big) logical step would be to use\n> some combination of delta islands, upcoming sparse indexes etc. to\n> actually share the object stores of the parent and submodule.\n\nInteresting - I'm trying to think of reasons not to and coming up blank,\nbut I also don't have much firsthand experience with the area of the\ncode that looks through the object store, so what do I know?\n\n> Things like \"git fsck\" which now just punt on COMMIT would need to\n> become smarter, but e.g. we could repack (or not, with islands)\n> between parent and submodule.\n> \n> I would think that this end goal makes more sense than the current\n> status quo of teaching every command that needs to e.g. grep the tree to\n> have a \"--recurse-submodules\". The distinction would be invisible to the\n> likes of \"git-grep\".\n\nYeah, I see where you're going, I think. Teaching everyone\n--recurse-submodules or to respect the config setting\n(core.recurseSubmodules? whatever it is) is inherently fragile, since it\nrelies on human reviewers to remember to chide patch authors to think of\nthe submodules use case. Neat.\n\n> It would mean more complexity in e.g. \"git commit\", but we can imagine\n> if you wanted a cross-submodule commit it could do those commits\n> recursively, update parent COMMIT entries etc. (and even, optionally,\n> push out the submodule changes). That particular thing being so ad-hoc\n> is a *very* frequent pain point in submodule use.\n\nYeah, it sounds like you're describing the approach I was hoping to use\nfor commit-with-recursion:\n\n - Note each submodule with staged changes, as well as the superproject\n - (Optional? but might be nice) Open an editor with all the commit\n messages separated by scissors, so you can easily refer back to or\n modify the submodule commit messages while writing the superproject\n commit message\n - Generate all the submodule commits with the supplied commit-msgs\n - Take the commit IDs of all the newly created commits, stage them in\n the superproject, and generate the superproject commit with the\n supplied commit-msg\n\nMaybe the editor bit is too much, but Jonathan Nieder at least really\nliked that idea :) But the bit you're talking about - generating the\nsubmodules first and then staging and committing in the superproject \"on\nthe fly\" - was the approach I was hoping to take.\n\n> \n> But why am I talking about this here when all you're suggesting is\n> another config level?\n> \n> Well, I think (but have not carefully thought about) that this\n> CONFIG_SCOPE_GITMODULES is probably a narrow net improvement now. If you\n> set most options in your .git/config to you that's the same logical\n> project, why shouldn't you get your diff setting or whatever because you\n> cd'd to a submodule \"in the same project\" (from the view of the user).\n> \n> But I think that for a wider \"improve submodules\" effort it's worth\n> someone (and right now, that sounds like it's you) thinking about where\n> we're going with the feature. Maybe with some technical doc identifying\n> the most common pain points, what we propose (or could envision) doing\n> about them.\n> \n> So e.g. in this case, having per-submodule config could be a step\n> forward, but it could also be one more step of walking in a circle.\n> \n> I.e. don't think any user asked for or wanted to stitch together\n> multiple .git directories into one linked pseudo-checkout, that's\n> ultimately something we're exposing as an implementation detail. If we\n> no longer expose that implementation detail, would we be stuck\n> supporting what's ultimately a workaround feature?\n> \n> None of that means we shouldn't have that one step forward that solves\n> real problems today.\n> \n> But I think we should think about the end goal(s) sooner than\n> later.\n\nYeah, this is actually a good nudge for me. Internally we've got a big\nnice doc explaining all our submodule plans for the next 6-9 months -\nbut I should probably get to sharing that with the list ;) I'd say to\nlook for it either this Friday or next Friday.\n\n> E.g. in your case, do you *really* want another config level, or\n> is it just the easiest way to get what you actually want, which is for a\n> \"git config\" in the submodule dir to perhaps consider its .git/config\n> and .git/modules/sha1collisiondetection/config as the same file for the\n> purposes of config parsing? Sans things like the remote URLs etc.\n\nAs for this specific case, I want what is in the patch. Using a new\nconfig file doesn't feel like a compromise to me - I actually would\nprefer users to be able to explicitly choose shared vs. repo-specific\nconfigs, rather than for we Git devs to implicitly decide which configs\nare fine to share and which aren't. (I could also see having explicitly\nshared or non-shared configs making it easier for wrappers to leverage\nthe Git config infrastructure, without mirroring our own \"list of\nconfigs to not share to submodule\" for themselves.)\n\nThis RFC is mostly here to enable shared hooks, as you might have\nguessed - but even with hooks, it's easy to imagine wanting a blend of\ninherited vs. per-repo hooks. For example, I want to inherit a hook to\ncreate a Gerrit Change-Id footer in my superproject and all my\nsubmodules, definitely - but if my superproject is written in C and\nincludes a submodule which is in, I dunno, Rust or Zig or Perl or\nwhatever people are writing these days, I definitely don't want to try\nand run my C linter from my superproject on my 15 Rust submodules - and\nI definitely don't want to disable it in each one.\n\n - Emily\n"},{"id":"422064","messageId":"xmqq8s5jp6o8.fsf@gitster.g","threadId":"55458","inReplyTo":"87v98p17im.fsf@evledraar.gmail.com","subject":"Re: Future structure of submodules and .git/, .git/modules/* organization","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-15T21:41:11Z","receivedAt":"2021-04-15T21:41:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> I very much wish that we could eventually make the use of submodules\n> totally transparent, i.e. (taking the example of git.git):\n>\n> * You clone, and we just get objects from\n> https://github.com/cr-marcstevens/sha1collisiondetection.git too\n>\n> * The fact that we have:\n>\n> 160000 commit 855827c583bc30645ba427885caa40c5b81764d2 sha1collisiondetection\n> ...\n\nI am afraid that the story is not that simple (I wish it were).\n\nThere are at last two opposing ways submodules are to be used. The\noriginal motivation was to borrow an external project as part of\nyour project, and the way we use SHA1DC is fairly close to it (but\nnot quite). In the context of such a usage\n\n\tgit commit -m \"message\" --recurse-submodules\n\nwould often not be an appropriate operation. A message that is\nsuitable in the context of the entire project would not be in the\ncontext of the project that is bound to your project as a submodule,\nand for your changes to be reusable by the folks who own the borrowed\nproject to make sense, your change should be defensible on its own,\n\"it helps this project that happens to use you as a submodule\" by\nitself is not all that convincing.\n\nThe other way submodule often gets used is to artificially split a\nlogically single project into many subdirectories and make them into\nseparate repositories, the top-level project binding them as\nsubmodules. An submodule in such an arrangement may not even make\nsense as a standalone project---this pattern was only brought into\nusage because without the more recent inventions like lazy/partial\nclones and sparse checkouts, large projects did not fit within a\nsingle repository.\n\nWith such an arrangement, of course it makes perfect sense for\nthings like\n\n\tgit commit -m \"message\" --recurse-submodules\n\tgit grep --recurse-submodules\n\nto work as if you are working inside a single repository, by\ndefinition. You are splitting a logically single project into\nmultiple submodules as a workaround, and then still wanting to\ntreat them as a single project, after all.\n\nSupporting those who want to use \"collection of submodules as if it\nwere a single monolithic project\" well is a worthy goal, but I do\nnot think it is healthy to assume that is the only use and forget\nabout use cases that would benefit from a clear boundary at\nsubmodules (e.g. not sharing commit log message, a change at the\ntoplevel project may consist of multiple changes at the submoudle\nlevel, etc.).\n\nThanks.\n\n"},{"id":"422702","messageId":"20210423001539.4059524-1-emilyshaffer@google.com","threadId":"55458","inReplyTo":"20210408233936.533342-1-emilyshaffer@google.com","subject":"[RFC PATCH v2 0/4] share a config between submodule and superproject","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-04-23T00:15:35Z","receivedAt":"2021-04-23T00:15:45Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Hi all,\n\nWith the second iteration, I bothered to make the tests pass (oops) -\nand I'm actually fairly unhappy with what I found. Patches 2 and 3 of\nthis iteration clean up tests which were (accidentally) explicitly\nchecking that no child processes were invoked besides the ones they\nexpected, by way of strictly grepping traces (by test_cmp or by line\ncounting). I don't like those tests as they were - to me, they felt like\nthe brittle kind of white-box test - but I also got a stronger feeling\nthat adding an additional child process or two during every Git\ninvocation is a bad idea. I also saw a pretty significant increase in\ntest run time:\n\nAll tests successful.\nFiles=927, Tests=24148, 693 wallclock secs ( 8.71 usr 2.05 sys +\n3254.41 cusr 1571.78 csys = 4836.95 CPU)\nResult: PASS\n\nreal 11m33.029s\nuser 54m23.187s\nsys 26m13.857s\n\nvs before:\n\nAll tests successful.\nFiles=926, Tests=24138, 144 wallclock secs ( 8.14 usr 2.03 sys + 882.29\ncusr 535.61 csys = 1428.07 CPU)\nResult: PASS\n\nreal 2m24.116s\nuser 14m50.499s\nsys 8m57.649s\n\nAnd that's on a Linux machine; as I understand it, invoking child\nprocesses can be even more painful on other operating systems.\n\nIf we could be assured that this extra step (finding the parent's gitdir\nand checking that config) was only running when we know we're in a\nsubmodule, I'd be less worried, I think. And there are a couple other\npieces in the big picture submodule plan I sent[1] around which would\nrequire repos to answer \"am I a submodule?\"\n\nSo I think this series may need to be shelved pending an answer to that\nquestion - whether we *should* let submodules know they are\nsubmodules[2] to turn on more behavior, and if so, how we should\nimplement that.\n\n - Emily\n\n[1] https://lore.kernel.org/git/YHofmWcIAidkvJiD@google.com\n[2] https://lore.kernel.org/git/xmqqk0pbm6qt.fsf@gitster.g\n\nEmily Shaffer (4):\n config: rename \"submodule\" scope to \"gitmodules\"\n t1510-repo-setup: don't use exact matching on traces\n t7006-pager.sh: more lenient trace checking\n config: add 'config.superproject' file\n\n Documentation/git-config.txt | 21 +++++-\n builtin/config.c | 9 ++-\n config.c | 28 +++++++-\n config.h | 5 +-\n submodule-config.c | 2 +-\n submodule.c | 29 +++++++++\n submodule.h | 8 +++\n t/t1311-superproject-config.sh | 116 +++++++++++++++++++++++++++++++++\n t/t1510-repo-setup.sh | 2 +-\n t/t7006-pager.sh | 24 +++++--\n 10 files changed, 230 insertions(+), 14 deletions(-)\n create mode 100755 t/t1311-superproject-config.sh\n\n-- \n2.31.1.498.g6c1eba8ee3d-goog\n\n"},{"id":"422703","messageId":"20210423001539.4059524-2-emilyshaffer@google.com","threadId":"55458","inReplyTo":"20210423001539.4059524-1-emilyshaffer@google.com","subject":"[RFC PATCH v2 1/4] config: rename \"submodule\" scope to \"gitmodules\"","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-04-23T00:15:36Z","receivedAt":"2021-04-23T00:15:48Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"To prepare for the addition of a new config scope which only applies to\nsubmodule projects, disambiguate \"CONFIG_SCOPE_SUBMODULES\". This scope\nrefers to configs gathered from the .gitmodules file in the\nsuperproject, so just call it \"gitmodules.\"\n\nSigned-off-by: Emily Shaffer \n---\n config.c | 4 ++--\n config.h | 2 +-\n submodule-config.c | 2 +-\n 3 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex 870d9534de..c8426ef3f3 100644\n--- a/config.c\n+++ b/config.c\n@@ -3499,8 +3499,8 @@ const char *config_scope_name(enum config_scope scope)\n \t\treturn \"worktree\";\n \tcase CONFIG_SCOPE_COMMAND:\n \t\treturn \"command\";\n-\tcase CONFIG_SCOPE_SUBMODULE:\n-\t\treturn \"submodule\";\n+\tcase CONFIG_SCOPE_GITMODULES:\n+\t\treturn \"gitmodules\";\n \tdefault:\n \t\treturn \"unknown\";\n \t}\ndiff --git a/config.h b/config.h\nindex 19a9adbaa9..535f5517b8 100644\n--- a/config.h\n+++ b/config.h\n@@ -42,7 +42,7 @@ enum config_scope {\n \tCONFIG_SCOPE_LOCAL,\n \tCONFIG_SCOPE_WORKTREE,\n \tCONFIG_SCOPE_COMMAND,\n-\tCONFIG_SCOPE_SUBMODULE,\n+\tCONFIG_SCOPE_GITMODULES,\n };\n const char *config_scope_name(enum config_scope scope);\n \ndiff --git a/submodule-config.c b/submodule-config.c\nindex f502505566..0e435e6fdd 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -637,7 +637,7 @@ static void config_from_gitmodules(config_fn_t fn, struct repository *repo, void\n {\n \tif (repo->worktree) {\n \t\tstruct git_config_source config_source = {\n-\t\t\t0, .scope = CONFIG_SCOPE_SUBMODULE\n+\t\t\t0, .scope = CONFIG_SCOPE_GITMODULES\n \t\t};\n \t\tconst struct config_options opts = { 0 };\n \t\tstruct object_id oid;\n-- \n2.31.1.498.g6c1eba8ee3d-goog\n\n"},{"id":"422704","messageId":"20210423001539.4059524-3-emilyshaffer@google.com","threadId":"55458","inReplyTo":"20210423001539.4059524-1-emilyshaffer@google.com","subject":"[RFC PATCH v2 2/4] t1510-repo-setup: don't use exact matching on traces","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-04-23T00:15:37Z","receivedAt":"2021-04-23T00:15:49Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Tests which interrogate the exact underlying behavior of the code under\ntest, instead of checking for the presence of desired side effects or\ncalls, are a known testing antipattern. They are flaky as they need to\nbe updated every time the underlying implementation changes.\n\nBy using 'grep --fixed-strings --file ' instead, we can check\nfor the positive presence of lines we are sure should be happening, and\nignore any additional things which may be happening around us (for\nexample, additional child processes which are occurring unrelated to the\ncode under test).\n\nSigned-off-by: Emily Shaffer \n---\n t/t1510-repo-setup.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh\nindex bbfe05b8e4..8bd4f54d03 100755\n--- a/t/t1510-repo-setup.sh\n+++ b/t/t1510-repo-setup.sh\n@@ -63,7 +63,7 @@ test_repo () {\n \t\trm -f trace &&\n \t\tGIT_TRACE_SETUP=\"$(pwd)/trace\" git symbolic-ref HEAD >/dev/null &&\n \t\tgrep '^setup: ' trace >result &&\n-\t\ttest_cmp expected result\n+\t\tgrep -Ff expected result\n \t)\n }\n \n-- \n2.31.1.498.g6c1eba8ee3d-goog\n\n"},{"id":"422705","messageId":"20210423001539.4059524-4-emilyshaffer@google.com","threadId":"55458","inReplyTo":"20210423001539.4059524-1-emilyshaffer@google.com","subject":"[RFC PATCH v2 3/4] t7006-pager.sh: more lenient trace checking","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-04-23T00:15:38Z","receivedAt":"2021-04-23T00:15:50Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"A number of tests in t7006-pager.sh are, as a side effect, checking that\n'git log' does not invoke any child processes besides the pager. There\nis no reason for that guarantee, and it is not explicitly the purpose of\nthese tests, so let's make the checking more intelligent and flexible.\n\nchild_start and child_exit events share a child ID - using that child\nID, we can try to disambiguate which child_exit belongs to which\nchild_start and limit our validation to only the pager's child process.\n\nSigned-off-by: Emily Shaffer \n---\n t/t7006-pager.sh | 24 ++++++++++++++++++------\n 1 file changed, 18 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nindex 0e7cf75435..ac2d91d56b 100755\n--- a/t/t7006-pager.sh\n+++ b/t/t7006-pager.sh\n@@ -676,7 +676,9 @@ test_expect_success TTY 'git returns SIGPIPE on early pager exit' '\n \t\ttest_terminal git log\n \tfi &&\n \n-\tgrep child_exit trace.normal >child-exits &&\n+\tPAGER_CHILD_ID=$(grep pager-used trace.normal | \\\n+\t\t\t sed -n \"s/child_start\\[\\([0-9]\\+\\)\\].*/\\1/p\") &&\n+\tgrep -F \"child_exit[\"$PAGER_CHILD_ID\"]\" trace.normal >child-exits &&\n \ttest_line_count = 1 child-exits &&\n \tgrep \" code:0 \" child-exits &&\n \ttest_path_is_file pager-used\n@@ -697,7 +699,9 @@ test_expect_success TTY 'git returns SIGPIPE on early pager non-zero exit' '\n \t\ttest_terminal git log\n \tfi &&\n \n-\tgrep child_exit trace.normal >child-exits &&\n+\tPAGER_CHILD_ID=$(grep pager-used trace.normal | \\\n+\t\t\t sed -n \"s/child_start\\[\\([0-9]\\+\\)\\].*/\\1/p\") &&\n+\tgrep -F \"child_exit[\"$PAGER_CHILD_ID\"]\" trace.normal >child-exits &&\n \ttest_line_count = 1 child-exits &&\n \tgrep \" code:1 \" child-exits &&\n \ttest_path_is_file pager-used\n@@ -718,7 +722,9 @@ test_expect_success TTY 'git discards pager non-zero exit without SIGPIPE' '\n \t\ttest_terminal git log\n \tfi &&\n \n-\tgrep child_exit trace.normal >child-exits &&\n+\tPAGER_CHILD_ID=$(grep pager-used trace.normal | \\\n+\t\t\t sed -n \"s/child_start\\[\\([0-9]\\+\\)\\].*/\\1/p\") &&\n+\tgrep -F \"child_exit[\"$PAGER_CHILD_ID\"]\" trace.normal >child-exits &&\n \ttest_line_count = 1 child-exits &&\n \tgrep \" code:1 \" child-exits &&\n \ttest_path_is_file pager-used\n@@ -739,7 +745,9 @@ test_expect_success TTY 'git discards nonexisting pager without SIGPIPE' '\n \t\ttest_terminal git log\n \tfi &&\n \n-\tgrep child_exit trace.normal >child-exits &&\n+\tPAGER_CHILD_ID=$(grep does-not-exist trace.normal | \\\n+\t\t\t sed -n \"s/child_start\\[\\([0-9]\\+\\)\\].*/\\1/p\") &&\n+\tgrep -F \"child_exit[\"$PAGER_CHILD_ID\"]\" trace.normal >child-exits &&\n \ttest_line_count = 1 child-exits &&\n \tgrep \" code:127 \" child-exits &&\n \ttest_path_is_file pager-used\n@@ -760,7 +768,9 @@ test_expect_success TTY 'git attempts to page to nonexisting pager command, gets\n \t\ttest_terminal git log\n \tfi &&\n \n-\tgrep child_exit trace.normal >child-exits &&\n+\tPAGER_CHILD_ID=$(grep does-not-exist trace.normal | \\\n+\t\t\t sed -n \"s/child_start\\[\\([0-9]\\+\\)\\].*/\\1/p\") &&\n+\tgrep -F \"child_exit[\"$PAGER_CHILD_ID\"]\" trace.normal >child-exits &&\n \ttest_line_count = 1 child-exits &&\n \tgrep \" code:-1 \" child-exits\n '\n@@ -780,7 +790,9 @@ test_expect_success TTY 'git returns SIGPIPE on propagated signals from pager' '\n \t\ttest_terminal git log\n \tfi &&\n \n-\tgrep child_exit trace.normal >child-exits &&\n+\tPAGER_CHILD_ID=$(grep pager-used trace.normal | \\\n+\t\t\t sed -n \"s/child_start\\[\\([0-9]\\+\\)\\].*/\\1/p\") &&\n+\tgrep -F \"child_exit[\"$PAGER_CHILD_ID\"]\" trace.normal >child-exits &&\n \ttest_line_count = 1 child-exits &&\n \tgrep \" code:143 \" child-exits &&\n \ttest_path_is_file pager-used\n-- \n2.31.1.498.g6c1eba8ee3d-goog\n\n"},{"id":"422706","messageId":"20210423001539.4059524-5-emilyshaffer@google.com","threadId":"55458","inReplyTo":"20210423001539.4059524-1-emilyshaffer@google.com","subject":"[RFC PATCH v2 4/4] config: add 'config.superproject' file","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-04-23T00:15:39Z","receivedAt":"2021-04-23T00:15:52Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Some configs, such as wrapper directives like gerrit.createChangeId, or\nforthcoming hook configs, should apply to a superproject as well as all\nits submodules. It may not be appropriate to apply them globally - for\nexample, if the user also contributes to many projects which do not use\nthe configs necessary for one project-with-submodules - and it may be\nburdensome to apply them locally to the superproject and each of its\nsubmodules. Even if the user runs 'git submodule foreach \"git config\n--local foo.bar', if a new submodule is added later on, that config is\nnot applied to the new submodule.\n\nIt is also inappropriate to share the entire superproject config, since\nsome items - like remote URLs or partial-clone filters - would not apply\nto a submodule.\n\nTo make life easier for projects with many submodules, then, create a\nnew \"config.superproject\" config scope, which is included in the config\nparse for the superproject as well as for all the submodules of that\nsuperproject.\n\nFor the superproject, this new config file is equally local to the local\nconfig; for the submodule, the new config file is less local than the\nlocal config. So let's include it directly before the local config\nduring the config parse.\n\nSigned-off-by: Emily Shaffer \n---\n\nNotes:\n Since v1:\n \n - Used test_when_finished liberally in tests instead of bare cleanup steps\n - Fixed some nits regarding xstrdup instead of strbuf_detach.\n \n One thing that I thought about but did not implement: rather than finding the\n path to the superproject's gitdir, you could imagine gathering the config by\n making a call out to 'git -C ../ config' - but on second thought, it seems like\n that will make it harder to edit. However, if we don't want to be able to edit\n superproject config from a submodule, that might be okay... (This approach could\n make 'git config --show-origin' harder to implement, though, I think.) So I did\n not make any changes about that.\n\n Documentation/git-config.txt | 21 +++++-\n builtin/config.c | 9 ++-\n config.c | 24 +++++++\n config.h | 3 +\n submodule.c | 29 +++++++++\n submodule.h | 8 +++\n t/t1311-superproject-config.sh | 116 +++++++++++++++++++++++++++++++++\n 7 files changed, 207 insertions(+), 3 deletions(-)\n create mode 100755 t/t1311-superproject-config.sh\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex 4b4cc5c5e8..a33136fb08 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -48,7 +48,7 @@ unset an existing `--type` specifier with `--no-type`.\n \n When reading, the values are read from the system, global and\n repository local configuration files by default, and options\n-`--system`, `--global`, `--local`, `--worktree` and\n+`--system`, `--global`, `--superproject`, `--local`, `--worktree` and\n `--file ` can be used to tell the command to read from only\n that location (see <>).\n \n@@ -127,6 +127,17 @@ rather than from all available files.\n +\n See also <>.\n \n+--superproject::\n+\tFor writing options: write to the superproject's\n+\t`.git/config.superproject` file, even if run from a submodule of that\n+\tsuperproject.\n++\n+For reading options: read only from the superproject's\n+`.git/config.superproject` file, even if run from a submodule of that\n+superproject, rather than from all available files.\n++\n+See also <>.\n+\n --local::\n \tFor writing options: write to the repository `.git/config` file.\n \tThis is the default behavior.\n@@ -283,7 +294,7 @@ The default is to use a pager.\n FILES\n -----\n \n-If not set explicitly with `--file`, there are four files where\n+If not set explicitly with `--file`, there are five files where\n 'git config' will search for configuration options:\n \n $(prefix)/etc/gitconfig::\n@@ -301,6 +312,12 @@ $XDG_CONFIG_HOME/git/config::\n \tUser-specific configuration file. Also called \"global\"\n \tconfiguration file.\n \n+$GIT_DIR/config.superproject::\n+\tWhen `git config` is run from a project which is a submodule of another\n+\tproject, that superproject's $GIT_DIR will be used. Use this config file\n+\tto set configurations which need to be the same across a superproject\n+\tand all its submodules.\n+\n $GIT_DIR/config::\n \tRepository specific configuration file.\n \ndiff --git a/builtin/config.c b/builtin/config.c\nindex f71fa39b38..f38a6823c7 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -26,7 +26,7 @@ static char key_delim = ' ';\n static char term = '\\n';\n \n static int use_global_config, use_system_config, use_local_config;\n-static int use_worktree_config;\n+static int use_worktree_config, use_superproject_config;\n static struct git_config_source given_config_source;\n static int actions, type;\n static char *default_value;\n@@ -130,6 +130,8 @@ static struct option builtin_config_options[] = {\n \tOPT_GROUP(N_(\"Config file location\")),\n \tOPT_BOOL(0, \"global\", &use_global_config, N_(\"use global config file\")),\n \tOPT_BOOL(0, \"system\", &use_system_config, N_(\"use system config file\")),\n+\tOPT_BOOL(0, \"superproject\",\n+\t\t &use_superproject_config, N_(\"use superproject config file\")),\n \tOPT_BOOL(0, \"local\", &use_local_config, N_(\"use repository config file\")),\n \tOPT_BOOL(0, \"worktree\", &use_worktree_config, N_(\"use per-worktree config file\")),\n \tOPT_STRING('f', \"file\", &given_config_source.file, N_(\"file\"), N_(\"use given config file\")),\n@@ -697,6 +699,11 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \telse if (use_system_config) {\n \t\tgiven_config_source.file = git_etc_gitconfig();\n \t\tgiven_config_source.scope = CONFIG_SCOPE_SYSTEM;\n+\t} else if (use_superproject_config) {\n+\t\tstruct strbuf superproject_cfg = STRBUF_INIT;\n+\t\tgit_config_superproject(&superproject_cfg, get_git_dir());\n+\t\tgiven_config_source.file = strbuf_detach(&superproject_cfg, NULL);\n+\t\tgiven_config_source.scope = CONFIG_SCOPE_SUPERPROJECT;\n \t} else if (use_local_config) {\n \t\tgiven_config_source.file = git_pathdup(\"config\");\n \t\tgiven_config_source.scope = CONFIG_SCOPE_LOCAL;\ndiff --git a/config.c b/config.c\nindex c8426ef3f3..3f62cd815b 100644\n--- a/config.c\n+++ b/config.c\n@@ -21,6 +21,7 @@\n #include \"dir.h\"\n #include \"color.h\"\n #include \"refs.h\"\n+#include \"submodule.h\"\n \n struct config_source {\n \tstruct config_source *prev;\n@@ -1838,6 +1839,15 @@ const char *git_etc_gitconfig(void)\n \treturn system_wide;\n }\n \n+void git_config_superproject(struct strbuf *sb, const char *gitdir)\n+{\n+\tif (!get_superproject_gitdir(sb))\n+\t\t/* not a submodule */\n+\t\tstrbuf_addstr(sb, gitdir);\n+\n+\tstrbuf_addstr(sb, \"/config.superproject\");\n+}\n+\n /*\n * Parse environment variable 'k' as a boolean (in various\n * possible spellings); if missing, use the default value 'def'.\n@@ -1895,6 +1905,17 @@ static int do_git_config_sequence(const struct config_options *opts,\n \tif (user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK))\n \t\tret += git_config_from_file(fn, user_config, data);\n \n+\tcurrent_parsing_scope = CONFIG_SCOPE_SUPERPROJECT;\n+\tif (opts->git_dir && !opts->ignore_superproject) {\n+\n+\t\tstruct strbuf superproject_gitdir = STRBUF_INIT;\n+\t\tgit_config_superproject(&superproject_gitdir, opts->git_dir);\n+\t\tif (!access_or_die(superproject_gitdir.buf, R_OK, 0))\n+\t\t\tret += git_config_from_file(fn, superproject_gitdir.buf, data);\n+\n+\t\tstrbuf_release(&superproject_gitdir);\n+\t}\n+\n \tcurrent_parsing_scope = CONFIG_SCOPE_LOCAL;\n \tif (!opts->ignore_repo && repo_config &&\n \t !access_or_die(repo_config, R_OK, 0))\n@@ -2013,6 +2034,7 @@ void read_very_early_config(config_fn_t cb, void *data)\n \n \topts.respect_includes = 1;\n \topts.ignore_repo = 1;\n+\topts.ignore_superproject = 1;\n \topts.ignore_worktree = 1;\n \topts.ignore_cmdline = 1;\n \topts.system_gently = 1;\n@@ -3501,6 +3523,8 @@ const char *config_scope_name(enum config_scope scope)\n \t\treturn \"command\";\n \tcase CONFIG_SCOPE_GITMODULES:\n \t\treturn \"gitmodules\";\n+\tcase CONFIG_SCOPE_SUPERPROJECT:\n+\t\treturn \"superproject\";\n \tdefault:\n \t\treturn \"unknown\";\n \t}\ndiff --git a/config.h b/config.h\nindex 535f5517b8..b42e1d13eb 100644\n--- a/config.h\n+++ b/config.h\n@@ -43,6 +43,7 @@ enum config_scope {\n \tCONFIG_SCOPE_WORKTREE,\n \tCONFIG_SCOPE_COMMAND,\n \tCONFIG_SCOPE_GITMODULES,\n+\tCONFIG_SCOPE_SUPERPROJECT,\n };\n const char *config_scope_name(enum config_scope scope);\n \n@@ -84,6 +85,7 @@ typedef int (*config_parser_event_fn_t)(enum config_event_t type,\n struct config_options {\n \tunsigned int respect_includes : 1;\n \tunsigned int ignore_repo : 1;\n+\tunsigned int ignore_superproject : 1;\n \tunsigned int ignore_worktree : 1;\n \tunsigned int ignore_cmdline : 1;\n \tunsigned int system_gently : 1;\n@@ -319,6 +321,7 @@ int git_config_rename_section_in_file(const char *, const char *, const char *);\n int git_config_copy_section(const char *, const char *);\n int git_config_copy_section_in_file(const char *, const char *, const char *);\n const char *git_etc_gitconfig(void);\n+void git_config_superproject(struct strbuf *, const char *);\n int git_env_bool(const char *, int);\n unsigned long git_env_ulong(const char *, unsigned long);\n int git_config_system(void);\ndiff --git a/submodule.c b/submodule.c\nindex 9767ba9893..92b00f8697 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -2178,6 +2178,35 @@ void absorb_git_dir_into_superproject(const char *path,\n \t}\n }\n \n+int get_superproject_gitdir(struct strbuf *buf)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tint rc = 0;\n+\n+\t/* NEEDSWORK: this call also calls out to a subprocess! */\n+\trc = get_superproject_working_tree(&sb);\n+\tstrbuf_release(&sb);\n+\n+\tif (!rc)\n+\t\treturn rc;\n+\n+\tprepare_submodule_repo_env_no_git_dir(&cp.env_array);\n+\n+\tstrvec_pushl(&cp.args, \"-C\", \"..\", \"rev-parse\", \"--absolute-git-dir\", NULL);\n+\tcp.git_cmd = 1;\n+\n+\trc = capture_command(&cp, buf, 0);\n+\tstrbuf_trim_trailing_newline(buf);\n+\n+\t/* leave buf empty if we didn't have a superproject gitdir */\n+\tif (rc)\n+\t\tstrbuf_reset(buf);\n+\n+\t/* rc reflects the exit code of the rev-parse; invert into a bool */\n+\treturn !rc;\n+}\n+\n int get_superproject_working_tree(struct strbuf *buf)\n {\n \tstruct child_process cp = CHILD_PROCESS_INIT;\ndiff --git a/submodule.h b/submodule.h\nindex 4ac6e31cf1..cc89d5e3fa 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -149,6 +149,14 @@ void prepare_submodule_repo_env(struct strvec *out);\n void absorb_git_dir_into_superproject(const char *path,\n \t\t\t\t unsigned flags);\n \n+/*\n+ * Return the gitdir of the superproject, which this project is a submodule of.\n+ * If this repository is a submodule, return 1 and fill buf with the absolute\n+ * path to the superproject's gitdir. If this repository is not a submodule of\n+ * another repository, return 0 and leave buf untouched.\n+ */\n+int get_superproject_gitdir(struct strbuf *buf);\n+\n /*\n * Return the absolute path of the working tree of the superproject, which this\n * project is a submodule of. If this repository is not a submodule of\ndiff --git a/t/t1311-superproject-config.sh b/t/t1311-superproject-config.sh\nnew file mode 100755\nindex 0000000000..74b6ca2cac\n--- /dev/null\n+++ b/t/t1311-superproject-config.sh\n@@ -0,0 +1,116 @@\n+#!/bin/sh\n+\n+test_description='Test git config --superproject in different settings'\n+\n+. ./test-lib.sh\n+\n+# follow t7400's example and use the trash dir repo as a submodule to add\n+submodurl=$(pwd -P)\n+\n+# since only the configs are modified, set up the repo structure only once\n+test_expect_success 'setup repo structure' '\n+\ttest_commit \"base\" &&\n+\tgit submodule add \"${submodurl}\" sub/ &&\n+\tgit commit -m \"add a submodule\"\n+'\n+\n+test_expect_success 'superproject config applies to super and submodule' '\n+\tcat >.git/config.superproject <<-EOF &&\n+\t[foo]\n+\t\tbar = baz\n+\tEOF\n+\ttest_when_finished rm .git/config.superproject &&\n+\n+\tgit config --get foo.bar &&\n+\tgit -C sub config --get foo.bar\n+\n+'\n+\n+test_expect_success 'can add from super or sub' '\n+\tgit config --superproject apple.species honeycrisp &&\n+\tgit -C sub config --superproject banana.species cavendish &&\n+\ttest_when_finished rm .git/config.superproject &&\n+\n+\tcat >expect <<-EOF &&\n+\tapple.species=honeycrisp\n+\tbanana.species=cavendish\n+\tEOF\n+\n+\tgit config --list >actual &&\n+\tgrep -Ff expect actual &&\n+\n+\tgit -C sub config --list >actual &&\n+\tgrep -Ff expect actual\n+'\n+\n+test_expect_success 'can --unset from super or sub' '\n+\tgit config --superproject apple.species honeycrisp &&\n+\tgit -C sub config --superproject banana.species cavendish &&\n+\ttest_when_finished rm .git/config.superproject &&\n+\n+\tgit config --unset --superproject banana.species &&\n+\tgit -C sub config --unset --superproject apple.species\n+'\n+\n+test_expect_success 'can --edit superproject config' '\n+\ttest_config core.editor \"echo [foo]bar=baz >\" &&\n+\tgit config --edit --superproject &&\n+\ttest_when_finished rm .git/config.superproject &&\n+\n+\tgit config --get foo.bar\n+'\n+\n+test_expect_success 'can --show-origin the superproject config' '\n+\tgit config --superproject --add foo.bar baz &&\n+\ttest_when_finished rm .git/config.superproject &&\n+\n+\tgit config --list --show-origin >actual &&\n+\tgrep -F \"config.superproject\" actual\n+'\n+\n+test_expect_success 'can --show-scope the superproject config' '\n+\tgit config --superproject --add foo.bar baz &&\n+\ttest_when_finished rm .git/config.superproject &&\n+\n+\tgit config --list --show-scope >actual &&\n+\tgrep \"superproject\" actual\n+'\n+\n+test_expect_success 'pre-existing config applies to new submodule' '\n+\tgit config --superproject --add foo.bar baz &&\n+\ttest_when_finished rm .git/config.superproject &&\n+\n+\tgit submodule add \"${submodurl}\" sub2/ &&\n+\ttest_when_finished rm -fr sub2 &&\n+\tgit commit -m \"add a second submodule\" &&\n+\ttest_when_finished git reset HEAD^ &&\n+\n+\tgit -C sub2 config --get foo.bar\n+'\n+\n+# NEEDSWORK: submodule.c:get_superproject_working_tree doesn't support worktree\n+test_expect_failure 'worktrees can still access config.superproject' '\n+\tgit config --superproject --add foo.bar baz &&\n+\ttest_when_finished rm .git/config.superproject &&\n+\n+\tgit worktree add wt &&\n+\ttest_when_finished git worktree remove wt &&\n+\t(\n+\t\tcd wt &&\n+\t\tgit config --get foo.bar\n+\t)\n+'\n+\n+# This test deletes the submodule! Keep it at the end of the test suite.\n+test_expect_success 'config.superproject works even with no submodules' '\n+\t# get rid of the submodule\n+\tgit reset HEAD^ &&\n+\trm -fr sub &&\n+\n+\tgit config --superproject --add foo.bar baz &&\n+\ttest_when_finished rm .git/config.superproject &&\n+\n+\tgit config --get foo.bar\n+'\n+\n+test_done\n-- \n2.31.1.498.g6c1eba8ee3d-goog\n\n"},{"id":"422740","messageId":"8d27af1f-7fa3-e3a7-f042-2559fc24bc18@gmail.com","threadId":"55458","inReplyTo":"20210423001539.4059524-2-emilyshaffer@google.com","subject":"Re: [RFC PATCH v2 1/4] config: rename \"submodule\" scope to \"gitmodules\"","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-04-23T09:46:23Z","receivedAt":"2021-04-23T09:46:29Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Emily\n\nOn 23/04/2021 01:15, Emily Shaffer wrote:\n> To prepare for the addition of a new config scope which only applies to\n> submodule projects, disambiguate \"CONFIG_SCOPE_SUBMODULES\". This scope\n> refers to configs gathered from the .gitmodules file in the\n> superproject, so just call it \"gitmodules.\"\n\nAm I right in thinking that this changes the output of `git config \n--show-scope`? If so I'm not sure it's a good idea as it would break any \nscripts that are parsing that output\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Emily Shaffer \n> ---\n> config.c | 4 ++--\n> config.h | 2 +-\n> submodule-config.c | 2 +-\n> 3 files changed, 4 insertions(+), 4 deletions(-)\n> \n> diff --git a/config.c b/config.c\n> index 870d9534de..c8426ef3f3 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -3499,8 +3499,8 @@ const char *config_scope_name(enum config_scope scope)\n> \t\treturn \"worktree\";\n> \tcase CONFIG_SCOPE_COMMAND:\n> \t\treturn \"command\";\n> -\tcase CONFIG_SCOPE_SUBMODULE:\n> -\t\treturn \"submodule\";\n> +\tcase CONFIG_SCOPE_GITMODULES:\n> +\t\treturn \"gitmodules\";\n> \tdefault:\n> \t\treturn \"unknown\";\n> \t}\n> diff --git a/config.h b/config.h\n> index 19a9adbaa9..535f5517b8 100644\n> --- a/config.h\n> +++ b/config.h\n> @@ -42,7 +42,7 @@ enum config_scope {\n> \tCONFIG_SCOPE_LOCAL,\n> \tCONFIG_SCOPE_WORKTREE,\n> \tCONFIG_SCOPE_COMMAND,\n> -\tCONFIG_SCOPE_SUBMODULE,\n> +\tCONFIG_SCOPE_GITMODULES,\n> };\n> const char *config_scope_name(enum config_scope scope);\n> \n> diff --git a/submodule-config.c b/submodule-config.c\n> index f502505566..0e435e6fdd 100644\n> --- a/submodule-config.c\n> +++ b/submodule-config.c\n> @@ -637,7 +637,7 @@ static void config_from_gitmodules(config_fn_t fn, struct repository *repo, void\n> {\n> \tif (repo->worktree) {\n> \t\tstruct git_config_source config_source = {\n> -\t\t\t0, .scope = CONFIG_SCOPE_SUBMODULE\n> +\t\t\t0, .scope = CONFIG_SCOPE_GITMODULES\n> \t\t};\n> \t\tconst struct config_options opts = { 0 };\n> \t\tstruct object_id oid;\n> \n\n"},{"id":"422741","messageId":"f4c3688e-8316-7f66-3bcb-d8aaeef3a7d3@gmail.com","threadId":"55458","inReplyTo":"20210423001539.4059524-4-emilyshaffer@google.com","subject":"Re: [RFC PATCH v2 3/4] t7006-pager.sh: more lenient trace checking","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-04-23T09:54:55Z","receivedAt":"2021-04-23T09:55:00Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Emily\n\nOn 23/04/2021 01:15, Emily Shaffer wrote:\n> A number of tests in t7006-pager.sh are, as a side effect, checking that\n> 'git log' does not invoke any child processes besides the pager. There\n> is no reason for that guarantee, and it is not explicitly the purpose of\n> these tests, so let's make the checking more intelligent and flexible.\n> \n> child_start and child_exit events share a child ID - using that child\n> ID, we can try to disambiguate which child_exit belongs to which\n> child_start and limit our validation to only the pager's child process.\n\nI've not looked at this test file properly but if we want to check that \nthe pager is invoked can we use a script that writes a file when it has \nbeen run as the pager and just cleanup that file at the end of each test \ncase?\n\n> Signed-off-by: Emily Shaffer \n> ---\n> t/t7006-pager.sh | 24 ++++++++++++++++++------\n> 1 file changed, 18 insertions(+), 6 deletions(-)\n> \n> diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\n> index 0e7cf75435..ac2d91d56b 100755\n> --- a/t/t7006-pager.sh\n> +++ b/t/t7006-pager.sh\n> @@ -676,7 +676,9 @@ test_expect_success TTY 'git returns SIGPIPE on early pager exit' '\n> \t\ttest_terminal git log\n> \tfi &&\n> \n> -\tgrep child_exit trace.normal >child-exits &&\n> +\tPAGER_CHILD_ID=$(grep pager-used trace.normal | \\\n> +\t\t\t sed -n \"s/child_start\\[\\([0-9]\\+\\)\\].*/\\1/p\") &&\n\nIf you want to save a process you could use sed to do the job of the \ngrep command. I think this should do it\n\nsed -n -e \"/child_exit/ {\" -e \"s/child_start\\[\\([0-9]\\+\\)\\].*/\\1/p\" -e \"}\"\n\n> +\tgrep -F \"child_exit[\"$PAGER_CHILD_ID\"]\" trace.normal >child-exits &&\n\nWhy is $PAGER_CHILD_ID unquoted?\n\nBest Wishes\n\nPhillip\n\n> \ttest_line_count = 1 child-exits &&\n> \tgrep \" code:0 \" child-exits &&\n> \ttest_path_is_file pager-used\n> @@ -697,7 +699,9 @@ test_expect_success TTY 'git returns SIGPIPE on early pager non-zero exit' '\n> \t\ttest_terminal git log\n> \tfi &&\n> \n> -\tgrep child_exit trace.normal >child-exits &&\n> +\tPAGER_CHILD_ID=$(grep pager-used trace.normal | \\\n> +\t\t\t sed -n \"s/child_start\\[\\([0-9]\\+\\)\\].*/\\1/p\") &&\n> +\tgrep -F \"child_exit[\"$PAGER_CHILD_ID\"]\" trace.normal >child-exits &&\n> \ttest_line_count = 1 child-exits &&\n> \tgrep \" code:1 \" child-exits &&\n> \ttest_path_is_file pager-used\n> @@ -718,7 +722,9 @@ test_expect_success TTY 'git discards pager non-zero exit without SIGPIPE' '\n> \t\ttest_terminal git log\n> \tfi &&\n> \n> -\tgrep child_exit trace.normal >child-exits &&\n> +\tPAGER_CHILD_ID=$(grep pager-used trace.normal | \\\n> +\t\t\t sed -n \"s/child_start\\[\\([0-9]\\+\\)\\].*/\\1/p\") &&\n> +\tgrep -F \"child_exit[\"$PAGER_CHILD_ID\"]\" trace.normal >child-exits &&\n> \ttest_line_count = 1 child-exits &&\n> \tgrep \" code:1 \" child-exits &&\n> \ttest_path_is_file pager-used\n> @@ -739,7 +745,9 @@ test_expect_success TTY 'git discards nonexisting pager without SIGPIPE' '\n> \t\ttest_terminal git log\n> \tfi &&\n> \n> -\tgrep child_exit trace.normal >child-exits &&\n> +\tPAGER_CHILD_ID=$(grep does-not-exist trace.normal | \\\n> +\t\t\t sed -n \"s/child_start\\[\\([0-9]\\+\\)\\].*/\\1/p\") &&\n> +\tgrep -F \"child_exit[\"$PAGER_CHILD_ID\"]\" trace.normal >child-exits &&\n> \ttest_line_count = 1 child-exits &&\n> \tgrep \" code:127 \" child-exits &&\n> \ttest_path_is_file pager-used\n> @@ -760,7 +768,9 @@ test_expect_success TTY 'git attempts to page to nonexisting pager command, gets\n> \t\ttest_terminal git log\n> \tfi &&\n> \n> -\tgrep child_exit trace.normal >child-exits &&\n> +\tPAGER_CHILD_ID=$(grep does-not-exist trace.normal | \\\n> +\t\t\t sed -n \"s/child_start\\[\\([0-9]\\+\\)\\].*/\\1/p\") &&\n> +\tgrep -F \"child_exit[\"$PAGER_CHILD_ID\"]\" trace.normal >child-exits &&\n> \ttest_line_count = 1 child-exits &&\n> \tgrep \" code:-1 \" child-exits\n> '\n> @@ -780,7 +790,9 @@ test_expect_success TTY 'git returns SIGPIPE on propagated signals from pager' '\n> \t\ttest_terminal git log\n> \tfi &&\n> \n> -\tgrep child_exit trace.normal >child-exits &&\n> +\tPAGER_CHILD_ID=$(grep pager-used trace.normal | \\\n> +\t\t\t sed -n \"s/child_start\\[\\([0-9]\\+\\)\\].*/\\1/p\") &&\n> +\tgrep -F \"child_exit[\"$PAGER_CHILD_ID\"]\" trace.normal >child-exits &&\n> \ttest_line_count = 1 child-exits &&\n> \tgrep \" code:143 \" child-exits &&\n> \ttest_path_is_file pager-used\n> \n\n"},{"id":"422742","messageId":"c31c30f0-2613-0a12-1025-6043ef5f2962@gmail.com","threadId":"55458","inReplyTo":"20210423001539.4059524-3-emilyshaffer@google.com","subject":"Re: [RFC PATCH v2 2/4] t1510-repo-setup: don't use exact matching on traces","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-04-23T09:59:12Z","receivedAt":"2021-04-23T09:59:18Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Emily\n\nOn 23/04/2021 01:15, Emily Shaffer wrote:\n> Tests which interrogate the exact underlying behavior of the code under\n> test, instead of checking for the presence of desired side effects or\n> calls, are a known testing antipattern. They are flaky as they need to\n> be updated every time the underlying implementation changes.\n> \n> By using 'grep --fixed-strings --file ' instead, we can check\n> for the positive presence of lines we are sure should be happening, and\n> ignore any additional things which may be happening around us (for\n> example, additional child processes which are occurring unrelated to the\n> code under test).\n >\n> Signed-off-by: Emily Shaffer \n> ---\n> t/t1510-repo-setup.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh\n> index bbfe05b8e4..8bd4f54d03 100755\n> --- a/t/t1510-repo-setup.sh\n> +++ b/t/t1510-repo-setup.sh\n> @@ -63,7 +63,7 @@ test_repo () {\n> \t\trm -f trace &&\n> \t\tGIT_TRACE_SETUP=\"$(pwd)/trace\" git symbolic-ref HEAD >/dev/null &&\n> \t\tgrep '^setup: ' trace >result &&\n> -\t\ttest_cmp expected result\n> +\t\tgrep -Ff expected result\n\nIf there is more than one line in expected (it's not clear from the \nlimited context if there is) then grep will succeed if any of the lines \nmatch rather than requiring all the lines to match as test_cmp does.\n\nBest wishes\n\nPhillip\n\n> \t)\n> }\n> \n> \n\n"},{"id":"422748","messageId":"nycvar.QRO.7.76.6.2104231405210.54@tvgsbejvaqbjf.bet","threadId":"55458","inReplyTo":"20210423001539.4059524-5-emilyshaffer@google.com","subject":"Re: [RFC PATCH v2 4/4] config: add 'config.superproject' file","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-04-23T12:08:29Z","receivedAt":"2021-04-23T12:08:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Emily,\n\nOn Thu, 22 Apr 2021, Emily Shaffer wrote:\n\n> Some configs, such as wrapper directives like gerrit.createChangeId, or\n> forthcoming hook configs, should apply to a superproject as well as all\n> its submodules. It may not be appropriate to apply them globally - for\n> example, if the user also contributes to many projects which do not use\n> the configs necessary for one project-with-submodules - and it may be\n> burdensome to apply them locally to the superproject and each of its\n> submodules. Even if the user runs 'git submodule foreach \"git config\n> --local foo.bar', if a new submodule is added later on, that config is\n> not applied to the new submodule.\n>\n> It is also inappropriate to share the entire superproject config, since\n> some items - like remote URLs or partial-clone filters - would not apply\n> to a submodule.\n>\n> To make life easier for projects with many submodules, then, create a\n> new \"config.superproject\" config scope, which is included in the config\n> parse for the superproject as well as for all the submodules of that\n> superproject.\n>\n> For the superproject, this new config file is equally local to the local\n> config; for the submodule, the new config file is less local than the\n> local config. So let's include it directly before the local config\n> during the config parse.\n>\n> Signed-off-by: Emily Shaffer \n> ---\n>\n> Notes:\n> Since v1:\n>\n> - Used test_when_finished liberally in tests instead of bare cleanup steps\n> - Fixed some nits regarding xstrdup instead of strbuf_detach.\n>\n> One thing that I thought about but did not implement: rather than finding the\n> path to the superproject's gitdir, you could imagine gathering the config by\n> making a call out to 'git -C ../ config' - but on second thought, it seems like\n> that will make it harder to edit. However, if we don't want to be able to edit\n> superproject config from a submodule, that might be okay... (This approach could\n> make 'git config --show-origin' harder to implement, though, I think.) So I did\n> not make any changes about that.\n\nHmm. Have you thought about worktrees of subprojects that happen to be\noutside the superproject's directory tree?\n\nI also wonder whether it is necessary to change Git at all, as a\nwell-crafted `[includeIf \"gitdir:/path/to/superproject/**\"]` should do the\ntrick, but without complicating the config code even further.\n\nCiao,\nJohannes\n"},{"id":"422750","messageId":"27d2ffe7-6f5b-726e-eed0-438a4efb8f9f@gmail.com","threadId":"55458","inReplyTo":"f4c3688e-8316-7f66-3bcb-d8aaeef3a7d3@gmail.com","subject":"Re: [RFC PATCH v2 3/4] t7006-pager.sh: more lenient trace checking","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-04-23T12:45:07Z","receivedAt":"2021-04-23T12:45:14Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Emily\n\nOn 23/04/2021 10:54, Phillip Wood wrote:\n> Hi Emily\n> [...]\n>> diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\n>> index 0e7cf75435..ac2d91d56b 100755\n>> --- a/t/t7006-pager.sh\n>> +++ b/t/t7006-pager.sh\n>> @@ -676,7 +676,9 @@ test_expect_success TTY 'git returns SIGPIPE on \n>> early pager exit' '\n>>           test_terminal git log\n>>       fi &&\n>> -    grep child_exit trace.normal >child-exits &&\n>> +    PAGER_CHILD_ID=$(grep pager-used trace.normal | \\\n>> +             sed -n \"s/child_start\\[\\([0-9]\\+\\)\\].*/\\1/p\") &&\n> \n> If you want to save a process you could use sed to do the job of the \n> grep command. I think this should do it\n> \n> sed -n -e \"/child_exit/ {\" -e \"s/child_start\\[\\([0-9]\\+\\)\\].*/\\1/p\" -e \"}\"\n\nI must have been half asleep when I wrote that, there is no need for the \nbraces and the initial match is wrong it should be\n\nsed -n \"/pager-used/s/child_start\\[\\([0-9]\\+\\)\\].*/\\1/p\"\n\n\nBest Wishes\n\nPhillip\n\n>> +    grep -F \"child_exit[\"$PAGER_CHILD_ID\"]\" trace.normal >child-exits &&\n> \n> Why is $PAGER_CHILD_ID unquoted?\n> \n> Best Wishes\n> \n> Phillip\n> \n>>       test_line_count = 1 child-exits &&\n>>       grep \" code:0 \" child-exits &&\n>>       test_path_is_file pager-used\n>> @@ -697,7 +699,9 @@ test_expect_success TTY 'git returns SIGPIPE on \n>> early pager non-zero exit' '\n>>           test_terminal git log\n>>       fi &&\n>> -    grep child_exit trace.normal >child-exits &&\n>> +    PAGER_CHILD_ID=$(grep pager-used trace.normal | \\\n>> +             sed -n \"s/child_start\\[\\([0-9]\\+\\)\\].*/\\1/p\") &&\n>> +    grep -F \"child_exit[\"$PAGER_CHILD_ID\"]\" trace.normal >child-exits &&\n>>       test_line_count = 1 child-exits &&\n>>       grep \" code:1 \" child-exits &&\n>>       test_path_is_file pager-used\n>> @@ -718,7 +722,9 @@ test_expect_success TTY 'git discards pager \n>> non-zero exit without SIGPIPE' '\n>>           test_terminal git log\n>>       fi &&\n>> -    grep child_exit trace.normal >child-exits &&\n>> +    PAGER_CHILD_ID=$(grep pager-used trace.normal | \\\n>> +             sed -n \"s/child_start\\[\\([0-9]\\+\\)\\].*/\\1/p\") &&\n>> +    grep -F \"child_exit[\"$PAGER_CHILD_ID\"]\" trace.normal >child-exits &&\n>>       test_line_count = 1 child-exits &&\n>>       grep \" code:1 \" child-exits &&\n>>       test_path_is_file pager-used\n>> @@ -739,7 +745,9 @@ test_expect_success TTY 'git discards nonexisting \n>> pager without SIGPIPE' '\n>>           test_terminal git log\n>>       fi &&\n>> -    grep child_exit trace.normal >child-exits &&\n>> +    PAGER_CHILD_ID=$(grep does-not-exist trace.normal | \\\n>> +             sed -n \"s/child_start\\[\\([0-9]\\+\\)\\].*/\\1/p\") &&\n>> +    grep -F \"child_exit[\"$PAGER_CHILD_ID\"]\" trace.normal >child-exits &&\n>>       test_line_count = 1 child-exits &&\n>>       grep \" code:127 \" child-exits &&\n>>       test_path_is_file pager-used\n>> @@ -760,7 +768,9 @@ test_expect_success TTY 'git attempts to page to \n>> nonexisting pager command, gets\n>>           test_terminal git log\n>>       fi &&\n>> -    grep child_exit trace.normal >child-exits &&\n>> +    PAGER_CHILD_ID=$(grep does-not-exist trace.normal | \\\n>> +             sed -n \"s/child_start\\[\\([0-9]\\+\\)\\].*/\\1/p\") &&\n>> +    grep -F \"child_exit[\"$PAGER_CHILD_ID\"]\" trace.normal >child-exits &&\n>>       test_line_count = 1 child-exits &&\n>>       grep \" code:-1 \" child-exits\n>>   '\n>> @@ -780,7 +790,9 @@ test_expect_success TTY 'git returns SIGPIPE on \n>> propagated signals from pager' '\n>>           test_terminal git log\n>>       fi &&\n>> -    grep child_exit trace.normal >child-exits &&\n>> +    PAGER_CHILD_ID=$(grep pager-used trace.normal | \\\n>> +             sed -n \"s/child_start\\[\\([0-9]\\+\\)\\].*/\\1/p\") &&\n>> +    grep -F \"child_exit[\"$PAGER_CHILD_ID\"]\" trace.normal >child-exits &&\n>>       test_line_count = 1 child-exits &&\n>>       grep \" code:143 \" child-exits &&\n>>       test_path_is_file pager-used\n>>\n> \n\n"}]}