Re: [BUG] git submodule update is not fail safe

From: Jens Lehmann <jens.lehmann@web.de>
Date: Jan 5, 2013, 14:01 UTC
Message-ID: <50E83224.2070701@web.de>
In-Reply-To: <7vzk0osjli.fsf@alter.siamese.dyndns.org>

Am 04.01.2013 22:51, schrieb Junio C Hamano:

Show 20 quoted lines

> Manlio Perillo <manlio.perillo@gmail.com> writes:
> 
>> $ git submodule update --init
>> ...
>> Submodule 'roms/vgabios' (git://git.qemu.org/vgabios.git/) registered
>> for path 'roms/vgabios'
>> fatal: unable to connect to anongit.freedesktop.org:
>> anongit.freedesktop.org[0: 131.252.210.161]: errno=Connection timed out
>>
>> Unable to fetch in submodule path 'pixman'
>>
>> $ git submodule update --init
>> fatal: Needed a single revision
>> Unable to find current revision in submodule path 'pixman'
>>
>> The problem is easy to solve: manually remove the pixman directory;
>> however IMHO git submodule update should not fail this way since it may
>> confuse the user.
> 
> Sounds like a reasonable observation.  Jens, Heiko, comments?

The reason seems to be that clone leaves a partial initialized .git directory in case of connection problems. The next time submodule update runs it tries to revive the submodule from .git/modules/<name> but fails as there are no objects in it.

This looks like a bug in clone to me, as it takes precautions to clean up if something goes wrong but doesn't do that in this case. But while glancing over the code I didn't find out what goes wrong here.

If this isn't seen as a bug in clone, we could also remove the .git/modules/<name> directory in module_clone() of git-submodule.s h when the clone fails. Manilo, does the following patch remove the problems you are seeing (after removing .git/modules/pixman manually)?

diff --git a/git-submodule.sh b/git-submodule.sh
index 2365149..4098702 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -208,7 +208,10 @@ module_clone()
                        git clone $quiet -n ${reference:+"$reference"} \
                                --separate-git-dir "$gitdir" "$url" "$sm_path"
                ) ||
-               die "$(eval_gettext "Clone of '\$url' into submodule path '\$sm_path' failed")"
+               (
+                       rm -rf "$gitdir" &&
+                       die "$(eval_gettext "Clone of '\$url' into submodule path '\$sm_path' failed")"
+               )
        fi

        # We already are at the root of the work tree but cd_to_toplevel will