{"thread":{"id":"29440","subject":"[BUG] Fail to add a module in a subdirectory if module is already cloned","startedAt":"2012-01-24T19:11:55Z","lastAt":"2012-01-25T01:48:46Z","messageCount":9,"participants":["Jehan Bing","Jens Lehmann","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"183027","messageId":"jfmvpp$4v7$1@dough.gmane.org","threadId":"29440","inReplyTo":null,"subject":"[BUG] Fail to add a module in a subdirectory if module is already cloned","fromName":"Jehan Bing","fromEmail":"jehan@orb.com","sentAt":"2012-01-24T19:11:55Z","receivedAt":"2012-01-24T19:11:55Z","isPatch":false,"sender":{"key":"jehan@orb.com","avatar":null},"body":"Hi,\n\nI'm getting an error if I try to add a module in a subdirectory and that \nmodule is already cloned.\nHere are the steps to reproduce (git 1.7.8.3):\n\ngit init module\ncd module\necho foo > foo\ngit add foo\ngit commit -m \"init\"\ncd ..\ngit init super\ncd super\necho foo > foo\ngit add foo\ngit commit -m \"init\"\ngit branch b1\ngit branch b2\ngit checkout b1\ngit submodule add ../module lib/module\ngit commit -m \"module\"\ngit checkout b2\nrm -rf lib\ngit submodule add ../module lib/module\n\nThe last command returns:\n     fatal: Not a git repository: ../.git/modules/lib/module\n     Unable to checkout submodule 'lib/module'\n\nThe file lib/modules/.git contains:\n     gitdir: ../.git/modules/lib/module\n(missing an additional \"../\")\n\nIn branch b1, after adding the module, the file contained the full path:\n     gitdir: /[...]/super/.git/modules/lib/module\nOr contains the correct relative path after checking out b1 later:\n     gitdir: ../../.git/modules/lib/module\n\n\nRegards,\n\tJehan\n"},{"id":"183038","messageId":"4F1F1E5F.2030509@web.de","threadId":"29440","inReplyTo":"jfmvpp$4v7$1@dough.gmane.org","subject":"Re: [BUG] Fail to add a module in a subdirectory if module is already cloned","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-01-24T21:10:55Z","receivedAt":"2012-01-24T21:10:55Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 24.01.2012 20:11, schrieb Jehan Bing:\n> I'm getting an error if I try to add a module in a subdirectory and that module is already cloned.\n> Here are the steps to reproduce (git 1.7.8.3):\n> \n> git init module\n> cd module\n> echo foo > foo\n> git add foo\n> git commit -m \"init\"\n> cd ..\n> git init super\n> cd super\n> echo foo > foo\n> git add foo\n> git commit -m \"init\"\n> git branch b1\n> git branch b2\n> git checkout b1\n> git submodule add ../module lib/module\n> git commit -m \"module\"\n> git checkout b2\n> rm -rf lib\n> git submodule add ../module lib/module\n> \n> The last command returns:\n>     fatal: Not a git repository: ../.git/modules/lib/module\n>     Unable to checkout submodule 'lib/module'\n> \n> The file lib/modules/.git contains:\n>     gitdir: ../.git/modules/lib/module\n> (missing an additional \"../\")\n> \n> In branch b1, after adding the module, the file contained the full path:\n>     gitdir: /[...]/super/.git/modules/lib/module\n> Or contains the correct relative path after checking out b1 later:\n>     gitdir: ../../.git/modules/lib/module\n\nThanks for your detailed report, I can reproduce that on current master.\n\nThe reason for this bug seems to be that in module_clonse() the name is\nnot properly initialized for added submodules (it gets set to the path\nlater), so the correct amount of leading \"../\"s for the git directory\nis not computed properly. The attached diff fixes that for me, I will\nsend a patch as soon as I have extended a test case for this breakage.\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 3adab93..9bb2e13 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -131,6 +131,7 @@ module_clone()\n        gitdir=\n        gitdir_base=\n        name=$(module_name \"$path\" 2>/dev/null)\n+       test -n \"$name\" || name=\"$path\"\n        base_path=$(dirname \"$path\")\n\n        gitdir=$(git rev-parse --git-dir)\n"},{"id":"183039","messageId":"4F1F1F10.7020907@web.de","threadId":"29440","inReplyTo":"4F1F1E5F.2030509@web.de","subject":"Re: [BUG] Fail to add a module in a subdirectory if module is already cloned","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-01-24T21:13:52Z","receivedAt":"2012-01-24T21:13:52Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 24.01.2012 22:10, schrieb Jens Lehmann:\n> The reason for this bug seems to be that in module_clonse() the name is\n> not properly initialized for added submodules ...\n\nThis should have read \"for re-added submodules\" ...\n"},{"id":"183040","messageId":"7vhazk3ibk.fsf@alter.siamese.dyndns.org","threadId":"29440","inReplyTo":"4F1F1E5F.2030509@web.de","subject":"Re: [BUG] Fail to add a module in a subdirectory if module is already cloned","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-24T21:24:15Z","receivedAt":"2012-01-24T21:24:15Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann <Jens.Lehmann@web.de> writes:\n\n> The reason for this bug seems to be that in module_clonse() the name is\n> not properly initialized for added submodules (it gets set to the path\n> later), so the correct amount of leading \"../\"s for the git directory\n> is not computed properly. The attached diff fixes that for me, I will\n> send a patch as soon as I have extended a test case for this breakage.\n>\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 3adab93..9bb2e13 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -131,6 +131,7 @@ module_clone()\n>         gitdir=\n>         gitdir_base=\n>         name=$(module_name \"$path\" 2>/dev/null)\n> +       test -n \"$name\" || name=\"$path\"\n\nThis somehow smells like sweeping a problem under the rug. Why doesn't\nmodule_name find the already registered path in the first place?\n\nI see \"module_name\" calls \"git config -f .gitmodules\" and I do not see any\ncd_to_toplevel in git-submodule.sh that would ensure this call to access\nthe gitmodules file at the top-level of the superproject. Is that the real\nreason why it is not finding what it should be finding?\n"},{"id":"183041","messageId":"4F1F2642.1070707@web.de","threadId":"29440","inReplyTo":"7vhazk3ibk.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG] Fail to add a module in a subdirectory if module is already cloned","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-01-24T21:44:34Z","receivedAt":"2012-01-24T21:44:34Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 24.01.2012 22:24, schrieb Junio C Hamano:\n> Jens Lehmann <Jens.Lehmann@web.de> writes:\n> \n>> The reason for this bug seems to be that in module_clonse() the name is\n>> not properly initialized for added submodules (it gets set to the path\n>> later), so the correct amount of leading \"../\"s for the git directory\n>> is not computed properly. The attached diff fixes that for me, I will\n>> send a patch as soon as I have extended a test case for this breakage.\n>>\n>> diff --git a/git-submodule.sh b/git-submodule.sh\n>> index 3adab93..9bb2e13 100755\n>> --- a/git-submodule.sh\n>> +++ b/git-submodule.sh\n>> @@ -131,6 +131,7 @@ module_clone()\n>>         gitdir=\n>>         gitdir_base=\n>>         name=$(module_name \"$path\" 2>/dev/null)\n>> +       test -n \"$name\" || name=\"$path\"\n> \n> This somehow smells like sweeping a problem under the rug. Why doesn't\n> module_name find the already registered path in the first place?\n> \n> I see \"module_name\" calls \"git config -f .gitmodules\" and I do not see any\n> cd_to_toplevel in git-submodule.sh that would ensure this call to access\n> the gitmodules file at the top-level of the superproject. Is that the real\n> reason why it is not finding what it should be finding?\n\nNope, it's the fact that the .gitmodules file doesn't contain this name\nbecause the branch was rewound. Please see my post where I proposed the\nsame change for a slightly different problem:\n  http://permalink.gmane.org/gmane.comp.version-control.git/187823\n(just fast forward to the first hunk of my diff at the end)\n\nI just didn't realize back then that this is needed even without the\nother changes to work properly. The possibly missing cd_to_toplevel is\nanother problem, the OP started the submodule add in the top level\ndirectory anyways.\n"},{"id":"183042","messageId":"4F1F2784.1020904@web.de","threadId":"29440","inReplyTo":"4F1F1E5F.2030509@web.de","subject":"[PATCH] submodule add: fix breakage when re-adding a deep submodule","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-01-24T21:49:56Z","receivedAt":"2012-01-24T21:49:56Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Since recently a submodule with name <name> has its git directory in the\n.git/modules/<name> directory of the superproject while the work tree\ncontains a gitfile pointing there.\n\nWhen the same submodule is added on a branch where it wasn't present so\nfar (it is not found in the .gitmodules file), the name is not initialized\nfrom the path as it should. This leads to a wrong path entered in the\ngitfile when the .git/modules/<name> directory is found, as this happily\nuses the - now empty - name. It then always points only a single directory\nup, even if we have a path deeper in the directory hierarchy.\n\nFix that by initializing the name of the submodule early in module_clone()\nif module_name() returned an empty name and add a test to catch that bug.\n\nReported-by: Jehan Bing <jehan@orb.com>\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n\nAm 24.01.2012 22:10, schrieb Jens Lehmann:\n> Am 24.01.2012 20:11, schrieb Jehan Bing:\n>> I'm getting an error if I try to add a module in a subdirectory and that module is already cloned.\n>> Here are the steps to reproduce (git 1.7.8.3):\n\n...\n\n> The reason for this bug seems to be that in module_clonse() the name is\n> not properly initialized for added submodules (it gets set to the path\n> later), so the correct amount of leading \"../\"s for the git directory\n> is not computed properly. The attached diff fixes that for me, I will\n> send a patch as soon as I have extended a test case for this breakage.\n\nWhich I now have.\n\n\n git-submodule.sh            |    1 +\n t/t7406-submodule-update.sh |    8 ++++++++\n 2 files changed, 9 insertions(+), 0 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 3adab93..9bb2e13 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -131,6 +131,7 @@ module_clone()\n \tgitdir=\n \tgitdir_base=\n \tname=$(module_name \"$path\" 2>/dev/null)\n+\ttest -n \"$name\" || name=\"$path\"\n \tbase_path=$(dirname \"$path\")\n\n \tgitdir=$(git rev-parse --git-dir)\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex 33b292b..5b97222 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -611,4 +611,12 @@ test_expect_success 'submodule update places git-dir in superprojects git-dir re\n \t)\n '\n\n+test_expect_success 'submodule add properly re-creates deeper level submodules' '\n+\t(cd super &&\n+\t git reset --hard master &&\n+\t rm -rf deeper/ &&\n+\t git submodule add ../submodule deeper/submodule\n+\t)\n+'\n+\n test_done\n-- \n1.7.9.rc2.3.g18574a\n"},{"id":"183043","messageId":"4F1F2D38.9050909@web.de","threadId":"29440","inReplyTo":"4F1F2642.1070707@web.de","subject":"Re: [BUG] Fail to add a module in a subdirectory if module is already cloned","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-01-24T22:14:16Z","receivedAt":"2012-01-24T22:14:16Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 24.01.2012 22:44, schrieb Jens Lehmann:\n> Am 24.01.2012 22:24, schrieb Junio C Hamano:\n>> I see \"module_name\" calls \"git config -f .gitmodules\" and I do not see any\n>> cd_to_toplevel in git-submodule.sh that would ensure this call to access\n>> the gitmodules file at the top-level of the superproject. Is that the real\n>> reason why it is not finding what it should be finding?\n\nJust for the record: I checked that and git-submodule does not set the\nSUBDIRECTORY_OK environment variable so every time it is not run in the\ntop level directory it aborts with:\n\"You need to run this command from the toplevel of the working tree.\"\n"},{"id":"183050","messageId":"7vd3a83ewi.fsf@alter.siamese.dyndns.org","threadId":"29440","inReplyTo":"4F1F2D38.9050909@web.de","subject":"Re: [BUG] Fail to add a module in a subdirectory if module is already cloned","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-24T22:38:05Z","receivedAt":"2012-01-24T22:38:05Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann <Jens.Lehmann@web.de> writes:\n\n> Just for the record: I checked that and git-submodule does not set the\n> SUBDIRECTORY_OK environment variable so every time it is not run in the\n> top level directory it aborts with:\n> \"You need to run this command from the toplevel of the working tree.\"\n\nAh, Ok.\n"},{"id":"183061","messageId":"4F1F5F7E.2000506@orb.com","threadId":"29440","inReplyTo":"4F1F2784.1020904@web.de","subject":"Re: [PATCH] submodule add: fix breakage when re-adding a deep submodule","fromName":"Jehan Bing","fromEmail":"jehan@orb.com","sentAt":"2012-01-25T01:48:46Z","receivedAt":"2012-01-25T01:48:46Z","isPatch":true,"sender":{"key":"jehan@orb.com","avatar":null},"body":"On 2012-01-24 13:49, Jens Lehmann wrote:\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 3adab93..9bb2e13 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -131,6 +131,7 @@ module_clone()\n>   \tgitdir=\n>   \tgitdir_base=\n>   \tname=$(module_name \"$path\" 2>/dev/null)\n> +\ttest -n \"$name\" || name=\"$path\"\n>   \tbase_path=$(dirname \"$path\")\n>\n>   \tgitdir=$(git rev-parse --git-dir)\n\nThat fixed my problem. Thanks Jens.\n\n\tJehan\n"}]}