Skip to content

Commit f3c520e

Browse files
gitstergitster
committed
Merge branch 'sg/name-rev-wo-recursion'
Redo "git name-rev" to avoid recursive calls. * sg/name-rev-wo-recursion: name-rev: cleanup name_ref() name-rev: eliminate recursion in name_rev() name-rev: use 'name->tip_name' instead of 'tip_name' name-rev: drop name_rev()'s 'generation' and 'distance' parameters name-rev: restructure creating/updating 'struct rev_name' instances name-rev: restructure parsing commits and applying date cutoff name-rev: pull out deref handling from the recursion name-rev: extract creating/updating a 'struct name_rev' into a helper t6120: add a test to cover inner conditions in 'git name-rev's name_rev() name-rev: use sizeof(*ptr) instead of sizeof(type) in allocation name-rev: avoid unnecessary cast in name_ref() name-rev: use strbuf_strip_suffix() in get_rev_name() t6120-describe: modernize the 'check_describe' helper t6120-describe: correct test repo history graph in comment
2 parents 6514ad4 + 2866fd2 commit f3c520e

File tree

2 files changed

+153
-66
lines changed

2 files changed

+153
-66
lines changed

‎builtin/name-rev.c

Lines changed: 97 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "tag.h"
77
#include "refs.h"
88
#include "parse-options.h"
9+
#include "prio-queue.h"
910
#include "sha1-lookup.h"
1011
#include "commit-slab.h"
1112

@@ -79,30 +80,16 @@ static int is_better_name(struct rev_name *name,
7980
return 0;
8081
}
8182

82-
static void name_rev(struct commit *commit,
83-
const char *tip_name, timestamp_t taggerdate,
84-
int generation, int distance, int from_tag,
85-
int deref)
83+
static struct rev_name *create_or_update_name(struct commit *commit,
84+
const char *tip_name,
85+
timestamp_t taggerdate,
86+
int generation, int distance,
87+
int from_tag)
8688
{
8789
struct rev_name *name = get_commit_rev_name(commit);
88-
struct commit_list *parents;
89-
int parent_number = 1;
90-
char *to_free = NULL;
91-
92-
parse_commit(commit);
93-
94-
if (commit->date < cutoff)
95-
return;
96-
97-
if (deref) {
98-
tip_name = to_free = xstrfmt("%s^0", tip_name);
99-
100-
if (generation)
101-
die("generation: %d, but deref?", generation);
102-
}
10390

10491
if (name == NULL) {
105-
name = xmalloc(sizeof(rev_name));
92+
name = xmalloc(sizeof(*name));
10693
set_commit_rev_name(commit, name);
10794
goto copy_data;
10895
} else if (is_better_name(name, taggerdate, distance, from_tag)) {
@@ -112,35 +99,97 @@ static void name_rev(struct commit *commit,
11299
name->generation = generation;
113100
name->distance = distance;
114101
name->from_tag = from_tag;
115-
} else {
102+
103+
return name;
104+
} else
105+
return NULL;
106+
}
107+
108+
static void name_rev(struct commit *start_commit,
109+
const char *tip_name, timestamp_t taggerdate,
110+
int from_tag, int deref)
111+
{
112+
struct prio_queue queue;
113+
struct commit *commit;
114+
struct commit **parents_to_queue = NULL;
115+
size_t parents_to_queue_nr, parents_to_queue_alloc = 0;
116+
char *to_free = NULL;
117+
118+
parse_commit(start_commit);
119+
if (start_commit->date < cutoff)
120+
return;
121+
122+
if (deref)
123+
tip_name = to_free = xstrfmt("%s^0", tip_name);
124+
125+
if (!create_or_update_name(start_commit, tip_name, taggerdate, 0, 0,
126+
from_tag)) {
116127
free(to_free);
117128
return;
118129
}
119130

120-
for (parents = commit->parents;
121-
parents;
122-
parents = parents->next, parent_number++) {
123-
if (parent_number > 1) {
124-
size_t len;
125-
char *new_name;
126-
127-
strip_suffix(tip_name, "^0", &len);
128-
if (generation > 0)
129-
new_name = xstrfmt("%.*s~%d^%d", (int)len, tip_name,
130-
generation, parent_number);
131-
else
132-
new_name = xstrfmt("%.*s^%d", (int)len, tip_name,
133-
parent_number);
134-
135-
name_rev(parents->item, new_name, taggerdate, 0,
136-
distance + MERGE_TRAVERSAL_WEIGHT,
137-
from_tag, 0);
138-
} else {
139-
name_rev(parents->item, tip_name, taggerdate,
140-
generation + 1, distance + 1,
141-
from_tag, 0);
131+
memset(&queue, 0, sizeof(queue)); /* Use the prio_queue as LIFO */
132+
prio_queue_put(&queue, start_commit);
133+
134+
while ((commit = prio_queue_get(&queue))) {
135+
struct rev_name *name = get_commit_rev_name(commit);
136+
struct commit_list *parents;
137+
int parent_number = 1;
138+
139+
parents_to_queue_nr = 0;
140+
141+
for (parents = commit->parents;
142+
parents;
143+
parents = parents->next, parent_number++) {
144+
struct commit *parent = parents->item;
145+
const char *new_name;
146+
int generation, distance;
147+
148+
parse_commit(parent);
149+
if (parent->date < cutoff)
150+
continue;
151+
152+
if (parent_number > 1) {
153+
size_t len;
154+
155+
strip_suffix(name->tip_name, "^0", &len);
156+
if (name->generation > 0)
157+
new_name = xstrfmt("%.*s~%d^%d",
158+
(int)len,
159+
name->tip_name,
160+
name->generation,
161+
parent_number);
162+
else
163+
new_name = xstrfmt("%.*s^%d", (int)len,
164+
name->tip_name,
165+
parent_number);
166+
generation = 0;
167+
distance = name->distance + MERGE_TRAVERSAL_WEIGHT;
168+
} else {
169+
new_name = name->tip_name;
170+
generation = name->generation + 1;
171+
distance = name->distance + 1;
172+
}
173+
174+
if (create_or_update_name(parent, new_name, taggerdate,
175+
generation, distance,
176+
from_tag)) {
177+
ALLOC_GROW(parents_to_queue,
178+
parents_to_queue_nr + 1,
179+
parents_to_queue_alloc);
180+
parents_to_queue[parents_to_queue_nr] = parent;
181+
parents_to_queue_nr++;
182+
}
142183
}
184+
185+
/* The first parent must come out first from the prio_queue */
186+
while (parents_to_queue_nr)
187+
prio_queue_put(&queue,
188+
parents_to_queue[--parents_to_queue_nr]);
143189
}
190+
191+
clear_prio_queue(&queue);
192+
free(parents_to_queue);
144193
}
145194

146195
static int subpath_matches(const char *path, const char *filter)
@@ -272,10 +321,9 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo
272321
int from_tag = starts_with(path, "refs/tags/");
273322

274323
if (taggerdate == TIME_MAX)
275-
taggerdate = ((struct commit *)o)->date;
324+
taggerdate = commit->date;
276325
path = name_ref_abbrev(path, can_abbreviate_output);
277-
name_rev(commit, xstrdup(path), taggerdate, 0, 0,
278-
from_tag, deref);
326+
name_rev(commit, xstrdup(path), taggerdate, from_tag, deref);
279327
}
280328
return 0;
281329
}
@@ -321,11 +369,10 @@ static const char *get_rev_name(const struct object *o, struct strbuf *buf)
321369
if (!n->generation)
322370
return n->tip_name;
323371
else {
324-
int len = strlen(n->tip_name);
325-
if (len > 2 && !strcmp(n->tip_name + len - 2, "^0"))
326-
len -= 2;
327372
strbuf_reset(buf);
328-
strbuf_addf(buf, "%.*s~%d", len, n->tip_name, n->generation);
373+
strbuf_addstr(buf, n->tip_name);
374+
strbuf_strip_suffix(buf, "^0");
375+
strbuf_addf(buf, "~%d", n->generation);
329376
return buf->buf;
330377
}
331378
}

