{"thread":{"id":"45188","subject":"[PATCH] submodule init: warn about falling back to a local path","startedAt":"2017-02-21T23:18:32Z","lastAt":"2017-02-25T01:31:57Z","messageCount":9,"participants":["Stefan Beller","Philip Oakley","Johannes Sixt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"312237","messageId":"20170221231815.4123-1-sbeller@google.com","threadId":"45188","inReplyTo":null,"subject":"[PATCH] submodule init: warn about falling back to a local path","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-02-21T23:18:15Z","receivedAt":"2017-02-21T23:18:32Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When a submodule is initialized, the config variable 'submodule..url'\nis set depending on the value of the same variable in the .gitmodules\nfile. When the URL indicates to be relative, then the url is computed\nrelative to its default remote. The default remote cannot be determined\naccurately in all cases, such that it falls back to 'origin'.\n\nThe 'origin' remote may not exist, though. In that case we give up looking\nfor a suitable remote and we'll just assume it to be a local relative path.\n\nThis can be confusing to users as there is a lot of guessing involved,\nwhich is not obvious to the user.\n\nSo in the corner case of assuming a local autoritative truth, warn the\nuser to lessen the confusion.\n\nThis behavior was introduced in 4d6893200 (submodule add: allow relative\nrepository path even when no url is set, 2011-06-06), which shared the\ncode with submodule-init and then ported to C in 3604242f080a (submodule:\nport init from shell to C, 2016-04-15).\n\nIn case of submodule-add, this behavior makes sense in some use cases[1],\nhowever for submodule-init there does not seem to be an immediate obvious\nuse case to fall back to a local submodule. However there might be, so\nwarn instead of die here.\n\n[1] e.g. http://stackoverflow.com/questions/8721984/git-ignore-files-for-public-repository-but-not-for-private\n\"store a secret locally in a submodule, with no intention to publish it\"\n\nReported-by: Shawn Pearce \nSigned-off-by: Stefan Beller \n---\n builtin/submodule--helper.c | 8 +++-----\n 1 file changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 899dc334e3..44c11dd91e 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -356,12 +356,10 @@ static void init_submodule(const char *path, const char *prefix, int quiet)\n \t\t\tstrbuf_addf(&remotesb, \"remote.%s.url\", remote);\n \t\t\tfree(remote);\n \n-\t\t\tif (git_config_get_string(remotesb.buf, &remoteurl))\n-\t\t\t\t/*\n-\t\t\t\t * The repository is its own\n-\t\t\t\t * authoritative upstream\n-\t\t\t\t */\n+\t\t\tif (git_config_get_string(remotesb.buf, &remoteurl)) {\n \t\t\t\tremoteurl = xgetcwd();\n+\t\t\t\twarning(_(\"could not lookup configuration '%s'. Assuming this repository is its own authoritative upstream.\"), remotesb.buf);\n+\t\t\t}\n \t\t\trelurl = relative_url(remoteurl, url, NULL);\n \t\t\tstrbuf_release(&remotesb);\n \t\t\tfree(remoteurl);\n-- \n2.12.0.rc1.16.ge4278d41a0.dirty\n\n"},{"id":"312337","messageId":"F50FAF9C5F724A87B5424494262E1BAD@PhilipOakley","threadId":"45188","inReplyTo":"20170221231815.4123-1-sbeller@google.com","subject":"Re: [PATCH] submodule init: warn about falling back to a local path","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2017-02-22T20:45:13Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Stefan Beller\" \n> When a submodule is initialized, the config variable \n> 'submodule..url'\n> is set depending on the value of the same variable in the .gitmodules\n> file. When the URL indicates to be relative, then the url is computed\n> relative to its default remote. The default remote cannot be determined\n> accurately in all cases, such that it falls back to 'origin'.\n>\n> The 'origin' remote may not exist, though. In that case we give up looking\n> for a suitable remote and we'll just assume it to be a local relative \n> path.\n>\n> This can be confusing to users as there is a lot of guessing involved,\n> which is not obvious to the user.\n\nWould a note in the user docs about the fallback you list above also be \nuseful?\n\n>\n> So in the corner case of assuming a local autoritative truth, warn the\n> user to lessen the confusion.\n>\n> This behavior was introduced in 4d6893200 (submodule add: allow relative\n> repository path even when no url is set, 2011-06-06), which shared the\n> code with submodule-init and then ported to C in 3604242f080a (submodule:\n> port init from shell to C, 2016-04-15).\n>\n> In case of submodule-add, this behavior makes sense in some use cases[1],\n> however for submodule-init there does not seem to be an immediate obvious\n> use case to fall back to a local submodule. However there might be, so\n> warn instead of die here.\n>\n> [1] e.g. \n> http://stackoverflow.com/questions/8721984/git-ignore-files-for-public-repository-but-not-for-private\n> \"store a secret locally in a submodule, with no intention to publish it\"\n>\n> Reported-by: Shawn Pearce \n> Signed-off-by: Stefan Beller \n> ---\n> builtin/submodule--helper.c | 8 +++-----\n> 1 file changed, 3 insertions(+), 5 deletions(-)\n>\n--\nPhilip \n\n"},{"id":"312363","messageId":"CAGZ79kYXyk=giuM6CnU=gnvkCw3aaVEn=WbJ6tjhn8kZumrmTA@mail.gmail.com","threadId":"45188","inReplyTo":"F50FAF9C5F724A87B5424494262E1BAD@PhilipOakley","subject":"Re: [PATCH] submodule init: warn about falling back to a local path","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-02-23T00:24:51Z","receivedAt":"2017-02-23T00:26:23Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Feb 22, 2017 at 7:01 AM, Philip Oakley wrote:\n>\n> Would a note in the user docs about the fallback you list above also be\n> useful?\n\nduh! yes it would! Will look at the documentation and reroll.\n"},{"id":"312488","messageId":"20170224001704.23854-1-sbeller@google.com","threadId":"45188","inReplyTo":"CAGZ79kYXyk=giuM6CnU=gnvkCw3aaVEn=WbJ6tjhn8kZumrmTA@mail.gmail.com","subject":"[PATCH] submodule init: warn about falling back to a local path","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-02-24T00:17:04Z","receivedAt":"2017-02-24T00:17:29Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When a submodule is initialized, the config variable 'submodule..url'\nis set depending on the value of the same variable in the .gitmodules\nfile. When the URL indicates to be relative, then the url is computed\nrelative to its default remote. The default remote cannot be determined\naccurately in all cases, such that it falls back to 'origin'.\n\nThe 'origin' remote may not exist, though. In that case we give up looking\nfor a suitable remote and we'll just assume it to be a local relative path.\n\nThis can be confusing to users as there is a lot of guessing involved,\nwhich is not obvious to the user.\n\nSo in the corner case of assuming a local autoritative truth, warn the\nuser to lessen the confusion.\n\nThis behavior was introduced in 4d6893200 (submodule add: allow relative\nrepository path even when no url is set, 2011-06-06), which shared the\ncode with submodule-init and then ported to C in 3604242f080a (submodule:\nport init from shell to C, 2016-04-15).\n\nIn case of submodule-add, this behavior makes sense in some use cases[1],\nhowever for submodule-init there does not seem to be an immediate obvious\nuse case to fall back to a local submodule. However there might be, so\nwarn instead of die here.\n\nWhile adding the warning, also clarify the behavior of relative URLs in\nthe documentation.\n\n[1] e.g. http://stackoverflow.com/questions/8721984/git-ignore-files-for-public-repository-but-not-for-private\n\"store a secret locally in a submodule, with no intention to publish it\"\n\nReported-by: Shawn Pearce \nSigned-off-by: Stefan Beller \n---\n Documentation/git-submodule.txt | 22 +++++++++++++++-------\n builtin/submodule--helper.c | 8 +++-----\n 2 files changed, 18 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 8acc72ebb8..d23dee2a5b 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -73,13 +73,17 @@ configuration entries unless `--name` is used to specify a logical name.\n +\n is the URL of the new submodule's origin repository.\n This may be either an absolute URL, or (if it begins with ./\n-or ../), the location relative to the superproject's origin\n+or ../), the location relative to the superproject's default remote\n repository (Please note that to specify a repository 'foo.git'\n which is located right next to a superproject 'bar.git', you'll\n have to use '../foo.git' instead of './foo.git' - as one might expect\n when following the rules for relative URLs - because the evaluation\n of relative URLs in Git is identical to that of relative directories).\n-If the superproject doesn't have an origin configured\n++\n+The default remote is the remote of the remote tracking branch\n+of the current branch. If no such remote tracking branch exists or\n+the in detached HEAD mode, \"origin\" is assumed to be the default remote.\n+If the superproject doesn't have a default remote configured\n the superproject is its own authoritative upstream and the current\n working directory is used instead.\n +\n@@ -118,18 +122,22 @@ too (and can also report changes to a submodule's work tree).\n \n init [--] [...]::\n \tInitialize the submodules recorded in the index (which were\n-\tadded and committed elsewhere) by copying submodule\n-\tnames and urls from .gitmodules to .git/config.\n+\tadded and committed elsewhere) by copying `submodule.$name.url`\n+\tfrom .gitmodules to .git/config, resolving relative urls to be\n+\trelative to the default remote.\n++\n \tOptional arguments limit which submodules will be initialized.\n-\tIt will also copy the value of `submodule.$name.update` into\n-\t.git/config.\n-\tThe key used in .git/config is `submodule.$name.url`.\n+\tIf no path is specified all submodules are initialized.\n++\n+\tWhen present, it will also copy the value of `submodule.$name.update`.\n \tThis command does not alter existing information in .git/config.\n \tYou can then customize the submodule clone URLs in .git/config\n \tfor your local setup and proceed to `git submodule update`;\n \tyou can also just use `git submodule update --init` without\n \tthe explicit 'init' step if you do not intend to customize\n \tany submodule locations.\n++\n+\tSee the add subcommand for the defintion of default remote.\n \n deinit [-f|--force] (--all|[--] ...)::\n \tUnregister the given submodules, i.e. remove the whole\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 899dc334e3..44c11dd91e 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -356,12 +356,10 @@ static void init_submodule(const char *path, const char *prefix, int quiet)\n \t\t\tstrbuf_addf(&remotesb, \"remote.%s.url\", remote);\n \t\t\tfree(remote);\n \n-\t\t\tif (git_config_get_string(remotesb.buf, &remoteurl))\n-\t\t\t\t/*\n-\t\t\t\t * The repository is its own\n-\t\t\t\t * authoritative upstream\n-\t\t\t\t */\n+\t\t\tif (git_config_get_string(remotesb.buf, &remoteurl)) {\n \t\t\t\tremoteurl = xgetcwd();\n+\t\t\t\twarning(_(\"could not lookup configuration '%s'. Assuming this repository is its own authoritative upstream.\"), remotesb.buf);\n+\t\t\t}\n \t\t\trelurl = relative_url(remoteurl, url, NULL);\n \t\t\tstrbuf_release(&remotesb);\n \t\t\tfree(remoteurl);\n-- \n2.12.0.rc1.16.ge4278d41a0.dirty\n\n"},{"id":"312505","messageId":"ae377bda-0776-f98b-5b6f-afa198647400@kdbg.org","threadId":"45188","inReplyTo":"20170224001704.23854-1-sbeller@google.com","subject":"Re: [PATCH] submodule init: warn about falling back to a local path","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2017-02-24T08:51:09Z","receivedAt":"2017-02-24T08:53:58Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 24.02.2017 um 01:17 schrieb Stefan Beller:\n> --- a/Documentation/git-submodule.txt\n> +++ b/Documentation/git-submodule.txt\n> @@ -73,13 +73,17 @@ configuration entries unless `--name` is used to specify a logical name.\n> ...\n> +The default remote is the remote of the remote tracking branch\n> +of the current branch. If no such remote tracking branch exists or\n> +the in detached HEAD mode, \"origin\" is assumed to be the default remote.\n\nThe part after \"or\" does not quite parse.\n\n> +If the superproject doesn't have a default remote configured\n> the superproject is its own authoritative upstream and the current\n> working directory is used instead.\n> +\n> @@ -118,18 +122,22 @@ too (and can also report changes to a submodule's work tree).\n>\n> init [--] [...]::\n> \tInitialize the submodules recorded in the index (which were\n> -\tadded and committed elsewhere) by copying submodule\n> -\tnames and urls from .gitmodules to .git/config.\n> +\tadded and committed elsewhere) by copying `submodule.$name.url`\n> +\tfrom .gitmodules to .git/config, resolving relative urls to be\n> +\trelative to the default remote.\n> ++\n> \tOptional arguments limit which submodules will be initialized.\n> -\tIt will also copy the value of `submodule.$name.update` into\n> -\t.git/config.\n> -\tThe key used in .git/config is `submodule.$name.url`.\n> +\tIf no path is specified all submodules are initialized.\n> ++\n> +\tWhen present, it will also copy the value of `submodule.$name.update`.\n> \tThis command does not alter existing information in .git/config.\n> \tYou can then customize the submodule clone URLs in .git/config\n> \tfor your local setup and proceed to `git submodule update`;\n> \tyou can also just use `git submodule update --init` without\n> \tthe explicit 'init' step if you do not intend to customize\n> \tany submodule locations.\n> ++\n> +\tSee the add subcommand for the defintion of default remote.\n\nTo be rendered correctly, I think you must remove the indentation from \ncontinuation paragraphs.\n\n>\n> deinit [-f|--force] (--all|[--] ...)::\n> \tUnregister the given submodules, i.e. remove the whole\n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index 899dc334e3..44c11dd91e 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -356,12 +356,10 @@ static void init_submodule(const char *path, const char *prefix, int quiet)\n> \t\t\tstrbuf_addf(&remotesb, \"remote.%s.url\", remote);\n> \t\t\tfree(remote);\n>\n> -\t\t\tif (git_config_get_string(remotesb.buf, &remoteurl))\n> -\t\t\t\t/*\n> -\t\t\t\t * The repository is its own\n> -\t\t\t\t * authoritative upstream\n> -\t\t\t\t */\n> +\t\t\tif (git_config_get_string(remotesb.buf, &remoteurl)) {\n> \t\t\t\tremoteurl = xgetcwd();\n> +\t\t\t\twarning(_(\"could not lookup configuration '%s'. Assuming this repository is its own authoritative upstream.\"), remotesb.buf);\n> +\t\t\t}\n\nIf you re-roll this patch, please place the warning before xgetcwd, \nwhich can potentially fail.\n\n-- Hannes\n\n"},{"id":"312530","messageId":"20170224182237.3696-1-sbeller@google.com","threadId":"45188","inReplyTo":"ae377bda-0776-f98b-5b6f-afa198647400@kdbg.org","subject":"[PATCH] submodule init: warn about falling back to a local path","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-02-24T18:22:37Z","receivedAt":"2017-02-24T18:23:46Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When a submodule is initialized, the config variable 'submodule..url'\nis set depending on the value of the same variable in the .gitmodules\nfile. When the URL indicates to be relative, then the url is computed\nrelative to its default remote. The default remote cannot be determined\naccurately in all cases, such that it falls back to 'origin'.\n\nThe 'origin' remote may not exist, though. In that case we give up looking\nfor a suitable remote and we'll just assume it to be a local relative path.\n\nThis can be confusing to users as there is a lot of guessing involved,\nwhich is not obvious to the user.\n\nSo in the corner case of assuming a local autoritative truth, warn the\nuser to lessen the confusion.\n\nThis behavior was introduced in 4d6893200 (submodule add: allow relative\nrepository path even when no url is set, 2011-06-06), which shared the\ncode with submodule-init and then ported to C in 3604242f080a (submodule:\nport init from shell to C, 2016-04-15).\n\nIn case of submodule-add, this behavior makes sense in some use cases[1],\nhowever for submodule-init there does not seem to be an immediate obvious\nuse case to fall back to a local submodule. However there might be, so\nwarn instead of die here.\n\nWhile adding the warning, also clarify the behavior of relative URLs in\nthe documentation.\n\n[1] e.g. http://stackoverflow.com/questions/8721984/git-ignore-files-for-public-repository-but-not-for-private\n\"store a secret locally in a submodule, with no intention to publish it\"\n\nReported-by: Shawn Pearce \nSigned-off-by: Stefan Beller \n---\n\n* reworded sentence to not omit half of it.\n* tested rendering to be correct\n* warn before xgetcwd\n\nThanks,\nStefan\n\n Documentation/git-submodule.txt | 36 ++++++++++++++++++++++--------------\n builtin/submodule--helper.c | 8 +++-----\n 2 files changed, 25 insertions(+), 19 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 8acc72ebb8..b810d37aa2 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -73,13 +73,17 @@ configuration entries unless `--name` is used to specify a logical name.\n +\n is the URL of the new submodule's origin repository.\n This may be either an absolute URL, or (if it begins with ./\n-or ../), the location relative to the superproject's origin\n+or ../), the location relative to the superproject's default remote\n repository (Please note that to specify a repository 'foo.git'\n which is located right next to a superproject 'bar.git', you'll\n have to use '../foo.git' instead of './foo.git' - as one might expect\n when following the rules for relative URLs - because the evaluation\n of relative URLs in Git is identical to that of relative directories).\n-If the superproject doesn't have an origin configured\n++\n+The default remote is the remote of the remote tracking branch\n+of the current branch. If no such remote tracking branch exists or\n+the HEAD is detached, \"origin\" is assumed to be the default remote.\n+If the superproject doesn't have a default remote configured\n the superproject is its own authoritative upstream and the current\n working directory is used instead.\n +\n@@ -118,18 +122,22 @@ too (and can also report changes to a submodule's work tree).\n \n init [--] [...]::\n \tInitialize the submodules recorded in the index (which were\n-\tadded and committed elsewhere) by copying submodule\n-\tnames and urls from .gitmodules to .git/config.\n-\tOptional arguments limit which submodules will be initialized.\n-\tIt will also copy the value of `submodule.$name.update` into\n-\t.git/config.\n-\tThe key used in .git/config is `submodule.$name.url`.\n-\tThis command does not alter existing information in .git/config.\n-\tYou can then customize the submodule clone URLs in .git/config\n-\tfor your local setup and proceed to `git submodule update`;\n-\tyou can also just use `git submodule update --init` without\n-\tthe explicit 'init' step if you do not intend to customize\n-\tany submodule locations.\n+\tadded and committed elsewhere) by copying `submodule.$name.url`\n+\tfrom .gitmodules to .git/config, resolving relative urls to be\n+\trelative to the default remote.\n++\n+Optional arguments limit which submodules will be initialized.\n+If no path is specified all submodules are initialized.\n++\n+When present, it will also copy the value of `submodule.$name.update`.\n+This command does not alter existing information in .git/config.\n+You can then customize the submodule clone URLs in .git/config\n+for your local setup and proceed to `git submodule update`;\n+you can also just use `git submodule update --init` without\n+the explicit 'init' step if you do not intend to customize\n+any submodule locations.\n++\n+See the add subcommand for the defintion of default remote.\n \n deinit [-f|--force] (--all|[--] ...)::\n \tUnregister the given submodules, i.e. remove the whole\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 899dc334e3..15a5430c00 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -356,12 +356,10 @@ static void init_submodule(const char *path, const char *prefix, int quiet)\n \t\t\tstrbuf_addf(&remotesb, \"remote.%s.url\", remote);\n \t\t\tfree(remote);\n \n-\t\t\tif (git_config_get_string(remotesb.buf, &remoteurl))\n-\t\t\t\t/*\n-\t\t\t\t * The repository is its own\n-\t\t\t\t * authoritative upstream\n-\t\t\t\t */\n+\t\t\tif (git_config_get_string(remotesb.buf, &remoteurl)) {\n+\t\t\t\twarning(_(\"could not lookup configuration '%s'. Assuming this repository is its own authoritative upstream.\"), remotesb.buf);\n \t\t\t\tremoteurl = xgetcwd();\n+\t\t\t}\n \t\t\trelurl = relative_url(remoteurl, url, NULL);\n \t\t\tstrbuf_release(&remotesb);\n \t\t\tfree(remoteurl);\n-- \n2.12.0.rc1.16.ge4278d41a0.dirty\n\n"},{"id":"312547","messageId":"xmqqlgsv1guq.fsf@gitster.mtv.corp.google.com","threadId":"45188","inReplyTo":"20170224182237.3696-1-sbeller@google.com","subject":"Re: [PATCH] submodule init: warn about falling back to a local path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-24T20:19:57Z","receivedAt":"2017-02-24T20:27:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> When a submodule is initialized, the config variable 'submodule..url'\n> is set depending on the value of the same variable in the .gitmodules\n> file. When the URL indicates to be relative, then the url is computed\n> relative to its default remote. The default remote cannot be determined\n> accurately in all cases, such that it falls back to 'origin'.\n>\n> The 'origin' remote may not exist, though. In that case we give up looking\n> for a suitable remote and we'll just assume it to be a local relative path.\n\nIOW we keep the .url to be relative, the same as the original one we\ntook from .gitmodules? That sounds like a sensible thing to do and\nI agree it makes sense to warn when it happens.\n\n> is the URL of the new submodule's origin repository.\n> This may be either an absolute URL, or (if it begins with ./\n> -or ../), the location relative to the superproject's origin\n> +or ../), the location relative to the superproject's default remote\n> repository (Please note that to specify a repository 'foo.git'\n> which is located right next to a superproject 'bar.git', you'll\n> have to use '../foo.git' instead of './foo.git' - as one might expect\n> when following the rules for relative URLs - because the evaluation\n> of relative URLs in Git is identical to that of relative directories).\n> -If the superproject doesn't have an origin configured\n> ++\n> +The default remote is the remote of the remote tracking branch\n> +of the current branch. If no such remote tracking branch exists or\n> +the HEAD is detached, \"origin\" is assumed to be the default remote.\n> +If the superproject doesn't have a default remote configured\n\nOK.\n\n> @@ -118,18 +122,22 @@ too (and can also report changes to a submodule's work tree).\n> \n> init [--] [...]::\n> \tInitialize the submodules recorded in the index (which were\n> -\tadded and committed elsewhere) by copying submodule\n> -\tnames and urls from .gitmodules to .git/config.\n> -\tOptional arguments limit which submodules will be initialized.\n> -\tIt will also copy the value of `submodule.$name.update` into\n> -\t.git/config.\n> -\tThe key used in .git/config is `submodule.$name.url`.\n> -\tThis command does not alter existing information in .git/config.\n> -\tYou can then customize the submodule clone URLs in .git/config\n> -\tfor your local setup and proceed to `git submodule update`;\n> -\tyou can also just use `git submodule update --init` without\n> -\tthe explicit 'init' step if you do not intend to customize\n> -\tany submodule locations.\n> +\tadded and committed elsewhere) by copying `submodule.$name.url`\n> +\tfrom .gitmodules to .git/config, resolving relative urls to be\n> +\trelative to the default remote.\n\nI read this as \"copying..., resolving relative to the default remote\n(if exists).\" A reader would wonder what happens if the default\nremote does not exist---don't we want to describe what happens in\nthat case, like, \"recording . (the current repository) as the\nupstream\" or something?\n\n> ++\n> +Optional arguments limit which submodules will be initialized.\n> +If no path is specified all submodules are initialized.\n\nPerhaps s/ all submodules/,&/?\n\n> deinit [-f|--force] (--all|[--] ...)::\n> \tUnregister the given submodules, i.e. remove the whole\n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index 899dc334e3..15a5430c00 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -356,12 +356,10 @@ static void init_submodule(const char *path, const char *prefix, int quiet)\n> \t\t\tstrbuf_addf(&remotesb, \"remote.%s.url\", remote);\n> \t\t\tfree(remote);\n> \n> -\t\t\tif (git_config_get_string(remotesb.buf, &remoteurl))\n> -\t\t\t\t/*\n> -\t\t\t\t * The repository is its own\n> -\t\t\t\t * authoritative upstream\n> -\t\t\t\t */\n> +\t\t\tif (git_config_get_string(remotesb.buf, &remoteurl)) {\n> +\t\t\t\twarning(_(\"could not lookup configuration '%s'. Assuming this repository is its own authoritative upstream.\"), remotesb.buf);\n\nSounds sensible (it might be a bit too long but it should be OK).\n\n> \t\t\t\tremoteurl = xgetcwd();\n> +\t\t\t}\n> \t\t\trelurl = relative_url(remoteurl, url, NULL);\n> \t\t\tstrbuf_release(&remotesb);\n> \t\t\tfree(remoteurl);\n\nThanks.\n"},{"id":"312548","messageId":"CAGZ79kbzhHaV4-cVvqwodwXSBstRfH1FrOh=iYMvU6cqYUcUng@mail.gmail.com","threadId":"45188","inReplyTo":"xmqqlgsv1guq.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] submodule init: warn about falling back to a local path","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-02-24T20:27:32Z","receivedAt":"2017-02-24T20:27:39Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Feb 24, 2017 at 12:19 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> When a submodule is initialized, the config variable 'submodule..url'\n>> is set depending on the value of the same variable in the .gitmodules\n>> file. When the URL indicates to be relative, then the url is computed\n>> relative to its default remote. The default remote cannot be determined\n>> accurately in all cases, such that it falls back to 'origin'.\n>>\n>> The 'origin' remote may not exist, though. In that case we give up looking\n>> for a suitable remote and we'll just assume it to be a local relative path.\n>\n> IOW we keep the .url to be relative, the same as the original one we\n> took from .gitmodules?\n\nWell that is one way to say that. Another way is:\nIf we cannot construct a URL based on the given relative URL, we\ndemote the relative URL to be a relative PATH and resolve that instead.\n\n> That sounds like a sensible thing to do and\n> I agree it makes sense to warn when it happens.\n\nok.\n\n>\n>> @@ -118,18 +122,22 @@ too (and can also report changes to a submodule's work tree).\n>>\n>> init [--] [...]::\n>> Initialize the submodules recorded in the index (which were\n>> - added and committed elsewhere) by copying submodule\n>> - names and urls from .gitmodules to .git/config.\n>> - Optional arguments limit which submodules will be initialized.\n>> - It will also copy the value of `submodule.$name.update` into\n>> - .git/config.\n>> - The key used in .git/config is `submodule.$name.url`.\n>> - This command does not alter existing information in .git/config.\n>> - You can then customize the submodule clone URLs in .git/config\n>> - for your local setup and proceed to `git submodule update`;\n>> - you can also just use `git submodule update --init` without\n>> - the explicit 'init' step if you do not intend to customize\n>> - any submodule locations.\n>> + added and committed elsewhere) by copying `submodule.$name.url`\n>> + from .gitmodules to .git/config, resolving relative urls to be\n>> + relative to the default remote.\n>\n> I read this as \"copying..., resolving relative to the default remote\n> (if exists).\" A reader would wonder what happens if the default\n> remote does not exist---don't we want to describe what happens in\n> that case, like, \"recording . (the current repository) as the\n> upstream\" or something?\n\neh, a better approach is s/copying//\nWe will resolve the relative URL no matter what. It's just that\nwe may end up with an absolute path instead of an absolute URL.\n\n>\n>> ++\n>> +Optional arguments limit which submodules will be initialized.\n>> +If no path is specified all submodules are initialized.\n>\n> Perhaps s/ all submodules/,&/?\n\nok\n\nWill reroll, once I have found a better wording.\n"},{"id":"312604","messageId":"20170225013147.23663-1-sbeller@google.com","threadId":"45188","inReplyTo":"CAGZ79kbzhHaV4-cVvqwodwXSBstRfH1FrOh=iYMvU6cqYUcUng@mail.gmail.com","subject":"[PATCH] submodule init: warn about falling back to a local path","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-02-25T01:31:47Z","receivedAt":"2017-02-25T01:31:57Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When a submodule is initialized, the config variable 'submodule..url'\nis set depending on the value of the same variable in the .gitmodules\nfile. When the URL indicates to be relative, then the url is computed\nrelative to its default remote. The default remote cannot be determined\naccurately in all cases, such that it falls back to 'origin'.\n\nThe 'origin' remote may not exist, though. In that case we give up looking\nfor a suitable remote and we'll just assume it to be a local relative path.\n\nThis can be confusing to users as there is a lot of guessing involved,\nwhich is not obvious to the user.\n\nSo in the corner case of assuming a local autoritative truth, warn the\nuser to lessen the confusion.\n\nThis behavior was introduced in 4d6893200 (submodule add: allow relative\nrepository path even when no url is set, 2011-06-06), which shared the\ncode with submodule-init and then ported to C in 3604242f080a (submodule:\nport init from shell to C, 2016-04-15).\n\nIn case of submodule-add, this behavior makes sense in some use cases[1],\nhowever for submodule-init there does not seem to be an immediate obvious\nuse case to fall back to a local submodule. However there might be, so\nwarn instead of die here.\n\nWhile adding the warning, also clarify the behavior of relative URLs in\nthe documentation.\n\n[1] e.g. http://stackoverflow.com/questions/8721984/git-ignore-files-for-public-repository-but-not-for-private\n\"store a secret locally in a submodule, with no intention to publish it\"\n\nReported-by: Shawn Pearce \nSigned-off-by: Stefan Beller \n---\n\n> Perhaps s/ all submodules/,&/?\n\ndone\n\n> I read this as \"copying..., resolving relative to the default remote\n> (if exists).\"\n\nreworded with shorter sentences:\n Initialize the submodules recorded in the index (which were\n added and committed elsewhere) by setting `submodule.$name.url`\n in .git/config. It uses the same setting from .gitmodules as\n a template. If the URL is relative, it will be resolved using\n the default remote. If there is no default remote, the current\n repository will be assumed to be upstream.\n...\n next paragraph\n \n\n Documentation/git-submodule.txt | 38 ++++++++++++++++++++++++--------------\n builtin/submodule--helper.c | 8 +++-----\n 2 files changed, 27 insertions(+), 19 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 8acc72ebb8..e05d0cddef 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -73,13 +73,17 @@ configuration entries unless `--name` is used to specify a logical name.\n +\n is the URL of the new submodule's origin repository.\n This may be either an absolute URL, or (if it begins with ./\n-or ../), the location relative to the superproject's origin\n+or ../), the location relative to the superproject's default remote\n repository (Please note that to specify a repository 'foo.git'\n which is located right next to a superproject 'bar.git', you'll\n have to use '../foo.git' instead of './foo.git' - as one might expect\n when following the rules for relative URLs - because the evaluation\n of relative URLs in Git is identical to that of relative directories).\n-If the superproject doesn't have an origin configured\n++\n+The default remote is the remote of the remote tracking branch\n+of the current branch. If no such remote tracking branch exists or\n+the HEAD is detached, \"origin\" is assumed to be the default remote.\n+If the superproject doesn't have a default remote configured\n the superproject is its own authoritative upstream and the current\n working directory is used instead.\n +\n@@ -118,18 +122,24 @@ too (and can also report changes to a submodule's work tree).\n \n init [--] [...]::\n \tInitialize the submodules recorded in the index (which were\n-\tadded and committed elsewhere) by copying submodule\n-\tnames and urls from .gitmodules to .git/config.\n-\tOptional arguments limit which submodules will be initialized.\n-\tIt will also copy the value of `submodule.$name.update` into\n-\t.git/config.\n-\tThe key used in .git/config is `submodule.$name.url`.\n-\tThis command does not alter existing information in .git/config.\n-\tYou can then customize the submodule clone URLs in .git/config\n-\tfor your local setup and proceed to `git submodule update`;\n-\tyou can also just use `git submodule update --init` without\n-\tthe explicit 'init' step if you do not intend to customize\n-\tany submodule locations.\n+\tadded and committed elsewhere) by setting `submodule.$name.url`\n+\tin .git/config. It uses the same setting from .gitmodules as\n+\ta template. If the URL is relative, it will be resolved using\n+\tthe default remote. If there is no default remote, the current\n+\trepository will be assumed to be upstream.\n++\n+Optional arguments limit which submodules will be initialized.\n+If no path is specified, all submodules are initialized.\n++\n+When present, it will also copy the value of `submodule.$name.update`.\n+This command does not alter existing information in .git/config.\n+You can then customize the submodule clone URLs in .git/config\n+for your local setup and proceed to `git submodule update`;\n+you can also just use `git submodule update --init` without\n+the explicit 'init' step if you do not intend to customize\n+any submodule locations.\n++\n+See the add subcommand for the defintion of default remote.\n \n deinit [-f|--force] (--all|[--] ...)::\n \tUnregister the given submodules, i.e. remove the whole\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 899dc334e3..15a5430c00 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -356,12 +356,10 @@ static void init_submodule(const char *path, const char *prefix, int quiet)\n \t\t\tstrbuf_addf(&remotesb, \"remote.%s.url\", remote);\n \t\t\tfree(remote);\n \n-\t\t\tif (git_config_get_string(remotesb.buf, &remoteurl))\n-\t\t\t\t/*\n-\t\t\t\t * The repository is its own\n-\t\t\t\t * authoritative upstream\n-\t\t\t\t */\n+\t\t\tif (git_config_get_string(remotesb.buf, &remoteurl)) {\n+\t\t\t\twarning(_(\"could not lookup configuration '%s'. Assuming this repository is its own authoritative upstream.\"), remotesb.buf);\n \t\t\t\tremoteurl = xgetcwd();\n+\t\t\t}\n \t\t\trelurl = relative_url(remoteurl, url, NULL);\n \t\t\tstrbuf_release(&remotesb);\n \t\t\tfree(remoteurl);\n-- \n2.12.0.rc1.16.ge4278d41a0.dirty\n\n"}]}