# [PATCH] git-submodule: a small fix

3 messages from 2011-09-26 to 2011-09-27. Participants: Roy Liu, Andrew Ardill, Jens Lehmann.
Thread: https://gitlist.dev/t/28506

## Roy Liu, 2011-09-26 22:00

Subject: [PATCH] git-submodule: a small fix
Message-ID: <CAOOg04z0UjOQCBMTuii37y3ykNU17hDTcD9E81C+r1whFpMaCg@mail.gmail.com>
URL: https://gitlist.dev/e/CAOOg04z0UjOQCBMTuii37y3ykNU17hDTcD9E81C%2Br1whFpMaCg%40mail.gmail.com

```
In git-submodule.sh, the "url" variable may contain a stale value from
the previous loop iteration, so clear it.

--- git-submodule.sh.orig   2011-09-26 17:50:45.000000000 -0400
+++ git-submodule.sh    2011-09-26 17:51:18.000000000 -0400
@@ -370,6 +370,8 @@
           esac
           git config submodule."$name".url "$url" ||
           die "Failed to register url for submodule path '$path'"
+        else
+            url=""
       fi

       # Copy "update" setting when it is not set yet

```

## Andrew Ardill, 2011-09-27 03:22

Subject: Re: [PATCH] git-submodule: a small fix
Message-ID: <CAH5451k-6HHx2xxddJauE8=P1umjG=TnrcOKmQfeh=4GOzpCKQ@mail.gmail.com>
URL: https://gitlist.dev/e/CAH5451k-6HHx2xxddJauE8%3DP1umjG%3DTnrcOKmQfeh%3D4GOzpCKQ%40mail.gmail.com
In-Reply-To: <CAOOg04z0UjOQCBMTuii37y3ykNU17hDTcD9E81C+r1whFpMaCg@mail.gmail.com>

```
On 27 September 2011 08:00, Roy Liu <carsomyr@gmail.com> wrote:
> In git-submodule.sh, the "url" variable may contain a stale value from
> the previous loop iteration, so clear it.
>
> --- git-submodule.sh.orig   2011-09-26 17:50:45.000000000 -0400
> +++ git-submodule.sh    2011-09-26 17:51:18.000000000 -0400
> @@ -370,6 +370,8 @@
>            esac
>            git config submodule."$name".url "$url" ||
>            die "Failed to register url for submodule path '$path'"
> +        else
> +            url=""
>        fi
>
>        # Copy "update" setting when it is not set yet

Perhaps a better commit description would be:

git-submodule: clear the url variable when not set to avoid using stale values

otherwise looks clean enough

Andrew

```

## Jens Lehmann, 2011-09-27 07:24

Subject: Re: [PATCH] git-submodule: a small fix
Message-ID: <4E817A1D.8050902@web.de>
URL: https://gitlist.dev/e/4E817A1D.8050902%40web.de
In-Reply-To: <CAH5451k-6HHx2xxddJauE8=P1umjG=TnrcOKmQfeh=4GOzpCKQ@mail.gmail.com>

```
Am 27.09.2011 05:22, schrieb Andrew Ardill:
> On 27 September 2011 08:00, Roy Liu <carsomyr@gmail.com> wrote:
>> In git-submodule.sh, the "url" variable may contain a stale value from
>> the previous loop iteration, so clear it.
>>
>> --- git-submodule.sh.orig   2011-09-26 17:50:45.000000000 -0400
>> +++ git-submodule.sh    2011-09-26 17:51:18.000000000 -0400
>> @@ -370,6 +370,8 @@
>>            esac
>>            git config submodule."$name".url "$url" ||
>>            die "Failed to register url for submodule path '$path'"
>> +        else
>> +            url=""
>>        fi
>>
>>        # Copy "update" setting when it is not set yet
> 
> Perhaps a better commit description would be:
> 
> git-submodule: clear the url variable when not set to avoid using stale values

Yes, the commit description needs to describe what was changed (and you can
also drop the "git-", to start with "submodule:" is sufficient here). Also
it would be nice if the message would describe under what circumstances this
happens (how did you notice this problem?).

But I wonder if the patch does it the right way. While it fixes the basic
issue that "url" might not be set, I doubt it does what the user expects.
The place where the - sometimes uninitialized - variable "url" is used is
a few lines down:

		say "$(eval_gettext "Submodule '\$name' (\$url) registered for path '\$path'")"

I doesn't make much sense to say "Submodule 'foo' () registered for path 'foo'"
here. Shouldn't "url" be set to "$(git config "submodule.$name.url")"? And
when looking at the if you added the else to it might make sense to set it
unconditionally before the if and then test "$url" there instead of adding an
extra else.

```