Re: [PATCH 2/2] Let deny.currentBranch=updateInstead ignore submodules

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 7, 2014, 19:20 UTC
Message-ID: <xmqqh9yag6mt.fsf@gitster.dls.corp.google.com>
In-Reply-To: <84dba8872922da96e99953eea0ccff5f5af9dd4a.1415368490.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

Show 20 quoted lines

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

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?