Re: [PATCH 1/2] git-commit: Disallow unchanged tree in non-merge mode

["Shawn O. Pearce" <spearce@spearce.org>]

"Dmitry V. Levin" <ldv@altlinux.org> wrote:
> Do not commit an unchanged tree in non-merge mode.

A laudable goal.  git-gui also does this.  Turns out the other
checks within git-gui prevent the user from ever getting that far.
I probably should remove the empty tree check as it costs CPU time
to get the old tree.  But I'd rather have the safety check.
 
> The idea is that amend should not commit an unchanged tree,
> one should just remove the top commit using git-reset instead.

NO.  `git commit --amend` is *often* used for fixing the commit
message.  Or adding additional detail.  Forcing the user to do
a `git reset --soft HEAD^ && git commit --amend` just because
you don't want git-commit to make an "empty commit" (which it
doesn't usually like to do now anyway!) is a major step back
in functionality.

NACK.
 
> diff --git a/git-commit.sh b/git-commit.sh
> index 1d04f1f..800f96c 100755
> --- a/git-commit.sh
> +++ b/git-commit.sh
> @@ -629,6 +629,16 @@ then
>  		tree=$(GIT_INDEX_FILE="$TMP_INDEX" git write-tree) &&
>  		rm -f "$TMP_INDEX"
>  	fi &&
> +	if test -n "$current" -a ! -f "$GIT_DIR/MERGE_HEAD"
> +	then
> +		current_tree="$(git cat-file commit "$current${amend:+^}" 2>/dev/null |
> +				sed -e '/^tree \+/!d' -e 's///' -e q)"

The better way to get the old tree would be this:

		current_tree="$(git rev-parse "$current${amend:+^}^{tree}" 2>/dev/null

as it avoids the tool from needing to know about the internal
representation of a commit object.  It also avoids an entire
fork+exec of a sed process.

> +		if test "$tree" = "$current_tree"
> +		then
> +			echo >&2 "nothing to commit${amend:+ (use \"git reset HEAD^\" to remove the top commit)}"

That message is a bad idea.  Doing a mixed mode reset will also
reset the index, causing the user to lose any changes that had
already been staged.  This may actually be difficult for him/her to
recover from if they have used `git add -i` or git-gui to stage only
certain hunks of files, or if their working tree has been further
modified after the commit but they want to go back and amend the
message only of the prior commit.

-- 
Shawn.