{"thread":{"id":"42172","subject":"[PATCH 2/2] submodule--helper update-clone: abort gracefully on missing .gitmodules","startedAt":"2016-04-28T20:02:45Z","lastAt":"2016-04-29T17:06:51Z","messageCount":3,"participants":["Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"284868","messageId":"1461873766-14703-1-git-send-email-sbeller@google.com","threadId":"42172","inReplyTo":null,"subject":"[PATCHv2 1/2] submodule init: fail gracefully with a missing .gitmodules file","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-28T20:02:45Z","receivedAt":"2016-04-28T20:02:45Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When there is no .gitmodules file availabe to initialize a submodule\nfrom, `submodule_from_path` just returns NULL. We need to check for\nthat and abort gracefully. When `submodule init` was implemented in shell,\na missing .gitmodules file would result in an error message\n\n    No url found for submodule path '%s' in .gitmodules\n\nReplicate that error message for now.\n\nWhen the .gitmodules file is missing we can probably fail even earlier\nfor all of the submodules with an improved error message.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\n  applies on submodule-init.\n  \n  Thanks,\n  Stefan\n\n builtin/submodule--helper.c | 10 +++++++---\n t/t7400-submodule-basic.sh  |  8 ++++++++\n 2 files changed, 15 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex b6d4f27..ce9d11e 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -314,13 +314,17 @@ static void init_submodule(const char *path, const char *prefix, int quiet)\n \t/* Only loads from .gitmodules, no overlay with .git/config */\n \tgitmodules_config();\n \n-\tsub = submodule_from_path(null_sha1, path);\n-\n \tif (prefix) {\n \t\tstrbuf_addf(&sb, \"%s%s\", prefix, path);\n \t\tdisplaypath = strbuf_detach(&sb, NULL);\n \t} else\n-\t\tdisplaypath = xstrdup(sub->path);\n+\t\tdisplaypath = xstrdup(path);\n+\n+\tsub = submodule_from_path(null_sha1, path);\n+\n+\tif (!sub)\n+\t\tdie(_(\"No url found for submodule path '%s' in .gitmodules\"),\n+\t\t\tdisplaypath);\n \n \t/*\n \t * Copy url setting when it is not set yet.\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex f99f674..df6b4da 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -18,6 +18,14 @@ test_expect_success 'setup - initial commit' '\n \tgit branch initial\n '\n \n+test_expect_success 'submodule init aborts on missing .gitmodules file' '\n+\ttest_when_finished \"git update-index --remove sub\" &&\n+\tgit update-index --add --cacheinfo 160000,$(git rev-parse HEAD),sub &&\n+\t# missing the .gitmodules file here\n+\ttest_must_fail git submodule init 2>actual &&\n+\ttest_i18ngrep \"No url found for submodule path\" actual\n+'\n+\n test_expect_success 'configuration parsing' '\n \ttest_when_finished \"rm -f .gitmodules\" &&\n \tcat >.gitmodules <<-\\EOF &&\n-- \n2.8.0.28.ga4e36c9\n"},{"id":"284867","messageId":"1461873766-14703-2-git-send-email-sbeller@google.com","threadId":"42172","inReplyTo":"1461873766-14703-1-git-send-email-sbeller@google.com","subject":"[PATCH 2/2] submodule--helper update-clone: abort gracefully on missing .gitmodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-28T20:02:46Z","receivedAt":"2016-04-28T20:02:46Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When there is no .gitmodules file availabe to initialize a submodule\nfrom, `submodule_from_path` just returns NULL. We need to check for\nthat and abort gracefully.\n\nWhen `git submodule update` was implemented in shell, this error out\nwith the warning\n\n    Submodule path '%s' not initialized\n    Maybe you want to use 'update --init'?\n\nReplicate that behavior for now instead of crashing.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/submodule--helper.c | 38 +++++++++++++++++++++++++-------------\n t/t7400-submodule-basic.sh  |  8 ++++++++\n 2 files changed, 33 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex ce9d11e..5d05393 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -593,6 +593,25 @@ struct submodule_update_clone {\n \tSUBMODULE_UPDATE_STRATEGY_INIT, 0, NULL, NULL, NULL, NULL, \\\n \tSTRING_LIST_INIT_DUP, 0}\n \n+\n+static void next_submodule_warn_missing(struct submodule_update_clone *suc,\n+\t\tstruct strbuf *out, const char *displaypath)\n+{\n+\t/*\n+\t * Only mention uninitialized submodules when their\n+\t * paths have been specified.\n+\t */\n+\tif (suc->warn_if_uninitialized) {\n+\t\tstrbuf_addf(out,\n+\t\t\t_(\"Submodule path '%s' not initialized\"),\n+\t\t\tdisplaypath);\n+\t\tstrbuf_addch(out, '\\n');\n+\t\tstrbuf_addstr(out,\n+\t\t\t_(\"Maybe you want to use 'update --init'?\"));\n+\t\tstrbuf_addch(out, '\\n');\n+\t}\n+}\n+\n /**\n  * Determine whether 'ce' needs to be cloned. If so, prepare the 'child' to\n  * run the clone. Returns 1 if 'ce' needs to be cloned, 0 otherwise.\n@@ -627,6 +646,11 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \telse\n \t\tdisplaypath = ce->name;\n \n+\tif (!sub) {\n+\t\tnext_submodule_warn_missing(suc, out, displaypath);\n+\t\tgoto cleanup;\n+\t}\n+\n \tif (suc->update.type == SM_UPDATE_NONE\n \t    || (suc->update.type == SM_UPDATE_UNSPECIFIED\n \t\t&& sub->update_strategy.type == SM_UPDATE_NONE)) {\n@@ -644,19 +668,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \tstrbuf_addf(&sb, \"submodule.%s.url\", sub->name);\n \tgit_config_get_string(sb.buf, &url);\n \tif (!url) {\n-\t\t/*\n-\t\t * Only mention uninitialized submodules when their\n-\t\t * path have been specified\n-\t\t */\n-\t\tif (suc->warn_if_uninitialized) {\n-\t\t\tstrbuf_addf(out,\n-\t\t\t\t_(\"Submodule path '%s' not initialized\"),\n-\t\t\t\tdisplaypath);\n-\t\t\tstrbuf_addch(out, '\\n');\n-\t\t\tstrbuf_addstr(out,\n-\t\t\t\t_(\"Maybe you want to use 'update --init'?\"));\n-\t\t\tstrbuf_addch(out, '\\n');\n-\t\t}\n+\t\tnext_submodule_warn_missing(suc, out, displaypath);\n \t\tgoto cleanup;\n \t}\n \ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex df6b4da..814ee63 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -26,6 +26,14 @@ test_expect_success 'submodule init aborts on missing .gitmodules file' '\n \ttest_i18ngrep \"No url found for submodule path\" actual\n '\n \n+test_expect_success 'submodule update aborts on missing .gitmodules file' '\n+\ttest_when_finished \"git update-index --remove sub\" &&\n+\tgit update-index --add --cacheinfo 160000,$(git rev-parse HEAD),sub &&\n+\t# missing the .gitmodules file here\n+\tgit submodule update sub 2>actual &&\n+\ttest_i18ngrep \"Submodule path .sub. not initialized\" actual\n+'\n+\n test_expect_success 'configuration parsing' '\n \ttest_when_finished \"rm -f .gitmodules\" &&\n \tcat >.gitmodules <<-\\EOF &&\n-- \n2.8.0.28.ga4e36c9\n"},{"id":"284948","messageId":"xmqq4mak1h04.fsf@gitster.mtv.corp.google.com","threadId":"42172","inReplyTo":"1461873766-14703-1-git-send-email-sbeller@google.com","subject":"Re: [PATCHv2 1/2] submodule init: fail gracefully with a missing .gitmodules file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-29T17:06:51Z","receivedAt":"2016-04-29T17:06:51Z","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 there is no .gitmodules file availabe to initialize a submodule\n> from, `submodule_from_path` just returns NULL. We need to check for\n> that and abort gracefully. When `submodule init` was implemented in shell,\n> a missing .gitmodules file would result in an error message\n>\n>     No url found for submodule path '%s' in .gitmodules\n>\n> Replicate that error message for now.\n\nGood to start from a faithful conversion.  Thanks for adding a test,\ntoo.\n\nWill queue.\n"}]}