path: root/builtin
diff options
authorMichael Haggerty <>2015-06-22 14:03:10 (GMT)
committerJunio C Hamano <>2015-06-22 20:17:14 (GMT)
commit1c03c4d34771db20b78231359caa6fda28e2d9fe (patch)
tree32d5851eb76677cdf990e10a23884d3fed5d8364 /builtin
parente2991c80485c646c86f5d80423f9ae983bed120b (diff)
delete_ref(): use the usual convention for old_sha1
The ref_transaction_update() family of functions use the following convention for their old_sha1 parameters: * old_sha1 == NULL: Don't check the old value at all. * is_null_sha1(old_sha1): Ensure that the reference didn't exist before the transaction. * otherwise: Ensure that the reference had the specified value before the transaction. delete_ref() had a different convention, namely treating is_null_sha1(old_sha1) as "don't care". Change it to adhere to the standard convention to reduce the scope for confusion. Please note that it is now a bug to pass old_sha1=NULL_SHA1 to delete_ref() (because it doesn't make sense to delete a reference that you already know doesn't exist). This is consistent with the behavior of ref_transaction_delete(). Most of the callers of delete_ref() never pass old_sha1=NULL_SHA1 to delete_ref(), and are therefore unaffected by this change. The two exceptions are: * The call in cmd_update_ref(), which passed NULL_SHA1 if the old value passed in on the command line was 0{40} or the empty string. Change that caller to pass NULL in those cases. Arguably, it should be an error to call "update-ref -d" with the old value set to "does not exist", just as it is for the `--stdin` command "delete". But since this usage was accepted until now, continue to accept it. * The call in delete_branches(), which could pass NULL_SHA1 if deleting a broken or symbolic ref. Change it to pass NULL in these cases. Signed-off-by: Michael Haggerty <> Signed-off-by: Junio C Hamano <>
Diffstat (limited to 'builtin')
2 files changed, 9 insertions, 2 deletions
diff --git a/builtin/branch.c b/builtin/branch.c
index 47e3eb9..58aa84f 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -253,7 +253,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
- if (delete_ref(name, sha1, REF_NODEREF)) {
+ if (delete_ref(name, is_null_sha1(sha1) ? NULL : sha1,
? _("Error deleting remote-tracking branch '%s'")
: _("Error deleting branch '%s'"),
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 160c7ac..6763cf1 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -422,7 +422,13 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
if (no_deref)
flags = REF_NODEREF;
if (delete)
- return delete_ref(refname, oldval ? oldsha1 : NULL, flags);
+ /*
+ * For purposes of backwards compatibility, we treat
+ * NULL_SHA1 as "don't care" here:
+ */
+ return delete_ref(refname,
+ (oldval && !is_null_sha1(oldsha1)) ? oldsha1 : NULL,
+ flags);
return update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,