Hi Junio,
On Fri, 7 Nov 2014, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schinde...@gmx.de> writes:
>
> > Signed-off-by: Johannes Schindelin <johannes.schinde...@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.
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
