[BUG] Fail to add a module in a subdirectory if module is already cloned

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

Am 24.01.2012 20:11, schrieb Jehan Bing:

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.

Am 24.01.2012 22:10, schrieb Jens Lehmann:

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

Jens Lehmann <Jens.Lehmann@web.de> writes:

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?

Am 24.01.2012 22:24, schrieb Junio C Hamano:

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.

Am 24.01.2012 22:44, schrieb 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."

Jens Lehmann <Jens.Lehmann@web.de> writes:

Ah, Ok.

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 <jehan@orb.com>
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

Am 24.01.2012 22:10, schrieb Jens Lehmann:

...

Which I now have.

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

On 2012-01-24 13:49, Jens Lehmann wrote:

That fixed my problem. Thanks Jens.

	Jehan