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

Junio C Hamano [off-list ref] writes:

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.

Just to make sure.  I am *not* saying that you do not need to run
"update-index --refresh".  It is necessary before running read-tree
to avoid false dirtyness, so you do need to run it.

I am only saying that it is too strict to fail the operation when
the command reports that you have a local modification in the
working tree.

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.

Hi Junio,

On Mon, 10 Nov 2014, Junio C Hamano wrote:

Junio C Hamano [off-list ref] writes:

quoted

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.

Just to make sure.  I am *not* saying that you do not need to run
"update-index --refresh".  It is necessary before running read-tree
to avoid false dirtyness, so you do need to run it.

I am only saying that it is too strict to fail the operation when
the command reports that you have a local modification in the
working tree.

Okay, now I am even more puzzled. I guess you actually meant to say that I
need to convert the die() into a return? If so, I agree fully.

Ciao,
Johannes