[PATCH 0/3] Make old sha1 optional with git update-ref -d

[PATCH 0/3] Make old sha1 optional with git update-ref -d

[Karl Hasselström]

This series makes the old ref value optional for update-ref -d (it's
already optional for update-ref without -d). But first it adds some
test and refactors a bit.

---

Karl Hasselström (3):
      Make old sha1 optional with git update-ref -d
      Clean up builtin-update-ref's option parsing
      Add some tests for git update-ref -d


 Documentation/git-update-ref.txt |    2 +-
 builtin-update-ref.c             |   32 ++++++++++++++++++--------------
 t/t1400-update-ref.sh            |   24 ++++++++++++++++++++++++
 3 files changed, 43 insertions(+), 15 deletions(-)

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

[PATCH 1/3] Add some tests for git update-ref -d

[Karl Hasselström]

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 t/t1400-update-ref.sh |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)


diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 78cd412..b88e767 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -32,6 +32,14 @@ test_expect_success \
 	"create $m" \
 	"git update-ref $m $B $A &&
 	 test $B"' = $(cat .git/'"$m"')'
+test_expect_success "fail to delete $m with stale ref" "
+	test_must_fail git update-ref -d $m $A &&
+	test $B = \$(cat .git/$m)
+"
+test_expect_success "delete $m" "
+	git update-ref -d $m $B &&
+	! test -f .git/$m
+"
 rm -f .git/$m
 
 test_expect_success \
@@ -49,6 +57,14 @@ test_expect_success \
 	"create $m (by HEAD)" \
 	"git update-ref HEAD $B $A &&
 	 test $B"' = $(cat .git/'"$m"')'
+test_expect_success "fail to delete $m (by HEAD) with stale ref" "
+	test_must_fail git update-ref -d HEAD $A &&
+	test $B = \$(cat .git/$m)
+"
+test_expect_success "delete $m (by HEAD)" "
+	git update-ref -d HEAD $B &&
+	! test -f .git/$m
+"
 rm -f .git/$m
 
 test_expect_success '(not) create HEAD with old sha1' "

[PATCH 2/3] Clean up builtin-update-ref's option parsing

[Karl Hasselström]

builtin-update-ref's option parsing was somewhat tricky to follow,
especially if the -d option was given. This patch cleans it upp a bit
(including fixing an out-of-bounds argv access), at the expense of
making it a bit longer.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 builtin-update-ref.c |   30 +++++++++++++++++-------------
 1 files changed, 17 insertions(+), 13 deletions(-)


diff --git a/builtin-update-ref.c b/builtin-update-ref.c
index e90737c..0d3eb8e 100644
--- a/builtin-update-ref.c
+++ b/builtin-update-ref.c
@@ -27,25 +27,29 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
 	if (msg && !*msg)
 		die("Refusing to perform update with empty message.");
 
-	if (argc < 2 || argc > 3)
-		usage_with_options(git_update_ref_usage, options);
-	refname = argv[0];
-	value   = argv[1];
-	oldval  = argv[2];
-
-	if (get_sha1(value, sha1))
-		die("%s: not a valid SHA1", value);
-
 	if (delete) {
-		if (oldval)
+		if (argc != 2)
 			usage_with_options(git_update_ref_usage, options);
-		return delete_ref(refname, sha1);
+		refname = argv[0];
+		value = NULL;
+		oldval = argv[1];
+	} else {
+		if (argc < 2 || argc > 3)
+			usage_with_options(git_update_ref_usage, options);
+		refname = argv[0];
+		value = argv[1];
+		oldval = argc > 2 ? argv[2] : NULL;
 	}
 
+	if (value && *value && get_sha1(value, sha1))
+		die("%s: not a valid SHA1", value);
 	hashclr(oldsha1);
 	if (oldval && *oldval && get_sha1(oldval, oldsha1))
 		die("%s: not a valid old SHA1", oldval);
 
