From: Jens Lehmann <jens.lehmann@web.de>
Date: Sun, 09 Nov 2014 16:42:56 GMT
Subject: Re: [PATCH 2/2] Let deny.currentBranch=updateInstead ignore submodules
Message-ID: <545F9990.9050505@web.de>
In-Reply-To: <xmqqh9yag6mt.fsf@gitster.dls.corp.google.com>

Am 07.11.2014 um 20:20 schrieb Junio C Hamano:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
>> They are not affected by the update anyway.
>>
>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>> ---
>>   builtin/receive-pack.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
>> index be4172f..4ba51df 100644
>> --- a/builtin/receive-pack.c
>> +++ b/builtin/receive-pack.c
>> @@ -740,7 +740,7 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
>>   static void merge_worktree(unsigned char *sha1)
>>   {
>>   	const char *update_refresh[] = {
>> -		"update-index", "--refresh", NULL
>> +		"update-index", "--ignore-submodules", "--refresh", NULL
>>   	};
>>   	const char *read_tree[] = {
>>   		"read-tree", "-u", "-m", sha1_to_hex(sha1), NULL
>
> I suspect that you did not squash this into 1/2 on purpose, and I am
> guessing the reason is because you were unsure what should happen
> when there were differences in submodules' working trees (otherwise,
> you would have simply squashed without "oops it was a thinko to
> forget passing this option" as a separate patch).  I am not sure
> either.

I think --ignore-submodules is currently the right thing to do here
and would rather squash this into the first commit.

> By the way, if the expected use case of updateInstead is what I
> outlined in the previous message, would it make more sense not to
> fail with "update-index --refresh" failure (i.e. the working tree
> files have no changes since the index)?
>
> Thinking about it a bit more, checking with "update-index --refresh"
> feels doubly wrong.  You not just want the working tree files to be
> pristine with respect to the index, but also you do not want to see
> any change between the index and the original HEAD, i.e.
>
> 	$ git reset --hard && echo >>Makefile ; git add Makefile
>          $ git update-index --refresh ; echo $?
>          0
>
> this is not a good state from which you would want to update the
> working tree.
>
> Wouldn't the two-tree form "read-tree -u -m" that is the equivalent
> to branch switching do a sufficient check?
>
> Also, regarding the new calls to die() in the main patch, shouldn't
> they just be returning the error reason in string form, just like
> DENY_REFUSE returns "branch is currently checked out" to signal a
> push failure to the caller?
>
>