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

From: Shawn O. Pearce <spearce@spearce.org>
Date: Sep 6, 2007, 02:25 UTC
Message-ID: <20070906022539.GG18160@spearce.org>
In-Reply-To: <20070905234941.GA643@nomad.office.altlinux.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.

Show 12 quoted lines

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