On Thu, Jun 22, 2017 at 05:52:35PM -0400, Jeff King wrote:

> Which really makes me feel like this patch is going in the right
> direction, as it makes all of this behave conceptually like:
> 
>   while read old new etc ...
>   do
>     git show $new
>   done <.git/logs/$ref
> 
> which is simple to explain and is what I'd expect (and is certainly the
> direction we went with the "diff uses real parents" commit).
> 
> We just need to hit the simplify_commit() code path here.

So here's a patch on top of what I posted before that pushes the reflog
check into the loop (it just decides whether to pull from the reflogs or
from the commit queue at the top of the loop).

I was surprised to find, though, that simplify_commit() does not
actually do the pathspec limiting! It's done by
try_to_simplify_commit(), which is part of add_parents_to_list(). I
hacked around it in the later part of the loop by calling
try_to_simplify myself and checking the TREESAME flag. But I have a
feeling I'm missing something about how the traversal is supposed to
work.

This does behave sensibly with "--no-merges" and "--merges", as well as
pathspec limiting.

diff --git a/revision.c b/revision.c
index 675247cd9..203468ddf 100644
--- a/revision.c
+++ b/revision.c
@@ -3112,19 +3112,19 @@ static void track_linear(struct rev_info *revs, struct 
commit *commit)
 
 static struct commit *get_revision_1(struct rev_info *revs)
 {
-       if (revs->reflog_info) {
-               struct commit *commit = next_reflog_entry(revs->reflog_info);
-               if (commit) {
-                       commit->object.flags &= ~(ADDED | SEEN | SHOWN);
-                       return commit;
-               }
-       }
+       while (1) {
+               struct commit *commit;
 
-       if (!revs->commits)
-               return NULL;
+               if (revs->reflog_info)
+                       commit = next_reflog_entry(revs->reflog_info);
+               else
+                       commit = pop_commit(&revs->commits);
 
-       do {
-               struct commit *commit = pop_commit(&revs->commits);
+               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
@@ -3135,7 +3135,8 @@ static struct commit *get_revision_1(struct rev_info 
*revs)
                        if (revs->max_age != -1 &&
                            (commit->date < revs->max_age))
                                continue;
-                       if (add_parents_to_list(revs, commit, &revs->commits, 
NULL) < 0) {
+                       if (!revs->reflog_info &&
+                           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));
@@ -3151,10 +3152,14 @@ static struct commit *get_revision_1(struct rev_info 
*revs)
                default:
                        if (revs->track_linear)
                                track_linear(revs, commit);
+                       if (revs->reflog_info) {
+                               try_to_simplify_commit(revs, commit);
+                               if (commit->object.flags & TREESAME)
+                                       continue;
+                       }
                        return commit;
                }
-       } while (revs->commits);
-       return NULL;
+       }
 }
 
 /*

Reply via email to