-	return update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,
-			  no_deref ? REF_NODEREF : 0, DIE_ON_ERR);
+	if (delete)
+		return delete_ref(refname, oldsha1);
+	else
+		return update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,
+				  no_deref ? REF_NODEREF : 0, DIE_ON_ERR);
 }

Re: [PATCH 2/3] Clean up builtin-update-ref's option parsing

[Junio C Hamano]

Karl Hasselström <kha@treskal.com> writes:

> builtin-update-ref's option parsing was somewhat tricky to follow,
> especially if the -d option was given. This patch cleans it upp a bit
> (including fixing an out-of-bounds argv access), at the expense of
> making it a bit longer.
>
> Signed-off-by: Karl Hasselström <kha@treskal.com>

Longer is fine but I am afraid that this patch is not much easier to
follow than the original, does not fix anything, and introduces a new bug.

> diff --git a/builtin-update-ref.c b/builtin-update-ref.c
> index e90737c..0d3eb8e 100644
> --- a/builtin-update-ref.c
> +++ b/builtin-update-ref.c
> @@ -27,25 +27,29 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
>  	if (msg && !*msg)
>  		die("Refusing to perform update with empty message.");
>  
> -	if (argc < 2 || argc > 3)
> -		usage_with_options(git_update_ref_usage, options);
> -	refname = argv[0];
> -	value   = argv[1];
> -	oldval  = argv[2];
> -
> -	if (get_sha1(value, sha1))
> -		die("%s: not a valid SHA1", value);
> -
>  	if (delete) {
> -		if (oldval)
> +		if (argc != 2)
>  			usage_with_options(git_update_ref_usage, options);
> -		return delete_ref(refname, sha1);
> +		refname = argv[0];
> +		value = NULL;
> +		oldval = argv[1];
> +	} else {
> +		if (argc < 2 || argc > 3)
> +			usage_with_options(git_update_ref_usage, options);
> +		refname = argv[0];
> +		value = argv[1];
> +		oldval = argc > 2 ? argv[2] : NULL;

When (argc == 3), argv[2] has the old value string given on the command
line.  When (argc == 2), argv[2] has the terminating NULL pointer.  So
either case you can safely use argv[2].  You do not allow other cases
upfront, so I do not understand why you need this conditional expression?

IOW, I do not see "an out-of-bounds argv access" in the original, and you
are making this assignment harder to follow.

>  	}
>  
> +	if (value && *value && get_sha1(value, sha1))
> +		die("%s: not a valid SHA1", value);

Dropping *value in the sequence may fix it but I think this is wrong.

We used to barf if you said "git update-ref refs/heads/master '' master"
because it would be nonsense to give an empty string as the new value of
the ref, didn't we?  Doesn't this change break it?  Does your set of
additional tests in [1/3] catch it?

>  	hashclr(oldsha1);
>  	if (oldval && *oldval && get_sha1(oldval, oldsha1))
>  		die("%s: not a valid old SHA1", oldval);
>  
> -	return update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,
> -			  no_deref ? REF_NODEREF : 0, DIE_ON_ERR);
> +	if (delete)
> +		return delete_ref(refname, oldsha1);
> +	else
> +		return update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,
> +				  no_deref ? REF_NODEREF : 0, DIE_ON_ERR);
>  }

Re: [PATCH 2/3] Clean up builtin-update-ref's option parsing

[Karl Hasselström]

On 2008-05-25 11:40:24 -0700, Junio C Hamano wrote:

