On Thu, Jul 06, 2017 at 11:00:13PM -0700, Junio C Hamano wrote:

> Jeff King <p...@peff.net> writes:
> 
> > I suspect that "--since 3.days" is still quite buggy (even with a single
> > reflog) because it checks commit timestamps and stops traversing when we
> > go too bar back. But in a reflog, the commits may be totally out of
> > order. I'm not sure what it should do. Either:
> >
> >   1. During a reflog walk, --since and --until should respect reflog
> >      timestamps instead of commit timestamps. You can already do
> >      "--until" there by simply starting the traversal later, but there's
> >      no way to cut it off with --since.
> >
> >   2. Limit commits shown by --since/--until as usual, but skip the "stop
> >      traversing" optimization when we see too many "old" commits. I.e.,
> >      omit a 4.days.ago commit, but keep walking to find other recent
> >      commits.
> 
> I think 1. is more logical, and I was imagining that it should be
> doable, now we are not constrained by (ab)using the commit_list with
> the fake-parent thing, but are pulling the entries directly from the
> reflog iterator and the timestamp would be already available to the
> iterator.
> 
> But I recall that the max_age and min_age cutoff is done long after
> a commit is pulled out of the "iterator mechanism" (be it the
> commit_list or your direct reflog iterator) by comparing
> commit->date with the cut-off, so it may be a bit more involved to
> arrange than I imagined X-<.  Hmph...

It's probably not too bad.

We do some of the limiting in limit_list(), which tries to mark commits
as UNINTERESTING. But I think in general that limit_list is incompatible
with reflog traversals (though I wouldn't be surprised if we fail to
flag all the options that set revs->limited as incompatible).

We handle max_age in get_revision_1() itself, which should be pretty
straightforward. For min_age, we do it in get_commit_action(), which
would need access to the reflog timestamp. But we do have the rev_info
there.

So something like the patch below would work, I think.

diff --git a/reflog-walk.c b/reflog-walk.c
index fbee9e0126..74ebe5148f 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -264,6 +264,18 @@ const char *get_reflog_ident(struct reflog_walk_info 
*reflog_info)
        return info->email;
 }
 
+timestamp_t get_reflog_timestamp(struct reflog_walk_info *reflog_info)
+{
+       struct commit_reflog *commit_reflog = reflog_info->last_commit_reflog;
+       struct reflog_info *info;
+
+       if (!commit_reflog)
+               return 0;
+
+       info = &commit_reflog->reflogs->items[commit_reflog->recno+1];
+       return info->timestamp;
+}
+
 void show_reflog_message(struct reflog_walk_info *reflog_info, int oneline,
                         const struct date_mode *dmode, int force_date)
 {
diff --git a/reflog-walk.h b/reflog-walk.h
index 373388cd14..7553c448fe 100644
--- a/reflog-walk.h
+++ b/reflog-walk.h
@@ -13,6 +13,7 @@ extern void show_reflog_message(struct reflog_walk_info 
*info, int,
 extern void get_reflog_message(struct strbuf *sb,
                struct reflog_walk_info *reflog_info);
 extern const char *get_reflog_ident(struct reflog_walk_info *reflog_info);
+extern timestamp_t get_reflog_timestamp(struct reflog_walk_info *reflog_info);
 extern void get_reflog_selector(struct strbuf *sb,
                struct reflog_walk_info *reflog_info,
                const struct date_mode *dmode, int force_date,
diff --git a/revision.c b/revision.c
index 5fc01f2d26..c248a16974 100644
--- a/revision.c
+++ b/revision.c
@@ -2973,8 +2973,13 @@ enum commit_action get_commit_action(struct rev_info 
*revs, struct commit *commi
                return commit_show;
        if (commit->object.flags & UNINTERESTING)
                return commit_ignore;
-       if (revs->min_age != -1 && (commit->date > revs->min_age))
-               return commit_ignore;
+       if (revs->min_age != -1) {
+               timestamp_t date = revs->reflog_info ?
+                                  get_reflog_timestamp(revs->reflog_info) :
+                                  commit->date;
+               if (date > revs->min_age)
+                       return commit_ignore;
+       }
        if (revs->min_parents || (revs->max_parents >= 0)) {
                int n = commit_list_count(commit->parents);
                if ((n < revs->min_parents) ||
@@ -3127,9 +3132,13 @@ static struct commit *get_revision_1(struct rev_info 
*revs)
                 * that we'd otherwise have done in limit_list().
                 */
                if (!revs->limited) {
-                       if (revs->max_age != -1 &&
-                           (commit->date < revs->max_age))
-                               continue;
+                       if (revs->max_age != -1) {
+                               timestamp_t date = revs->reflog_info ?
+                                                  
get_reflog_timestamp(revs->reflog_info) :
+                                                  commit->date;
+                               if (date < revs->max_age)
+                                       continue;
+                       }
                        if (!revs->reflog_info &&
                            add_parents_to_list(revs, commit, &revs->commits, 
NULL) < 0) {
                                if (!revs->ignore_missing_links)

Reply via email to