Re: [PATCH 3/3] update a few Porcelain-ish for ref lock safety.

[Andy Whitcroft <apw@shadowen.org>]

Junio C Hamano wrote:
> This updates the use of git-update-ref in git-branch, git-tag
> and git-commit to make them safer in a few corner cases as
> demonstration.
> 
>  - git-tag makes sure that the named tag does not exist, allows
>    you to edit tag message and then creates the tag.  If a tag
>    with the same name was created by somebody else in the
>    meantime, it used to happily overwrote it.  Now it notices
>    the situation.
> 
>  - git-branch -d and git-commit (for the initial commit) had the
>    same issue but with smaller race window, which is plugged
>    with this.
> 
> Signed-off-by: Junio C Hamano <junkio@cox.net>
> ---
> 
>  * Obviously I would need to update this on top of Linus's
>    packed-refs, but this 3-patch series applies on top of the
>    current "master". 
> 
>  git-branch.sh |    9 ++++++---
>  git-commit.sh |    2 +-
>  git-tag.sh    |    9 ++++++---
>  3 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/git-branch.sh b/git-branch.sh
> index e0501ec..2b58d20 100755
> --- a/git-branch.sh
> +++ b/git-branch.sh
> @@ -42,8 +42,7 @@ If you are sure you want to delete it, r
>  	    esac
>  	    ;;
>  	esac
> -	rm -f "$GIT_DIR/logs/refs/heads/$branch_name"
> -	rm -f "$GIT_DIR/refs/heads/$branch_name"
> +	git update-ref -d "refs/heads/$branch_name" "$branch"
>  	echo "Deleted branch $branch_name."
>      done
>      exit 0
> @@ -112,6 +111,7 @@ rev=$(git-rev-parse --verify "$head") ||
>  git-check-ref-format "heads/$branchname" ||
>  	die "we do not like '$branchname' as a branch name."
>  
> +prev=0000000000000000000000000000000000000000
>  if [ -e "$GIT_DIR/refs/heads/$branchname" ]
>  then
>  	if test '' = "$force"
> @@ -121,10 +121,13 @@ then
>  	then
>  		die "cannot force-update the current branch."
>  	fi
> +	prev=`git rev-parse --verify "refs/heads/$branchname"`
>  fi
>  if test "$create_log" = 'yes'
>  then
>  	mkdir -p $(dirname "$GIT_DIR/logs/refs/heads/$branchname")
>  	touch "$GIT_DIR/logs/refs/heads/$branchname"
>  fi
> -git update-ref -m "branch: Created from $head" "refs/heads/$branchname" $rev
> +git update-ref -m "branch: Created from $head" \
> +	"refs/heads/$branchname" $rev $prev
> +
> diff --git a/git-commit.sh b/git-commit.sh
> index 5a4c659..87b13ef 100755
> --- a/git-commit.sh
> +++ b/git-commit.sh
> @@ -554,8 +554,8 @@ else
>  		exit 1
>  	fi
>  	PARENTS=""
> -	current=
>  	rloga='commit (initial)'
> +	current=0000000000000000000000000000000000000000
>  fi
>  
>  if test -z "$no_edit"
> diff --git a/git-tag.sh b/git-tag.sh
> index a0afa25..2bde3c0 100755
> --- a/git-tag.sh
> +++ b/git-tag.sh
> @@ -63,8 +63,11 @@ done
>  
>  name="$1"
>  [ "$name" ] || usage
> -if [ -e "$GIT_DIR/refs/tags/$name" -a -z "$force" ]; then
> -    die "tag '$name' already exists"
> +prev=0000000000000000000000000000000000000000

It seems a little odd to need to use such a large 'none' thing.  Will
linus' updates start returning this when there is no tag?  If so then it
makes sense.  Else perhaps it would be nice to have a short cut for it.
 Such as 'none'.

-apw