git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/6] t1414: document some reflog-walk oddities

From
Jeff King <peff@peff.net>
Date
Jul 7, 2017, 06:22 UTC
Message-ID
<20170707062211.a7336zi7rn7pal3z@sigill.intra.peff.net>
In-Reply-To
<xmqq37a8eqhe.fsf@gitster.mtv.corp.google.com>
On Thu, Jul 06, 2017 at 11:00:13PM -0700, Junio C Hamano wrote:
Show 28 quoted lines
> Jeff King <peff@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)
Previous: Junio C HamanoNext: Ramsay Jones
Message 32 of 69 in “Truncating HEAD reflog on branch move”
  1. brian m. carlsonJun 21, 2017
  2. Junio C HamanoJun 21, 2017
  3. Kyle MeyerJun 21, 2017
  4. Kyle MeyerJun 21, 2017
  5. Jeff KingJun 22, 2017
  6. Junio C HamanoJun 22, 2017
  7. Jeff KingJun 22, 2017
  8. Junio C HamanoJun 22, 2017
  9. Jeff KingJun 22, 2017
  10. Junio C HamanoJun 22, 2017
  11. Jeff KingJun 22, 2017
  12. Jeff KingJun 22, 2017
  13. Jeff KingJun 23, 2017
  14. brian m. carlsonJul 4, 2017
  15. Jeff KingJul 4, 2017
  16. brian m. carlsonJul 4, 2017
  17. Jeff KingJul 4, 2017
  18. Junio C HamanoJul 5, 2017
  19. 0/6 fixing reflog-walk odditiesJeff King, Jul 5, 2017
  20. 1/6 reflog-walk: skip over double-null oid due to HEAD renameJeff King, Jul 5, 2017
  21. Junio C HamanoJul 5, 2017
  22. Jeff KingJul 5, 2017
  23. 2/6 t1414: document some reflog-walk odditiesJeff King, Jul 5, 2017
  24. Junio C HamanoJul 5, 2017
  25. Jeff KingJul 5, 2017
  26. Junio C HamanoJul 5, 2017
  27. Jeff KingJul 6, 2017
  28. Jeff KingJul 6, 2017
  29. Junio C HamanoJul 6, 2017
  30. Jeff KingJul 7, 2017
  31. Junio C HamanoJul 7, 2017
  32. Jeff KingJul 7, 2017
  33. Ramsay JonesJul 5, 2017
  34. Jeff KingJul 5, 2017
  35. 3/6 log: do not free parents when walking reflogJeff King, Jul 5, 2017
  36. 4/6 get_revision_1(): replace do-while with an early returnJeff King, Jul 5, 2017
  37. 5/6 rev-list: check reflog_info before showing usageJeff King, Jul 5, 2017
  38. Junio C HamanoJul 5, 2017
  39. Jeff KingJul 5, 2017
  40. 6/6 reflog-walk: stop using fake parentsJeff King, Jul 5, 2017
  41. Eric WongJul 7, 2017
  42. Jeff KingJul 7, 2017
  43. Jeff KingJul 7, 2017
  44. Eric WongJul 10, 2017
  45. Jeff KingJul 10, 2017
  46. Junio C HamanoJul 10, 2017
  47. 0/4 reflog-walk fixes for maintJeff King, Jul 7, 2017
  48. 1/4 reflog-walk: skip over double-null oid due to HEAD renameJeff King, Jul 7, 2017
  49. 2/4 reflog-walk: duplicate strings in complete_reflogs listJeff King, Jul 7, 2017
  50. 3/4 reflog-walk: don't free reflogs added to cacheJeff King, Jul 7, 2017
  51. 4/4 reflog-walk: include all fields when freeing complete_reflogsJeff King, Jul 7, 2017
  52. 0/7 fixing reflog-walk odditiesJeff King, Jul 7, 2017
  53. 1/7 t1414: document some reflog-walk odditiesJeff King, Jul 7, 2017
  54. Jeff KingJul 7, 2017
  55. Kyle MeyerJul 7, 2017
  56. 2/7 revision: disallow reflog walking with revs->limitedJeff King, Jul 7, 2017
  57. 4/7 get_revision_1(): replace do-while with an early returnJeff King, Jul 7, 2017
  58. 5/7 rev-list: check reflog_info before showing usageJeff King, Jul 7, 2017
  59. 3/7 log: do not free parents when walking reflogJeff King, Jul 7, 2017
  60. Junio C HamanoJul 7, 2017
  61. Jeff KingJul 9, 2017
  62. Junio C HamanoJul 9, 2017
  63. Jeff KingJul 10, 2017
  64. 6/7 reflog-walk: stop using fake parentsJeff King, Jul 7, 2017
  65. Jeff KingJul 7, 2017
  66. Kyle MeyerJul 7, 2017
  67. Jeff KingJul 9, 2017
  68. 7/7 reflog-walk: apply --since/--until to reflog datesJeff King, Jul 7, 2017
  69. Junio C HamanoJul 7, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.