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 <mhag...@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 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to