{"thread":{"id":"14246","subject":"[PATCH] fix \"git-submodule add a/b/c/repository\"","startedAt":"2008-07-01T15:00:25Z","lastAt":"2008-07-10T01:05:41Z","messageCount":15,"participants":["Sylvain Joyeux","Junio C Hamano","Mark Levedahl"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"81881","messageId":"20080701150025.GD5852@joyeux","threadId":"14246","inReplyTo":null,"subject":"[PATCH] fix \"git-submodule add a/b/c/repository\"","fromName":"Sylvain Joyeux","fromEmail":"sylvain.joyeux@dfki.de","sentAt":"2008-07-01T15:00:25Z","receivedAt":"2008-07-01T15:00:25Z","isPatch":true,"sender":{"key":"sylvain.joyeux@dfki.de","avatar":null},"body":"The 'master' version of git-submodule.sh fails to see that\na/b/c/repository is an already existing repository and messes up the\nwhole thing. The following patch fixes that.\n-- \nSylvain\n\n>From 2bca2e17a01cd81ce30f81750583ab943ab57ff0 Mon Sep 17 00:00:00 2001\nFrom: Sylvain Joyeux \nDate: Tue, 1 Jul 2008 16:45:04 +0200\nSubject: [PATCH] fix submodule add for non-toplevel in-project directories\n\nThis patch fixes git-submodule add for submodules that\nalready exist in the current package tree, in a folder\nwhich is not at toplevel, i.e.:\n\n git submodule add a/b/c/repository\n\nSigned-off-by: Sylvain Joyeux \n---\n git-submodule.sh | 6 +++++-\n t/t7400-submodule-basic.sh | 9 +++++++++\n 2 files changed, 14 insertions(+), 1 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex e2b91f6..3fa8ff3 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -156,7 +156,11 @@ cmd_add()\n \n \t# Guess path from repo if not specified or strip trailing slashes\n \tif test -z \"$path\"; then\n-\t\tpath=$(echo \"$repo\" | sed -e 's|/*$||' -e 's|:*/*\\.git$||' -e 's|.*[/:]||g')\n+ if echo \"$repo\" | grep -q \"^\\(\\.\\.\\|\\/\\)\"; then\n+ path=$(echo \"$repo\" | sed -e 's|/*$||' -e 's|:*/*\\.git$||' -e 's|.*[/:]||g')\n+ else\n+ path=$(echo \"$repo\" | sed -e 's|/*$||')\n+ fi\n \telse\n \t\tpath=$(echo \"$path\" | sed -e 's|/*$||')\n \tfi\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex ffaa932..84ea6e9 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -215,4 +215,13 @@ test_expect_success 'update --init' '\n \n '\n \n+test_expect_success 'adding an already-existing repository deep in the directory hierarchy' '\n+\n+ mkdir dir0 &&\n+ mkdir dir0/dir1 &&\n+ git clone init dir0/dir1/init &&\n+ git-submodule add dir0/dir1/init &&\n+ git-submodule status | grep \"dir0/dir1/init\"\n+'\n+\n test_done\n-- \n1.5.6\n"},{"id":"82340","messageId":"7vd4lro7ct.fsf@gitster.siamese.dyndns.org","threadId":"14246","inReplyTo":"20080701150025.GD5852@joyeux","subject":"Re: [PATCH] fix \"git-submodule add a/b/c/repository\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-06T06:27:14Z","receivedAt":"2008-07-06T06:27:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sylvain Joyeux writes:\n\n> The 'master' version of git-submodule.sh fails to see that\n> a/b/c/repository is an already existing repository and messes up the\n> whole thing. The following patch fixes that.\n> -- \n> Sylvain\n>\n>>From 2bca2e17a01cd81ce30f81750583ab943ab57ff0 Mon Sep 17 00:00:00 2001\n> From: Sylvain Joyeux \n> Date: Tue, 1 Jul 2008 16:45:04 +0200\n> Subject: [PATCH] fix submodule add for non-toplevel in-project directories\n>\n> This patch fixes git-submodule add for submodules that\n> already exist in the current package tree, in a folder\n> which is not at toplevel, i.e.:\n>\n> git submodule add a/b/c/repository\n\nWhich one is the commit log message ;-)?\n\nPerhaps Documentation/SubmittingPatches needs a review?\n\n> +test_expect_success 'adding an already-existing repository deep in the directory hierarchy' '\n> +\n> + mkdir dir0 &&\n> + mkdir dir0/dir1 &&\n> + git clone init dir0/dir1/init &&\n> + git-submodule add dir0/dir1/init &&\n> + git-submodule status | grep \"dir0/dir1/init\"\n> +'\n\nI am not sure if this is fixing a sane use case. \"submodule add\" is\ndocumented to take:\n\n 'git submodule' [--quiet] add [-b branch] [--] []\n\nand you are adding at dir0/dir1/init a submodule that will interact with \"init\"\nrepository, so shouldn't that command line be something like:\n\n\tgit submodule add init dir0/dir1/init\n\n???\n\nConfused..\n"},{"id":"82378","messageId":"20080706161101.GB23385@jhaampe.org","threadId":"14246","inReplyTo":"7vd4lro7ct.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] fix \"git-submodule add a/b/c/repository\"","fromName":"Sylvain Joyeux","fromEmail":"sylvain.joyeux@dfki.de","sentAt":"2008-07-06T16:11:01Z","receivedAt":"2008-07-06T16:11:01Z","isPatch":true,"sender":{"key":"sylvain.joyeux@dfki.de","avatar":null},"body":"> > +test_expect_success 'adding an already-existing repository deep in the directory hierarchy' '\n> > +\n> > + mkdir dir0 &&\n> > + mkdir dir0/dir1 &&\n> > + git clone init dir0/dir1/init &&\n> > + git-submodule add dir0/dir1/init &&\n> > + git-submodule status | grep \"dir0/dir1/init\"\n> > +'\n> \n> I am not sure if this is fixing a sane use case. \"submodule add\" is\n> documented to take:\n> \n> 'git submodule' [--quiet] add [-b branch] [--] []\n> \n> and you are adding at dir0/dir1/init a submodule that will interact with \"init\"\n> repository, so shouldn't that command line be something like:\n> \n> \tgit submodule add init dir0/dir1/init\n\n \tgit submodule add dir0/dir1/init\n\nIs supposed to add the repository already checked-out in dir0/dir1/init as a\nsubmodule, at the same location.\n\n \tgit submodule add init dir0/dir1/init\n \nWould clone dir0/dir1/init at ./init and add ./init as a submodule. This is\nactually what the current git-submodule (wrongly) does.\n\nSylvain\n"},{"id":"82382","messageId":"48711782.6090609@gmail.com","threadId":"14246","inReplyTo":"20080706161101.GB23385@jhaampe.org","subject":"Re: [PATCH] fix \"git-submodule add a/b/c/repository\"","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2008-07-06T19:05:38Z","receivedAt":"2008-07-06T19:05:38Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Sylvain Joyeux wrote:\n> \tgit submodule add init dir0/dir1/init\n> \n> Would clone dir0/dir1/init at ./init and add ./init as a submodule. This is\n> actually what the current git-submodule (wrongly) does.\n>\n> Sylvain\n> \n...after some prep work...\n\n >git submodule add init dir0/dir1/init\nAdding existing repo at 'dir0/dir1/init' to the index\n\nSo, what's the problem?\n\nMark\n"},{"id":"82424","messageId":"20080707063424.GB5506@jhaampe.org","threadId":"14246","inReplyTo":"48711782.6090609@gmail.com","subject":"Re: [PATCH] fix \"git-submodule add a/b/c/repository\"","fromName":"Sylvain Joyeux","fromEmail":"sylvain.joyeux@dfki.de","sentAt":"2008-07-07T06:34:25Z","receivedAt":"2008-07-07T06:34:25Z","isPatch":true,"sender":{"key":"sylvain.joyeux@dfki.de","avatar":null},"body":"On Sun, Jul 06, 2008 at 03:05:38PM -0400, Mark Levedahl wrote:\n> Sylvain Joyeux wrote:\n>> \tgit submodule add init dir0/dir1/init\n>> Would clone dir0/dir1/init at ./init and add ./init as a submodule. \n>> This is\n>> actually what the current git-submodule (wrongly) does.\n>>\n>> Sylvain\n>> \n> ...after some prep work...\n>\n> >git submodule add init dir0/dir1/init\n> Adding existing repo at 'dir0/dir1/init' to the index\n>\n> So, what's the problem?\n\nRedo the prep work, the clone and now\n\ngit submodule add dir0/dir1/init\n\n(i.e. don't expect dir0/dir1/init to be the clone of ./init, that was just a\nshortcut for the test. Expect it to be a clone of \"something, somewhere\")\n\n[~/tmp/test]% git-submodule add dir0/dir1/init\nInitialize init/.git\nInitialized empty Git repository in /home/doudou/tmp/test/init/.git/\n\n[~/tmp/test]% git status\n# On branch master\n# Changes to be committed:\n# (use \"git reset HEAD ...\" to unstage)\n#\n# modified: .gitmodules\n# new file: init\n#\n# Untracked files:\n# (use \"git add ...\" to include in what will be committed)\n#\n# dir0/\n\nWhat it is supposed to do (according to the documentation) is register\ndir0/dir1/init as the submodule, not clone it and register init/.\n\nSylvain\n"},{"id":"82524","messageId":"4872CF86.5050702@gmail.com","threadId":"14246","inReplyTo":"20080707063424.GB5506@jhaampe.org","subject":"Re: [PATCH] fix \"git-submodule add a/b/c/repository\"","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2008-07-08T02:23:02Z","receivedAt":"2008-07-08T02:23:02Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Sylvain Joyeux wrote:\n>\n> Redo the prep work, the clone and now\n>\n> git submodule add dir0/dir1/init\n>\n> (i.e. don't expect dir0/dir1/init to be the clone of ./init, that was just a\n> shortcut for the test. Expect it to be a clone of \"something, somewhere\")\n>\n> \nPer the man-page,\n git submodule [--quiet] add [-b branch] [--] []\n\nwhich means, that the *repository* url is mandatory, the path is \noptional. What you specifically asked git-submodule to do was to *clone* \nfrom dir0/dir1/init, and because you gave no path to put the submodule \nin, git-submodule deduced the name as \"init\", and cloned to there.\n\nMark\n"},{"id":"82528","messageId":"7v7ibxxfje.fsf@gitster.siamese.dyndns.org","threadId":"14246","inReplyTo":"4872CF86.5050702@gmail.com","subject":"Re: [PATCH] fix \"git-submodule add a/b/c/repository\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-08T02:42:29Z","receivedAt":"2008-07-08T02:42:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Levedahl writes:\n\n> Sylvain Joyeux wrote:\n>>\n>> Redo the prep work, the clone and now\n>>\n>> git submodule add dir0/dir1/init\n>>\n>> (i.e. don't expect dir0/dir1/init to be the clone of ./init, that was just a\n>> shortcut for the test. Expect it to be a clone of \"something, somewhere\")\n>>\n>>\n> Per the man-page,\n> git submodule [--quiet] add [-b branch] [--] []\n>\n> which means, that the *repository* url is mandatory, the path is\n> optional. What you specifically asked git-submodule to do was to\n> *clone* from dir0/dir1/init, and because you gave no path to put the\n> submodule in, git-submodule deduced the name as \"init\", and cloned to\n> there.\n\nI'd like to hear clarifications on two counts, please?\n\n (1) If Sylvain wanted to have that appear at dir0/dir1/init not init,\n would it have been sufficient to give that path twice (once for\n and another for parameter) to make things work as\n expected?\n\n (2) Is it generally considered a sane use case to specify an existing\n repository inside the working tree of a superproject as a submodule\n using \"git submodule add\" like Sylvain's example did?\n\n I would have understood if the command were \"git add dir0/dir1/init\",\n but I have this vague recolleciton that \"git submodule add\" is about\n telling our repository about a submodule that comes from _outside_.\n"},{"id":"82532","messageId":"4872DE54.5010804@gmail.com","threadId":"14246","inReplyTo":"7v7ibxxfje.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] fix \"git-submodule add a/b/c/repository\"","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2008-07-08T03:26:12Z","receivedAt":"2008-07-08T03:26:12Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Junio C Hamano wrote:\n> I'd like to hear clarifications on two counts, please?\n> (1) If Sylvain wanted to have that appear at dir0/dir1/init not init,\n> would it have been sufficient to give that path twice (once for\n> and another for parameter) to make things work as\n> expected?\n> \ngit-submodule really requires two arguments:\n\n $ git submodule add \n\nand supports two modes:\n\n1) relative-path exists and is a valid repo: just add the module, it was \ncreated in tree, the user is expected to eventually push this to the \ngiven URL so other users will get this as normal. This exists to \nsimplify the process of creating a repo to begin with.\n\n2) relative-path doesn't exist: clone from the URL. This is the normal use.\nsubmodule supports adding a module in one of two ways:\n\nSo,\n\n $ git submodule add dir0/dir1/init dir0/dir1/init\n\nwill add the repo, but also makes the repo its own origin. I don't think \nthis makes sense.\n> (2) Is it generally considered a sane use case to specify an existing\n> repository inside the working tree of a superproject as a submodule\n> using \"git submodule add\" like Sylvain's example did?\n>\n> I would have understood if the command were \"git add dir0/dir1/init\",\n> but I have this vague recolleciton that \"git submodule add\" is about\n> telling our repository about a submodule that comes from _outside_.\n>\n>\n> \nAdding an existing in-tree repo, ala\n\n $ git submodule add \n\nis there to ease the initial creation of a submodule. It can be created \nand registered in-tree, and later pushed to the server. This is sane, \nbut is not the normal usage (makes sense only on creation).\n\nMark\n"},{"id":"82559","messageId":"7vhcb0x6ak.fsf@gitster.siamese.dyndns.org","threadId":"14246","inReplyTo":"4872DE54.5010804@gmail.com","subject":"Re: [PATCH] fix \"git-submodule add a/b/c/repository\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-08T06:02:11Z","receivedAt":"2008-07-08T06:02:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Levedahl writes:\n\n> Junio C Hamano wrote:\n>> I'd like to hear clarifications on two counts, please?\n>> (1) If Sylvain wanted to have that appear at dir0/dir1/init not init,\n>> would it have been sufficient to give that path twice (once for\n>> and another for parameter) to make things work as\n>> expected?\n>>\n> git-submodule really requires two arguments:\n>\n> $ git submodule add \n>\n> and supports two modes:\n>\n> 1) relative-path exists and is a valid repo: just add the module, it\n> was created in tree, the user is expected to eventually push this to\n> the given URL so other users will get this as normal. This exists to\n> simplify the process of creating a repo to begin with.\n>\n> 2) relative-path doesn't exist: clone from the URL. This is the normal use.\n> submodule supports adding a module in one of two ways:\n>\n> So,\n>\n> $ git submodule add dir0/dir1/init dir0/dir1/init\n>\n> will add the repo, but also makes the repo its own origin. I don't\n> think this makes sense.\n>> (2) Is it generally considered a sane use case to specify an existing\n>> repository inside the working tree of a superproject as a submodule\n>> using \"git submodule add\" like Sylvain's example did?\n>>\n>> I would have understood if the command were \"git add dir0/dir1/init\",\n>> but I have this vague recolleciton that \"git submodule add\" is about\n>> telling our repository about a submodule that comes from _outside_.\n>>\n>>\n>>\n> Adding an existing in-tree repo, ala\n>\n> $ git submodule add \n>\n> is there to ease the initial creation of a submodule. It can be\n> created and registered in-tree, and later pushed to the server. This\n> is sane, but is not the normal usage (makes sense only on creation).\n\nThanks.\n\nThe above is quite a bit more information than I can read from\nDocumentation/git-submodule.txt; care to send it in in a patch form?\n"},{"id":"82569","messageId":"20080708080848.GC3628@joyeux","threadId":"14246","inReplyTo":"4872DE54.5010804@gmail.com","subject":"Re: [PATCH] fix \"git-submodule add a/b/c/repository\"","fromName":"Sylvain Joyeux","fromEmail":"sylvain.joyeux@dfki.de","sentAt":"2008-07-08T08:08:48Z","receivedAt":"2008-07-08T08:08:48Z","isPatch":true,"sender":{"key":"sylvain.joyeux@dfki.de","avatar":null},"body":"On Mon, Jul 07, 2008 at 11:26:12PM -0400, Mark Levedahl wrote:\n> Junio C Hamano wrote:\n>> I'd like to hear clarifications on two counts, please?\n>> (1) If Sylvain wanted to have that appear at dir0/dir1/init not init,\n>> would it have been sufficient to give that path twice (once for\n>> and another for parameter) to make things work as\n>> expected?\n>> \n> git-submodule really requires two arguments:\n\nThen it should raise an error when the following is given\n\n git submodule add ./relative-path-in-repo\n\nwhich, for now, is accepted as\n\n git submodule add ./relative-path-in-repo ./relative-path-in-repo\n\n(and confused me into thinking it was a normal behaviour)\n\nSylvain\n"},{"id":"82651","messageId":"4873FEF7.7000005@gmail.com","threadId":"14246","inReplyTo":"7vhcb0x6ak.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] fix \"git-submodule add a/b/c/repository\"","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2008-07-08T23:57:43Z","receivedAt":"2008-07-08T23:57:43Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Junio C Hamano wrote:\n> Thanks.\n>\n> The above is quite a bit more information than I can read from\n> Documentation/git-submodule.txt; care to send it in in a patch form?\n>\n> \nWill do, but I think it makes more sense to clean things up a bit so \nthey are more explicable:\n\n1) *require* two arguments for add: \n2) Remove one option for URL. Currently, we accept:\n a) absolute URL : origin is at URL\n b) top-level-relative URL (./foo | ../foo) : locates origin relative \nto top-level origin\n c) path-relative URL : locates origin relative to current working \ndirectory\n\nI don't understand the use-case for item (c), and in any case it is \neasily replaced as an abolute url (e.g., $(pwd)/URL). So, I propose to \nrestrict to (a) or (b) only, and reject (c).\n\nI think these two changes in concert will reduce a lot of confusion \nwithout removing any real capability. Absent negative comments, I will \nprepare a patch to do this, *and* update the documentation to better \ndefine the options.\n\nMark\n"},{"id":"82676","messageId":"1215575965-3588-1-git-send-email-mlevedahl@gmail.com","threadId":"14246","inReplyTo":"7vhcb0x6ak.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] git-submodule - make \"submodule add\" more strict, and document it","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2008-07-09T03:59:25Z","receivedAt":"2008-07-09T03:59:25Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"This change makes \"submodule add\" much more strict in the arguments it\ntakes, and is intended to address confusion as recently noted on the\ngit-list. With this change, the required syntax is:\n\t$ git submodule add URL path\n\nSpecifically, this eliminates the form\n\t$git submodule add URL\nwhich was confused by more than one person as\n\t$git submodule add path\n\nThe change eliminates one form of URL: a path relative to the current\nworking directory. This form was confusing, and does not seem to\ncorrespond to an important use-case. Specifically, no-one has identified\nthe use to clone from a repository already in the tree, but if this is\nneeded it is easily done using an absolute URL: $(pwd)/relative-path. So,\nno functionality is lost.\n\nFollowing this change, there are exactly four variants of\nsubmodule-add, as both arguments have two flavors:\n\nURL can be absolute, or can begin with ./|../ and thus name the origin\nrelative to the top-level origin.\n\nNote: With this patch, \"submodule add\" discerns an absolute URL as\nmatching /*|*:*: e.g., URL begins with /, or it contains a :. This works\nfor all valid URLs, an absolute path in POSIX, as well as an absolute\npath on Windows).\n\npath can either already exist as a valid git repo, or will be cloned from\nthe given URL. The first form here allows easy addition of a new\nsubmodule to an existing project as the module can be added and tested\nin-tree before pushing to the public repository. However, the more\nusual form is the second, where the repo is cloned from the given URL.\n\nThis specifically addresses the issue of\n\t$ git submodule add a/b/c\nattempting to clone from a repository at \"a/b/c\" to create a new module\nin \"c\". This also simplifies description of \"relative URL\" as there is now\nexactly *one* form: a URL relative to the parent's origin repo.\n\nSigned-off-by: Mark Levedahl \n---\n Documentation/git-submodule.txt | 31 +++++++++++++++------\n git-submodule.sh | 55 ++++++++++++++------------------------\n 2 files changed, 42 insertions(+), 44 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 105fc2d..6f24f92 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -9,7 +9,7 @@ git-submodule - Initialize, update or inspect submodules\n SYNOPSIS\n --------\n [verse]\n-'git submodule' [--quiet] add [-b branch] [--] []\n+'git submodule' [--quiet] add [-b branch] [--] \n 'git submodule' [--quiet] status [--cached] [--] [...]\n 'git submodule' [--quiet] init [--] [...]\n 'git submodule' [--quiet] update [--init] [--] [...]\n@@ -20,14 +20,26 @@ COMMANDS\n --------\n add::\n \tAdd the given repository as a submodule at the given path\n-\tto the changeset to be committed next. If path is a valid\n-\trepository within the project, it is added as is. Otherwise,\n-\trepository is cloned at the specified path. path is added to the\n-\tchangeset and registered in .gitmodules. If no path is\n-\tspecified, the path is deduced from the repository specification.\n-\tIf the repository url begins with ./ or ../, it is stored as\n-\tgiven but resolved as a relative path from the main project's\n-\turl when cloning.\n+\tto the changeset to be committed next. This requires two arguments,\n+\t and . is the URL of the new\n+\tsubmodule's origin repository. This may be either an absolute URL,\n+\tor (if it begins with ./ or ../), the location relative\n+\tto the parent repository's origin.\n+\n+\t is the relative location for the cloned submodule to\n+\texist in the current tree. If does not exist, then the\n+\tmodule is created by cloning from the named URL. If does\n+\texist and is already a valid git repository, then this is added\n+\tto the changeset without cloning. This second form is provided\n+\tto ease adding a submodule to a project the first time, and presumes\n+\tthe user will later push the new submodule to the given URL.\n+\n+\tIn either case, the given URL is recorded into .gitmodules for\n+\tuse by subsequent users cloning the project. If the URL is given\n+\trelative to the parent, the presumption is the main and sub-modules\n+\twill be kept together in the same relative location, and only the\n+\ttop-level URL need be provided: git-submodule will correctly\n+\tlocat the submodules using the hint in .gitmodules.\n \n status::\n \tShow the status of the submodules. This will print the SHA-1 of the\n@@ -85,6 +97,7 @@ OPTIONS\n ::\n \tPath to submodule(s). When specified this will restrict the command\n \tto only operate on the submodules found at the specified paths.\n+\t(This argument is required with add).\n \n FILES\n -----\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 3eb78cc..d227907 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -5,7 +5,7 @@\n # Copyright (c) 2007 Lars Hjemli\n \n USAGE=\"[--quiet] [--cached] \\\n-[add [-b branch]|status|init|update [-i|--init]|summary [-n|--summary-limit ] []] \\\n+[add [-b branch] ]|[status|init|update [-i|--init]|summary [-n|--summary-limit ] []] \\\n [--] [...]\"\n OPTIONS_SPEC=\n . git-sh-setup\n@@ -27,18 +27,6 @@ say()\n \tfi\n }\n \n-# NEEDSWORK: identical function exists in get_repo_base in clone.sh\n-get_repo_base() {\n-\t(\n-\t\tcd \"`/bin/pwd`\" &&\n-\t\tcd \"$1\" || cd \"$1.git\" &&\n-\t\t{\n-\t\t\tcd .git\n-\t\t\tpwd\n-\t\t}\n-\t) 2>/dev/null\n-}\n-\n # Resolve relative url by appending to parent's url\n resolve_relative_url ()\n {\n@@ -115,7 +103,7 @@ module_clone()\n #\n # Add a new submodule to the working tree, .gitmodules and the index\n #\n-# $@ = repo [path]\n+# $@ = repo path\n #\n # optional branch is stored in global branch variable\n #\n@@ -150,16 +138,27 @@ cmd_add()\n \trepo=$1\n \tpath=$2\n \n-\tif test -z \"$repo\"; then\n+\tif test -z \"$repo\" -o -z \"$path\"; then\n \t\tusage\n \tfi\n \n-\t# Guess path from repo if not specified or strip trailing slashes\n-\tif test -z \"$path\"; then\n-\t\tpath=$(echo \"$repo\" | sed -e 's|/*$||' -e 's|:*/*\\.git$||' -e 's|.*[/:]||g')\n-\telse\n-\t\tpath=$(echo \"$path\" | sed -e 's|/*$||')\n-\tfi\n+\t# assure repo is absolute or relative to parent\n+\tcase \"$repo\" in\n+\t\t./*|../*)\n+\t\t\t# dereference source url relative to parent's url\n+\t\t\trealrepo=$(resolve_relative_url \"$repo\") || exit\n+\t\t\t;;\n+\t *:*|/*)\n+\t\t\t# absolute url\n+\t\t\trealrepo=$repo\n+\t\t\t;;\n+\t\t*)\n+\t\t\tdie \"repo URL: '$repo' must be absolute or begin with ./|../\"\n+\t ;;\n+\tesac\n+\n+\t# strip trailing slashes from path\n+\tpath=$(echo \"$path\" | sed -e 's|/*$||')\n \n \tgit ls-files --error-unmatch \"$path\" > /dev/null 2>&1 &&\n \tdie \"'$path' already exists in the index\"\n@@ -175,20 +174,6 @@ cmd_add()\n \t\t\tdie \"'$path' already exists and is not a valid git repo\"\n \t\tfi\n \telse\n-\t\tcase \"$repo\" in\n-\t\t./*|../*)\n-\t\t\t# dereference source url relative to parent's url\n-\t\t\trealrepo=$(resolve_relative_url \"$repo\") || exit\n-\t\t\t;;\n-\t\t*)\n-\t\t\t# Turn the source into an absolute path if\n-\t\t\t# it is local\n-\t\t\tif base=$(get_repo_base \"$repo\"); then\n-\t\t\t\trepo=\"$base\"\n-\t\t\tfi\n-\t\t\trealrepo=$repo\n-\t\t\t;;\n-\t\tesac\n \n \t\tmodule_clone \"$path\" \"$realrepo\" || exit\n \t\t(unset GIT_DIR; cd \"$path\" && git checkout -q ${branch:+-b \"$branch\" \"origin/$branch\"}) ||\n-- \n1.5.6.2.271.g73ad8\n"},{"id":"82689","messageId":"7vwsjvlhjw.fsf@gitster.siamese.dyndns.org","threadId":"14246","inReplyTo":"1215575965-3588-1-git-send-email-mlevedahl@gmail.com","subject":"Re: [PATCH] git-submodule - make \"submodule add\" more strict, and document it","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-09T06:04:19Z","receivedAt":"2008-07-09T06:04:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Levedahl writes:\n\n> This change makes \"submodule add\" much more strict in the arguments it\n> takes, and is intended to address confusion as recently noted on the\n> git-list. With this change, the required syntax is:\n> \t$ git submodule add URL path\n>\n\nPlease have an extra blank line on both sides of examples.\n\n> Specifically, this eliminates the form\n> \t$git submodule add URL\n> which was confused by more than one person as\n> \t$git submodule add path\n\ns/\\$/& /;\n\n> The change eliminates one form of URL: a path relative to the current\n> working directory....\n> ...\n> Following this change, there are exactly four variants of\n> submodule-add, as both arguments have two flavors:\n>\n> URL can be absolute, or can begin with ./|../ and thus name the origin\n> relative to the top-level origin.\n\nNow this _is_ confusing. Examples of a path relative to the current\nworking directory would be ./foo or ../../foo but the previous paragraph\nsays the form is eliminated. I think I know what you want to say, but it\nstill is confusing.\n\n> add::\n> \tAdd the given repository as a submodule at the given path\n> +\tto the changeset to be committed next. This requires two arguments,\n> +\t and . is the URL of the new\n> +\tsubmodule's origin repository. This may be either an absolute URL,\n> +\tor (if it begins with ./ or ../), the location relative\n> +\tto the parent repository's origin.\n\nThis is much better than the part I found confusing above. Here, \"parent\nrepository\" actually means \"this repository\", right? It is the one that\nowns the index you are adding the gitlink to and tracks the .gitmodules\nfile you are adding an entry for this submodule to.\n\n> +\t is the relative location for the cloned submodule to\n> +\texist in the current tree. If does not exist, then the\n> +\tmodule is created by cloning from the named URL. If does\n> +\texist and is already a valid git repository, then this is added\n> +\tto the changeset without cloning. This second form is provided\n> +\tto ease adding a submodule to a project the first time, and presumes\n> +\tthe user will later push the new submodule to the given URL.\n> +\n> +\tIn either case, the given URL is recorded into .gitmodules for\n> +\tuse by subsequent users cloning the project. If the URL is given\n> +\trelative to the parent, the presumption is the main and sub-modules\n\nWe seem to say \"superproject\" and \"submodule\" elsewhere, including Linus's\ntalk.\n\n> +\twill be kept together in the same relative location, and only the\n> +\ttop-level URL need be provided: git-submodule will correctly\n> +\tlocat the submodules using the hint in .gitmodules.\n\ns/locat/&e/;\n\n> @@ -150,16 +138,27 @@ cmd_add()\n> ...\n> +\t# assure repo is absolute or relative to parent\n> +\tcase \"$repo\" in\n> +\t\t./*|../*)\n> +\t\t\t# dereference source url relative to parent's url\n> +\t\t\trealrepo=$(resolve_relative_url \"$repo\") || exit\n> +\t\t\t;;\n> +\t *:*|/*)\n\nFunny indentation; \"case/esac\" and its arms align, like so:\n\n\tcase \"$repo\" in\n ./* | ../*)\n \t...\n\t\t;;\n\tesac\n"},{"id":"82763","messageId":"1215651941-3460-1-git-send-email-mlevedahl@gmail.com","threadId":"14246","inReplyTo":"7vwsjvlhjw.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] git-submodule - make \"submodule add\" more strict, and document it","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2008-07-10T01:05:40Z","receivedAt":"2008-07-10T01:05:40Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"This change makes \"submodule add\" much more strict in the arguments it\ntakes, and is intended to address confusion as recently noted on the\ngit-list. With this change, the required syntax is:\n\n\t$ git submodule add URL path\n\nSpecifically, this eliminates the form\n\n\t$ git submodule add URL\n\nwhich was confused by more than one person as\n\n\t$ git submodule add path\n\nWith this patch, the URL locating the submodule's origin repository can be\neither an absolute URL, or (if it begins with ./ or ../) can express the\nsubmodule's repository location relative to the superproject's origin.\n\nThis patch also eliminates a third form of URL, which was relative to the\nsuperproject's top-level directory (not its repository). Any URL that was\nneither absolute nor matched ./*|../* was assumed to point to a\nsubdirectory of the superproject as the location of the submodule's origin\nrepository. This URL form was confusing and does not seem to correspond\nto an important use-case. Specifically, no-one has identified the need to\nclone from a repository already in the superproject's tree, but if this is\nneeded it is easily done using an absolute URL: $(pwd)/relative-path. So,\nno functionality is lost with this patch. (t6008-rev-list-submodule.sh did\nrely upon this relative URL, fixed by using $(pwd).)\n\nFollowing this change, there are exactly four variants of\nsubmodule-add, as both arguments have two flavors:\n\nURL can be absolute, or can begin with ./|../ and thus names the\nsubmodule's origin relative to the superproject's origin.\n\nNote: With this patch, \"submodule add\" discerns an absolute URL as\nmatching /*|*:*: e.g., URL begins with /, or it contains a :. This works\nfor all valid URLs, an absolute path in POSIX, as well as an absolute path\non Windows).\n\npath can either already exist as a valid git repo, or will be cloned from\nthe given URL. The first form here eases creation of a new submodule in\nan existing superproject as the submodule can be added and tested in-tree\nbefore pushing to the public repository. However, the more usual form is\nthe second, where the repo is cloned from the given URL.\n\nThis specifically addresses the issue of\n\n\t$ git submodule add a/b/c\n\nattempting to clone from a repository at \"a/b/c\" to create a new module\nin \"c\". This also simplifies description of \"relative URL\" as there is now\nexactly *one* form: a URL relative to the parent's origin repo.\n\nSigned-off-by: Mark Levedahl \n---\nThis should address Junio's issues. In addition, I found one test\naffected by this change and fixed that (the test actually used\nthe relative path URL to clone, replaced with $(pwd)).\n\n Documentation/git-submodule.txt | 36 +++++++++++++++++++------\n git-submodule.sh | 55 ++++++++++++++------------------------\n t/t6008-rev-list-submodule.sh | 2 +-\n 3 files changed, 48 insertions(+), 45 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 105fc2d..1fe13a6 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -9,7 +9,7 @@ git-submodule - Initialize, update or inspect submodules\n SYNOPSIS\n --------\n [verse]\n-'git submodule' [--quiet] add [-b branch] [--] []\n+'git submodule' [--quiet] add [-b branch] [--] \n 'git submodule' [--quiet] status [--cached] [--] [...]\n 'git submodule' [--quiet] init [--] [...]\n 'git submodule' [--quiet] update [--init] [--] [...]\n@@ -20,14 +20,31 @@ COMMANDS\n --------\n add::\n \tAdd the given repository as a submodule at the given path\n-\tto the changeset to be committed next. If path is a valid\n-\trepository within the project, it is added as is. Otherwise,\n-\trepository is cloned at the specified path. path is added to the\n-\tchangeset and registered in .gitmodules. If no path is\n-\tspecified, the path is deduced from the repository specification.\n-\tIf the repository url begins with ./ or ../, it is stored as\n-\tgiven but resolved as a relative path from the main project's\n-\turl when cloning.\n+\tto the changeset to be committed next to the current\n+\tproject: the current project is termed termed the \"superproject\".\n+\n+\tThis requires two arguments: and .\n+\n+\t is the URL of the new submodule's origin repository.\n+\tThis may be either an absolute URL, or (if it begins with ./\n+\tor ../), the location relative to the superproject's origin\n+\trepository.\n+\n+\t is the relative location for the cloned submodule to\n+\texist in the superproject. If does not exist, then the\n+\tsubmodule is created by cloning from the named URL. If does\n+\texist and is already a valid git repository, then this is added\n+\tto the changeset without cloning. This second form is provided\n+\tto ease creating a new submodule from scratch, and presumes\n+\tthe user will later push the submodule to the given URL.\n+\n+\tIn either case, the given URL is recorded into .gitmodules for\n+\tuse by subsequent users cloning the superproject. If the URL is\n+\tgiven relative to the superproject's repository, the presumption\n+\tis the superproject and submodule repositories will be kept\n+\ttogether in the same relative location, and only the\n+\tsuperproject's URL need be provided: git-submodule will correctly\n+\tlocate the submodule using the relative URL in .gitmodules.\n\n status::\n \tShow the status of the submodules. This will print the SHA-1 of the\n@@ -85,6 +102,7 @@ OPTIONS\n ::\n \tPath to submodule(s). When specified this will restrict the command\n \tto only operate on the submodules found at the specified paths.\n+\t(This argument is required with add).\n\n FILES\n -----\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 099a7d7..c2ce2fb 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -5,7 +5,7 @@\n # Copyright (c) 2007 Lars Hjemli\n\n USAGE=\"[--quiet] [--cached] \\\n-[add [-b branch]|status|init|update [-i|--init]|summary [-n|--summary-limit ] []] \\\n+[add [-b branch] ]|[status|init|update [-i|--init]|summary [-n|--summary-limit ] []] \\\n [--] [...]\"\n OPTIONS_SPEC=\n . git-sh-setup\n@@ -27,18 +27,6 @@ say()\n \tfi\n }\n\n-# NEEDSWORK: identical function exists in get_repo_base in clone.sh\n-get_repo_base() {\n-\t(\n-\t\tcd \"`/bin/pwd`\" &&\n-\t\tcd \"$1\" || cd \"$1.git\" &&\n-\t\t{\n-\t\t\tcd .git\n-\t\t\tpwd\n-\t\t}\n-\t) 2>/dev/null\n-}\n-\n # Resolve relative url by appending to parent's url\n resolve_relative_url ()\n {\n@@ -115,7 +103,7 @@ module_clone()\n #\n # Add a new submodule to the working tree, .gitmodules and the index\n #\n-# $@ = repo [path]\n+# $@ = repo path\n #\n # optional branch is stored in global branch variable\n #\n@@ -150,16 +138,27 @@ cmd_add()\n \trepo=$1\n \tpath=$2\n\n-\tif test -z \"$repo\"; then\n+\tif test -z \"$repo\" -o -z \"$path\"; then\n \t\tusage\n \tfi\n\n-\t# Guess path from repo if not specified or strip trailing slashes\n-\tif test -z \"$path\"; then\n-\t\tpath=$(echo \"$repo\" | sed -e 's|/*$||' -e 's|:*/*\\.git$||' -e 's|.*[/:]||g')\n-\telse\n-\t\tpath=$(echo \"$path\" | sed -e 's|/*$||')\n-\tfi\n+\t# assure repo is absolute or relative to parent\n+\tcase \"$repo\" in\n+\t./*|../*)\n+\t\t# dereference source url relative to parent's url\n+\t\trealrepo=$(resolve_relative_url \"$repo\") || exit\n+\t\t;;\n+\t*:*|/*)\n+\t\t# absolute url\n+\t\trealrepo=$repo\n+\t\t;;\n+\t*)\n+\t\tdie \"repo URL: '$repo' must be absolute or begin with ./|../\"\n+\t;;\n+\tesac\n+\n+\t# strip trailing slashes from path\n+\tpath=$(echo \"$path\" | sed -e 's|/*$||')\n\n \tgit ls-files --error-unmatch \"$path\" > /dev/null 2>&1 &&\n \tdie \"'$path' already exists in the index\"\n@@ -174,20 +173,6 @@ cmd_add()\n \t\t\tdie \"'$path' already exists and is not a valid git repo\"\n \t\tfi\n \telse\n-\t\tcase \"$repo\" in\n-\t\t./*|../*)\n-\t\t\t# dereference source url relative to parent's url\n-\t\t\trealrepo=$(resolve_relative_url \"$repo\") || exit\n-\t\t\t;;\n-\t\t*)\n-\t\t\t# Turn the source into an absolute path if\n-\t\t\t# it is local\n-\t\t\tif base=$(get_repo_base \"$repo\"); then\n-\t\t\t\trepo=\"$base\"\n-\t\t\tfi\n-\t\t\trealrepo=$repo\n-\t\t\t;;\n-\t\tesac\n\n \t\tmodule_clone \"$path\" \"$realrepo\" || exit\n \t\t(unset GIT_DIR; cd \"$path\" && git checkout -q ${branch:+-b \"$branch\" \"origin/$branch\"}) ||\ndiff --git a/t/t6008-rev-list-submodule.sh b/t/t6008-rev-list-submodule.sh\nindex 88e96fb..c4af9ca 100755\n--- a/t/t6008-rev-list-submodule.sh\n+++ b/t/t6008-rev-list-submodule.sh\n@@ -23,7 +23,7 @@ test_expect_success 'setup' '\n\n \t: > super-file &&\n \tgit add super-file &&\n-\tgit submodule add . sub &&\n+\tgit submodule add \"$(pwd)\" sub &&\n \tgit symbolic-ref HEAD refs/heads/super &&\n \ttest_tick &&\n \tgit commit -m super-initial &&\n--\n1.5.6.2.271.g73ad8\n"},{"id":"82764","messageId":"1215651941-3460-2-git-send-email-mlevedahl@gmail.com","threadId":"14246","inReplyTo":"1215651941-3460-1-git-send-email-mlevedahl@gmail.com","subject":"[PATCH] git-submodule - register submodule URL if adding in place","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2008-07-10T01:05:41Z","receivedAt":"2008-07-10T01:05:41Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"When adding a new submodule in place, meaning the user created the\nsubmodule as a git repo in the superproject's tree first, we don't go\nthrough \"git submodule init\" to register the module. Thus, the\nsubmodule's origin repository URL is not stored in .git/config, and no\nsubsequent submodule operation will ever do so. In this case, assume the\nURL the user supplies to \"submodule add\" is the one that should be\nregistered, and do so.\n\nSigned-off-by: Mark Levedahl \n---\nDscho's issue, so far as I can tell, was not really with this patch\nbut is addressed by the preceding patch eliminating the\nconfusing single argument \"submodule add\" format that lead to the\nerror he wanted to avoid. This patch is not changed except that it\nis rebased.\n\n git-submodule.sh | 10 ++++++++++\n 1 files changed, 10 insertions(+), 0 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex c2ce2fb..9228f56 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -172,6 +172,16 @@ cmd_add()\n \t\telse\n \t\t\tdie \"'$path' already exists and is not a valid git repo\"\n \t\tfi\n+\n+\t\tcase \"$repo\" in\n+\t\t./*|../*)\n+\t\t\turl=$(resolve_relative_url \"$repo\") || exit\n+\t\t ;;\n+\t\t*)\n+\t\t\turl=\"$repo\"\n+\t\t\t;;\n+\t\tesac\n+\t\tgit config submodule.\"$path\".url \"$url\"\n \telse\n\n \t\tmodule_clone \"$path\" \"$realrepo\" || exit\n--\n1.5.6.2.271.g73ad8\n"}]}