{"thread":{"id":"44237","subject":"[PATCH 0/2] submodule pushes be extra careful","startedAt":"2016-10-06T19:37:38Z","lastAt":"2016-10-11T16:38:13Z","messageCount":10,"participants":["Stefan Beller","Junio C Hamano","Heiko Voigt"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"303524","messageId":"20161006193725.31553-1-sbeller@google.com","threadId":"44237","inReplyTo":null,"subject":"[PATCH 0/2] submodule pushes be extra careful","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-10-06T19:37:23Z","receivedAt":"2016-10-06T19:37:38Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This is a reroll of\nhttp://public-inbox.org/git/20161004210359.15266-1-sbeller@google.com/\n([PATCHv3 1/2] push: change submodule default to check when submodules exist)\nbut with a test.\n\nAs we only have a heuristic, the test failed initially as these tests don't\nhave any configuration at all nor do they have the submodule repos in the\nsuperprojects git dir. So I was looking for a cheap way to add a config.\n\nI could have written the config myself via git config, I think it is more\nidiomatic to use a submodule command for that. However just getting the\nconfiguration added is not possible with a submodule command, so I made one\nin the first patch.\n\nThanks,\nStefan\n\nStefan Beller (2):\n  submodule add: extend force flag to add existing repos\n  push: change submodule default to check when submodules exist\n\n builtin/push.c                 | 15 ++++++++++++++-\n git-submodule.sh               | 10 ++++++++--\n t/t5531-deep-submodule-push.sh |  6 +++++-\n t/t7400-submodule-basic.sh     | 14 ++++++++++++++\n 4 files changed, 41 insertions(+), 4 deletions(-)\n\n-- \n2.10.1.353.g1629400\n\n"},{"id":"303525","messageId":"20161006193725.31553-2-sbeller@google.com","threadId":"44237","inReplyTo":"20161006193725.31553-1-sbeller@google.com","subject":"[PATCH 1/2] submodule add: extend force flag to add existing repos","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-10-06T19:37:24Z","receivedAt":"2016-10-06T19:37:42Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Currently the force flag in `git submodule add` takes care of possibly\nignored files or when a name collision occurs.\n\nHowever there is another situation where submodule add comes in handy:\nWhen you already have a gitlink recorded, but no configuration was\ndone (i.e. no .gitmodules file nor any entry in .git/config) and you\nwant to generate these config entries. For this situation allow\n`git submodule add` to proceed if there is already a submodule at the\ngiven path in the index.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n git-submodule.sh           | 10 ++++++++--\n t/t7400-submodule-basic.sh | 14 ++++++++++++++\n 2 files changed, 22 insertions(+), 2 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex a024a13..3762616 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -207,8 +207,14 @@ cmd_add()\n \t\t\ttstart\n \t\t\ts|/*$||\n \t\t')\n-\tgit ls-files --error-unmatch \"$sm_path\" > /dev/null 2>&1 &&\n-\tdie \"$(eval_gettext \"'\\$sm_path' already exists in the index\")\"\n+\tif test -z \"$force\"\n+\tthen\n+\t\tgit ls-files --error-unmatch \"$sm_path\" > /dev/null 2>&1 &&\n+\t\tdie \"$(eval_gettext \"'\\$sm_path' already exists in the index\")\"\n+\telse\n+\t\tgit ls-files -s \"$sm_path\" | sane_grep -v \"^160000\" > /dev/null 2>&1 &&\n+\t\tdie \"$(eval_gettext \"'\\$sm_path' already exists in the index and is not a submodule\")\"\n+\tfi\n \n \tif test -z \"$force\" && ! git add --dry-run --ignore-missing \"$sm_path\" > /dev/null 2>&1\n \tthen\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex b77cce8..c09ce0d 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -152,6 +152,20 @@ test_expect_success 'submodule add to .gitignored path with --force' '\n \t)\n '\n \n+test_expect_success 'submodule add to reconfigure existing submodule with --force' '\n+\t(\n+\t\tcd addtest-ignore &&\n+\t\tgit submodule add --force bogus-url submod &&\n+\t\tgit submodule add -b initial \"$submodurl\" submod-branch &&\n+\t\ttest \"bogus-url\" = \"$(git config -f .gitmodules submodule.submod.url)\" &&\n+\t\ttest \"bogus-url\" = \"$(git config submodule.submod.url)\" &&\n+\t\t# Restore the url\n+\t\tgit submodule add --force \"$submodurl\" submod\n+\t\ttest \"$submodurl\" = \"$(git config -f .gitmodules submodule.submod.url)\" &&\n+\t\ttest \"$submodurl\" = \"$(git config submodule.submod.url)\"\n+\t)\n+'\n+\n test_expect_success 'submodule add --branch' '\n \techo \"refs/heads/initial\" >expect-head &&\n \tcat <<-\\EOF >expect-heads &&\n-- \n2.10.1.353.g1629400\n\n"},{"id":"303526","messageId":"20161006193725.31553-3-sbeller@google.com","threadId":"44237","inReplyTo":"20161006193725.31553-1-sbeller@google.com","subject":"[PATCHv4 2/2] push: change submodule default to check when submodules exist","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-10-06T19:37:25Z","receivedAt":"2016-10-06T19:37:46Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When working with submodules, it is easy to forget to push the submodules.\nThe setting 'check', which checks if any existing submodule is present on\nat least one remote of the submodule remotes, is designed to prevent this\nmistake.\n\nFlipping the default to check for submodules is safer than the current\ndefault of ignoring submodules while pushing.\n\nHowever checking for submodules requires additional work[1], which annoys\nusers that do not use submodules, so we turn on the check for submodules\nbased on a cheap heuristic, the existence of the .git/modules directory.\nThat directory doesn't exist when no submodules are used and is only\ncreated and populated when submodules are cloned/added.\n\nWhen the submodule directory doesn't exist, a user may have changed the\ngitlinks via plumbing commands. Currently the default is to not check.\nRECURSE_SUBMODULES_DEFAULT is effectively RECURSE_SUBMODULES_OFF currently,\nthough it may change in the future. When no submodules exist such a check\nis pointless as it would fail anyway, so let's just turn it off.\n\n[1] https://public-inbox.org/git/CA+55aFyos78qODyw57V=w13Ux5-8SvBqObJFAq22K+XKPWVbAA@mail.gmail.com/\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/push.c                 | 15 ++++++++++++++-\n t/t5531-deep-submodule-push.sh |  6 +++++-\n 2 files changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 3bb9d6b..683f270 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -3,6 +3,7 @@\n  */\n #include \"cache.h\"\n #include \"refs.h\"\n+#include \"dir.h\"\n #include \"run-command.h\"\n #include \"builtin.h\"\n #include \"remote.h\"\n@@ -22,6 +23,7 @@ static int deleterefs;\n static const char *receivepack;\n static int verbosity;\n static int progress = -1;\n+static int has_submodules_configured;\n static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n static enum transport_family family;\n \n@@ -31,6 +33,15 @@ static const char **refspec;\n static int refspec_nr;\n static int refspec_alloc;\n \n+static void preset_submodule_default(void)\n+{\n+\tif (has_submodules_configured || file_exists(git_path(\"modules\")) ||\n+\t    (!is_bare_repository() && file_exists(\".gitmodules\")))\n+\t\trecurse_submodules = RECURSE_SUBMODULES_CHECK;\n+\telse\n+\t\trecurse_submodules = RECURSE_SUBMODULES_OFF;\n+}\n+\n static void add_refspec(const char *ref)\n {\n \trefspec_nr++;\n@@ -495,7 +506,8 @@ static int git_push_config(const char *k, const char *v, void *cb)\n \t\tconst char *value;\n \t\tif (!git_config_get_value(\"push.recursesubmodules\", &value))\n \t\t\trecurse_submodules = parse_push_recurse_submodules_arg(k, value);\n-\t}\n+\t} else if (starts_with(k, \"submodule.\") && ends_with(k, \".url\"))\n+\t\thas_submodules_configured = 1;\n \n \treturn git_default_config(k, v, NULL);\n }\n@@ -552,6 +564,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t};\n \n \tpacket_trace_identity(\"push\");\n+\tpreset_submodule_default();\n \tgit_config(git_push_config, &flags);\n \targc = parse_options(argc, argv, prefix, options, push_usage, 0);\n \tset_push_cert_flags(&flags, push_cert);\ndiff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh\nindex 198ce84..e690749 100755\n--- a/t/t5531-deep-submodule-push.sh\n+++ b/t/t5531-deep-submodule-push.sh\n@@ -65,7 +65,11 @@ test_expect_success 'push fails if submodule commit not on remote' '\n \t\tgit add gar/bage &&\n \t\tgit commit -m \"Third commit for gar/bage\" &&\n \t\t# the push should fail with --recurse-submodules=check\n-\t\t# on the command line...\n+\t\t# on the command line. \"check\" is the default for repos in\n+\t\t# which submodules are detected by existence of config,\n+\t\t# .gitmodules file or an internal .git/modules/<submodule-repo>\n+\t\tgit submodule add -f ../submodule.git gar/bage &&\n+\t\ttest_must_fail git push ../pub.git master &&\n \t\ttest_must_fail git push --recurse-submodules=check ../pub.git master &&\n \n \t\t# ...or if specified in the configuration..\n-- \n2.10.1.353.g1629400\n\n"},{"id":"303530","messageId":"xmqqshs9i6fr.fsf@gitster.mtv.corp.google.com","threadId":"44237","inReplyTo":"20161006193725.31553-2-sbeller@google.com","subject":"Re: [PATCH 1/2] submodule add: extend force flag to add existing repos","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-06T20:11:20Z","receivedAt":"2016-10-06T20:11:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> Currently the force flag in `git submodule add` takes care of possibly\n> ignored files or when a name collision occurs.\n>\n> However there is another situation where submodule add comes in handy:\n> When you already have a gitlink recorded, but no configuration was\n> done (i.e. no .gitmodules file nor any entry in .git/config) and you\n> want to generate these config entries. For this situation allow\n> `git submodule add` to proceed if there is already a submodule at the\n> given path in the index.\n>\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n\nYup, the goal makes perfect sense.  \n\nI vaguely recall discussing this exact issue of \"git submodule add\"\nthat refuses to add a path that already is a gitlink (via \"git add\"\nthat has previously been run) elsewhere on this list some time ago.\n\n"},{"id":"303532","messageId":"xmqqoa2xi5rd.fsf@gitster.mtv.corp.google.com","threadId":"44237","inReplyTo":"20161006193725.31553-3-sbeller@google.com","subject":"Re: [PATCHv4 2/2] push: change submodule default to check when submodules exist","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-06T20:25:58Z","receivedAt":"2016-10-06T20:26:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> When working with submodules, it is easy to forget to push the submodules.\n> The setting 'check', which checks if any existing submodule is present on\n> at least one remote of the submodule remotes, is designed to prevent this\n> mistake.\n>\n> Flipping the default to check for submodules is safer than the current\n> default of ignoring submodules while pushing.\n>\n> However checking for submodules requires additional work[1], which annoys\n> users that do not use submodules, so we turn on the check for submodules\n> based on a cheap heuristic, the existence of the .git/modules directory.\n\n... and other things like .gitmodules, submodule stuff in\n.git/config, etc.?\n\n> +\t} else if (starts_with(k, \"submodule.\") && ends_with(k, \".url\"))\n> +\t\thas_submodules_configured = 1;\n\nAn in-code comment to explain why \".url\" is so special would be\nhelpful.  Documentation/config.txt says .path and .url are both\ninitially populated by 'git submodule init', which might be outdated\ninformation that confuses readers of the above code.\n\n> @@ -552,6 +564,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n>  \t};\n>  \n>  \tpacket_trace_identity(\"push\");\n> +\tpreset_submodule_default();\n>  \tgit_config(git_push_config, &flags);\n>  \targc = parse_options(argc, argv, prefix, options, push_usage, 0);\n>  \tset_push_cert_flags(&flags, push_cert);\n> diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh\n> index 198ce84..e690749 100755\n> --- a/t/t5531-deep-submodule-push.sh\n> +++ b/t/t5531-deep-submodule-push.sh\n> @@ -65,7 +65,11 @@ test_expect_success 'push fails if submodule commit not on remote' '\n>  \t\tgit add gar/bage &&\n>  \t\tgit commit -m \"Third commit for gar/bage\" &&\n>  \t\t# the push should fail with --recurse-submodules=check\n> -\t\t# on the command line...\n> +\t\t# on the command line. \"check\" is the default for repos in\n> +\t\t# which submodules are detected by existence of config,\n> +\t\t# .gitmodules file or an internal .git/modules/<submodule-repo>\n> +\t\tgit submodule add -f ../submodule.git gar/bage &&\n> +\t\ttest_must_fail git push ../pub.git master &&\n>  \t\ttest_must_fail git push --recurse-submodules=check ../pub.git master &&\n>  \n>  \t\t# ...or if specified in the configuration..\n"},{"id":"303534","messageId":"CAGZ79kYij=7kfQa8cc1btRowBXbia01QgYTKTgz93ZBdx249Jw@mail.gmail.com","threadId":"44237","inReplyTo":"xmqqoa2xi5rd.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv4 2/2] push: change submodule default to check when submodules exist","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-10-06T21:29:30Z","receivedAt":"2016-10-06T21:30:12Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Oct 6, 2016 at 1:25 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> When working with submodules, it is easy to forget to push the submodules.\n>> The setting 'check', which checks if any existing submodule is present on\n>> at least one remote of the submodule remotes, is designed to prevent this\n>> mistake.\n>>\n>> Flipping the default to check for submodules is safer than the current\n>> default of ignoring submodules while pushing.\n>>\n>> However checking for submodules requires additional work[1], which annoys\n>> users that do not use submodules, so we turn on the check for submodules\n>> based on a cheap heuristic, the existence of the .git/modules directory.\n>\n> ... and other things like .gitmodules, submodule stuff in\n> .git/config, etc.?\n\nOh right, I need to update this commit message to mention this, too\n\n>\n>> +     } else if (starts_with(k, \"submodule.\") && ends_with(k, \".url\"))\n>> +             has_submodules_configured = 1;\n>\n> An in-code comment to explain why \".url\" is so special would be\n> helpful.  Documentation/config.txt says .path and .url are both\n\n       submodule.<name>.path, submodule.<name>.url\n           The path within this project and URL for a submodule. These\n           variables are initially populated by git submodule init. See git-\n           submodule(1) and gitmodules(5) for details.\n\nThe correct version is:\n\nsubmodule.<name>.path::\n    This is a config variable only found in .gitmodules files that\nindicate at which\n    path relative to the repository root the submodule is found.\n    It is used to resolve the mapping (submodule name)<->path throughout all\n    time, i.e. also later when a submodule is initialized and checked out, any\n    subsequent operation that needs a submodule name/repsoitory uses it for\n    lookup.\n\nsubmodule.<name>.url::\n    This is a config variable found both in .gitmodules files as well\nas .git/config.\n    In the .gitmodules files it serves as a hint where to obtain a\nrepository from\n    to populate the gitlinks within the repository.\n    On submodule-init the value will be copied over to the .git/config\nfile and there\n    it serves as a holder of the value only until the first\nsubmodule-update is run,\n    as the initial submodule-update is using that url to clone the submodule.\n    From then on it is only used as a boolean flag in the .git/config\nto indicate\n    whether the submodule is considered interesting for further submodule\n    commands. (See the similar competing thing with submodule.<name>.ignore)\n\nGiven that it is obvious to my current me, that .url is the only\nsensible thing as .path\nis not found in the .git/config which we are searching in here.\nSo for the purpose of this patch I'd just add a line like this to\nremind my future self:\n\n    /* See if any submodule is considered interesting: */\n\nI would like to avoid references to .path as that is not helping here.\n(Why does that\ncomment point to a variable name that is not supposed to exist in the file?)\nSo I think we really need to think how to reduce confusion in the\nfuture by readers that\nhave more or less overview of the submodules concept.\n\nA future version of the man page may read:\n\nsubmodule.<name>.path::\n    <same as before>, we may want to consider if we copy it over to\n.git/config, so the\n    repository may trash the .gitmodules file and the submodules keep working.\n\nsubmodule.<name>.url::\n    indication in .gitmodules where the submodule is cloned from. It\nwill be copied over\n    to .git/config on submodule-init. The user may adapt the\nconfigured urls there before\n    proceeding to submodule-update, which will delete the url setting\nagain, as the\n    cloned submodule repository will hold the authoritative setting\nwhere the remote\n    of the submodule is.\n    So it only exists in .git/config for submodules that are\ninitialized but have never\n    been cloned. Even when the submodule is deinit'd and reinited we\ndon't even need\n    to copy the url, as we still have the old submodule repository in\n.git/module/<name>\n\nsubmodule.<name>.exists::\n    This setting is per working tree (unlike the previous 2) that\nindicates if submodule\n    related commands pay attention to the given submodule. It is still\nunclear how this\n    works together with the .ignore setting.\n\n\nNote for this future to come true, we'd have to have only 2 big series\nOne that Duy started to introduce per working tree configs, see\n    https://public-inbox.org/git/20160720172419.25473-1-pclouds@gmail.com/\nThe other one is a series that hasn't seen the mailing list yet, that\nI started to\nseparate the 2 modes of the url both as a value holder (to point at a URL) as\nwell as just a boolean flag for commands to acknowledge the existence of the\nsubmodule.\n\n\n> initially populated by 'git submodule init', which might be outdated\n> information that confuses readers of the above code.\n\nA lot of text but I guess we can use some ideas from here to update the\n.path and .url documentation. (The wording in this email is not of man\npage quality).\n\nThanks,\nStefan\n"},{"id":"303540","messageId":"20161006234100.8979-1-sbeller@google.com","threadId":"44237","inReplyTo":"xmqqoa2xi5rd.fsf@gitster.mtv.corp.google.com","subject":"[PATCHv5] push: change submodule default to check when submodules exist","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-10-06T23:41:00Z","receivedAt":"2016-10-06T23:41:08Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When working with submodules, it is easy to forget to push the submodules.\nThe setting 'check', which checks if any existing submodule is present on\nat least one remote of the submodule remotes, is designed to prevent this\nmistake.\n\nFlipping the default to check for submodules is safer than the current\ndefault of ignoring submodules while pushing.\n\nHowever checking for submodules requires additional work[1], which annoys\nusers that do not use submodules, so we turn on the check for submodules\nbased on a cheap heuristic, the existence of\n* any initialized submodules via checking submodule.<name>.url.\n* the .git/modules directory. That directory doesn't exist when no\n  submodules are used and is only created and populated when submodules\n  are cloned/added.\n* the existence of the .gitmodules file.\n\nWhen the submodule directory doesn't exist, a user may have changed the\ngitlinks via plumbing commands. Currently the default is to not check.\nRECURSE_SUBMODULES_DEFAULT is effectively RECURSE_SUBMODULES_OFF currently,\nthough it may change in the future. When no submodules exist such a check\nis pointless as it would fail anyway, so let's just turn it off.\n\n[1] https://public-inbox.org/git/CA+55aFyos78qODyw57V=w13Ux5-8SvBqObJFAq22K+XKPWVbAA@mail.gmail.com/\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\n * Reworded the commit message\n * added comment on why we only check submodule.<name>.url\n\n The first patch of the series is still good, so please use\n https://public-inbox.org/git/20161006193725.31553-2-sbeller@google.com/\n first and then build this on top.\n \n Thanks,\n Stefan \n\n builtin/push.c                 | 16 +++++++++++++++-\n t/t5531-deep-submodule-push.sh |  6 +++++-\n 2 files changed, 20 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 3bb9d6b..9e0b8db 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -3,6 +3,7 @@\n  */\n #include \"cache.h\"\n #include \"refs.h\"\n+#include \"dir.h\"\n #include \"run-command.h\"\n #include \"builtin.h\"\n #include \"remote.h\"\n@@ -22,6 +23,7 @@ static int deleterefs;\n static const char *receivepack;\n static int verbosity;\n static int progress = -1;\n+static int has_submodules_configured;\n static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n static enum transport_family family;\n \n@@ -31,6 +33,15 @@ static const char **refspec;\n static int refspec_nr;\n static int refspec_alloc;\n \n+static void preset_submodule_default(void)\n+{\n+\tif (has_submodules_configured || file_exists(git_path(\"modules\")) ||\n+\t    (!is_bare_repository() && file_exists(\".gitmodules\")))\n+\t\trecurse_submodules = RECURSE_SUBMODULES_CHECK;\n+\telse\n+\t\trecurse_submodules = RECURSE_SUBMODULES_OFF;\n+}\n+\n static void add_refspec(const char *ref)\n {\n \trefspec_nr++;\n@@ -495,7 +506,9 @@ static int git_push_config(const char *k, const char *v, void *cb)\n \t\tconst char *value;\n \t\tif (!git_config_get_value(\"push.recursesubmodules\", &value))\n \t\t\trecurse_submodules = parse_push_recurse_submodules_arg(k, value);\n-\t}\n+\t} else if (starts_with(k, \"submodule.\") && ends_with(k, \".url\"))\n+\t\t/* The submodule.<name>.url is used as a bit to indicate existence */\n+\t\thas_submodules_configured = 1;\n \n \treturn git_default_config(k, v, NULL);\n }\n@@ -552,6 +565,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t};\n \n \tpacket_trace_identity(\"push\");\n+\tpreset_submodule_default();\n \tgit_config(git_push_config, &flags);\n \targc = parse_options(argc, argv, prefix, options, push_usage, 0);\n \tset_push_cert_flags(&flags, push_cert);\ndiff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh\nindex 198ce84..e690749 100755\n--- a/t/t5531-deep-submodule-push.sh\n+++ b/t/t5531-deep-submodule-push.sh\n@@ -65,7 +65,11 @@ test_expect_success 'push fails if submodule commit not on remote' '\n \t\tgit add gar/bage &&\n \t\tgit commit -m \"Third commit for gar/bage\" &&\n \t\t# the push should fail with --recurse-submodules=check\n-\t\t# on the command line...\n+\t\t# on the command line. \"check\" is the default for repos in\n+\t\t# which submodules are detected by existence of config,\n+\t\t# .gitmodules file or an internal .git/modules/<submodule-repo>\n+\t\tgit submodule add -f ../submodule.git gar/bage &&\n+\t\ttest_must_fail git push ../pub.git master &&\n \t\ttest_must_fail git push --recurse-submodules=check ../pub.git master &&\n \n \t\t# ...or if specified in the configuration..\n-- \n2.10.1.353.g1629400\n\n"},{"id":"303557","messageId":"20161007125239.GB61543@book.hvoigt.net","threadId":"44237","inReplyTo":"xmqqshs9i6fr.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/2] submodule add: extend force flag to add existing repos","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2016-10-07T12:52:39Z","receivedAt":"2016-10-07T12:52:51Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Thu, Oct 06, 2016 at 01:11:20PM -0700, Junio C Hamano wrote:\n> Stefan Beller <sbeller@google.com> writes:\n> \n> > Currently the force flag in `git submodule add` takes care of possibly\n> > ignored files or when a name collision occurs.\n> >\n> > However there is another situation where submodule add comes in handy:\n> > When you already have a gitlink recorded, but no configuration was\n> > done (i.e. no .gitmodules file nor any entry in .git/config) and you\n> > want to generate these config entries. For this situation allow\n> > `git submodule add` to proceed if there is already a submodule at the\n> > given path in the index.\n\nIs it important that the submodule is in the index? How about worktree?\nFrom the index entry alone we can not deduce the values anyway. So I\nwould say the submodule has to be in the worktree, no matter what is in\nthe index. If its not in the index we can also add it.\n\nBTW, that is the way I imagined submodules would work in the first\nplace: just clone and add them, like I described here[1].\n\n> > Signed-off-by: Stefan Beller <sbeller@google.com>\n> > ---\n> \n> Yup, the goal makes perfect sense.  \n> \n> I vaguely recall discussing this exact issue of \"git submodule add\"\n> that refuses to add a path that already is a gitlink (via \"git add\"\n> that has previously been run) elsewhere on this list some time ago.\n\nYes there was a discussion, see the link.\n\nCheers Heiko\n\n[1] http://public-inbox.org/git/%3C20160916141143.GA47240@book.hvoigt.net%3E/\n"},{"id":"303589","messageId":"CAGZ79kbg1KaeL14DMcV=ObaZOBpJpxOZAEYY1mmf8eqDUUqYmA@mail.gmail.com","threadId":"44237","inReplyTo":"20161007125239.GB61543@book.hvoigt.net","subject":"Re: [PATCH 1/2] submodule add: extend force flag to add existing repos","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-10-07T17:25:04Z","receivedAt":"2016-10-07T17:26:26Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Oct 7, 2016 at 5:52 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n> On Thu, Oct 06, 2016 at 01:11:20PM -0700, Junio C Hamano wrote:\n>> Stefan Beller <sbeller@google.com> writes:\n>>\n>> > Currently the force flag in `git submodule add` takes care of possibly\n>> > ignored files or when a name collision occurs.\n>> >\n>> > However there is another situation where submodule add comes in handy:\n>> > When you already have a gitlink recorded, but no configuration was\n>> > done (i.e. no .gitmodules file nor any entry in .git/config) and you\n>> > want to generate these config entries. For this situation allow\n>> > `git submodule add` to proceed if there is already a submodule at the\n>> > given path in the index.\n>\n> Is it important that the submodule is in the index?\n\nIf it is not in the index, it already works.\n\n> How about worktree?\n> From the index entry alone we can not deduce the values anyway.\n\nRight, but as of now this is the only show stopper, i.e.\n* you have an existing repo? -> fine, it works with --force\n* you even ignored that repo -> --force knows how to do it.\n* you already have a gitlink -> Sorry, you're out of luck.\n\nSo that is why I stressed index in this commit message, as it is only about this\ncase.\n\n>\n> [1] http://public-inbox.org/git/%3C20160916141143.GA47240@book.hvoigt.net%3E/\n\nCurrent situation:\n\n> clone the submodule into a directory\n> git submodule add --force\n> git commit everything\n\nworks fine, but:\n\n> clone the submodule into a directory\n> git add <gitlink>\n> git commit <gitlink> -m \"Add submodule\"\n> # me: \"Oh crap! I did forget the configuration.\"\n> git submodule add --force <url> <gitlink>\n> # Git: \"It already exists in the index, I am not going to produce the config for you.\"\n\nThe last step is changed with this patch, as\nit will just work fine then.\n"},{"id":"303884","messageId":"20161011163647.GA78554@book.hvoigt.net","threadId":"44237","inReplyTo":"CAGZ79kbg1KaeL14DMcV=ObaZOBpJpxOZAEYY1mmf8eqDUUqYmA@mail.gmail.com","subject":"Re: [PATCH 1/2] submodule add: extend force flag to add existing repos","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2016-10-11T16:36:47Z","receivedAt":"2016-10-11T16:38:13Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi,\n\nOn Fri, Oct 07, 2016 at 10:25:04AM -0700, Stefan Beller wrote:\n> On Fri, Oct 7, 2016 at 5:52 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n> > On Thu, Oct 06, 2016 at 01:11:20PM -0700, Junio C Hamano wrote:\n> >> Stefan Beller <sbeller@google.com> writes:\n> >>\n> >> > Currently the force flag in `git submodule add` takes care of possibly\n> >> > ignored files or when a name collision occurs.\n> >> >\n> >> > However there is another situation where submodule add comes in handy:\n> >> > When you already have a gitlink recorded, but no configuration was\n> >> > done (i.e. no .gitmodules file nor any entry in .git/config) and you\n> >> > want to generate these config entries. For this situation allow\n> >> > `git submodule add` to proceed if there is already a submodule at the\n> >> > given path in the index.\n> >\n> > Is it important that the submodule is in the index?\n> \n> If it is not in the index, it already works.\n\nAh ok I was not aware of that, sorry.\n\n> > How about worktree?\n> > From the index entry alone we can not deduce the values anyway.\n> \n> Right, but as of now this is the only show stopper, i.e.\n> * you have an existing repo? -> fine, it works with --force\n> * you even ignored that repo -> --force knows how to do it.\n> * you already have a gitlink -> Sorry, you're out of luck.\n> \n> So that is why I stressed index in this commit message, as it is only about this\n> case.\n\nForget what I wrote. As said above I was not aware that there is only an\nerror when it is already in the index.\n\n> > [1] http://public-inbox.org/git/%3C20160916141143.GA47240@book.hvoigt.net%3E/\n> \n> Current situation:\n> \n> > clone the submodule into a directory\n> > git submodule add --force\n> > git commit everything\n> \n> works fine, but:\n> \n> > clone the submodule into a directory\n> > git add <gitlink>\n> > git commit <gitlink> -m \"Add submodule\"\n> > # me: \"Oh crap! I did forget the configuration.\"\n> > git submodule add --force <url> <gitlink>\n> > # Git: \"It already exists in the index, I am not going to produce the config for you.\"\n> \n> The last step is changed with this patch, as\n> it will just work fine then.\n\nThanks.\n\nCheers Heiko\n"}]}