Skip to content

Commit

Permalink
Merge branch 'sg/commit-graph-validate'
Browse files Browse the repository at this point in the history 
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'
  • Loading branch information
@gitster
gitster committed Aug 22, 2019
2 parents 072735e + 7c5c9b9 commit 6ba06b5
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 30 deletions.
10 changes: 6 additions & 4 deletions builtin/commit-graph.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ static int graph_write(int argc, const char **argv)
struct string_list *commit_hex = NULL;
struct string_list lines;
int result = 0;
unsigned int flags = COMMIT_GRAPH_PROGRESS;
enum commit_graph_write_flags flags = COMMIT_GRAPH_WRITE_PROGRESS;

static struct option builtin_commit_graph_write_options[] = {
OPT_STRING(0, "object-dir", &opts.obj_dir,
Expand Down Expand Up @@ -192,9 +192,9 @@ static int graph_write(int argc, const char **argv)
if (!opts.obj_dir)
opts.obj_dir = get_object_directory();
if (opts.append)
flags |= COMMIT_GRAPH_APPEND;
flags |= COMMIT_GRAPH_WRITE_APPEND;
if (opts.split)
flags |= COMMIT_GRAPH_SPLIT;
flags |= COMMIT_GRAPH_WRITE_SPLIT;

read_replace_refs = 0;

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

if (opts.stdin_packs)
pack_indexes = &lines;
if (opts.stdin_commits)
if (opts.stdin_commits) {
commit_hex = &lines;
flags |= COMMIT_GRAPH_WRITE_CHECK_OIDS;
}

UNLEAK(buf);
}
Expand Down
2 changes: 1 addition & 1 deletion builtin/gc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -687,7 +687,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)

if (gc_write_commit_graph &&
write_commit_graph_reachable(get_object_directory(),
!quiet && !daemonized ? COMMIT_GRAPH_PROGRESS : 0,
!quiet && !daemonized ? COMMIT_GRAPH_WRITE_PROGRESS : 0,
NULL))
return 1;

Expand Down
40 changes: 23 additions & 17 deletions commit-graph.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -783,7 +783,8 @@ struct write_commit_graph_context {

unsigned append:1,
report_progress:1,
split:1;
split:1,
check_oids:1;

const struct split_commit_graph_opts *split_opts;
};
Expand Down Expand Up @@ -1134,7 +1135,8 @@ static int add_ref_to_list(const char *refname,
return 0;
}

