Skip to content

Commit 0404c50

Browse files
gitstergitster
committed
Merge branch 'sg/commit-graph-validate' into jch
The code to write commit-graph over given commit object names has been made a bit more robust. * sg/commit-graph-validate: commit-graph: error out on invalid commit oids in 'write --stdin-commits' commit-graph: turn a group of write-related macro flags into an enum t5318-commit-graph: use 'test_expect_code'
2 parents 9c6e30d + 7c5c9b9 commit 0404c50

5 files changed

Lines changed: 51 additions & 30 deletions

File tree

‎builtin/commit-graph.c‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ static int graph_write(int argc, const char **argv)
154154
struct string_list *commit_hex = NULL;
155155
struct string_list lines;
156156
int result = 0;
157-
unsigned int flags = COMMIT_GRAPH_PROGRESS;
157+
enum commit_graph_write_flags flags = COMMIT_GRAPH_WRITE_PROGRESS;
158158

159159
static struct option builtin_commit_graph_write_options[] = {
160160
OPT_STRING(0, "object-dir", &opts.obj_dir,
@@ -192,9 +192,9 @@ static int graph_write(int argc, const char **argv)
192192
if (!opts.obj_dir)
193193
opts.obj_dir = get_object_directory();
194194
if (opts.append)
195-
flags |= COMMIT_GRAPH_APPEND;
195+
flags |= COMMIT_GRAPH_WRITE_APPEND;
196196
if (opts.split)
197-
flags |= COMMIT_GRAPH_SPLIT;
197+
flags |= COMMIT_GRAPH_WRITE_SPLIT;
198198

199199
read_replace_refs = 0;
200200

@@ -213,8 +213,10 @@ static int graph_write(int argc, const char **argv)
213213

214214
if (opts.stdin_packs)
215215
pack_indexes = &lines;
216-
if (opts.stdin_commits)
216+
if (opts.stdin_commits) {
217217
commit_hex = &lines;
218+
flags |= COMMIT_GRAPH_WRITE_CHECK_OIDS;
219+
}
218220

219221
UNLEAK(buf);
220222
}

‎builtin/gc.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -687,7 +687,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
687687

688688
if (gc_write_commit_graph &&
689689
write_commit_graph_reachable(get_object_directory(),
690-
!quiet && !daemonized ? COMMIT_GRAPH_PROGRESS : 0,
690+
!quiet && !daemonized ? COMMIT_GRAPH_WRITE_PROGRESS : 0,
691691
NULL))
692692
return 1;
693693

‎commit-graph.c‎

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -783,7 +783,8 @@ struct write_commit_graph_context {
783783

784784
unsigned append:1,
785785
report_progress:1,
786-
split:1;
786+
split:1,
787+
check_oids:1;
787788

788789
const struct split_commit_graph_opts *split_opts;
789790
};
@@ -1134,7 +1135,8 @@ static int add_ref_to_list(const char *refname,
11341135
return 0;
11351136
}
11361137

1137-
int write_commit_graph_reachable(const char *obj_dir, unsigned int flags,
1138+
int write_commit_graph_reachable(const char *obj_dir,
1139+
enum commit_graph_write_flags flags,
11381140
const struct split_commit_graph_opts *split_opts)
11391141
{
11401142
struct string_list list = STRING_LIST_INIT_DUP;
@@ -1193,8 +1195,8 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,
11931195
return 0;
11941196
}
11951197

1196-
static void fill_oids_from_commit_hex(struct write_commit_graph_context *ctx,
1197-
struct string_list *commit_hex)
1198+
static int fill_oids_from_commit_hex(struct write_commit_graph_context *ctx,
1199+
struct string_list *commit_hex)
11981200
{
11991201
uint32_t i;
12001202
struct strbuf progress_title = STRBUF_INIT;
@@ -1215,20 +1217,21 @@ static void fill_oids_from_commit_hex(struct write_commit_graph_context *ctx,
12151217
struct commit *result;
12161218

12171219
display_progress(ctx->progress, i + 1);
1218-
if (commit_hex->items[i].string &&
1219-
parse_oid_hex(commit_hex->items[i].string, &oid, &end))
1220-
continue;
1221-
1222-
result = lookup_commit_reference_gently(ctx->r, &oid, 1);
1223-
1224-
if (result) {
1220+
if (!parse_oid_hex(commit_hex->items[i].string, &oid, &end) &&
1221+
(result = lookup_commit_reference_gently(ctx->r, &oid, 1))) {
12251222
ALLOC_GROW(ctx->oids.list, ctx->oids.nr + 1, ctx->oids.alloc);
12261223
oidcpy(&ctx->oids.list[ctx->oids.nr], &(result->object.oid));
12271224
ctx->oids.nr++;
1225+
} else if (ctx->check_oids) {
1226+
error(_("invalid commit object id: %s"),
1227+
commit_hex->items[i].string);
1228+
return -1;
12281229
}
12291230
}
12301231
stop_progress(&ctx->progress);
12311232
strbuf_release(&progress_title);
1233+
1234+
return 0;
12321235
}
12331236

12341237
static void fill_oids_from_all_packs(struct write_commit_graph_context *ctx)
@@ -1752,7 +1755,7 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx)
17521755
int write_commit_graph(const char *obj_dir,
17531756
struct string_list *pack_indexes,
17541757
struct string_list *commit_hex,
1755-
unsigned int flags,
1758+
enum commit_graph_write_flags flags,
17561759
const struct split_commit_graph_opts *split_opts)
17571760
{
17581761
struct write_commit_graph_context *ctx;
@@ -1773,9 +1776,10 @@ int write_commit_graph(const char *obj_dir,
17731776
if (len && ctx->obj_dir[len - 1] == '/')
17741777
ctx->obj_dir[len - 1] = 0;
17751778

1776-
ctx->append = flags & COMMIT_GRAPH_APPEND ? 1 : 0;
1777-
ctx->report_progress = flags & COMMIT_GRAPH_PROGRESS ? 1 : 0;
1778-
ctx->split = flags & COMMIT_GRAPH_SPLIT ? 1 : 0;
1779+
ctx->append = flags & COMMIT_GRAPH_WRITE_APPEND ? 1 : 0;
1780+
ctx->report_progress = flags & COMMIT_GRAPH_WRITE_PROGRESS ? 1 : 0;
1781+
ctx->split = flags & COMMIT_GRAPH_WRITE_SPLIT ? 1 : 0;
1782+
ctx->check_oids = flags & COMMIT_GRAPH_WRITE_CHECK_OIDS ? 1 : 0;
17791783
ctx->split_opts = split_opts;
17801784

17811785
if (ctx->split) {
@@ -1830,8 +1834,10 @@ int write_commit_graph(const char *obj_dir,
18301834
goto cleanup;
18311835
}
18321836

1833-
if (commit_hex)
1834-
fill_oids_from_commit_hex(ctx, commit_hex);
1837+
if (commit_hex) {
1838+
if ((res = fill_oids_from_commit_hex(ctx, commit_hex)))
1839+
goto cleanup;
1840+
}
18351841

18361842
if (!pack_indexes && !commit_hex)
18371843
fill_oids_from_all_packs(ctx);

‎commit-graph.h‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,13 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd,
7171
*/
7272
int generation_numbers_enabled(struct repository *r);
7373

74-
#define COMMIT_GRAPH_APPEND (1 << 0)
75-
#define COMMIT_GRAPH_PROGRESS (1 << 1)
76-
#define COMMIT_GRAPH_SPLIT (1 << 2)
74+
enum commit_graph_write_flags {
75+
COMMIT_GRAPH_WRITE_APPEND = (1 << 0),
76+
COMMIT_GRAPH_WRITE_PROGRESS = (1 << 1),
77+
COMMIT_GRAPH_WRITE_SPLIT = (1 << 2),
78+
/* Make sure that each OID in the input is a valid commit OID. */
79+
COMMIT_GRAPH_WRITE_CHECK_OIDS = (1 << 3)
80+
};
7781

7882
struct split_commit_graph_opts {
7983
int size_multiple;
@@ -87,12 +91,13 @@ struct split_commit_graph_opts {
8791
* is not compatible with the commit-graph feature, then the
8892
* methods will return 0 without writing a commit-graph.
8993
*/
90-
int write_commit_graph_reachable(const char *obj_dir, unsigned int flags,
94+
int write_commit_graph_reachable(const char *obj_dir,
95+
enum commit_graph_write_flags flags,
9196
const struct split_commit_graph_opts *split_opts);
9297
int write_commit_graph(const char *obj_dir,
9398
struct string_list *pack_indexes,
9499
struct string_list *commit_hex,
95-
unsigned int flags,
100+
enum commit_graph_write_flags flags,
96101
const struct split_commit_graph_opts *split_opts);
97102

98103
#define COMMIT_GRAPH_VERIFY_SHALLOW (1 << 0)

‎t/t5318-commit-graph.sh‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,10 @@ test_expect_success 'write graph with no packs' '
2323
test_path_is_missing info/commit-graph
2424
'
2525

26-
test_expect_success 'close with correct error on bad input' '
26+
test_expect_success 'exit with correct error on bad input to --stdin-packs' '
2727
cd "$TRASH_DIRECTORY/full" &&
2828
echo doesnotexist >in &&
29-
{ git commit-graph write --stdin-packs <in 2>stderr; ret=$?; } &&
30-
test "$ret" = 1 &&
29+
test_expect_code 1 git commit-graph write --stdin-packs <in 2>stderr &&
3130
test_i18ngrep "error adding pack" stderr
3231
'
3332

@@ -41,6 +40,15 @@ test_expect_success 'create commits and repack' '
4140
git repack
4241
'
4342

43+
test_expect_success 'exit with correct error on bad input to --stdin-commits' '
44+
cd "$TRASH_DIRECTORY/full" &&
45+
echo HEAD | test_expect_code 1 git commit-graph write --stdin-commits 2>stderr &&
46+
test_i18ngrep "invalid commit object id" stderr &&
47+
# valid tree OID, but not a commit OID
48+
git rev-parse HEAD^{tree} | test_expect_code 1 git commit-graph write --stdin-commits 2>stderr &&
49+
test_i18ngrep "invalid commit object id" stderr
50+
'
51+
4452
graph_git_two_modes() {
4553
git -c core.commitGraph=true $1 >output
4654
git -c core.commitGraph=false $1 >expect

0 commit comments

Comments
 (0)