[PATCH 7/8] reflog_expire(): never update a reference to null_sha1

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Feb 9, 2015, 09:12 UTC
Message-ID: <1423473164-6011-8-git-send-email-mhagger@alum.mit.edu>
In-Reply-To: <1423473164-6011-1-git-send-email-mhagger@alum.mit.edu>

Currently, if --updateref is specified and the very last reflog entry is expired or deleted, the reference's value is set to 0{40}. This is an invalid state of the repository, and breaks, for example, "git fsck" and "git for-each-ref".

The only place we use --updateref in our own code is when dropping stash entries. In that code, the very next step is to check if the reflog has been made empty, and if so, delete the "refs/stash" reference entirely. Thus that code path ultimately leaves the repository in a valid state.

But we don't want the repository in an invalid state even temporarily, and we don't want leave an invalid state if other callers of "git reflog expire|delete --updateref" don't think to do the extra cleanup step.

So, if "git reflog expire|delete" leaves no more entries in the reflog, just leave the reference un-updated.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
Another alternative would be to delete the reference in this
situation. This behavior would be kindof logically consistent and
happens to be just the thing that "git stash drop" wants. It wouldn't
even really be backwards-incompatible, given that the current code
leaves a broken repository in this situation. However, this change
would require some special casing if the reference whose reflog is
being expired is the current reference, and it somehow seems too
"lossy" to me for some reason. I'm open to feedback on this point.

 refs.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index c0001da..1b2a497 100644
--- a/refs.c
+++ b/refs.c
@@ -4077,10 +4077,13 @@ int reflog_expire(const char *refname, const unsigned char *sha1,
 		/*
 		 * It doesn't make sense to adjust a reference pointed
 		 * to by a symbolic ref based on expiring entries in
-		 * the symbolic reference's reflog.
+		 * the symbolic reference's reflog. Nor can we update
+		 * a reference if there are no remaining reflog
+		 * entries.
 		 */
 		int update = (flags & EXPIRE_REFLOGS_UPDATE_REF) &&
-			~(type & REF_ISSYMREF);
+			~(type & REF_ISSYMREF) &&
+			!is_null_sha1(cb.last_kept_sha1);
 
 		if (close_lock_file(&reflog_lock)) {
 			status |= error("couldn't write %s: %s", log_file,

-- 
2.1.4