‎t/t6120-describe.sh

Lines changed: 56 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,27 @@
11
#!/bin/sh
22

3-
test_description='test describe
3+
test_description='test describe'
4+
5+
# o---o-----o----o----o-------o----x
6+
# \ D,R e /
7+
# \---o-------------o-'
8+
# \ B /
9+
# `-o----o----o-'
10+
# A c
11+
#
12+
# First parent of a merge commit is on the same line, second parent below.
413

5-
B
6-
.--------------o----o----o----x
7-
/ / /
8-
o----o----o----o----o----. /
9-
\ A c /
10-
.------------o---o---o
11-
D,R e
12-
'
1314
. ./test-lib.sh
1415

1516
check_describe () {
1617
expect="$1"
1718
shift
18-
R=$(git describe "$@" 2>err.actual)
19-
S=$?
20-
cat err.actual >&3
21-
test_expect_success "describe $*" '
22-
test $S = 0 &&
19+
describe_opts="$@"
20+
test_expect_success "describe $describe_opts" '
21+
R=$(git describe $describe_opts 2>err.actual) &&
2322
case "$R" in
2423
$expect) echo happy ;;
25-
*) echo "Oops - $R is not $expect";
24+
*) echo "Oops - $R is not $expect" &&
2625
false ;;
2726
esac
2827
'
@@ -382,7 +381,7 @@ test_expect_success 'describe tag object' '
382381
test_i18ngrep "fatal: test-blob-1 is neither a commit nor blob" actual
383382
'
384383

385-
test_expect_failure ULIMIT_STACK_SIZE 'name-rev works in a deep repo' '
384+
test_expect_success ULIMIT_STACK_SIZE 'name-rev works in a deep repo' '
386385
i=1 &&
387386
while test $i -lt 8000
388387
do
@@ -439,4 +438,45 @@ test_expect_success 'name-rev a rev shortly after epoch' '
439438
test_cmp expect actual
440439
'
441440

441+
# A--------------master
442+
# \ /
443+
# \----------M2
444+
# \ /
445+
# \---M1-C
446+
# \ /
447+
# B
448+
test_expect_success 'name-rev covers all conditions while looking at parents' '
449+
git init repo &&
450+
(
451+
cd repo &&
452+
453+
echo A >file &&
454+
git add file &&
455+
git commit -m A &&
456+
A=$(git rev-parse HEAD) &&
457+
458+
git checkout --detach &&
459+
echo B >file &&
460+
git commit -m B file &&
461+
B=$(git rev-parse HEAD) &&
462+
463+
git checkout $A &&
464+
git merge --no-ff $B && # M1
465+
466+
echo C >file &&
467+
git commit -m C file &&
468+
469+
git checkout $A &&
470+
git merge --no-ff HEAD@{1} && # M2
471+
472+
git checkout master &&
473+
git merge --no-ff HEAD@{1} &&
474+
475+
echo "$B master^2^2~1^2" >expect &&
476+
git name-rev $B >actual &&
477+
478+
test_cmp expect actual
479+
)
480+
'
481+
442482
test_done

0 commit comments

Comments
 (0)