git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Stefan Beller" <sbeller@google.com>,
	"Ronnie Sahlberg" <ronniesahlberg@gmail.com>,
	"Jonathan Nieder" <jrnieder@gmail.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	git@vger.kernel.org, "Michael Haggerty" <mhagger@alum.mit.edu>
Subject: [PATCH 7/8] reflog_expire(): never update a reference to null_sha1
Date: Mon,  9 Feb 2015 10:12:43 +0100	[thread overview]
Message-ID: <1423473164-6011-8-git-send-email-mhagger@alum.mit.edu> (raw)
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

  parent reply	other threads:[~2015-02-09  9:13 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-09  9:12 [PATCH 0/8] Fix some problems with reflog expiration Michael Haggerty
2015-02-09  9:12 ` [PATCH 1/8] write_ref_sha1(): remove check for lock == NULL Michael Haggerty
2015-02-10 22:52   ` Stefan Beller
2015-02-11  0:06     ` Jeff King
2015-02-09  9:12 ` [PATCH 2/8] write_ref_sha1(): Move write elision test to callers Michael Haggerty
2015-02-12 19:58   ` Junio C Hamano
2015-02-09  9:12 ` [PATCH 3/8] lock_ref_sha1_basic(): do not set force_write for missing references Michael Haggerty
2015-02-10 23:24   ` Stefan Beller
2015-02-11  0:05     ` Jeff King
2015-02-11  0:07       ` Stefan Beller
2015-02-12 12:09       ` Michael Haggerty
2015-02-09  9:12 ` [PATCH 4/8] reflog: fix documentation Michael Haggerty
2015-02-10 23:25   ` Stefan Beller
2015-02-09  9:12 ` [PATCH 5/8] reflog: rearrange the manpage Michael Haggerty
2015-02-10 23:42   ` Stefan Beller
2015-02-12 15:17     ` Michael Haggerty
2015-02-12 20:09       ` Junio C Hamano
2015-02-09  9:12 ` [PATCH 6/8] reflog_expire(): ignore --updateref for symbolic references Michael Haggerty
2015-02-11  0:44   ` Stefan Beller
2015-02-12 16:08     ` Michael Haggerty
2015-02-12 17:04       ` Stefan Beller
2015-02-12 20:16       ` Junio C Hamano
2015-02-12 21:54   ` Jeff King
2015-02-13 14:34     ` Michael Haggerty
2015-02-09  9:12 ` Michael Haggerty [this message]
2015-02-09 20:55   ` [PATCH 7/8] reflog_expire(): never update a reference to null_sha1 Eric Sunshine
2015-02-12 11:51     ` Michael Haggerty
2015-02-09  9:12 ` [PATCH 8/8] reflog_expire(): lock symbolic refs themselves, not their referent Michael Haggerty
2015-02-11  0:49   ` Stefan Beller
2015-02-11 22:49     ` Junio C Hamano
2015-02-11 23:25       ` Stefan Beller
2015-02-12 16:52         ` Michael Haggerty
2015-02-12 18:04           ` Stefan Beller
2015-02-13 16:26             ` Michael Haggerty
2015-02-13 17:16               ` Stefan Beller
2015-02-13 18:05               ` Junio C Hamano
2015-02-13 18:21                 ` Stefan Beller
2015-02-13 18:26                   ` Junio C Hamano
2015-02-13 18:32                     ` Stefan Beller
2015-02-13 19:12                       ` Junio C Hamano
2015-02-13 20:11                         ` Michael Haggerty
2015-02-13 21:53                           ` Junio C Hamano
2015-02-14  5:58                             ` Michael Haggerty
2015-02-09 18:57 ` [PATCH 0/8] Fix some problems with reflog expiration Stefan Beller
2015-02-10 23:12 ` [PATCH] refs.c: get rid of force_write flag Stefan Beller
2015-02-12 15:35   ` Michael Haggerty

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1423473164-6011-8-git-send-email-mhagger@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=ronniesahlberg@gmail.com \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).