Merge branch 'mh/expire-updateref-fixes'
authorJunio C Hamano <gitster@pobox.com>
Tue, 10 Mar 2015 20:52:39 +0000 (13:52 -0700)
committerJunio C Hamano <gitster@pobox.com>
Tue, 10 Mar 2015 20:52:40 +0000 (13:52 -0700)
Various issues around "reflog expire", e.g. using --updateref when
expiring a reflog for a symbolic reference, have been corrected
and/or made saner.

* mh/expire-updateref-fixes:
reflog_expire(): never update a reference to null_sha1
reflog_expire(): ignore --updateref for symbolic references
reflog: improve and update documentation
struct ref_lock: delete the force_write member
lock_ref_sha1_basic(): do not set force_write for missing references
write_ref_sha1(): move write elision test to callers
write_ref_sha1(): remove check for lock == NULL

Documentation/git-reflog.txt
builtin/reflog.c
refs.c
index 70791b9fd88b4c64260523a5fcadcb46ce504f8f..5e7908e4f705d858dca8483069fd3b89c8f7e31c 100644 (file)
@@ -17,85 +17,113 @@ The command takes various subcommands, and different options
 depending on the subcommand:
 
 [verse]
-'git reflog expire' [--dry-run] [--stale-fix] [--verbose]
-       [--expire=<time>] [--expire-unreachable=<time>] [--all] <refs>...
-'git reflog delete' ref@\{specifier\}...
 'git reflog' ['show'] [log-options] [<ref>]
