Fail to add a module in a subdirectory if module is already cloned

BUG In review Jehan Bing 3 participants 9 messages Jun 15, 2016

Share & tools

Jehan Bing

author

2016-06-15 22:52 UTC #0 lore raw

Hi,

I'm getting an error if I try to add a module in a subdirectory and that module is already cloned. Here are the steps to reproduce (git 1.7.8.3):

git init module
cd module
echo foo > foo
git add foo
git commit -m "init"
cd ..
git init super
cd super
echo foo > foo
git add foo
git commit -m "init"
git branch b1
git branch b2
git checkout b1
git submodule add ../module lib/module
git commit -m "module"
git checkout b2
rm -rf lib
git submodule add ../module lib/module

The last command returns:
fatal: Not a git repository: ../.git/modules/lib/module Unable to checkout submodule 'lib/module'

The file lib/modules/.git contains:
gitdir: ../.git/modules/lib/module
(missing an additional "../")

In branch b1, after adding the module, the file contained the full path:

    gitdir: /[...]/super/.git/modules/lib/module
Or contains the correct relative path after checking out b1 later:
    gitdir: ../../.git/modules/lib/module

Regards,
Jehan

Jens Lehmann

2016-06-15 22:52 UTC #1 lore raw

Am 24.01.2012 20:11, schrieb Jehan Bing:

Quoted 27 lines

I'm getting an error if I try to add a module in a subdirectory and that module is already cloned. Here are the steps to reproduce (git 1.7.8.3):
git init module
cd module
echo foo > foo
git add foo
git commit -m "init"
cd ..
git init super
cd super
echo foo > foo
git add foo
git commit -m "init"
git branch b1
git branch b2
git checkout b1
git submodule add ../module lib/module
git commit -m "module"
git checkout b2
rm -rf lib
git submodule add ../module lib/module
The last command returns:
fatal: Not a git repository: ../.git/modules/lib/module Unable to checkout submodule 'lib/module'

Quoted 8 lines

The file lib/modules/.git contains:
gitdir: ../.git/modules/lib/module
(missing an additional "../")
In branch b1, after adding the module, the file contained the full path:
    gitdir: /[...]/super/.git/modules/lib/module
Or contains the correct relative path after checking out b1 later:
    gitdir: ../../.git/modules/lib/module

Thanks for your detailed report, I can reproduce that on current master.

The reason for this bug seems to be that in module_clonse() the name is not properly initialized for added submodules (it gets set to the path later), so the correct amount of leading "../"s for the git directory is not computed properly. The attached diff fixes that for me, I will send a patch as soon as I have extended a test case for this breakage.

git-submodule.sh +1 -0

index 3adab93..9bb2e13 100755
@@ -131,6 +131,7 @@ module_clone()
        gitdir=
        gitdir_base=
        name=$(module_name "$path" 2>/dev/null)
+       test -n "$name" || name="$path"
        base_path=$(dirname "$path")
 
        gitdir=$(git rev-parse --git-dir)

Jens Lehmann

2016-06-15 22:52 UTC #2 lore raw

Am 24.01.2012 22:10, schrieb Jens Lehmann:

Quoted 2 lines

The reason for this bug seems to be that in module_clonse() the name is not properly initialized for added submodules ...

This should have read "for re-added submodules" ...

Jens Lehmann

2016-06-15 22:52 UTC #3 lore raw

PATCH 1/1 submodule add: fix breakage when re-adding a deep submodule

In reply to Jens Lehmann

Since recently a submodule with name <name> has its git directory in the .git/modules/<name> directory of the superproject while the work tree contains a gitfile pointing there.

When the same submodule is added on a branch where it wasn't present so far (it is not found in the .gitmodules file), the name is not initialized from the path as it should. This leads to a wrong path entered in the gitfile when the .git/modules/<name> directory is found, as this happily uses the - now empty - name. It then always points only a single directory up, even if we have a path deeper in the directory hierarchy.

Fix that by initializing the name of the submodule early in module_clone() if module_name() returned an empty name and add a test to catch that bug.

Reported-by Jehan Bing <[email protected]>

Signed-off-by Jens Lehmann <[email protected]>

---

Am 24.01.2012 22:10, schrieb Jens Lehmann:

Quoted 3 lines

Am 24.01.2012 20:11, schrieb Jehan Bing:
Quoted 2 lines
I'm getting an error if I try to add a module in a subdirectory and that module is already cloned. Here are the steps to reproduce (git 1.7.8.3):

...

Quoted 5 lines

The reason for this bug seems to be that in module_clonse() the name is not properly initialized for added submodules (it gets set to the path later), so the correct amount of leading "../"s for the git directory is not computed properly. The attached diff fixes that for me, I will send a patch as soon as I have extended a test case for this breakage.

Which I now have.

 git-submodule.sh            |    1 +
 t/t7406-submodule-update.sh |    8 ++++++++
 2 files changed, 9 insertions(+), 0 deletions(-)

git-submodule.sh +1 -0

index 3adab93..9bb2e13 100755
@@ -131,6 +131,7 @@ module_clone()
 	gitdir=
 	gitdir_base=
 	name=$(module_name "$path" 2>/dev/null)
