Hi Junio,
On Mon, 10 Nov 2014, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>
> >> 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.
>
> I know that ;-), but I think I wasn't clear enough.
>
> I am not saying you are not using two-tree read-tree. I am saying
> the check with update-index --refresh before read-tree seems
> dubious.
>
> There are three "cleanliness" we require in various occasions:
>
> (1) rebase asks you to have your index and the working tree match
> HEAD exactly, as if just after "reset --hard HEAD".
>
> (2) merge asks you to have your index match HEAD exactly (i.e. no
> output from "diff --cached HEAD"), and have no changes in the
> working tree at paths that are involved in the operation. It
> is OK to have local changes in the working tree (but not in the
> index) at paths that are not involved in a mergy operation.
>
> (3) checkout asks you to have your index and the working tree match
> either HEAD or the commit you are switching to exactly at paths
> that are involved in the operation. It is OK to have local
> changes in the working tree and in the index at paths that are
> not different between the commits you are moving between, and
> it is also OK to have the contents from the "new" commit in the
> working tree and the index at paths that are different between
> the two.
>
> Dying when "update-index --refresh" signals a difference is an
> attempt to mimic #1, but it is in line with the spirit of the reason
> why a user would want to use updateInstead, I think. The situation
> is more like the person who pushed into your repository from
> sideline did a "checkout -B $current_branch $new_commit" to update
> the HEAD, the index and the working tree, to let you pretend as if
> you based your work on the commit he pushed to you.
>
> While you still need to error out when your local work does not
> satisfy the cleanliness criteria #3 above, I do not think you would
> want to stop the operation when "checkout" would not fail, e.g. you
> have a local change that does not interfere with the update between
> the two commits, with this one:
>
> + if (run_command(&child))
> + die ("Could not refresh the index");
>
> When refreshed the index successfully, we signal that there were
> differences between the index and the working tree with a non-zero
> return value, so "Could not refresh" is not quite right, either.
>
> But this one that checks the exit status from two-tree read-tree
>
> + if (run_command(&child))
> + die ("Could not merge working tree with new HEAD. Good luck.");
>
> is checking the right condition, i.e. cleanliness #3. The
> disposition should not be "die", but an error return to tell the
> caller to abort the push as we discussed earlier.
I have to say that I am quite puzzled now: I guess that you want me to
drop the update-index --refresh call?
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