Merge branch 'jk/reflog-special-cases-fix'

The logic to access reflog entries by date and number had ugly
corner cases at the boundaries, which have been cleaned up.

* jk/reflog-special-cases-fix:
  read_ref_at(): special-case ref@{0} for an empty reflog
  get_oid_basic(): special-case ref@{n} for oldest reflog entry
  Revert "refs: allow @{n} to work with n-sized reflog"

diff --git a/object-name.c b/object-name.c
index 3a2ef5d6800173fa669bdfcb2612bf21a7c6417a..511f09bc0fea75301433fe888efbe10d5227fdc0 100644 (file)
--- a/object-name.c
+++ b/object-name.c
@@ -1034,6 +1034,15 @@ static int get_oid_basic(struct repository *r, const char *str, int len,
                                                len, str,
                                                show_date(co_time, co_tz, DATE_MODE(RFC2822)));
                                }
+                       } else if (nth == co_cnt && !is_null_oid(oid)) {
+                               /*
+                                * We were asked for the Nth reflog (counting
+                                * from 0), but there were only N entries.
+                                * read_ref_at() will have returned "1" to tell
+                                * us it did not find an entry, but it did
+                                * still fill in the oid with the "old" value,
+                                * which we can use.
+                                */
                        } else {
                                if (flags & GET_OID_QUIETLY) {
                                        exit(128);

diff --git a/refs.c b/refs.c
index f9261267f05efcee5be1f312a1a82b034bff61b6..c6d412877d00ae3767aacb0400904843dae7d5f3 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -1039,55 +1039,40 @@ static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,
                           const char *message, void *cb_data)
 {
        struct read_ref_at_cb *cb = cb_data;
-       int reached_count;
 
        cb->tz = tz;
        cb->date = timestamp;
 
-       /*
-        * It is not possible for cb->cnt == 0 on the first iteration because
-        * that special case is handled in read_ref_at().
-        */
-       if (cb->cnt > 0)
-               cb->cnt--;
-       reached_count = cb->cnt == 0 && !is_null_oid(ooid);
-       if (timestamp <= cb->at_time || reached_count) {
+       if (timestamp <= cb->at_time || cb->cnt == 0) {
                set_read_ref_cutoffs(cb, timestamp, tz, message);
                /*
                 * we have not yet updated cb->[n|o]oid so they still
                 * hold the values for the previous record.
                 */
-               if (!is_null_oid(&cb->ooid) && !oideq(&cb->ooid, noid))
-                       warning(_("log for ref %s has gap after %s"),
+               if (!is_null_oid(&cb->ooid)) {
+                       oidcpy(cb->oid, noid);
+                       if (!oideq(&cb->ooid, noid))
+                               warning(_("log for ref %s has gap after %s"),
                                        cb->refname, show_date(cb->date, cb->tz, DATE_MODE(RFC2822)));
-               if (reached_count)
-                       oidcpy(cb->oid, ooid);
-               else if (!is_null_oid(&cb->ooid) || cb->date == cb->at_time)
+               }
+               else if (cb->date == cb->at_time)
                        oidcpy(cb->oid, noid);
                else if (!oideq(noid, cb->oid))
                        warning(_("log for ref %s unexpectedly ended on %s"),
                                cb->refname, show_date(cb->date, cb->tz,
                                                       DATE_MODE(RFC2822)));
+               cb->reccnt++;
+               oidcpy(&cb->ooid, ooid);
+               oidcpy(&cb->noid, noid);
                cb->found_it = 1;
+               return 1;
        }
        cb->reccnt++;
        oidcpy(&cb->ooid, ooid);
        oidcpy(&cb->noid, noid);
-       return cb->found_it;
-}
-
-static int read_ref_at_ent_newest(struct object_id *ooid UNUSED,
-                                 struct object_id *noid,
-                                 const char *email UNUSED,
-                                 timestamp_t timestamp, int tz,
-                                 const char *message, void *cb_data)
-{
-       struct read_ref_at_cb *cb = cb_data;
-
-       set_read_ref_cutoffs(cb, timestamp, tz, message);
-       oidcpy(cb->oid, noid);
-       /* We just want the first entry */
-       return 1;
+       if (cb->cnt > 0)
+               cb->cnt--;
+       return 0;
 }
 
 static int read_ref_at_ent_oldest(struct object_id *ooid, struct object_id *noid,
@@ -1099,7 +1084,7 @@ static int read_ref_at_ent_oldest(struct object_id *ooid, struct object_id *noid
 
        set_read_ref_cutoffs(cb, timestamp, tz, message);
        oidcpy(cb->oid, ooid);
-       if (is_null_oid(cb->oid))
+       if (cb->at_time && is_null_oid(cb->oid))
                oidcpy(cb->oid, noid);
        /* We just want the first entry */
        return 1;
@@ -1122,14 +1107,24 @@ int read_ref_at(struct ref_store *refs, const char *refname,
        cb.cutoff_cnt = cutoff_cnt;
        cb.oid = oid;
 
-       if (cb.cnt == 0) {
-               refs_for_each_reflog_ent_reverse(refs, refname, read_ref_at_ent_newest, &cb);
-               return 0;
-       }
-
        refs_for_each_reflog_ent_reverse(refs, refname, read_ref_at_ent, &cb);
 
        if (!cb.reccnt) {
+               if (cnt == 0) {
+                       /*
+                        * The caller asked for ref@{0}, and we had no entries.
+                        * It's a bit subtle, but in practice all callers have
+                        * prepped the "oid" field with the current value of
+                        * the ref, which is the most reasonable fallback.
+                        *
+                        * We'll put dummy values into the out-parameters (so
+                        * they're not just uninitialized garbage), and the
+                        * caller can take our return value as a hint that
+                        * we did not find any such reflog.
+                        */
+                       set_read_ref_cutoffs(&cb, 0, 0, "empty reflog");
+                       return 1;
+               }
                if (flags & GET_OID_QUIETLY)
                        exit(128);
                else

diff --git a/refs.h b/refs.h
index 895579aeb7a9da56995550f7631caf55371c7951..0bd3fab468e7a703042999109b532fa4f2c51acb 100644 (file)
--- a/refs.h
+++ b/refs.h
@@ -440,7 +440,20 @@ int refs_create_reflog(struct ref_store *refs, const char *refname,
                       struct strbuf *err);
 int safe_create_reflog(const char *refname, struct strbuf *err);
 
-/** Reads log for the value of ref during at_time. **/
+/**
+ * Reads log for the value of ref during at_time (in which case "cnt" should be
+ * negative) or the reflog "cnt" entries from the top (in which case "at_time"
+ * should be 0).
+ *
+ * If we found the reflog entry in question, returns 0 (and details of the
+ * entry can be found in the out-parameters).
+ *
+ * If we ran out of reflog entries, the out-parameters are filled with the
+ * details of the oldest entry we did find, and the function returns 1. Note
+ * that there is one important special case here! If the reflog was empty
+ * and the caller asked for the 0-th cnt, we will return "1" but leave the
+ * "oid" field untouched.
+ **/
 int read_ref_at(struct ref_store *refs,
                const char *refname, unsigned int flags,
                timestamp_t at_time, int cnt,

diff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh
index 6a98b2df7611ed741be197640f20e1147794aa8d..a1139f79e2ccfdff2b562571bdd8bdf8aa974883 100755 (executable)
--- a/t/t3202-show-branch.sh
+++ b/t/t3202-show-branch.sh
@@ -4,9 +4,6 @@ test_description='test show-branch'
 
 . ./test-lib.sh
 
-# arbitrary reference time: 2009-08-30 19:20:00
-GIT_TEST_DATE_NOW=1251660000; export GIT_TEST_DATE_NOW
-
 test_expect_success 'error descriptions on empty repository' '
        current=$(git branch --show-current) &&
        cat >expect <<-EOF &&
@@ -187,18 +184,6 @@ test_expect_success 'show branch --merge-base with N arguments' '
        test_cmp expect actual
 '
 
-test_expect_success 'show branch --reflog=2' '
-       sed "s/^>       //" >expect <<-\EOF &&
-       >       ! [refs/heads/branch10@{0}] (4 years, 5 months ago) commit: branch10
-       >        ! [refs/heads/branch10@{1}] (4 years, 5 months ago) commit: branch10
-       >       --
-       >       +  [refs/heads/branch10@{0}] branch10
-       >       ++ [refs/heads/branch10@{1}] initial
-       EOF
-       git show-branch --reflog=2 >actual &&
-       test_cmp actual expect
-'
-
 # incompatible options
 while read combo
 do
@@ -264,4 +249,38 @@ test_expect_success 'error descriptions on orphan branch' '
        test_branch_op_in_wt -c new-branch
 '
 
+test_expect_success 'setup reflogs' '
+       test_commit base &&
+       git checkout -b branch &&
+       test_commit one &&
+       git reset --hard HEAD^ &&
+       test_commit two &&
+       test_commit three
+'
+
+test_expect_success '--reflog shows reflog entries' '
+       cat >expect <<-\EOF &&
+       ! [branch@{0}] (0 seconds ago) commit: three
+        ! [branch@{1}] (60 seconds ago) commit: two
+         ! [branch@{2}] (2 minutes ago) reset: moving to HEAD^
+          ! [branch@{3}] (2 minutes ago) commit: one
+       ----
+       +    [branch@{0}] three
+       ++   [branch@{1}] two
+          + [branch@{3}] one
+       ++++ [branch@{2}] base
+       EOF
+       # the output always contains relative timestamps; use
+       # a known time to get deterministic results
+       GIT_TEST_DATE_NOW=$test_tick \
+       git show-branch --reflog branch >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success '--reflog handles missing reflog' '
+       git reflog expire --expire=now branch &&
+       git show-branch --reflog branch >actual &&
+       test_must_be_empty actual
+'
+
 test_done