@matheusbsbs / matheus-bagliano / commits / 0bd52e27e3

Fork 0

commit-graph.h: store an odb in 'struct write_commit_graph_context'

There are lots of places in 'commit-graph.h' where a function either has (or almost has) a full 'struct object_directory *', accesses '->path', and then throws away the rest of the struct. This can cause headaches when comparing the locations of object directories across alternates (e.g., in the case of deciding if two commit-graph layers can be merged). These paths are normalized with 'normalize_path_copy()' which mitigates some comparison issues, but not all [1]. Replace usage of 'char *object_dir' with 'odb->path' by storing a 'struct object_directory *' in the 'write_commit_graph_context' structure. This is an intermediate step towards getting rid of all path normalization in 'commit-graph.c'. Resolving a user-provided '--object-dir' argument now requires that we compare it to the known alternates for equality. Prior to this patch, an unknown '--object-dir' argument would silently exit with status zero. This can clearly lead to unintended behavior, such as verifying commit-graphs that aren't in a repository's own object store (or one of its alternates), or causing a typo to mask a legitimate commit-graph verification failure. Make this error non-silent by 'die()'-ing when the given '--object-dir' does not match any known alternate object store. [1]: In my testing, for example, I can get one side of the commit-graph code to fill object_dir with "./objects" and the other with just "objects". Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed Feb 3, 2020 at 21:51 UTC 0bd52e27e315a76d6950fa30ce602eef15d90571

7 files changed +54 -36

Documentation/git-commit-graph.txt

+4 -1

@@ -26,7 +26,10 @@ OPTIONS

26 file. This parameter exists to specify the location of an alternate

27 that only has the objects directory, not a full `.git` directory. The

28 commit-graph file is expected to be in the `<dir>/info` directory and

29 - the packfiles are expected to be in `<dir>/pack`.

29 + the packfiles are expected to be in `<dir>/pack`. If the directory

30 + could not be made into an absolute path, or does not match any known

31 + object directory, `git commit-graph ...` will exit with non-zero

32 + status.

34 --[no-]progress::

35 Turn progress on/off explicitly. If neither is specified, progress is

builtin/commit-graph.c

+28 -5

@@ -34,9 +34,29 @@ static struct opts_commit_graph {

34 int progress;

35 } opts;

37 +static struct object_directory *find_odb(struct repository *r,

38 + const char *obj_dir)

39 +{

40 + struct object_directory *odb;

41 + char *obj_dir_real = real_pathdup(obj_dir, 1);

42 +

43 + prepare_alt_odb(r);

44 + for (odb = r->objects->odb; odb; odb = odb->next) {

45 + if (!strcmp(obj_dir_real, real_path(odb->path)))

46 + break;

47 + }

48 +

49 + free(obj_dir_real);

50 +

51 + if (!odb)

52 + die(_("could not find object directory matching %s"), obj_dir);

53 + return odb;

54 +}

55 +

56 static int graph_verify(int argc, const char **argv)

57 {

58 struct commit_graph *graph = NULL;

59 + struct object_directory *odb = NULL;

60 char *graph_name;

61 int open_ok;

62 int fd;

@@ -67,7 +87,8 @@ static int graph_verify(int argc, const char **argv)

87 if (opts.progress)

88 flags |= COMMIT_GRAPH_WRITE_PROGRESS;

70 - graph_name = get_commit_graph_filename(opts.obj_dir);

90 + odb = find_odb(the_repository, opts.obj_dir);

91 + graph_name = get_commit_graph_filename(odb->path);

92 open_ok = open_commit_graph(graph_name, &fd, &st);

93 if (!open_ok && errno != ENOENT)

94 die_errno(_("Could not open commit-graph '%s'"), graph_name);

@@ -76,8 +97,8 @@ static int graph_verify(int argc, const char **argv)

98 if (open_ok)

99 graph = load_commit_graph_one_fd_st(fd, &st);

79 - else

80 - graph = read_commit_graph_one(the_repository, opts.obj_dir);

100 + else

101 + graph = read_commit_graph_one(the_repository, odb->path);

102

103 /* Return failure if open_ok predicted success */

104 if (!graph)

@@ -94,6 +115,7 @@ static int graph_write(int argc, const char **argv)

115 {

116 struct string_list *pack_indexes = NULL;

117 struct string_list *commit_hex = NULL;

118 + struct object_directory *odb = NULL;

119 struct string_list lines;

120 int result = 0;

121 enum commit_graph_write_flags flags = 0;

@@ -145,9 +167,10 @@ static int graph_write(int argc, const char **argv)

167 flags |= COMMIT_GRAPH_WRITE_PROGRESS;

168

169 read_replace_refs = 0;

170 + odb = find_odb(the_repository, opts.obj_dir);

171

172 if (opts.reachable) {

150 - if (write_commit_graph_reachable(opts.obj_dir, flags, &split_opts))

173 + if (write_commit_graph_reachable(odb, flags, &split_opts))

174 return 1;

175 return 0;

176 }

@@ -169,7 +192,7 @@ static int graph_write(int argc, const char **argv)

192 UNLEAK(buf);

193 }

194

172 - if (write_commit_graph(opts.obj_dir,

195 + if (write_commit_graph(odb,

196 pack_indexes,

197 commit_hex,

198 flags,

builtin/commit.c

+1 -1

@@ -1688,7 +1688,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)

1688 "not exceeded, and then \"git restore --staged :/\" to recover."));

1689

1690 if (git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) &&

1691 - write_commit_graph_reachable(get_object_directory(), 0, NULL))

1691 + write_commit_graph_reachable(the_repository->objects->odb, 0, NULL))

