Skip to content

Commit 0cb526e

Browse files
gitstergitster
committed
Merge branch 'jk/reflog-walk' into maint
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
2 parents 72140a7 + de23944 commit 0cb526e

File tree

8 files changed

+253
-123
lines changed

8 files changed

+253
-123
lines changed

‎builtin/log.c‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -372,11 +372,14 @@ static int cmd_log_walk(struct rev_info *rev)
372372
*/
373373
rev->max_count++;
374374
if (!rev->reflog_info) {
375-
/* we allow cycles in reflog ancestry */
375+
/*
376+
* We may show a given commit multiple times when
377+
* walking the reflogs.
378+
*/
376379
free_commit_buffer(commit);
380+
free_commit_list(commit->parents);
381+
commit->parents = NULL;
377382
}
378-
free_commit_list(commit->parents);
379-
commit->parents = NULL;
380383
if (saved_nrl < rev->diffopt.needed_rename_limit)
381384
saved_nrl = rev->diffopt.needed_rename_limit;
382385
if (rev->diffopt.degraded_cc_to_c)

‎builtin/rev-list.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "graph.h"
1212
#include "bisect.h"
1313
#include "progress.h"
14+
#include "reflog-walk.h"
1415

1516
static const char rev_list_usage[] =
1617
"git rev-list [OPTION] <commit-id>... [ -- paths... ]\n"
@@ -349,7 +350,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
349350
/* Only --header was specified */
350351
revs.commit_format = CMIT_FMT_RAW;
351352

