git-submodule path computation bug with recursive submodules

git-submodule path computation bug with recursive submodules

[Bob Halley]

The git-submodule command has problems in some cases when there is a symlink
in the path returned by pwd.

I've created a repository at http://www.play-bow.org/repos/product to
demonstrate the issue.

$ git --version
git version 1.7.11.1.116.g8228a23

I copied and built the head of the master branch today.

First we need a symlink.  I discovered this problem because I had a symlink
in / to a directory under my home directory for typing convenience.

$ ls -al /bug
lrwxr-xr-x  1 root  admin  17  5 Jul 12:26 /bug@ -> /Users/halley/bug

$ cd /bug
$ pwd
/bug

Note that pwd does not resolve the symlink (though pwd -P would on many
platforms)

$ pwd -P
/Users/halley/bug/product

$ git clone http://www.play-bow.org/repos/product
Cloning into 'product'...

I'm purposely not using 'clone --recursive' as the bug doesn't appear if you
do that.

$ cd product/
$ ls
file1	file2	foo/

Let's get those submodules...

$ git submodule update --init --recursive
Submodule 'foo' (http://www.play-bow.org/repos/foo) registered for path 'foo'
Cloning into 'foo'...
Submodule path 'foo': checked out '2b02e1eb2e34961d807cfc5fc7e477e0ca844600'
Submodule 'bar' (http://www.play-bow.org/repos/bar) registered for path 'bar'
Cloning into 'bar'...
fatal: Not a git repository: ../../../../Users/halley/bug/product/.git/modules/foo/modules/bar
Failed to recurse into submodule path 'foo'

The first level of submodule clones successfully, but submodule 'foo'
contains submodule 'bar', and cloning it fails.

Now we'll try it again, but this time we'll take the symlink out of the picture.

$ cd /Users/halley/bug
$ rm -rf product
$ pwd
/Users/halley/bug
$ git clone http://www.play-bow.org/repos/product
Cloning into 'product'...
$ cd product
$ git submodule update --init --recursive
Submodule 'foo' (http://www.play-bow.org/repos/foo) registered for path 'foo'
Cloning into 'foo'...
Submodule path 'foo': checked out '2b02e1eb2e34961d807cfc5fc7e477e0ca844600'
Submodule 'bar' (http://www.play-bow.org/repos/bar) registered for path 'bar'
Cloning into 'bar'...
Submodule path 'bar': checked out '00f1d87d3bd6948ef861f2b134daafabb10f7d6f'

All good.

The problem occurs in git-submodule when it replaces path components
with '..'.  It's assuming this will create a valid relative path by
going up to the common root and then down, but this is an invalid
assumption if any of the path components are symlinks.

Here's some partial output of git-submodule running with sh -x set, in a
situation when the pwd is /bug:

++ git clone -n --separate-git-dir /Users/halley/bug/product/.git/modules/foo/modules/bar http://www.play-bow.org/repos/bar bar
Cloning into 'bar'...
+++ cd /Users/halley/bug/product/.git/modules/foo/modules/bar
+++ pwd
++ a=/Users/halley/bug/product/.git/modules/foo/modules/bar/
+++ cd bar
+++ pwd
++ b=/bug/product/foo/bar/
++ case $a in
++ case $b in
++ test /Users/halley/bug/product/.git/modules/foo/modules/bar/ '!=' /Users/halley/bug/product/.git/modules/foo/modules/bar/
++ test /bug/product/foo/bar/ '!=' /bug/product/foo/bar/
++ test '' = ''
++ a=Users/halley/bug/product/.git/modules/foo/modules/bar/
++ b=bug/product/foo/bar/
++ test Users = bug
++ a=Users/halley/bug/product/.git/modules/foo/modules/bar
++ b=bug/product/foo/bar
+++ echo bug/product/foo/bar
+++ sed -e 's|[^/][^/]*|..|g'
++ rel=../../../..
++ echo 'gitdir: ../../../../Users/halley/bug/product/.git/modules/foo/modules/bar'
+++ echo Users/halley/bug/product/.git/modules/foo/modules/bar
+++ sed -e 's|[^/][^/]*|..|g'
++ rel=../../../../../../../../..
++ clear_local_git_env
+++ git rev-parse --local-env-vars
++ unset GIT_ALTERNATE_OBJECT_DIRECTORIES GIT_CONFIG GIT_CONFIG_PARAMETERS GIT_OBJECT_DIRECTORY GIT_DIR GIT_WORK_TREE GIT_GRAFT_FILE GIT_INDEX_FILE GIT_NO_REPLACE_OBJECTS
++ cd bar
++ GIT_WORK_TREE=.
++ git config core.worktree ../../../../../../../../../bug/product/foo/bar
fatal: Not a git repository: ../../../../Users/halley/bug/product/.git/modules/foo/modules/bar

This bit:

++ rel=../../../..
++ echo 'gitdir: ../../../../Users/halley/bug/product/.git/modules/foo/modules/bar'

is wrong, because from /bug/product/foo/bar, ../../../.. is
/Users/halley, not /.

If I replace the calls to "pwd" with "pwd -P", the problem goes away.
Unfortunately I'm not sure how to fix the script portably, as neither pwd -P
nor readlink(1) seem to be universally available.

Regards,

/Bob

Re: git-submodule path computation bug with recursive submodules

[Phil Hord]

On Thu, Jul 5, 2012 at 8:09 AM, Bob Halley <halley@play-bow.org> wrote:
>
> The git-submodule command has problems in some cases when there is a symlink
> in the path returned by pwd.
>
> I've created a repository at http://www.play-bow.org/repos/product to
> demonstrate the issue.
>
> $ git --version
> git version 1.7.11.1.116.g8228a23
>
> I copied and built the head of the master branch today.
>
> First we need a symlink.  I discovered this problem because I had a symlink
> in / to a directory under my home directory for typing convenience.
>
> $ ls -al /bug
> lrwxr-xr-x  1 root  admin  17  5 Jul 12:26 /bug@ -> /Users/halley/bug
>
> $ cd /bug
> $ pwd
> /bug
>
> Note that pwd does not resolve the symlink (though pwd -P would on many
> platforms)
>
> $ pwd -P
> /Users/halley/bug/product
>
> $ git clone http://www.play-bow.org/repos/product
> Cloning into 'product'...
>
> I'm purposely not using 'clone --recursive' as the bug doesn't appear if you
> do that.
>
> $ cd product/
> $ ls
> file1   file2   foo/
>
> Let's get those submodules...
>
> $ git submodule update --init --recursive
> Submodule 'foo' (http://www.play-bow.org/repos/foo) registered for path 'foo'
> Cloning into 'foo'...
> Submodule path 'foo': checked out '2b02e1eb2e34961d807cfc5fc7e477e0ca844600'
> Submodule 'bar' (http://www.play-bow.org/repos/bar) registered for path 'bar'
> Cloning into 'bar'...
> fatal: Not a git repository: ../../../../Users/halley/bug/product/.git/modules/foo/modules/bar
> Failed to recurse into submodule path 'foo'
>
> The first level of submodule clones successfully, but submodule 'foo'
> contains submodule 'bar', and cloning it fails.

It fails for me too, running zsh and stock Linux.

Seems to originate here.

Commit: 69c3051780d6cacfe242563296160634dc667a90:
Author: Jens Lehmann <Jens.Lehmann@web.de>
Date:   Sun Mar 4 22:15:36 2012 +0100

    submodules: refactor computation of relative gitdir path

Phil

Re: git-submodule path computation bug with recursive submodules

[Jens Lehmann]

Am 05.07.2012 16:18, schrieb Phil Hord:
> On Thu, Jul 5, 2012 at 8:09 AM, Bob Halley <halley@play-bow.org> wrote:
>> The first level of submodule clones successfully, but submodule 'foo'
>> contains submodule 'bar', and cloning it fails.
> 
> It fails for me too, running zsh and stock Linux.
> 
> Seems to originate here.
> 
> Commit: 69c3051780d6cacfe242563296160634dc667a90:
> Author: Jens Lehmann <Jens.Lehmann@web.de>
> Date:   Sun Mar 4 22:15:36 2012 +0100
> 
>     submodules: refactor computation of relative gitdir path

Yup, thanks both for reporting and nailing that one down.

This diff fixes the problem for me and should be a portable way to
avoid "pwd -P" or "cd -P", I'll cook up a patch with test for that:


diff --git a/git-submodule.sh b/git-submodule.sh
index 5c61ae2..4a22555 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -150,8 +150,8 @@ module_clone()
                die "$(eval_gettext "Clone of '\$url' into submodule path '\$sm_
        fi

-       a=$(cd "$gitdir" && pwd)/
-       b=$(cd "$sm_path" && pwd)/
+       a=$(cd_to_toplevel && cd "$gitdir" && pwd)/
+       b=$(cd_to_toplevel && cd "$sm_path" && pwd)/
        # normalize Windows-style absolute paths to POSIX-style absolute paths
        case $a in [a-zA-Z]:/*) a=/${a%%:*}${a#*:} ;; esac
        case $b in [a-zA-Z]:/*) b=/${b%%:*}${b#*:} ;; esac