1692 return 1;

1693

1694 repo_rerere(the_repository, 0);

builtin/fetch.c

+1 -1

@@ -1870,7 +1870,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)

1870 if (progress)

1871 commit_graph_flags |= COMMIT_GRAPH_WRITE_PROGRESS;

1872

1873 - write_commit_graph_reachable(get_object_directory(),

1873 + write_commit_graph_reachable(the_repository->objects->odb,

1874 commit_graph_flags,

1875 NULL);

1876 }

builtin/gc.c

+1 -1

@@ -686,7 +686,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)

686

687 prepare_repo_settings(the_repository);

688 if (the_repository->settings.gc_write_commit_graph == 1)

689 - write_commit_graph_reachable(get_object_directory(),

689 + write_commit_graph_reachable(the_repository->objects->odb,

690 !quiet && !daemonized ? COMMIT_GRAPH_WRITE_PROGRESS : 0,

691 NULL);

692

commit-graph.c

+16 -25

@@ -772,7 +772,7 @@ struct packed_oid_list {

772

773 struct write_commit_graph_context {

774 struct repository *r;

775 - char *obj_dir;

775 + struct object_directory *odb;

776 char *graph_name;

777 struct packed_oid_list oids;

778 struct packed_commit_list commits;

@@ -1149,7 +1149,7 @@ static int add_ref_to_list(const char *refname,

1149 return 0;

1150 }

1151

1152 -int write_commit_graph_reachable(const char *obj_dir,

1152 +int write_commit_graph_reachable(struct object_directory *odb,

1153 enum commit_graph_write_flags flags,

1154 const struct split_commit_graph_opts *split_opts)

1155 {

@@ -1157,7 +1157,7 @@ int write_commit_graph_reachable(const char *obj_dir,

1157 int result;

1158

1159 for_each_ref(add_ref_to_list, &list);

1160 - result = write_commit_graph(obj_dir, NULL, &list,

1160 + result = write_commit_graph(odb, NULL, &list,

1161 flags, split_opts);

1162

1163 string_list_clear(&list, 0);

@@ -1172,7 +1172,7 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,

1172 struct strbuf packname = STRBUF_INIT;

1173 int dirlen;

1174

1175 - strbuf_addf(&packname, "%s/pack/", ctx->obj_dir);

1175 + strbuf_addf(&packname, "%s/pack/", ctx->odb->path);

1176 dirlen = packname.len;

1177 if (ctx->report_progress) {

1178 strbuf_addf(&progress_title,

@@ -1368,10 +1368,10 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)

1368

1369 strbuf_addf(&tmp_file,

1370 "%s/info/commit-graphs/tmp_graph_XXXXXX",

1371 - ctx->obj_dir);

1371 + ctx->odb->path);

1372 ctx->graph_name = strbuf_detach(&tmp_file, NULL);

1373 } else {

1374 - ctx->graph_name = get_commit_graph_filename(ctx->obj_dir);

1374 + ctx->graph_name = get_commit_graph_filename(ctx->odb->path);

1375 }

1376

1377 if (safe_create_leading_directories(ctx->graph_name)) {

@@ -1382,7 +1382,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)

1382 }

1383

1384 if (ctx->split) {

1385 - char *lock_name = get_chain_filename(ctx->obj_dir);

1385 + char *lock_name = get_chain_filename(ctx->odb->path);

1386

1387 hold_lock_file_for_update(&lk, lock_name, LOCK_DIE_ON_ERROR);

1388

@@ -1506,12 +1506,12 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)

1506 }

1507 }

1508 } else {

1509 - char *graph_name = get_commit_graph_filename(ctx->obj_dir);

1509 + char *graph_name = get_commit_graph_filename(ctx->odb->path);

1510 unlink(graph_name);

1511 }

1512

1513 ctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 1] = xstrdup(oid_to_hex(&file_hash));

1514 - final_graph_name = get_split_graph_filename(ctx->obj_dir,

1514 + final_graph_name = get_split_graph_filename(ctx->odb->path,

1515 ctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 1]);

1516 ctx->commit_graph_filenames_after[ctx->num_commit_graphs_after - 1] = final_graph_name;

1517

@@ -1553,7 +1553,7 @@ static void split_graph_merge_strategy(struct write_commit_graph_context *ctx)

1553

1554 while (g && (g->num_commits <= size_mult * num_commits ||

1555 (max_commits && num_commits > max_commits))) {

1556 - if (strcmp(g->obj_dir, ctx->obj_dir))

1556 + if (strcmp(g->obj_dir, ctx->odb->path))

1557 break;

1558

1559 num_commits += g->num_commits;

@@ -1568,7 +1568,7 @@ static void split_graph_merge_strategy(struct write_commit_graph_context *ctx)

1568 char *old_graph_name = get_commit_graph_filename(g->obj_dir);

1569

1570 if (!strcmp(g->filename, old_graph_name) &&

1571 - strcmp(g->obj_dir, ctx->obj_dir)) {

1571 + strcmp(g->obj_dir, ctx->odb->path)) {

1572 ctx->num_commit_graphs_after = 1;

1573 ctx->new_base_graph = NULL;

1574 }

@@ -1719,13 +1719,13 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx)

1719 if (ctx->split_opts && ctx->split_opts->expire_time)

1720 expire_time -= ctx->split_opts->expire_time;

1721 if (!ctx->split) {

1722 - char *chain_file_name = get_chain_filename(ctx->obj_dir);

1722 + char *chain_file_name = get_chain_filename(ctx->odb->path);

1723 unlink(chain_file_name);

1724 free(chain_file_name);

1725 ctx->num_commit_graphs_after = 0;

1726 }

1727

1728 - strbuf_addstr(&path, ctx->obj_dir);

1728 + strbuf_addstr(&path, ctx->odb->path);

1729 strbuf_addstr(&path, "/info/commit-graphs");

1730 dir = opendir(path.buf);

1731

@@ -1764,7 +1764,7 @@ out:

1764 strbuf_release(&path);

1765 }