> >  	if (delete) {
> > -		if (oldval)
> > +		if (argc != 2)
> >  			usage_with_options(git_update_ref_usage, options);
> > -		return delete_ref(refname, sha1);
> > +		refname = argv[0];
> > +		value = NULL;
> > +		oldval = argv[1];
> > +	} else {
> > +		if (argc < 2 || argc > 3)
> > +			usage_with_options(git_update_ref_usage, options);
> > +		refname = argv[0];
> > +		value = argv[1];
> > +		oldval = argc > 2 ? argv[2] : NULL;
>
> When (argc == 3), argv[2] has the old value string given on the
> command line. When (argc == 2), argv[2] has the terminating NULL
> pointer. So either case you can safely use argv[2]. You do not allow
> other cases upfront, so I do not understand why you need this
> conditional expression?
>
> IOW, I do not see "an out-of-bounds argv access" in the original,
> and you are making this assignment harder to follow.

Mmmph. The problem here was that I didn't know that argv is
NULL-terminated. I just assumed it was bad to access anything beyond
the given array length as is usually the case in C. But I really
should have taken a hint from the existing code. Sorry for wasting
your time with this.

> >  	}
> >  
> > +	if (value && *value && get_sha1(value, sha1))
> > +		die("%s: not a valid SHA1", value);
> 
> Dropping *value in the sequence may fix it but I think this is wrong.
> 
> We used to barf if you said "git update-ref refs/heads/master '' master"
> because it would be nonsense to give an empty string as the new value of
> the ref, didn't we?  Doesn't this change break it?  Does your set of
> additional tests in [1/3] catch it?

Aaah, I see what you mean. Good point. The empty string should
obviously not be allowed as the new ref. In the original code it _is_
allowed as the _old_ value, and interpreted as 0{40}.

I'll update the series. With, among other things, a comment that
explains this.

                                 -+-

Many thanks for the review. I'll be back.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

[PATCH v2 0/2] Make old sha1 optional with git update-ref -d

[Karl Hasselström]

This series makes the old ref value optional for update-ref -d (it's
already optional for update-ref without -d).

---

Patch 1/2 updated after comments from Junio.

Karl Hasselström (2):
      Make old sha1 optional with git update-ref -d
      Clean up builtin-update-ref's option parsing


 Documentation/git-update-ref.txt |    2 +-
 builtin-update-ref.c             |   36 ++++++++++++++++++++----------------
 t/t1400-update-ref.sh            |    8 ++++++++
 3 files changed, 29 insertions(+), 17 deletions(-)

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

[PATCH v2 1/2] Clean up builtin-update-ref's option parsing

[Karl Hasselström]

builtin-update-ref's option parsing was somewhat tricky to follow,
especially if the -d option was given. This patch cleans it upp a bit,
at the expense of making it a bit longer.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 builtin-update-ref.c |   34 +++++++++++++++++++---------------
 1 files changed, 19 insertions(+), 15 deletions(-)


diff --git a/builtin-update-ref.c b/builtin-update-ref.c
index 93c1271..cc8bc5c 100644
--- a/builtin-update-ref.c
+++ b/builtin-update-ref.c
@@ -11,7 +11,7 @@ static const char * const git_update_ref_usage[] = {
 
 int cmd_update_ref(int argc, const char **argv, const char *prefix)
 {
-	const char *refname, *value, *oldval, *msg=NULL;
+	const char *refname, *oldval, *msg=NULL;
 	unsigned char sha1[20], oldsha1[20];
 	int delete = 0, no_deref = 0;
 	struct option options[] = {
@@ -27,25 +27,29 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
 	if (msg && !*msg)
 		die("Refusing to perform update with empty message.");
 
-	if (argc < 2 || argc > 3)
-		usage_with_options(git_update_ref_usage, options);
-	refname = argv[0];
-	value   = argv[1];
-	oldval  = argv[2];
-
-	if (get_sha1(value, sha1))
-		die("%s: not a valid SHA1", value);
-
 	if (delete) {
-		if (oldval)
+		if (argc != 2)
+			usage_with_options(git_update_ref_usage, options);
+		refname = argv[0];
+		oldval = argv[1];
+	} else {
+                const char *value;
+		if (argc < 2 || argc > 3)
 			usage_with_options(git_update_ref_usage, options);
-		return delete_ref(refname, sha1);
+		refname = argv[0];
+		value = argv[1];
+		oldval = argv[2];
+		if (get_sha1(value, sha1))
+			die("%s: not a valid SHA1", value);
 	}
 
-	hashclr(oldsha1);
+	hashclr(oldsha1); /* all-zero hash in case oldval is the empty string */
 	if (oldval && *oldval && get_sha1(oldval, oldsha1))
 		die("%s: not a valid old SHA1", oldval);
 
-	return update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,
-			  no_deref ? REF_NODEREF : 0, DIE_ON_ERR);
+	if (delete)
+		return delete_ref(refname, oldsha1);
+	else
+		return update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,
+				  no_deref ? REF_NODEREF : 0, DIE_ON_ERR);
 }

