Subject: Re: [PATCH] submodule: Add --force option for git submodule update

From: Jens Lehmann <[email-masked]>
Date: Wed, 30 Mar 2011 21:05:45 +0200

Am 30.03.2011 20:50, schrieb Nicolas Morey-Chaisemartin:
> On 03/30/2011 08:32 PM, Jens Lehmann wrote:
>> Am 30.03.2011 09:56, schrieb Nicolas Morey-Chaisemartin:
> 
>> All looking good up to here. But I wonder if the rest of git-submodule.sh
>> could be changed a bit less invasive ... maybe as simple as this?
>>
>> @@ -458,7 +461,6 @@ cmd_update()
>>
>>  		if test "$subsha1" != "$sha1"
>>  		then
>> -			force=
>>  			if test -z "$subsha1"
>>  			then
>>  				force="-f"
>>
>> Now force will not be cleared and thus contain "-f" if the user provided
>> it on the command line. All tests (including your new ones) are running
>> fine with this simplification ... am I missing something?
> 
> Actually, I don't think this work.
> By doing that, if you run git submodule update without -f, it will set -f when you reached the first submodule not yet checked out ( -z $subsha1 ),
> and the following submodules will be checkout using --force which may throw away changes the user wanted to keep.

You are right, I just came to that conclusion myself ... but with a loop
local variable initialized from force on every iteration it should work.

> I know it is very intrusive. The main reason for that is I wanted the -f option to always behave the same (meaning throw away changes),
> whether the submodule is already on the right commit or not.

Hmm, I don't know if that is a good thing to do. People are used to
"git submodule update" to only touch those submodule where the HEAD
differs from the commit recorded in the superproject (And I often
find myself using "-f" if the command didn't succeed without it).
But when using "-f" touches other submodules than not using it the
user might experience a rather unpleasant surprise, I'm not sure we
want to go that way.

> If we accept to drop this and only drop the changes when subsha1 != sha1, the patch can be much sorter by simply keeping the force flags I used and without modifying all the case/while thing.

Yes.