+	test -n "$name" || name="$path"
 	base_path=$(dirname "$path")
 
 	gitdir=$(git rev-parse --git-dir)

t/t7406-submodule-update.sh +8 -0

index 33b292b..5b97222 100755
@@ -611,4 +611,12 @@ test_expect_success 'submodule update places git-dir in superprojects git-dir re
 	)
 '
 
+test_expect_success 'submodule add properly re-creates deeper level submodules' '
+	(cd super &&
+	 git reset --hard master &&
+	 rm -rf deeper/ &&
+	 git submodule add ../submodule deeper/submodule
+	)
+'
+
 test_done

Signature · 1 line

1.7.9.rc2.3.g18574a

Jehan Bing

author

2016-06-15 22:52 UTC #4 lore raw

Re: [PATCH] submodule add: fix breakage when re-adding a deep submodule

git-submodule.sh module_clone() · line 137

Quoted 12 lines · Jens Lehmann

git-submodule.sh +1 -0

index 3adab93..9bb2e13 100755
@@ -131,6 +131,7 @@ module_clone()
  	gitdir=
  	gitdir_base=
  	name=$(module_name "$path" 2>/dev/null)
+	test -n "$name" || name="$path"
  	base_path=$(dirname "$path")
 
  	gitdir=$(git rev-parse --git-dir)

That fixed my problem. Thanks Jens.

Jehan

Junio C Hamano

2016-06-15 22:52 UTC #5 lore raw

In reply to Jens Lehmann

Quoted 11 lines · Jens Lehmann

The reason for this bug seems to be that in module_clonse() the name is not properly initialized for added submodules (it gets set to the path later), so the correct amount of leading "../"s for the git directory is not computed properly. The attached diff fixes that for me, I will send a patch as soon as I have extended a test case for this breakage.
git-submodule.sh +0 -0
index 3adab93..9bb2e13 100755

git-submodule.sh module_clone() · line 134

Quoted 4 lines

git-submodule.sh +1 -0

@@ -131,6 +131,7 @@ module_clone()
        gitdir=
        gitdir_base=
        name=$(module_name "$path" 2>/dev/null)
+       test -n "$name" || name="$path"

This somehow smells like sweeping a problem under the rug. Why doesn't module_name find the already registered path in the first place?

I see "module_name" calls "git config -f .gitmodules" and I do not see any cd_to_toplevel in git-submodule.sh that would ensure this call to access the gitmodules file at the top-level of the superproject. Is that the real reason why it is not finding what it should be finding?

Jens Lehmann

2016-06-15 22:52 UTC #6 lore raw

Am 24.01.2012 22:24, schrieb Junio C Hamano:

Quoted 21 lines

Quoted 11 lines · Jens Lehmann
The reason for this bug seems to be that in module_clonse() the name is not properly initialized for added submodules (it gets set to the path later), so the correct amount of leading "../"s for the git directory is not computed properly. The attached diff fixes that for me, I will send a patch as soon as I have extended a test case for this breakage.
git-submodule.sh +0 -0
index 3adab93..9bb2e13 100755
git-submodule.sh module_clone() · line 134
Quoted 4 lines
git-submodule.sh +1 -0
@@ -131,6 +131,7 @@ module_clone()
        gitdir=
        gitdir_base=
        name=$(module_name "$path" 2>/dev/null)
+       test -n "$name" || name="$path"
This somehow smells like sweeping a problem under the rug. Why doesn't module_name find the already registered path in the first place?

Quoted 4 lines

I see "module_name" calls "git config -f .gitmodules" and I do not see any cd_to_toplevel in git-submodule.sh that would ensure this call to access the gitmodules file at the top-level of the superproject. Is that the real reason why it is not finding what it should be finding?

Nope, it's the fact that the .gitmodules file doesn't contain this name because the branch was rewound. Please see my post where I proposed the same change for a slightly different problem:
http://permalink.gmane.org/gmane.comp.version-control.git/187823
(just fast forward to the first hunk of my diff at the end)

I just didn't realize back then that this is needed even without the other changes to work properly. The possibly missing cd_to_toplevel is another problem, the OP started the submodule add in the top level directory anyways.

Jens Lehmann

2016-06-15 22:52 UTC #7 lore raw

Am 24.01.2012 22:44, schrieb Jens Lehmann:

Quoted 5 lines

Am 24.01.2012 22:24, schrieb Junio C Hamano:
Quoted 4 lines
I see "module_name" calls "git config -f .gitmodules" and I do not see any cd_to_toplevel in git-submodule.sh that would ensure this call to access the gitmodules file at the top-level of the superproject. Is that the real reason why it is not finding what it should be finding?

Just for the record: I checked that and git-submodule does not set the SUBDIRECTORY_OK environment variable so every time it is not run in the top level directory it aborts with:
"You need to run this command from the toplevel of the working tree."

Junio C Hamano

2016-06-15 22:52 UTC #8 lore raw

Quoted 4 lines · Jens Lehmann

Just for the record: I checked that and git-submodule does not set the SUBDIRECTORY_OK environment variable so every time it is not run in the top level directory it aborts with:
"You need to run this command from the toplevel of the working tree."

Ah, Ok.