Skip to content

Commit dc57a9b

Browse files
gitstergitster
committed
Merge branch 'tb/commit-graph-no-check-oids'
Clean-up the commit-graph codepath. * tb/commit-graph-no-check-oids: commit-graph: drop COMMIT_GRAPH_WRITE_CHECK_OIDS flag t5318: reorder test below 'graph_read_expect' commit-graph.c: simplify 'fill_oids_from_commits' builtin/commit-graph.c: dereference tags in builtin builtin/commit-graph.c: extract 'read_one_commit()' commit-graph.c: peel refs in 'add_ref_to_set' commit-graph.c: show progress of finding reachable commits commit-graph.c: extract 'refs_cb_data'
2 parents f4cec40 + 2f00c35 commit dc57a9b

File tree

5 files changed

+95
-77
lines changed

5 files changed

+95
-77
lines changed

‎Documentation/git-commit-graph.txt‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,10 @@ with `--stdin-commits` or `--reachable`.)
4747
+
4848
With the `--stdin-commits` option, generate the new commit graph by
4949
walking commits starting at the commits specified in stdin as a list
50-
of OIDs in hex, one OID per line. (Cannot be combined with
51-
`--stdin-packs` or `--reachable`.)
50+
of OIDs in hex, one OID per line. OIDs that resolve to non-commits
51+
(either directly, or by peeling tags) are silently ignored. OIDs that
52+
are malformed, or do not exist generate an error. (Cannot be combined
53+
with `--stdin-packs` or `--reachable`.)
5254
+
5355
With the `--reachable` option, generate the new commit graph by walking
5456
commits starting at all refs. (Cannot be combined with `--stdin-commits`

‎builtin/commit-graph.c‎

Lines changed: 47 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
#include "repository.h"
77
#include "commit-graph.h"
88
#include "object-store.h"
9+
#include "progress.h"
10+
#include "tag.h"
911

1012
static char const * const builtin_commit_graph_usage[] = {
1113
N_("git commit-graph verify [--object-dir <objdir>] [--shallow] [--[no-]progress]"),
@@ -138,14 +140,37 @@ static int write_option_parse_split(const struct option *opt, const char *arg,
138140
return 0;
139141
}
140142

143+
static int read_one_commit(struct oidset *commits, struct progress *progress,
144+
const char *hash)
145+
{
146+
struct object *result;
147+
struct object_id oid;
148+
const char *end;
149+
150+
if (parse_oid_hex(hash, &oid, &end))
151+
return error(_("unexpected non-hex object ID: %s"), hash);
152+
153+
result = deref_tag(the_repository, parse_object(the_repository, &oid),
154+
NULL, 0);
155+
if (!result)
156+
return error(_("invalid object: %s"), hash);
157+
else if (object_as_type(the_repository, result, OBJ_COMMIT, 1))
158+
oidset_insert(commits, &result->oid);
159+
160+
display_progress(progress, oidset_size(commits));
161+
162+
return 0;
163+
}
164+
141165
static int graph_write(int argc, const char **argv)
142166
{
143-
struct string_list *pack_indexes = NULL;
167+
struct string_list pack_indexes = STRING_LIST_INIT_NODUP;
168+
struct strbuf buf = STRBUF_INIT;
144169
struct oidset commits = OIDSET_INIT;
145170
struct object_directory *odb = NULL;
146-
struct string_list lines;
147171
int result = 0;
148172
enum commit_graph_write_flags flags = 0;
173+
struct progress *progress = NULL;
149174

150175
static struct option builtin_commit_graph_write_options[] = {
151176
OPT_STRING(0, "object-dir", &opts.obj_dir,
@@ -209,44 +234,38 @@ static int graph_write(int argc, const char **argv)
209234
return 0;
210235
}
211236

212-
string_list_init(&lines, 0);
213-
if (opts.stdin_packs || opts.stdin_commits) {
214-
struct strbuf buf = STRBUF_INIT;
215-
237+
if (opts.stdin_packs) {
216238
while (strbuf_getline(&buf, stdin) != EOF)
217-
string_list_append(&lines, strbuf_detach(&buf, NULL));
218-
219-
if (opts.stdin_packs)
220-
pack_indexes = &lines;
221-
if (opts.stdin_commits) {
222-
struct string_list_item *item;
223-
oidset_init(&commits, lines.nr);
224-
for_each_string_list_item(item, &lines) {
225-
struct object_id oid;
226-
const char *end;
227-
228-
if (parse_oid_hex(item->string, &oid, &end)) {
229-
error(_("unexpected non-hex object ID: "
230-
"%s"), item->string);
231-
return 1;
232-
}
233-
234-
oidset_insert(&commits, &oid);
239+
string_list_append(&pack_indexes,
240+
strbuf_detach(&buf, NULL));
241+
} else if (opts.stdin_commits) {
242+
oidset_init(&commits, 0);
243+
if (opts.progress)
244+
progress = start_delayed_progress(
245+
_("Collecting commits from input"), 0);
246+
247+
while (strbuf_getline(&buf, stdin) != EOF) {
248+
if (read_one_commit(&commits, progress, buf.buf)) {
249+
result = 1;
250+
goto cleanup;
235251
}
236-
flags |= COMMIT_GRAPH_WRITE_CHECK_OIDS;
237252
}
238253

239-
UNLEAK(buf);
254+
240255
}
241256

242257
if (write_commit_graph(odb,
243-
pack_indexes,
258+
opts.stdin_packs ? &pack_indexes : NULL,
244259
opts.stdin_commits ? &commits : NULL,
245260
flags,
246261
&split_opts))
247262
result = 1;
248263

249-
UNLEAK(lines);
264+
cleanup:
265+
string_list_clear(&pack_indexes, 0);
266+
strbuf_release(&buf);
267+
if (progress)
268+
stop_progress(&progress);
250269
return result;
251270
}
252271

‎commit-graph.c‎

Lines changed: 27 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -881,7 +881,6 @@ struct write_commit_graph_context {
881881
unsigned append:1,
882882
report_progress:1,
883883
split:1,
884-
check_oids:1,
885884
changed_paths:1,
886885
order_by_pack:1;
887886

@@ -1319,13 +1318,25 @@ static void compute_bloom_filters(struct write_commit_graph_context *ctx)
13191318
stop_progress(&progress);
13201319
}
13211320

1321+
struct refs_cb_data {
1322+
struct oidset *commits;
1323+
struct progress *progress;
1324+
};
1325+
13221326
static int add_ref_to_set(const char *refname,
13231327
const struct object_id *oid,
13241328
int flags, void *cb_data)
13251329
{
1326-
struct oidset *commits = (struct oidset *)cb_data;
1330+
struct object_id peeled;
1331+
struct refs_cb_data *data = (struct refs_cb_data *)cb_data;
1332+
1333+
if (!peel_ref(refname, &peeled))
1334+
oid = &peeled;
1335+
if (oid_object_info(the_repository, oid, NULL) == OBJ_COMMIT)
1336+
oidset_insert(data->commits, oid);
1337+
1338+
display_progress(data->progress, oidset_size(data->commits));
13271339

1328-
oidset_insert(commits, oid);
13291340
return 0;
13301341
}
13311342

@@ -1334,13 +1345,22 @@ int write_commit_graph_reachable(struct object_directory *odb,
13341345
const struct split_commit_graph_opts *split_opts)
13351346
{
13361347
struct oidset commits = OIDSET_INIT;
1348+
struct refs_cb_data data;
13371349
int result;
13381350

1339-
for_each_ref(add_ref_to_set, &commits);
1351+
memset(&data, 0, sizeof(data));
1352+
data.commits = &commits;
1353+
if (flags & COMMIT_GRAPH_WRITE_PROGRESS)
1354+
data.progress = start_delayed_progress(
1355+
_("Collecting referenced commits"), 0);
1356+
1357+
for_each_ref(add_ref_to_set, &data);
13401358
result = write_commit_graph(odb, NULL, &commits,
13411359
flags, split_opts);
13421360

13431361
oidset_clear(&commits);
1362+
if (data.progress)
1363+
stop_progress(&data.progress);
13441364
return result;
13451365
}
13461366

@@ -1392,46 +1412,19 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,
13921412
static int fill_oids_from_commits(struct write_commit_graph_context *ctx,
13931413
struct oidset *commits)
13941414
{
1395-
uint32_t i = 0;
1396-
struct strbuf progress_title = STRBUF_INIT;
13971415
struct oidset_iter iter;
13981416
struct object_id *oid;
13991417

14001418
if (!oidset_size(commits))
14011419
return 0;
14021420

1403-
if (ctx->report_progress) {
1404-
strbuf_addf(&progress_title,
1405-
Q_("Finding commits for commit graph from %d ref",
1406-
"Finding commits for commit graph from %d refs",
1407-
oidset_size(commits)),
1408-
oidset_size(commits));
1409-
ctx->progress = start_delayed_progress(
1410-
progress_title.buf,
1411-
oidset_size(commits));
1412-
}
1413-
14141421
oidset_iter_init(commits, &iter);
14151422
while ((oid = oidset_iter_next(&iter))) {
1416-
struct commit *result;
1417-
1418-
display_progress(ctx->progress, ++i);
1419-
1420-
result = lookup_commit_reference_gently(ctx->r, oid, 1);
1421-
if (result) {
1422-
ALLOC_GROW(ctx->oids.list, ctx->oids.nr + 1, ctx->oids.alloc);
1423-
oidcpy(&ctx->oids.list[ctx->oids.nr], &(result->object.oid));
1424-
ctx->oids.nr++;
1425-
} else if (ctx->check_oids) {
1426-
error(_("invalid commit object id: %s"),
1427-
oid_to_hex(oid));
1428-
return -1;
1429-
}
1423+
ALLOC_GROW(ctx->oids.list, ctx->oids.nr + 1, ctx->oids.alloc);
1424+
oidcpy(&ctx->oids.list[ctx->oids.nr], oid);
1425+
ctx->oids.nr++;
14301426
}
14311427

1432-
stop_progress(&ctx->progress);
1433-
strbuf_release(&progress_title);
1434-
14351428
return 0;
14361429
}
14371430

@@ -2017,7 +2010,6 @@ int write_commit_graph(struct object_directory *odb,
20172010
ctx->append = flags & COMMIT_GRAPH_WRITE_APPEND ? 1 : 0;
20182011
ctx->report_progress = flags & COMMIT_GRAPH_WRITE_PROGRESS ? 1 : 0;
20192012
ctx->split = flags & COMMIT_GRAPH_WRITE_SPLIT ? 1 : 0;
2020-
ctx->check_oids = flags & COMMIT_GRAPH_WRITE_CHECK_OIDS ? 1 : 0;
20212013
ctx->split_opts = split_opts;
20222014
ctx->changed_paths = flags & COMMIT_GRAPH_WRITE_BLOOM_FILTERS ? 1 : 0;
20232015
ctx->total_bloom_filter_data_size = 0;

‎commit-graph.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,7 @@ enum commit_graph_write_flags {
9191
COMMIT_GRAPH_WRITE_APPEND = (1 << 0),
9292
COMMIT_GRAPH_WRITE_PROGRESS = (1 << 1),
9393
COMMIT_GRAPH_WRITE_SPLIT = (1 << 2),
94-
/* Make sure that each OID in the input is a valid commit OID. */
95-
COMMIT_GRAPH_WRITE_CHECK_OIDS = (1 << 3),
96-
COMMIT_GRAPH_WRITE_BLOOM_FILTERS = (1 << 4),
94+
COMMIT_GRAPH_WRITE_BLOOM_FILTERS = (1 << 3),
9795
};
9896

9997
enum commit_graph_split_flags {

‎t/t5318-commit-graph.sh‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,6 @@ test_expect_success 'create commits and repack' '
4646
git repack
4747
'
4848

49-
test_expect_success 'exit with correct error on bad input to --stdin-commits' '
50-
cd "$TRASH_DIRECTORY/full" &&
51-
echo HEAD | test_expect_code 1 git commit-graph write --stdin-commits 2>stderr &&
52-
test_i18ngrep "unexpected non-hex object ID: HEAD" stderr &&
53-
# valid tree OID, but not a commit OID
54-
git rev-parse HEAD^{tree} | test_expect_code 1 git commit-graph write --stdin-commits 2>stderr &&
55-
test_i18ngrep "invalid commit object id" stderr
56-
'
57-
5849
graph_git_two_modes() {
5950
git -c core.commitGraph=true $1 >output
6051
git -c core.commitGraph=false $1 >expect
@@ -95,6 +86,22 @@ graph_read_expect() {
9586
test_cmp expect output
9687
}
9788

89+
test_expect_success 'exit with correct error on bad input to --stdin-commits' '
90+
cd "$TRASH_DIRECTORY/full" &&
91+
# invalid, non-hex OID
92+
echo HEAD >in &&
93+
test_expect_code 1 git commit-graph write --stdin-commits <in 2>stderr &&
94+
test_i18ngrep "unexpected non-hex object ID: HEAD" stderr &&
95+
# non-existent OID
96+
echo $ZERO_OID >in &&
97+
test_expect_code 1 git commit-graph write --stdin-commits <in 2>stderr &&
98+
test_i18ngrep "invalid object" stderr &&
99+
# valid commit and tree OID
100+
git rev-parse HEAD HEAD^{tree} >in &&
101+
git commit-graph write --stdin-commits <in &&
102+
graph_read_expect 3
103+
'
104+
98105
test_expect_success 'write graph' '
99106
cd "$TRASH_DIRECTORY/full" &&
100107
git commit-graph write &&

0 commit comments

Comments
 (0)