[PATCH v3 6/7] reflog_expire(): ignore --updateref for symbolic references
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: another problem in this code 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(-)
Show changes to 2 files +14 −4
Documentation/git-reflog.txt, refs.c
diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt
index 730106c..5e7908e 100644
--- a/Documentation/git-reflog.txt
+++ b/Documentation/git-reflog.txt
@@ -88,7 +88,8 @@ Options for `expire`
--updateref::
Update the reference to the value of the top reflog entry (i.e.
- <ref>@\{0\}) if the previous top entry was pruned.
+ <ref>@\{0\}) if the previous top entry was pruned. (This
+ option is ignored for symbolic references.)
--rewrite::
If a reflog entry's predecessor is pruned, adjust its "old"
diff --git a/refs.c b/refs.c
index 7a2f53f..48bb9e8 100644
--- a/refs.c
+++ b/refs.c
@@ -4028,6 +4028,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;
@@ -4039,7 +4040,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)) {
@@ -4076,10 +4077,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 ||
@@ -4090,7 +4099,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
Michael Haggerty