@matheusbsbs / matheus-bagliano / commits / 5e6f003ca8

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> Reviewed-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Michael Haggerty committed Mar 3, 2015 at 12:43 UTC 5e6f003ca8aed546d50a4f3fbf1e011186047bc0

2 files changed +14 -4

Documentation/git-reflog.txt

+2 -1

@@ -88,7 +88,8 @@ Options for `expire`

89 --updateref::

90 Update the reference to the value of the top reflog entry (i.e.

91 - <ref>@\{0\}) if the previous top entry was pruned.

91 + <ref>@\{0\}) if the previous top entry was pruned. (This

92 + option is ignored for symbolic references.)

94 --rewrite::

95 If a reflog entry's predecessor is pruned, adjust its "old"

refs.c

+12 -3

@@ -4029,6 +4029,7 @@ int reflog_expire(const char *refname, const unsigned char *sha1,

4029 struct ref_lock *lock;

4030 char *log_file;

4031 int status = 0;

4032 + int type;

4033

4034 memset(&cb, 0, sizeof(cb));

4035 cb.flags = flags;

@@ -4040,7 +4041,7 @@ int reflog_expire(const char *refname, const unsigned char *sha1,

4041 * reference itself, plus we might need to update the

4042 * reference if --updateref was specified:

4043 */

4043 - lock = lock_ref_sha1_basic(refname, sha1, NULL, 0, NULL);

4044 + lock = lock_ref_sha1_basic(refname, sha1, NULL, 0, &type);

4045 if (!lock)

4046 return error("cannot lock ref '%s'", refname);

4047 if (!reflog_exists(refname)) {

@@ -4077,10 +4078,18 @@ int reflog_expire(const char *refname, const unsigned char *sha1,

4078 (*cleanup_fn)(cb.policy_cb);

4079

4080 if (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {

4081 + /*

4082 + * It doesn't make sense to adjust a reference pointed

4083 + * to by a symbolic ref based on expiring entries in

4084 + * the symbolic reference's reflog.

4085 + */

4086 + int update = (flags & EXPIRE_REFLOGS_UPDATE_REF) &&

4087 + !(type & REF_ISSYMREF);

4088 +

4089 if (close_lock_file(&reflog_lock)) {

4090 status |= error("couldn't write %s: %s", log_file,

4091 strerror(errno));

4083 - } else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&

4092 + } else if (update &&

4093 (write_in_full(lock->lock_fd,

4094 sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||

4095 write_str_in_full(lock->lock_fd, "\n") != 1 ||

@@ -4091,7 +4100,7 @@ int reflog_expire(const char *refname, const unsigned char *sha1,

4100 } else if (commit_lock_file(&reflog_lock)) {

4101 status |= error("unable to commit reflog '%s' (%s)",

4102 log_file, strerror(errno));

4094 - } else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) && commit_ref(lock)) {

4103 + } else if (update && commit_ref(lock)) {

4104 status |= error("couldn't set %s", lock->ref_name);

4105 }

4106 }