Re: [PATCH] post-update hook: update working copy

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 2, 2007, 01:02 UTC
Message-ID: <7vd4ut7948.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1193964304-10847-1-git-send-email-sam.vilain@catalyst.net.nz>

Sam Vilain <sam.vilain@catalyst.net.nz> writes:

> Now that git-stash is available, it is not so unsafe to push to a
> non-bare repository, but care needs to be taken to preserve any dirty
> working copy or index state.  This hook script does that, using
> git-stash.

Honestly, I am reluctant to do things that _encourages_ pushing into a live tree.

 - Who guarantees that the reflog is enabled for the HEAD?

 - Who guarantees that a human user is not actively editing the
   work tree files without saving?  You would not see "dirty
   state", the editor would notice "the file was modified since
   you started editing it" and tell so to the user, but the user
   cannot recover from the situation without knowing to do the
   three-way merge between HEAD@{1}, HEAD and the index _anyway_.

Show 12 quoted lines

> +update_wc() {
> +	ref=$1
> +	echo "Push to checked out branch $ref" >&2
> +	if (cd $GIT_WORK_TREE; git-diff-files -q --exit-code >/dev/null)
> +	then
> +		wc_dirty=0
> +	else
> +		echo "W:unstaged changes found in working copy" >&2
> +		wc_dirty=1
> +		desc="working copy"
> +	fi
> +	if git diff-index HEAD@{1} >/dev/null

Are you missing "--cached" here?

Show 8 quoted lines

> +	if [ "$wc_dirty" -ne 0 -o "$index_dirty" -ne 0 ]
> +	then
> +		new=$(git rev-parse HEAD)
> +		git-update-ref --no-deref HEAD HEAD@{1}
> +		echo "W:stashing dirty $desc - see git-stash(1)" >&2
> +		(cd $GIT_WORK_TREE
> +		git stash save "dirty $desc before update to $new")
> +		git-symbolic-ref HEAD "$ref"

This part feels somewhat dangerous. What happens if we are interrupted in the middle of these commands?

Show 8 quoted lines

> +	fi
> +
> +	# eye candy - show the WC updates :)
> +	echo "Updating working copy" >&2
> +	(cd $GIT_WORK_TREE
> +	git-diff-index -R --name-status HEAD >&2
> +	git-reset --hard HEAD)
> +}

And I would have expected you would unstash the dirty state here. Are there any reason not to?