1766

1767 -int write_commit_graph(const char *obj_dir,

1767 +int write_commit_graph(struct object_directory *odb,

1768 struct string_list *pack_indexes,

1769 struct string_list *commit_hex,

1770 enum commit_graph_write_flags flags,

@@ -1772,7 +1772,6 @@ int write_commit_graph(const char *obj_dir,

1772 {

1773 struct write_commit_graph_context *ctx;

1774 uint32_t i, count_distinct = 0;

1775 - size_t len;

1775 int res = 0;

1776

1777 if (!commit_graph_compatible(the_repository))

@@ -1780,14 +1779,7 @@ int write_commit_graph(const char *obj_dir,

1779

1780 ctx = xcalloc(1, sizeof(struct write_commit_graph_context));

1781 ctx->r = the_repository;

1783 -

1784 - /* normalize object dir with no trailing slash */

1785 - ctx->obj_dir = xmallocz(strlen(obj_dir) + 1);

1786 - normalize_path_copy(ctx->obj_dir, obj_dir);

1787 - len = strlen(ctx->obj_dir);

1788 - if (len && ctx->obj_dir[len - 1] == '/')

1789 - ctx->obj_dir[len - 1] = 0;

1790 -

1782 + ctx->odb = odb;

1783 ctx->append = flags & COMMIT_GRAPH_WRITE_APPEND ? 1 : 0;

1784 ctx->report_progress = flags & COMMIT_GRAPH_WRITE_PROGRESS ? 1 : 0;

1785 ctx->split = flags & COMMIT_GRAPH_WRITE_SPLIT ? 1 : 0;

@@ -1824,7 +1816,7 @@ int write_commit_graph(const char *obj_dir,

1816 ctx->oids.alloc = split_opts->max_commits;

1817

1818 if (ctx->append) {

1827 - prepare_commit_graph_one(ctx->r, ctx->obj_dir);

1819 + prepare_commit_graph_one(ctx->r, ctx->odb->path);

1820 if (ctx->r->objects->commit_graph)

1821 ctx->oids.alloc += ctx->r->objects->commit_graph->num_commits;

1822 }

@@ -1898,7 +1890,6 @@ cleanup:

1890 free(ctx->graph_name);

1891 free(ctx->commits.list);

1892 free(ctx->oids.list);

1901 - free(ctx->obj_dir);

1893

1894 if (ctx->commit_graph_filenames_after) {

1895 for (i = 0; i < ctx->num_commit_graphs_after; i++) {

commit-graph.h

+3 -2

@@ -5,6 +5,7 @@

5 #include "repository.h"

6 #include "string-list.h"

7 #include "cache.h"

8 +#include "object-store.h"

10 #define GIT_TEST_COMMIT_GRAPH "GIT_TEST_COMMIT_GRAPH"

11 #define GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD "GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD"

@@ -91,10 +92,10 @@ struct split_commit_graph_opts {

92 * is not compatible with the commit-graph feature, then the

93 * methods will return 0 without writing a commit-graph.

94 */

94 -int write_commit_graph_reachable(const char *obj_dir,

95 +int write_commit_graph_reachable(struct object_directory *odb,

96 enum commit_graph_write_flags flags,

97 const struct split_commit_graph_opts *split_opts);

97 -int write_commit_graph(const char *obj_dir,

98 +int write_commit_graph(struct object_directory *odb,

99 struct string_list *pack_indexes,

100 struct string_list *commit_hex,

101 enum commit_graph_write_flags flags,