+'git reflog expire' [--expire=<time>] [--expire-unreachable=<time>]
+       [--rewrite] [--updateref] [--stale-fix]
+       [--dry-run] [--verbose] [--all | <refs>...]
+'git reflog delete' [--rewrite] [--updateref]
+       [--dry-run] [--verbose] ref@\{specifier\}...
+
+Reference logs, or "reflogs", record when the tips of branches and
+other references were updated in the local repository. Reflogs are
+useful in various Git commands, to specify the old value of a
+reference. For example, `HEAD@{2}` means "where HEAD used to be two
+moves ago", `master@{one.week.ago}` means "where master used to point
+to one week ago in this local repository", and so on. See
+linkgit:gitrevisions[7] for more details.
+
+This command manages the information recorded in the reflogs.
+
+The "show" subcommand (which is also the default, in the absence of
+any subcommands) shows the log of the reference provided in the
+command-line (or `HEAD`, by default). The reflog covers all recent
+actions, and in addition the `HEAD` reflog records branch switching.
+`git reflog show` is an alias for `git log -g --abbrev-commit
+--pretty=oneline`; see linkgit:git-log[1] for more information.
+
+The "expire" subcommand prunes older reflog entries. Entries older
+than `expire` time, or entries older than `expire-unreachable` time
+and not reachable from the current tip, are removed from the reflog.
+This is typically not used directly by end users -- instead, see
+linkgit:git-gc[1].
+
+The "delete" subcommand deletes single entries from the reflog. Its
+argument must be an _exact_ entry (e.g. "`git reflog delete
+master@{2}`"). This subcommand is also typically not used directly by
+end users.
 
-Reflog is a mechanism to record when the tip of branches are
-updated.  This command is to manage the information recorded in it.
 
-The subcommand "expire" is used to prune older reflog entries.
-Entries older than `expire` time, or entries older than
-`expire-unreachable` time and not reachable from the current
-tip, are removed from the reflog.  This is typically not used
-directly by the end users -- instead, see linkgit:git-gc[1].
-
-The subcommand "show" (which is also the default, in the absence of any
-subcommands) will take all the normal log options, and show the log of
-the reference provided in the command-line (or `HEAD`, by default).
-The reflog will cover all recent actions (HEAD reflog records branch switching
-as well).  It is an alias for `git log -g --abbrev-commit --pretty=oneline`;
-see linkgit:git-log[1].
+OPTIONS
+-------
 
-The reflog is useful in various Git commands, to specify the old value
-of a reference. For example, `HEAD@{2}` means "where HEAD used to be
-two moves ago", `master@{one.week.ago}` means "where master used to
-point to one week ago", and so on. See linkgit:gitrevisions[7] for
-more details.
+Options for `show`
+~~~~~~~~~~~~~~~~~~
 
-To delete single entries from the reflog, use the subcommand "delete"
-and specify the _exact_ entry (e.g. "`git reflog delete master@{2}`").
+`git reflog show` accepts any of the options accepted by `git log`.
 
 
-OPTIONS
--------
+Options for `expire`
+~~~~~~~~~~~~~~~~~~~~
 
---stale-fix::
-       This revamps the logic -- the definition of "broken commit"
-       becomes: a commit that is not reachable from any of the refs and
-       there is a missing object among the commit, tree, or blob
-       objects reachable from it that is not reachable from any of the
-       refs.
-+
-This computation involves traversing all the reachable objects, i.e. it
-has the same cost as 'git prune'.  Fortunately, once this is run, we
-should not have to ever worry about missing objects, because the current
-prune and pack-objects know about reflogs and protect objects referred by
-them.
+--all::
+       Process the reflogs of all references.
 
 --expire=<time>::
-       Entries older than this time are pruned.  Without the
-       option it is taken from configuration `gc.reflogExpire`,
-       which in turn defaults to 90 days.  --expire=all prunes
-       entries regardless of their age; --expire=never turns off
-       pruning of reachable entries (but see --expire-unreachable).
+       Prune entries older than the specified time. If this option is
+       not specified, the expiration time is taken from the
+       configuration setting `gc.reflogExpire`, which in turn
+       defaults to 90 days. `--expire=all` prunes entries regardless
+       of their age; `--expire=never` turns off pruning of reachable
+       entries (but see `--expire-unreachable`).
 
 --expire-unreachable=<time>::
-       Entries older than this time and not reachable from
-       the current tip of the branch are pruned.  Without the
-       option it is taken from configuration
-       `gc.reflogExpireUnreachable`, which in turn defaults to
-       30 days.  --expire-unreachable=all prunes unreachable
-       entries regardless of their age; --expire-unreachable=never
+       Prune entries older than `<time>` that are not reachable from
+       the current tip of the branch. If this option is not
+       specified, the expiration time is taken from the configuration
+       setting `gc.reflogExpireUnreachable`, which in turn defaults
+       to 30 days. `--expire-unreachable=all` prunes unreachable
+       entries regardless of their age; `--expire-unreachable=never`
        turns off early pruning of unreachable entries (but see
-       --expire).
-
---all::
-       Instead of listing <refs> explicitly, prune all refs.
+       `--expire`).
 
 --updateref::
-       Update the ref with the sha1 of the top reflog entry (i.e.
-       <ref>@\{0\}) after expiring or deleting.
+       Update the reference to the value of the top reflog entry (i.e.
+       <ref>@\{0\}) if the previous top entry was pruned.  (This
+       option is ignored for symbolic references.)
 
 --rewrite::
-       While expiring or deleting, adjust each reflog entry to ensure
-       that the `old` sha1 field points to the `new` sha1 field of the
-       previous entry.
+       If a reflog entry's predecessor is pruned, adjust its "old"
+       SHA-1 to be equal to the "new" SHA-1 field of the entry that
+       now precedes it.
+
+--stale-fix::
+       Prune any reflog entries that point to "broken commits". A
+       broken commit is a commit that is not reachable from any of
+       the reference tips and that refers, directly or indirectly, to
+       a missing commit, tree, or blob object.
++
+This computation involves traversing all the reachable objects, i.e. it
+has the same cost as 'git prune'.  It is primarily intended to fix
+corruption caused by garbage collecting using older versions of Git,
+which didn't protect objects referred to by reflogs.
+
+-n::
+--dry-run::
+       Do not actually prune any entries; just show what would have
+       been pruned.
 
 --verbose::
        Print extra information on screen.
 
+
+Options for `delete`
+~~~~~~~~~~~~~~~~~~~~
+
+`git reflog delete` accepts options `--updateref`, `--rewrite`, `-n`,
+`--dry-run`, and `--verbose`, with the same meanings as when they are
+used with `expire`.
+
+
 GIT
 ---
 Part of the linkgit:git[1] suite
index 49c64f96d8adba41261741cfa48f0e36fcd9f3bb..8182b648b9c9693ec6f23f6b2100e432bfa794ba 100644 (file)
@@ -8,14 +8,11 @@
 #include "revision.h"
 #include "reachable.h"
 
-/*
- * reflog expire
- */
-
+/* NEEDSWORK: switch to using parse_options */
 static const char reflog_expire_usage[] =
-"git reflog expire [--verbose] [--dry-run] [--stale-fix] [--expire=<time>] [--expire-unreachable=<time>] [--all] <refs>...";
+"git reflog expire [--expire=<time>] [--expire-unreachable=<time>] [--rewrite] [--updateref] [--stale-fix] [--dry-run | -n] [--verbose] [--all] <refs>...";
 static const char reflog_delete_usage[] =
-"git reflog delete [--verbose] [--dry-run] [--rewrite] [--updateref] <refs>...";
+"git reflog delete [--rewrite] [--updateref] [--dry-run | -n] [--verbose] <refs>...";
 
 static unsigned long default_reflog_expire;
 static unsigned long default_reflog_expire_unreachable;
diff --git a/refs.c b/refs.c
index 8d46b08055ef9cb151c1868a0354fd906b0bcddf..e23542b3869b38e47f59f102d28648d30d506574 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -12,7 +12,6 @@ struct ref_lock {
        struct lock_file *lk;
        unsigned char old_sha1[20];
        int lock_fd;
-       int force_write;
 };
 
 /*
@@ -2277,7 +2276,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
        int type, lflags;
        int mustexist = (old_sha1 && !is_null_sha1(old_sha1));
        int resolve_flags = 0;
-       int missing = 0;
        int attempts_remaining = 3;
 
        lock = xcalloc(1, sizeof(struct ref_lock));
@@ -2316,13 +2314,13 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
                        orig_refname, strerror(errno));
                goto error_return;
        }
-       missing = is_null_sha1(lock->old_sha1);
-       /* When the ref did not exist and we are creating it,
-        * make sure there is no existing ref that is packed
-        * whose name begins with our refname, nor a ref whose
-        * name is a proper prefix of our refname.
+       /*
+        * If the ref did not exist and we are creating it, make sure
+        * there is no existing packed ref whose name begins with our
+        * refname, nor a packed ref whose name is a proper prefix of
+        * our refname.
         */
-       if (missing &&
+       if (is_null_sha1(lock->old_sha1) &&
             !is_refname_available(refname, skip, get_packed_refs(&ref_cache))) {
                last_errno = ENOTDIR;
                goto error_return;
@@ -2338,10 +2336,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
        lock->ref_name = xstrdup(refname);
        lock->orig_ref_name = xstrdup(orig_refname);
        ref_file = git_path("%s", refname);
-       if (missing)
-               lock->force_write = 1;
-       if ((flags & REF_NODEREF) && (type & REF_ISSYMREF))
-               lock->force_write = 1;
 
  retry:
        switch (safe_create_leading_directories(ref_file)) {
@@ -2897,7 +2891,6 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
                error("unable to lock %s for update", newrefname);
                goto rollback;
        }
-       lock->force_write = 1;
        hashcpy(lock->old_sha1, orig_sha1);
        if (write_ref_sha1(lock, orig_sha1, logmsg)) {
                error("unable to write current sha1 into %s", newrefname);
@@ -2913,7 +2906,6 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
                goto rollbacklog;
        }
 
-       lock->force_write = 1;
        flag = log_all_ref_updates;
        log_all_ref_updates = 0;
        if (write_ref_sha1(lock, orig_sha1, NULL))
@@ -3099,14 +3091,6 @@ static int write_ref_sha1(struct ref_lock *lock,
        static char term = '\n';
        struct object *o;
 
-       if (!lock) {
-               errno = EINVAL;
-               return -1;
-       }
-       if (!lock->force_write && !hashcmp(lock->old_sha1, sha1)) {
-               unlock_ref(lock);
-               return 0;
-       }
        o = parse_object(sha1);
        if (!o) {
                error("Trying to write ref %s with nonexistent object %s",
@@ -3851,15 +3835,28 @@ int ref_transaction_commit(struct ref_transaction *transaction,
                int flags = update->flags;
 
                if ((flags & REF_HAVE_NEW) && !is_null_sha1(update->new_sha1)) {
-                       if (write_ref_sha1(update->lock, update->new_sha1,
-                                          update->msg)) {
+                       int overwriting_symref = ((update->type & REF_ISSYMREF) &&
+                                                 (update->flags & REF_NODEREF));
+
+                       if (!overwriting_symref
+                           && !hashcmp(update->lock->old_sha1, update->new_sha1)) {
+                               /*
+                                * The reference already has the desired
+                                * value, so we don't need to write it.
+                                */
+                               unlock_ref(update->lock);
+                               update->lock = NULL;
+                       } else if (write_ref_sha1(update->lock, update->new_sha1,
+                                                 update->msg)) {
                                update->lock = NULL; /* freed by write_ref_sha1 */
                                strbuf_addf(err, "Cannot update the ref '%s'.",
                                            update->refname);
                                ret = TRANSACTION_GENERIC_ERROR;
                                goto cleanup;
+                       } else {
+                               /* freed by write_ref_sha1(): */
+                               update->lock = NULL;
                        }
-                       update->lock = NULL; /* freed by write_ref_sha1 */
                }
        }
 
@@ -4083,6 +4080,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;
@@ -4094,7 +4092,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)) {
@@ -4131,10 +4129,21 @@ 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. Nor can we update
+                * a reference if there are no remaining reflog
+                * entries.
+                */
+               int update = (flags & EXPIRE_REFLOGS_UPDATE_REF) &&
+                       !(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,
                                        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 ||
@@ -4145,7 +4154,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);
                }
        }