Merge branch 'jk/reflog-walk'
authorJunio C Hamano <gitster@pobox.com>
Fri, 11 Aug 2017 20:27:00 +0000 (13:27 -0700)
committerJunio C Hamano <gitster@pobox.com>
Fri, 11 Aug 2017 20:27:01 +0000 (13:27 -0700)
Numerous bugs in walking of reflogs via "log -g" and friends have
been fixed.

* jk/reflog-walk:
reflog-walk: apply --since/--until to reflog dates
reflog-walk: stop using fake parents
rev-list: check reflog_info before showing usage
get_revision_1(): replace do-while with an early return
log: do not free parents when walking reflog
log: clarify comment about reflog cycles
revision: disallow reflog walking with revs->limited
t1414: document some reflog-walk oddities

1  2 
builtin/log.c
builtin/rev-list.c
revision.c
diff --combined builtin/log.c
index cb7e0e61d722e3ff5fb66528e4e7496c1d3c5680,630d6cff2ff969c280fd0f06f17ce7a960901008..725c7b8a1a40f3dd943e4f42c801121d9c8ea78f
@@@ -372,11 -372,14 +372,14 @@@ static int cmd_log_walk(struct rev_inf
                         */
                        rev->max_count++;
                if (!rev->reflog_info) {
-                       /* we allow cycles in reflog ancestry */
+                       /*
+                        * We may show a given commit multiple times when
+                        * walking the reflogs.
+                        */
                        free_commit_buffer(commit);
+                       free_commit_list(commit->parents);
+                       commit->parents = NULL;
                }
-               free_commit_list(commit->parents);
-               commit->parents = NULL;
                if (saved_nrl < rev->diffopt.needed_rename_limit)
                        saved_nrl = rev->diffopt.needed_rename_limit;
                if (rev->diffopt.degraded_cc_to_c)
@@@ -484,8 -487,8 +487,8 @@@ static int show_blob_object(const struc
            !DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV))
                return stream_blob_to_fd(1, oid, NULL, 0);
  
 -      if (get_sha1_with_context(obj_name, GET_SHA1_RECORD_PATH,
 -                                oidc.hash, &obj_context))
 +      if (get_oid_with_context(obj_name, GET_OID_RECORD_PATH,
 +                               &oidc, &obj_context))
                die(_("Not a valid object name %s"), obj_name);
        if (!obj_context.path ||
            !textconv_object(obj_context.path, obj_context.mode, &oidc, 1, &buf, &size)) {
@@@ -1308,7 -1311,7 +1311,7 @@@ static struct commit *get_base_commit(c
  
                if (rev_nr % 2)
                        rev[i] = rev[2 * i];
 -              rev_nr = (rev_nr + 1) / 2;
 +              rev_nr = DIV_ROUND_UP(rev_nr, 2);
        }
  
        if (!in_merge_bases(base, rev[0]))
diff --combined builtin/rev-list.c
index fee10d856787acce529f035ab421b1cec22bac82,53a746dd891419f01d7c26b4b783a3b0142c26d2..e8f50489038644ef7a6dc0d217563c65fa9d6083
@@@ -11,6 -11,7 +11,7 @@@
  #include "graph.h"
  #include "bisect.h"
  #include "progress.h"
+ #include "reflog-walk.h"
  
  static const char rev_list_usage[] =
  "git rev-list [OPTION] <commit-id>... [ -- paths... ]\n"
@@@ -122,7 -123,6 +123,7 @@@ static void show_commit(struct commit *
                ctx.date_mode_explicit = revs->date_mode_explicit;
                ctx.fmt = revs->commit_format;
                ctx.output_encoding = get_log_output_encoding();
 +              ctx.color = revs->diffopt.use_color;
                pretty_print_commit(&ctx, commit, &buf);
                if (buf.len) {
                        if (revs->commit_format != CMIT_FMT_ONELINE)
@@@ -349,7 -349,7 +350,7 @@@ int cmd_rev_list(int argc, const char *
                /* Only --header was specified */
                revs.commit_format = CMIT_FMT_RAW;
  
-       if ((!revs.commits &&
+       if ((!revs.commits && reflog_walk_empty(revs.reflog_info) &&
             (!(revs.tag_objects || revs.tree_objects || revs.blob_objects) &&
              !revs.pending.nr)) ||
            revs.diff)
diff --combined revision.c
index bfbf0514c20332d8c8d197081fb463a88053553d,41b4375c3c9cf5a6bb284b432a9f46091087ef4e..ab4af0c352928e85adb7697fba5eb9ecddf83a70
@@@ -148,16 -148,14 +148,14 @@@ static void add_pending_object_with_pat
        if (revs->reflog_info && obj->type == OBJ_COMMIT) {
                struct strbuf buf = STRBUF_INIT;
                int len = interpret_branch_name(name, 0, &buf, 0);
-               int st;
  
                if (0 < len && name[len] && buf.len)
                        strbuf_addstr(&buf, name + len);
-               st = add_reflog_for_walk(revs->reflog_info,
-                                        (struct commit *)obj,
-                                        buf.buf[0] ? buf.buf: name);
+               add_reflog_for_walk(revs->reflog_info,
+                                   (struct commit *)obj,
+                                   buf.buf[0] ? buf.buf: name);
                strbuf_release(&buf);
-               if (st)
-                       return;
+               return; /* do not add the commit itself */
        }
        add_object_array_with_path(obj, name, &revs->pending, mode, path);
  }
@@@ -1142,7 -1140,7 +1140,7 @@@ int ref_excluded(struct string_list *re
        if (!ref_excludes)
                return 0;
        for_each_string_list_item(item, ref_excludes) {
 -              if (!wildmatch(item->string, path, 0, NULL))
 +              if (!wildmatch(item->string, path, 0))
                        return 1;
        }
        return 0;
@@@ -1303,7 -1301,7 +1301,7 @@@ static int add_parents_only(struct rev_
                flags ^= UNINTERESTING | BOTTOM;
                arg++;
        }
 -      if (get_sha1_committish(arg, oid.hash))
 +      if (get_oid_committish(arg, &oid))
                return 0;
        while (1) {
                it = get_reference(revs, arg, &oid, 0);
@@@ -1362,6 -1360,7 +1360,6 @@@ void init_revisions(struct rev_info *re
        init_grep_defaults();
        grep_init(&revs->grep_filter, prefix);
        revs->grep_filter.status_only = 1;
 -      revs->grep_filter.regflags = REG_NEWLINE;
  
        diff_setup(&revs->diffopt);
        if (prefix && !revs->diffopt.prefix) {
@@@ -1452,7 -1451,7 +1450,7 @@@ static int handle_dotdot_1(const char *
        unsigned int a_flags, b_flags;
        int symmetric = 0;
        unsigned int flags_exclude = flags ^ (UNINTERESTING | BOTTOM);
 -      unsigned int oc_flags = GET_SHA1_COMMITTISH | GET_SHA1_RECORD_PATH;
 +      unsigned int oc_flags = GET_OID_COMMITTISH | GET_OID_RECORD_PATH;
  
        a_name = arg;
        if (!*a_name)
        if (!*b_name)
                b_name = "HEAD";
  
 -      if (get_sha1_with_context(a_name, oc_flags, a_oid.hash, a_oc) ||
 -          get_sha1_with_context(b_name, oc_flags, b_oid.hash, b_oc))
 +      if (get_oid_with_context(a_name, oc_flags, &a_oid, a_oc) ||
 +          get_oid_with_context(b_name, oc_flags, &b_oid, b_oc))
                return -1;
  
        if (!cant_be_filename) {
@@@ -1548,7 -1547,7 +1546,7 @@@ int handle_revision_arg(const char *arg
        int local_flags;
        const char *arg = arg_;
        int cant_be_filename = revarg_opt & REVARG_CANNOT_BE_FILENAME;
 -      unsigned get_sha1_flags = GET_SHA1_RECORD_PATH;
 +      unsigned get_sha1_flags = GET_OID_RECORD_PATH;
  
        flags = flags & UNINTERESTING ? flags | BOTTOM : flags & ~BOTTOM;
  
        }
  
        if (revarg_opt & REVARG_COMMITTISH)
 -              get_sha1_flags |= GET_SHA1_COMMITTISH;
 +              get_sha1_flags |= GET_OID_COMMITTISH;
  
 -      if (get_sha1_with_context(arg, get_sha1_flags, oid.hash, &oc))
 +      if (get_oid_with_context(arg, get_sha1_flags, &oid, &oc))
                return revs->ignore_missing ? 0 : -1;
        if (!cant_be_filename)
                verify_non_filename(revs->prefix, arg);
@@@ -2021,6 -2020,7 +2019,6 @@@ static int handle_revision_opt(struct r
                revs->grep_filter.pattern_type_option = GREP_PATTERN_TYPE_ERE;
        } else if (!strcmp(arg, "--regexp-ignore-case") || !strcmp(arg, "-i")) {
                revs->grep_filter.ignore_case = 1;
 -              revs->grep_filter.regflags |= REG_ICASE;
                DIFF_OPT_SET(&revs->diffopt, PICKAXE_IGNORE_CASE);
        } else if (!strcmp(arg, "--fixed-strings") || !strcmp(arg, "-F")) {
                revs->grep_filter.pattern_type_option = GREP_PATTERN_TYPE_FIXED;
@@@ -2319,7 -2319,7 +2317,7 @@@ int setup_revisions(int argc, const cha
                struct object_id oid;
                struct object *object;
                struct object_context oc;
 -              if (get_sha1_with_context(revs->def, 0, oid.hash, &oc))
 +              if (get_oid_with_context(revs->def, 0, &oid, &oc))
                        diagnose_missing_default(revs->def);
                object = get_reference(revs, revs->def, &oid, 0);
                add_pending_object_with_mode(revs, object, revs->def, oc.mode);
  
        if (revs->reverse && revs->reflog_info)
                die("cannot combine --reverse with --walk-reflogs");
+       if (revs->reflog_info && revs->limited)
+               die("cannot combine --walk-reflogs with history-limiting options");
        if (revs->rewrite_parents && revs->children.name)
                die("cannot combine --parents and --children");
  
@@@ -2963,6 -2965,18 +2963,18 @@@ static inline int want_ancestry(const s
        return (revs->rewrite_parents || revs->children.name);
  }
  
+ /*
+  * Return a timestamp to be used for --since/--until comparisons for this
+  * commit, based on the revision options.
+  */
+ static timestamp_t comparison_date(const struct rev_info *revs,
+                                  struct commit *commit)
+ {
+       return revs->reflog_info ?
+               get_reflog_timestamp(revs->reflog_info) :
+               commit->date;
+ }
  enum commit_action get_commit_action(struct rev_info *revs, struct commit *commit)
  {
        if (commit->object.flags & SHOWN)
                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 &&
+           comparison_date(revs, commit) > 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) ||
@@@ -3107,17 -3122,19 +3120,19 @@@ static void track_linear(struct rev_inf
  
  static struct commit *get_revision_1(struct rev_info *revs)
  {
-       if (!revs->commits)
-               return NULL;
+       while (1) {
+               struct commit *commit;
  
-       do {
-               struct commit *commit = pop_commit(&revs->commits);
+               if (revs->reflog_info)
+                       commit = next_reflog_entry(revs->reflog_info);
+               else
+                       commit = pop_commit(&revs->commits);
  
-               if (revs->reflog_info) {
-                       save_parents(revs, commit);
-                       fake_reflog_parent(revs->reflog_info, commit);
+               if (!commit)
+                       return NULL;
+               if (revs->reflog_info)
                        commit->object.flags &= ~(ADDED | SEEN | SHOWN);
-               }
  
                /*
                 * If we haven't done the list limiting, we need to look at
                 */
                if (!revs->limited) {
                        if (revs->max_age != -1 &&
-                           (commit->date < revs->max_age))
+                           comparison_date(revs, commit) < revs->max_age)
                                continue;
-                       if (add_parents_to_list(revs, commit, &revs->commits, NULL) < 0) {
+                       if (revs->reflog_info)
+                               try_to_simplify_commit(revs, commit);
+                       else if (add_parents_to_list(revs, commit, &revs->commits, NULL) < 0) {
                                if (!revs->ignore_missing_links)
                                        die("Failed to traverse parents of commit %s",
                                                oid_to_hex(&commit->object.oid));
                                track_linear(revs, commit);
                        return commit;
                }
-       } while (revs->commits);
-       return NULL;
+       }
  }
  
  /*