[PATCH v2 2/2] Make old sha1 optional with git update-ref -d

[Karl Hasselström]

Giving the old sha1 is already optional when changing a ref, and it's
quite handy when running update-ref manually. So make it optional for
deleting a ref too.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 Documentation/git-update-ref.txt |    2 +-
 builtin-update-ref.c             |    6 +++---
 t/t1400-update-ref.sh            |    8 ++++++++
 3 files changed, 12 insertions(+), 4 deletions(-)


diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt
index 4dc4759..80b94c3 100644
--- a/Documentation/git-update-ref.txt
+++ b/Documentation/git-update-ref.txt
@@ -7,7 +7,7 @@ git-update-ref - Update the object name stored in a ref safely
 
 SYNOPSIS
 --------
-'git-update-ref' [-m <reason>] (-d <ref> <oldvalue> | [--no-deref] <ref> <newvalue> [<oldvalue>])
+'git-update-ref' [-m <reason>] (-d <ref> [<oldvalue>] | [--no-deref] <ref> <newvalue> [<oldvalue>])
 
 DESCRIPTION
 -----------
diff --git a/builtin-update-ref.c b/builtin-update-ref.c
index cc8bc5c..9546488 100644
--- a/builtin-update-ref.c
+++ b/builtin-update-ref.c
@@ -4,7 +4,7 @@
 #include "parse-options.h"
 
 static const char * const git_update_ref_usage[] = {
-	"git-update-ref [options] -d <refname> <oldval>",
+	"git-update-ref [options] -d <refname> [<oldval>]",
 	"git-update-ref [options]    <refname> <newval> [<oldval>]",
 	NULL
 };
@@ -28,7 +28,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
 		die("Refusing to perform update with empty message.");
 
 	if (delete) {
-		if (argc != 2)
+		if (argc < 1 || argc > 2)
 			usage_with_options(git_update_ref_usage, options);
 		refname = argv[0];
 		oldval = argv[1];
@@ -48,7 +48,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
 		die("%s: not a valid old SHA1", oldval);
 
 	if (delete)
-		return delete_ref(refname, oldsha1);
+		return delete_ref(refname, oldval ? oldsha1 : NULL);
 	else
 		return update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,
 				  no_deref ? REF_NODEREF : 0, DIE_ON_ERR);
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index b8b7ab4..f387d46 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -42,6 +42,14 @@ test_expect_success "delete $m" '
 '
 rm -f .git/$m
 
+test_expect_success "delete $m without oldvalue verification" "
+	git update-ref $m $A &&
+	test $A = \$(cat .git/$m) &&
+	git update-ref -d $m &&
+	! test -f .git/$m
+"
+rm -f .git/$m
+
 test_expect_success \
 	"fail to create $n" \
 	"touch .git/$n_dir

Re: [PATCH v2 2/2] Make old sha1 optional with git update-ref -d

[Junio C Hamano]

Karl Hasselström <kha@treskal.com> writes:

> Giving the old sha1 is already optional when changing a ref, and it's
> quite handy when running update-ref manually. So make it optional for
> deleting a ref too.
>
> Signed-off-by: Karl Hasselström <kha@treskal.com>

