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

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

 



Hi Junio,

On Fri, 7 Nov 2014, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
> 
> > 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.

This change is squashed into the first patch in the next iteration.

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

That is indeed what the patched code calls.

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

Changed in the next iteration.

Ciao,
Johannes
--
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]