int write_commit_graph_reachable(const char *obj_dir, unsigned int flags,
int write_commit_graph_reachable(const char *obj_dir,
enum commit_graph_write_flags flags,
const struct split_commit_graph_opts *split_opts)
{
struct string_list list = STRING_LIST_INIT_DUP;
Expand Down Expand Up @@ -1193,8 +1195,8 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,
return 0;
}

static void fill_oids_from_commit_hex(struct write_commit_graph_context *ctx,
struct string_list *commit_hex)
static int fill_oids_from_commit_hex(struct write_commit_graph_context *ctx,
struct string_list *commit_hex)
{
uint32_t i;
struct strbuf progress_title = STRBUF_INIT;
Expand All @@ -1215,20 +1217,21 @@ static void fill_oids_from_commit_hex(struct write_commit_graph_context *ctx,
struct commit *result;

display_progress(ctx->progress, i + 1);
if (commit_hex->items[i].string &&
parse_oid_hex(commit_hex->items[i].string, &oid, &end))
continue;

result = lookup_commit_reference_gently(ctx->r, &oid, 1);

if (result) {
if (!parse_oid_hex(commit_hex->items[i].string, &oid, &end) &&
(result = lookup_commit_reference_gently(ctx->r, &oid, 1))) {
ALLOC_GROW(ctx->oids.list, ctx->oids.nr + 1, ctx->oids.alloc);
oidcpy(&ctx->oids.list[ctx->oids.nr], &(result->object.oid));
ctx->oids.nr++;
} else if (ctx->check_oids) {
error(_("invalid commit object id: %s"),
commit_hex->items[i].string);
return -1;
}
}
stop_progress(&ctx->progress);
strbuf_release(&progress_title);

return 0;
}

static void fill_oids_from_all_packs(struct write_commit_graph_context *ctx)
Expand Down Expand Up @@ -1752,7 +1755,7 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx)
int write_commit_graph(const char *obj_dir,
struct string_list *pack_indexes,
struct string_list *commit_hex,
unsigned int flags,
enum commit_graph_write_flags flags,
const struct split_commit_graph_opts *split_opts)
{
struct write_commit_graph_context *ctx;
Expand All @@ -1773,9 +1776,10 @@ int write_commit_graph(const char *obj_dir,
if (len && ctx->obj_dir[len - 1] == '/')
ctx->obj_dir[len - 1] = 0;

ctx->append = flags & COMMIT_GRAPH_APPEND ? 1 : 0;
ctx->report_progress = flags & COMMIT_GRAPH_PROGRESS ? 1 : 0;
ctx->split = flags & COMMIT_GRAPH_SPLIT ? 1 : 0;
ctx->append = flags & COMMIT_GRAPH_WRITE_APPEND ? 1 : 0;
ctx->report_progress = flags & COMMIT_GRAPH_WRITE_PROGRESS ? 1 : 0;
ctx->split = flags & COMMIT_GRAPH_WRITE_SPLIT ? 1 : 0;
ctx->check_oids = flags & COMMIT_GRAPH_WRITE_CHECK_OIDS ? 1 : 0;
ctx->split_opts = split_opts;

if (ctx->split) {
Expand Down Expand Up @@ -1830,8 +1834,10 @@ int write_commit_graph(const char *obj_dir,
goto cleanup;
}

if (commit_hex)
fill_oids_from_commit_hex(ctx, commit_hex);
if (commit_hex) {
if ((res = fill_oids_from_commit_hex(ctx, commit_hex)))
goto cleanup;
}

if (!pack_indexes && !commit_hex)
fill_oids_from_all_packs(ctx);
Expand Down
15 changes: 10 additions & 5 deletions commit-graph.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,13 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd,
*/
int generation_numbers_enabled(struct repository *r);

#define COMMIT_GRAPH_APPEND (1 << 0)
#define COMMIT_GRAPH_PROGRESS (1 << 1)
#define COMMIT_GRAPH_SPLIT (1 << 2)
enum commit_graph_write_flags {
COMMIT_GRAPH_WRITE_APPEND = (1 << 0),
COMMIT_GRAPH_WRITE_PROGRESS = (1 << 1),
COMMIT_GRAPH_WRITE_SPLIT = (1 << 2),
/* Make sure that each OID in the input is a valid commit OID. */
COMMIT_GRAPH_WRITE_CHECK_OIDS = (1 << 3)
};

struct split_commit_graph_opts {
int size_multiple;
Expand All @@ -87,12 +91,13 @@ struct split_commit_graph_opts {
* is not compatible with the commit-graph feature, then the
* methods will return 0 without writing a commit-graph.
*/
int write_commit_graph_reachable(const char *obj_dir, unsigned int flags,
int write_commit_graph_reachable(const char *obj_dir,
enum commit_graph_write_flags flags,
const struct split_commit_graph_opts *split_opts);
int write_commit_graph(const char *obj_dir,
struct string_list *pack_indexes,
struct string_list *commit_hex,
unsigned int flags,
enum commit_graph_write_flags flags,
const struct split_commit_graph_opts *split_opts);

#define COMMIT_GRAPH_VERIFY_SHALLOW (1 << 0)
Expand Down
14 changes: 11 additions & 3 deletions t/t5318-commit-graph.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,10 @@ test_expect_success 'write graph with no packs' '
test_path_is_missing info/commit-graph
'

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

Expand All @@ -41,6 +40,15 @@ test_expect_success 'create commits and repack' '
git repack
'

test_expect_success 'exit with correct error on bad input to --stdin-commits' '
cd "$TRASH_DIRECTORY/full" &&
echo HEAD | test_expect_code 1 git commit-graph write --stdin-commits 2>stderr &&
test_i18ngrep "invalid commit object id" stderr &&
# valid tree OID, but not a commit OID
git rev-parse HEAD^{tree} | test_expect_code 1 git commit-graph write --stdin-commits 2>stderr &&
test_i18ngrep "invalid commit object id" stderr
'

graph_git_two_modes() {
git -c core.commitGraph=true $1 >output
git -c core.commitGraph=false $1 >expect
Expand Down

0 comments on commit 6ba06b5

Please sign in to comment.