"Handy" is not a very good reason when talking about plumbing command like
update-ref that takes an extra parameter for safety of the last step in
read, operate, verify-and-update sequence.

Although it is not a reason _bad enough_ to make your patch a bad idea,
perhaps you should rethink the problem at the same time?  Whatever you are
trying to do cannot be done without manually invoking update-ref directly
by the end user, perhaps that needs to be addressed?

Re: [PATCH v2 2/2] Make old sha1 optional with git update-ref -d

[Karl Hasselström]

On 2008-06-02 22:51:33 -0700, Junio C Hamano wrote:

> Karl Hasselström <kha@treskal.com> writes:
>
> > Giving the old sha1 is already optional when changing a ref, and
> > it's quite handy when running update-ref manually. So make it
> > optional for deleting a ref too.
>
> "Handy" is not a very good reason when talking about plumbing
> command like update-ref that takes an extra parameter for safety of
> the last step in read, operate, verify-and-update sequence.
>
> Although it is not a reason _bad enough_ to make your patch a bad
> idea, perhaps you should rethink the problem at the same time?
> Whatever you are trying to do cannot be done without manually
> invoking update-ref directly by the end user, perhaps that needs to
> be addressed?

I need this from time to time when experimenting with StGit -- or,
more precisely, when cleaning up afterwards in case it didn't go so
well. Packed refs means I can't safely just edit/delete files under
.git/refs anymore, so I use update-ref instead. And having to use the
safety belt every time is just plain tedious. I'd say this is a
perfect example of when using the plumbing by hand is really what you
want.

(In this kind of situation, branch -D often doesn't work since my ref
isn't under refs/heads, and besides I really don't want any of it's
smarts right then -- I want to delete a ref, nothing more.)

Better suggestions welcome, of course.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: [PATCH v2 2/2] Make old sha1 optional with git update-ref -d

[Junio C Hamano]

Karl Hasselström <kha@treskal.com> writes:

> I need this from time to time when experimenting with StGit -- or,
> more precisely, when cleaning up afterwards in case it didn't go so
> well. Packed refs means I can't safely just edit/delete files under
> .git/refs anymore, so I use update-ref instead. And having to use the
> safety belt every time is just plain tedious. I'd say this is a
> perfect example of when using the plumbing by hand is really what you
> want.

Oh, I should have guessed, and I am relieved by your answer.

I was worried that in some workflow people may have to use "update-ref -d"
all the time because our Porcelain allows them to create some ref without
giving a clean way to remove it (e.g. "branch" and "tag" allow creation
and deletion with -d/-D, so they are Ok, but I was worried there might be
something else I overlooked).

Manually cleaning up what an experimental version of Porcelain left behind
does not fall into category of "normal user has to do this all the time",
so I do not have to worry.

Thanks.

[PATCH 3/3] Make old sha1 optional with git update-ref -d

[Karl Hasselström]

Giving the old sha1 is already optional when changing a ref, and it's
quite handy when running update-ref manually. So make it optional for
deleting a ref too.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 Documentation/git-update-ref.txt |    2 +-
 builtin-update-ref.c             |    8 ++++----
 t/t1400-update-ref.sh            |    8 ++++++++
 3 files changed, 13 insertions(+), 5 deletions(-)


diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt
index 4dc4759..80b94c3 100644
--- a/Documentation/git-update-ref.txt
+++ b/Documentation/git-update-ref.txt
@@ -7,7 +7,7 @@ git-update-ref - Update the object name stored in a ref safely
 
 SYNOPSIS
 --------
-'git-update-ref' [-m <reason>] (-d <ref> <oldvalue> | [--no-deref] <ref> <newvalue> [<oldvalue>])
+'git-update-ref' [-m <reason>] (-d <ref> [<oldvalue>] | [--no-deref] <ref> <newvalue> [<oldvalue>])
 
 DESCRIPTION
 -----------
