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

[Junio C Hamano <gitster@pobox.com>]

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.

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?