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

From: Nicolas Morey-Chaisemartin <devel-git@morey-chaisemartin.com>
Date: Mar 30, 2011, 18:50 UTC
Message-ID: <4D937B7E.10808@morey-chaisemartin.com>
In-Reply-To: <4D93773C.2010807@web.de>

On 03/30/2011 08:32 PM, Jens Lehmann wrote:

> Am 30.03.2011 09:56, schrieb Nicolas Morey-Chaisemartin:

Show 15 quoted lines

> 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.

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.

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.

Show 27 quoted lines

>>  
>> +test_expect_success 'submodule update should fail due to local changes' '
>> +	(cd super/submodule &&
>> +	 git reset --hard HEAD~1 &&
>> +	 echo "local change" > file
>> +	) &&
>> +	(cd super &&
>> +	 (cd submodule &&
>> +	  compare_head
>> +	 ) &&
>> +	 test_must_fail git submodule update submodule
>> +	)
>> +'
> 
> This test is shorter and might be easier to understand rewritten as:
> 
> +test_expect_success 'submodule update should fail due to local changes' '
> +	(cd super &&
> +	 (cd submodule &&
> +	  git reset --hard HEAD~1 &&
> +	  echo "local change" > file
> +	  compare_head
> +	 ) &&
> +	 test_must_fail git submodule update submodule
> +	)
> +'
>

Agreed.