diff --git a/builtin-update-ref.c b/builtin-update-ref.c
index 0d3eb8e..054e64d 100644
--- a/builtin-update-ref.c
+++ b/builtin-update-ref.c
@@ -4,7 +4,7 @@
 #include "parse-options.h"
 
 static const char * const git_update_ref_usage[] = {
-	"git-update-ref [options] -d <refname> <oldval>",
+	"git-update-ref [options] -d <refname> [<oldval>]",
 	"git-update-ref [options]    <refname> <newval> [<oldval>]",
 	NULL
 };
@@ -28,11 +28,11 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
 		die("Refusing to perform update with empty message.");
 
 	if (delete) {
-		if (argc != 2)
+		if (argc < 1 || argc > 2)
 			usage_with_options(git_update_ref_usage, options);
 		refname = argv[0];
 		value = NULL;
-		oldval = argv[1];
+		oldval = argc > 1 ? argv[1] : NULL;
 	} else {
 		if (argc < 2 || argc > 3)
 			usage_with_options(git_update_ref_usage, options);
@@ -48,7 +48,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
 		die("%s: not a valid old SHA1", oldval);
 
 	if (delete)
-		return delete_ref(refname, oldsha1);
+		return delete_ref(refname, oldval ? oldsha1 : NULL);
 	else
 		return update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,
 				  no_deref ? REF_NODEREF : 0, DIE_ON_ERR);
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index b88e767..da82645 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -42,6 +42,14 @@ test_expect_success "delete $m" "
 "
 rm -f .git/$m
 
+test_expect_success "delete $m without oldvalue verification" "
+	git update-ref $m $A &&
+	test $A = \$(cat .git/$m) &&
+	git update-ref -d $m &&
+	! test -f .git/$m
+"
+rm -f .git/$m
+
 test_expect_success \
 	"fail to create $n" \
 	"touch .git/$n_dir

Re: [PATCH 3/3] Make old sha1 optional with git update-ref -d

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 518 bytes --]

Hi,

On Sun, 25 May 2008, Karl Hasselström wrote:

> Giving the old sha1 is already optional when changing a ref, and it's
> quite handy when running update-ref manually. So make it optional for
> deleting a ref too.

Isn't this a bit dangerous, especially given that there is potentially 
_no_ reflog as a safeguard?

So no, it is not the same as when changing a ref.

I am mildly negative on this (having lost some remote branches, because 
they were deleted _together with the reflogs_ by "git push bla :x"),
Dscho

Re: [PATCH 3/3] Make old sha1 optional with git update-ref -d

[Karl Hasselström]

On 2008-05-26 11:51:12 +0100, Johannes Schindelin wrote:

> On Sun, 25 May 2008, Karl Hasselström wrote:
>
> > Giving the old sha1 is already optional when changing a ref, and
> > it's quite handy when running update-ref manually. So make it
> > optional for deleting a ref too.
>
> Isn't this a bit dangerous, especially given that there is
> potentially _no_ reflog as a safeguard?
>
> So no, it is not the same as when changing a ref.
>
> I am mildly negative on this (having lost some remote branches,
> because they were deleted _together with the reflogs_ by "git push
> bla :x"),

Deleting the wrong thing is never a fun experience. The thing is, this
patch doesn't really make it any easier to delete the wrong thing.
Before, you'd

  git update-ref -d <ref> <some-other-way-to-point-to-the-same-commit>

if you wanted to play it safe, and

  git update-ref -d <ref> <ref>

if you thought you knew what you were doing. All this patch does is
make the second usage mode easier to type.

(One might argue that a user who doesn't know what she's doing will
have an easier time shooting herself in the foot when the oldvalue is
optional, but I was under the impression that ease of use without
consulting the man page shouldn't be a factor when designing the
plumbing UI.)

I do agree with you that branch deletion is one of the more
"dangerous" operations in git. But I don't think making update-ref
hard to use is the answer there; what we'd really need is for old
reflogs to stay around for a while before being deleted.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle