[PATCH 6/8] reflog_expire(): ignore --updateref for symbolic references
- From
Michael Haggerty <mhagger@alum.mit.edu>
- Date
- Feb 9, 2015, 09:12 UTC
- Message-ID
- <1423473164-6011-7-git-send-email-mhagger@alum.mit.edu>
- In-Reply-To
- <1423473164-6011-1-git-send-email-mhagger@alum.mit.edu>
If we are expiring reflog entries for a symbolic reference, then how should --updateref be handled if the newest reflog entry is expired?
Option 1: Update the referred-to reference. (This is what the current code does.) This doesn't make sense, because the referred-to reference has its own reflog, which hasn't been rewritten.
Option 2: Update the symbolic reference itself (as in, REF_NODEREF). This would convert the symbolic reference into a non-symbolic reference (e.g., detaching HEAD), which is surely not what a user would expect.
Option 3: Error out. This is plausible, but it would make the following usage impossible:
git reflog expire ... --updateref --all
Option 4: Ignore --updateref for symbolic references.
We choose to implement option 4.
Note: there are still other problems in this code that will be fixed in a moment.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> --- Documentation/git-reflog.txt | 3 ++- refs.c | 15 ++++++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt index f15a48e..9b87b46 100644 --- a/Documentation/git-reflog.txt +++ b/Documentation/git-reflog.txt @@ -85,7 +85,8 @@ them. --updateref:: Update the ref with the sha1 of the top reflog entry (i.e. - <ref>@\{0\}) after expiring or deleting. + <ref>@\{0\}) after expiring or deleting. (This option is + ignored for symbolic references.) --rewrite:: While expiring or deleting, adjust each reflog entry to ensure diff --git a/refs.c b/refs.c index b083858..c0001da 100644 --- a/refs.c +++ b/refs.c @@ -4025,6 +4025,7 @@ int reflog_expire(const char *refname, const unsigned char *sha1, struct ref_lock *lock; char *log_file; int status = 0; + int type; memset(&cb, 0, sizeof(cb)); cb.flags = flags; @@ -4036,7 +4037,7 @@ int reflog_expire(const char *refname, const unsigned char *sha1, * reference itself, plus we might need to update the * reference if --updateref was specified: */ - lock = lock_ref_sha1_basic(refname, sha1, NULL, 0, NULL); + lock = lock_ref_sha1_basic(refname, sha1, NULL, 0, &type); if (!lock) return error("cannot lock ref '%s'", refname); if (!reflog_exists(refname)) { @@ -4073,10 +4074,18 @@ int reflog_expire(const char *refname, const unsigned char *sha1, (*cleanup_fn)(cb.policy_cb); if (!(flags & EXPIRE_REFLOGS_DRY_RUN)) { + /* + * 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. + */ + int update = (flags & EXPIRE_REFLOGS_UPDATE_REF) && + ~(type & REF_ISSYMREF); + if (close_lock_file(&reflog_lock)) { status |= error("couldn't write %s: %s", log_file, strerror(errno)); - } else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) && + } else if (update && (write_in_full(lock->lock_fd, sha1_to_hex(cb.last_kept_sha1), 40) != 40 || write_str_in_full(lock->lock_fd, "\n") != 1 || @@ -4087,7 +4096,7 @@ int reflog_expire(const char *refname, const unsigned char *sha1, } else if (commit_lock_file(&reflog_lock)) { status |= error("unable to commit reflog '%s' (%s)", log_file, strerror(errno)); - } else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) && commit_ref(lock)) { + } else if (update && commit_ref(lock)) { status |= error("couldn't set %s", lock->ref_name); } }
-- 2.1.4