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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Johannes Schindelin <johannes.schindelin@xxxxxx> writes:

> They are not affected by the update anyway.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> ---
>  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?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]