352-
if ((!revs.commits &&
353+
if ((!revs.commits && reflog_walk_empty(revs.reflog_info) &&
353354
(!(revs.tag_objects || revs.tree_objects || revs.blob_objects) &&
354355
!revs.pending.nr)) ||
355356
revs.diff)

‎reflog-walk.c‎

Lines changed: 67 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -94,45 +94,6 @@ static int get_reflog_recno_by_time(struct complete_reflogs *array,
9494
return -1;
9595
}
9696

97-
struct commit_info_lifo {
98-
struct commit_info {
99-
struct commit *commit;
100-
void *util;
101-
} *items;
102-
int nr, alloc;
103-
};
104-
105-
static struct commit_info *get_commit_info(struct commit *commit,
106-
struct commit_info_lifo *lifo, int pop)
107-
{
108-
int i;
109-
for (i = 0; i < lifo->nr; i++)
110-
if (lifo->items[i].commit == commit) {
111-
struct commit_info *result = &lifo->items[i];
112-
if (pop) {
113-
if (i + 1 < lifo->nr)
114-
memmove(lifo->items + i,
115-
lifo->items + i + 1,
116-
(lifo->nr - i) *
117-
sizeof(struct commit_info));
118-
lifo->nr--;
119-
}
120-
return result;
121-
}
122-
return NULL;
123-
}
124-
125-
static void add_commit_info(struct commit *commit, void *util,
126-
struct commit_info_lifo *lifo)
127-
{
128-
struct commit_info *info;
129-
ALLOC_GROW(lifo->items, lifo->nr + 1, lifo->alloc);
130-
info = lifo->items + lifo->nr;
131-
info->commit = commit;
132-
info->util = util;
133-
lifo->nr++;
134-
}
135-
13697
struct commit_reflog {
13798
int recno;
13899
enum selector_type {
@@ -144,7 +105,8 @@ struct commit_reflog {
144105
};
145106

146107
struct reflog_walk_info {
147-
struct commit_info_lifo reflogs;
108+
struct commit_reflog **logs;
109+
size_t nr, alloc;
148110
struct string_list complete_reflogs;
149111
struct commit_reflog *last_commit_reflog;
150112
};
@@ -233,52 +195,10 @@ int add_reflog_for_walk(struct reflog_walk_info *info,
233195
commit_reflog->selector = selector;
234196
commit_reflog->reflogs = reflogs;
235197

236-
add_commit_info(commit, commit_reflog, &info->reflogs);
237-
return 0;
238-
}
198+
ALLOC_GROW(info->logs, info->nr + 1, info->alloc);
199+
info->logs[info->nr++] = commit_reflog;
239200

240-
void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)
241-
{
242-
struct commit_info *commit_info =
243-
get_commit_info(commit, &info->reflogs, 0);
244-
struct commit_reflog *commit_reflog;
245-
struct object *logobj;
246-
struct reflog_info *reflog;
247-
248-
info->last_commit_reflog = NULL;
249-
if (!commit_info)
250-
return;
251-
252-
commit_reflog = commit_info->util;
253-
if (commit_reflog->recno < 0) {
254-
commit->parents = NULL;
255-
return;
256-
}
257-
info->last_commit_reflog = commit_reflog;
258-
259-
do {
260-
reflog = &commit_reflog->reflogs->items[commit_reflog->recno];
261-
commit_reflog->recno--;
262-
logobj = parse_object(&reflog->ooid);
263-
} while (commit_reflog->recno && (logobj && logobj->type != OBJ_COMMIT));
264-
265-
if (!logobj && commit_reflog->recno >= 0 && is_null_oid(&reflog->ooid)) {
266-
/* a root commit, but there are still more entries to show */
267-
reflog = &commit_reflog->reflogs->items[commit_reflog->recno];
268-
logobj = parse_object(&reflog->noid);
269-
if (!logobj)
270-
logobj = parse_object(&reflog->ooid);
271-
}
272-
273-
if (!logobj || logobj->type != OBJ_COMMIT) {
274-
commit_info->commit = NULL;
275-
commit->parents = NULL;
276-
return;
277-
}
278-
commit_info->commit = (struct commit *)logobj;
279-
280-
commit->parents = xcalloc(1, sizeof(struct commit_list));
281-
commit->parents->item = commit_info->commit;
201+
return 0;
282202
}
283203

284204
void get_reflog_selector(struct strbuf *sb,
@@ -344,6 +264,18 @@ const char *get_reflog_ident(struct reflog_walk_info *reflog_info)
344264
return info->email;
345265
}
346266

267+
timestamp_t get_reflog_timestamp(struct reflog_walk_info *reflog_info)
268+
{
269+
struct commit_reflog *commit_reflog = reflog_info->last_commit_reflog;
270+
struct reflog_info *info;
271+
272+
if (!commit_reflog)
273+
return 0;
274+
275+
info = &commit_reflog->reflogs->items[commit_reflog->recno+1];
276+
return info->timestamp;
277+
}
278+
347279
void show_reflog_message(struct reflog_walk_info *reflog_info, int oneline,
348280
const struct date_mode *dmode, int force_date)
349281
{
@@ -365,3 +297,53 @@ void show_reflog_message(struct reflog_walk_info *reflog_info, int oneline,
365297
strbuf_release(&selector);
366298
}
367299
}
300+
301+
int reflog_walk_empty(struct reflog_walk_info *info)
302+
{
303+
return !info || !info->nr;
304+
}
305+
306+
static struct commit *next_reflog_commit(struct commit_reflog *log)
307+
{
308+
for (; log->recno >= 0; log->recno--) {
309+
struct reflog_info *entry = &log->reflogs->items[log->recno];
310+
struct object *obj = parse_object(&entry->noid);
311+
312+
if (obj && obj->type == OBJ_COMMIT)
313+
return (struct commit *)obj;
314+
}
315+
return NULL;
316+
}
317+
318+
static timestamp_t log_timestamp(struct commit_reflog *log)
319+
{
320+
return log->reflogs->items[log->recno].timestamp;
321+
}
322+
323+
struct commit *next_reflog_entry(struct reflog_walk_info *walk)
324+
{
325+
struct commit_reflog *best = NULL;
326+
struct commit *best_commit = NULL;
327+
size_t i;
328+
329+
for (i = 0; i < walk->nr; i++) {
330+
struct commit_reflog *log = walk->logs[i];
331+
struct commit *commit = next_reflog_commit(log);
332+
333+
if (!commit)
334+
continue;
335+
336+
if (!best || log_timestamp(log) > log_timestamp(best)) {
337+
best = log;
338+
best_commit = commit;
339+
}
340+
}
341+
342+
if (best) {
343+
best->recno--;
344+
walk->last_commit_reflog = best;
345+
return best_commit;
346+
}
347+
348+
return NULL;
349+
}

‎reflog-walk.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,19 @@ struct reflog_walk_info;
88
extern void init_reflog_walk(struct reflog_walk_info **info);
99
extern int add_reflog_for_walk(struct reflog_walk_info *info,
1010
struct commit *commit, const char *name);
11-
extern void fake_reflog_parent(struct reflog_walk_info *info,
12-
struct commit *commit);
1311
extern void show_reflog_message(struct reflog_walk_info *info, int,
1412
const struct date_mode *, int force_date);
1513
extern void get_reflog_message(struct strbuf *sb,
1614
struct reflog_walk_info *reflog_info);
1715
extern const char *get_reflog_ident(struct reflog_walk_info *reflog_info);
16+
extern timestamp_t get_reflog_timestamp(struct reflog_walk_info *reflog_info);
1817
extern void get_reflog_selector(struct strbuf *sb,
1918
struct reflog_walk_info *reflog_info,
2019
const struct date_mode *dmode, int force_date,
2120
int shorten);
2221

22+
extern int reflog_walk_empty(struct reflog_walk_info *walk);
23+
24+
struct commit *next_reflog_entry(struct reflog_walk_info *reflog_info);
25+
2326
#endif

‎revision.c‎

Lines changed: 37 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -148,16 +148,14 @@ static void add_pending_object_with_path(struct rev_info *revs,
148148
if (revs->reflog_info && obj->type == OBJ_COMMIT) {
149149
struct strbuf buf = STRBUF_INIT;
150150
int len = interpret_branch_name(name, 0, &buf, 0);
151-
int st;
152151

153152
if (0 < len && name[len] && buf.len)
154153
strbuf_addstr(&buf, name + len);
155-
st = add_reflog_for_walk(revs->reflog_info,
156-
(struct commit *)obj,
157-
buf.buf[0] ? buf.buf: name);
154+
add_reflog_for_walk(revs->reflog_info,
155+
(struct commit *)obj,
156+
buf.buf[0] ? buf.buf: name);
158157
strbuf_release(&buf);
159-
if (st)
160-
return;
158+
return; /* do not add the commit itself */
161159
}
162160
add_object_array_with_path(obj, name, &revs->pending, mode, path);
163161
}
@@ -2364,6 +2362,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
23642362

23652363
if (revs->reverse && revs->reflog_info)
23662364
die("cannot combine --reverse with --walk-reflogs");
2365+
if (revs->reflog_info && revs->limited)
2366+
die("cannot combine --walk-reflogs with history-limiting options");
23672367
if (revs->rewrite_parents && revs->children.name)
23682368
die("cannot combine --parents and --children");
23692369

@@ -2963,6 +2963,18 @@ static inline int want_ancestry(const struct rev_info *revs)
29632963
return (revs->rewrite_parents || revs->children.name);
29642964
}
29652965

2966+
/*
2967+
* Return a timestamp to be used for --since/--until comparisons for this
2968+
* commit, based on the revision options.
2969+
*/
2970+
static timestamp_t comparison_date(const struct rev_info *revs,
2971+
struct commit *commit)
2972+
{
2973+
return revs->reflog_info ?
2974+
get_reflog_timestamp(revs->reflog_info) :
2975+
commit->date;
2976+
}
2977+
29662978
enum commit_action get_commit_action(struct rev_info *revs, struct commit *commit)
29672979
{
29682980
if (commit->object.flags & SHOWN)
@@ -2973,8 +2985,9 @@ enum commit_action get_commit_action(struct rev_info *revs, struct commit *commi
29732985
return commit_show;
29742986
if (commit->object.flags & UNINTERESTING)
29752987
return commit_ignore;
2976-
if (revs->min_age != -1 && (commit->date > revs->min_age))
2977-
return commit_ignore;
2988+
if (revs->min_age != -1 &&
2989+
comparison_date(revs, commit) > revs->min_age)
2990+
return commit_ignore;
29782991
if (revs->min_parents || (revs->max_parents >= 0)) {
29792992
int n = commit_list_count(commit->parents);
29802993
if ((n < revs->min_parents) ||
@@ -3107,17 +3120,19 @@ static void track_linear(struct rev_info *revs, struct commit *commit)
31073120

31083121
static struct commit *get_revision_1(struct rev_info *revs)
31093122
{
3110-
if (!revs->commits)
3111-
return NULL;
3123+
while (1) {
3124+
struct commit *commit;
31123125

3113-
do {
3114-
struct commit *commit = pop_commit(&revs->commits);
3126+
if (revs->reflog_info)
3127+
commit = next_reflog_entry(revs->reflog_info);
3128+
else
3129+
commit = pop_commit(&revs->commits);
31153130

3116-
if (revs->reflog_info) {
3117-
save_parents(revs, commit);
3118-
fake_reflog_parent(revs->reflog_info, commit);
3131+
if (!commit)
3132+
return NULL;
3133+
3134+
if (revs->reflog_info)
31193135
commit->object.flags &= ~(ADDED | SEEN | SHOWN);
3120-
}
31213136

31223137
/*
31233138
* If we haven't done the list limiting, we need to look at
@@ -3126,9 +3141,12 @@ static struct commit *get_revision_1(struct rev_info *revs)
31263141
*/
31273142
if (!revs->limited) {
31283143
if (revs->max_age != -1 &&
3129-
(commit->date < revs->max_age))
3144+
comparison_date(revs, commit) < revs->max_age)
31303145
continue;
3131-
if (add_parents_to_list(revs, commit, &revs->commits, NULL) < 0) {
3146+
3147+
if (revs->reflog_info)
3148+
try_to_simplify_commit(revs, commit);
3149+
else if (add_parents_to_list(revs, commit, &revs->commits, NULL) < 0) {
31323150
if (!revs->ignore_missing_links)
31333151
die("Failed to traverse parents of commit %s",
31343152
oid_to_hex(&commit->object.oid));
@@ -3146,8 +3164,7 @@ static struct commit *get_revision_1(struct rev_info *revs)
31463164
track_linear(revs, commit);
31473165
return commit;
31483166
}
3149-
} while (revs->commits);
3150-
return NULL;
3167+
}
31513168
}
31523169

31533170
/*

‎t/t1411-reflog-show.sh‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -171,14 +171,4 @@ test_expect_success 'reflog exists works' '
171171
! git reflog exists refs/heads/nonexistent
172172
'
173173

174-
# The behavior with two reflogs is buggy and the output is in flux; for now
175-
# we're just checking that the program works at all without segfaulting.
176-
test_expect_success 'showing multiple reflogs works' '
177-
git log -g HEAD HEAD >actual
178-
'
179-
180-
test_expect_success 'showing multiple reflogs with an old date' '
181-
git log -g HEAD@{1979-01-01} HEAD >actual
182-
'
183-
184174
test_done

0 commit comments

Comments
 (0)