sha1-file: use an object_directory for the main object dir

Our handling of alternate object directories is needlessly different from the main object directory. As a result, many places in the code basically look like this: do_something(r->objects->objdir); for (odb = r->objects->alt_odb_list; odb; odb = odb->next) do_something(odb->path); That gets annoying when do_something() is non-trivial, and we've resorted to gross hacks like creating fake alternates (see find_short_object_filename()). Instead, let's give each raw_object_store a unified list of object_directory structs. The first will be the main store, and everything after is an alternate. Very few callers even care about the distinction, and can just loop over the whole list (and those who care can just treat the first element differently). A few observations: - we don't need r->objects->objectdir anymore, and can just mechanically convert that to r->objects->odb->path - object_directory's path field needs to become a real pointer rather than a FLEX_ARRAY, in order to fill it with expand_base_dir() - we'll call prepare_alt_odb() earlier in many functions (i.e., outside of the loop). This may result in us calling it even when our function would be satisfied looking only at the main odb. But this doesn't matter in practice. It's not a very expensive operation in the first place, and in the majority of cases it will be a noop. We call it already (and cache its results) in prepare_packed_git(), and we'll generally check packs before loose objects. So essentially every program is going to call it immediately once per program. Arguably we should just prepare_alt_odb() immediately upon setting up the repository's object directory, which would save us sprinkling calls throughout the code base (and forgetting to do so has been a source of subtle bugs in the past). But I've stopped short of that here, since there are already a lot of other moving parts in this patch. - Most call sites just get shorter. The check_and_freshen() functions are an exception, because they have entry points to handle local and nonlocal directories separately. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Nov 12, 2018 at 09:50 UTC f0eaf638195a6510254b075026dcad955b8b607d
11 files changed +90 -147
builtin/fsck.c
+3 -18
@@ -725,13 +725,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
725 for_each_loose_object(mark_loose_for_connectivity, NULL, 0);
726 for_each_packed_object(mark_packed_for_connectivity, NULL, 0);
727 } else {
728 - struct object_directory *alt_odb_list;
729 -
730 - fsck_object_dir(get_object_directory());
731 -
728 prepare_alt_odb(the_repository);
733 - alt_odb_list = the_repository->objects->alt_odb_list;
734 - for (odb = alt_odb_list; odb; odb = odb->next)
729 + for (odb = the_repository->objects->odb; odb; odb = odb->next)
730 fsck_object_dir(odb->path);
731
732 if (check_full) {
@@ -834,13 +829,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
829 struct child_process commit_graph_verify = CHILD_PROCESS_INIT;
830 const char *verify_argv[] = { "commit-graph", "verify", NULL, NULL, NULL };
831
837 - commit_graph_verify.argv = verify_argv;
838 - commit_graph_verify.git_cmd = 1;
839 - if (run_command(&commit_graph_verify))
840 - errors_found |= ERROR_COMMIT_GRAPH;
841 -
832 prepare_alt_odb(the_repository);
843 - for (odb = the_repository->objects->alt_odb_list; odb; odb = odb->next) {
833 + for (odb = the_repository->objects->odb; odb; odb = odb->next) {
834 child_process_init(&commit_graph_verify);
835 commit_graph_verify.argv = verify_argv;
836 commit_graph_verify.git_cmd = 1;
@@ -855,13 +845,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
845 struct child_process midx_verify = CHILD_PROCESS_INIT;
846 const char *midx_argv[] = { "multi-pack-index", "verify", NULL, NULL, NULL };
847
858 - midx_verify.argv = midx_argv;
859 - midx_verify.git_cmd = 1;
860 - if (run_command(&midx_verify))
861 - errors_found |= ERROR_COMMIT_GRAPH;
862 -
848 prepare_alt_odb(the_repository);
864 - for (odb = the_repository->objects->alt_odb_list; odb; odb = odb->next) {
849 + for (odb = the_repository->objects->odb; odb; odb = odb->next) {
850 child_process_init(&midx_verify);
851 midx_verify.argv = midx_argv;
852 midx_verify.git_cmd = 1;
builtin/grep.c
+1 -1
@@ -441,7 +441,7 @@ static int grep_submodule(struct grep_opt *opt, struct repository *superproject,
441 * object.
442 */
443 grep_read_lock();
444 - add_to_alternates_memory(submodule.objects->objectdir);
444 + add_to_alternates_memory(submodule.objects->odb->path);
445 grep_read_unlock();
446
447 if (oid) {
commit-graph.c
+1 -4
@@ -231,7 +231,6 @@ static void prepare_commit_graph_one(struct repository *r, const char *obj_dir)
231 static int prepare_commit_graph(struct repository *r)
232 {
233 struct object_directory *odb;
234 - char *obj_dir;
234 int config_value;
235
236 if (r->objects->commit_graph_attempted)
@@ -252,10 +251,8 @@ static int prepare_commit_graph(struct repository *r)
251 if (!commit_graph_compatible(r))
252 return 0;
253
255 - obj_dir = r->objects->objectdir;
256 - prepare_commit_graph_one(r, obj_dir);
254 prepare_alt_odb(r);
258 - for (odb = r->objects->alt_odb_list;
255 + for (odb = r->objects->odb;
256 !r->objects->commit_graph && odb;
257 odb = odb->next)
258 prepare_commit_graph_one(r, odb->path);
environment.c
+2 -2
@@ -274,9 +274,9 @@ const char *get_git_work_tree(void)
274
275 char *get_object_directory(void)
276 {
277 - if (!the_repository->objects->objectdir)
277 + if (!the_repository->objects->odb)
278 BUG("git environment hasn't been setup");
279 - return the_repository->objects->objectdir;
279 + return the_repository->objects->odb->path;
280 }
281
282 int odb_mkstemp(struct strbuf *temp_filename, const char *pattern)
object-store.h
+13 -14
@@ -24,19 +24,14 @@ struct object_directory {
24 * Path to the alternative object store. If this is a relative path,
25 * it is relative to the current working directory.
26 */
27 - char path[FLEX_ARRAY];
27 + char *path;
28 };
29 +
30 void prepare_alt_odb(struct repository *r);
31 char *compute_alternate_path(const char *path, struct strbuf *err);
32 typedef int alt_odb_fn(struct object_directory *, void *);
33 int foreach_alt_odb(alt_odb_fn, void*);
34
34 -/*
35 - * Allocate a "struct alternate_object_database" but do _not_ actually
36 - * add it to the list of alternates.
37 - */
38 -struct object_directory *alloc_alt_odb(const char *dir);
39 -
35 /*
36 * Add the directory to the on-disk alternates file; the new entry will also
37 * take effect in the current process.
@@ -80,17 +75,21 @@ struct multi_pack_index;
75
76 struct raw_object_store {
77 /*
83 - * Path to the repository's object store.
84 - * Cannot be NULL after initialization.
78 + * Set of all object directories; the main directory is first (and
79 + * cannot be NULL after initialization). Subsequent directories are
80 + * alternates.
81 */
86 - char *objectdir;
82 + struct object_directory *odb;
83 + struct object_directory **odb_tail;
84 + int loaded_alternates;
85
88 - /* Path to extra alternate object database if not NULL */
86 + /*
87 + * A list of alternate object directories loaded from the environment;
88 + * this should not generally need to be accessed directly, but will
89 + * populate the "odb" list when prepare_alt_odb() is run.
90 + */
91 char *alternate_db;
92
91 - struct object_directory *alt_odb_list;
92 - struct object_directory **alt_odb_tail;
93 -
93 /*
94 * Objects that should be substituted by other objects
95 * (see git-replace(1)).
object.c
+10 -9
@@ -482,26 +482,26 @@ struct raw_object_store *raw_object_store_new(void)
482 return o;
483 }
484
485 -static void free_alt_odb(struct object_directory *odb)
485 +static void free_object_directory(struct object_directory *odb)
486 {
487 + free(odb->path);
488 oid_array_clear(&odb->loose_objects_cache);
489 free(odb);
490 }
491
491 -static void free_alt_odbs(struct raw_object_store *o)
492 +static void free_object_directories(struct raw_object_store *o)
493 {
493 - while (o->alt_odb_list) {
494 + while (o->odb) {
495 struct object_directory *next;
496
496 - next = o->alt_odb_list->next;
497 - free_alt_odb(o->alt_odb_list);
498 - o->alt_odb_list = next;
497 + next = o->odb->next;
498 + free_object_directory(o->odb);
499 + o->odb = next;
500 }
501 }
502
503 void raw_object_store_clear(struct raw_object_store *o)
504 {
504 - FREE_AND_NULL(o->objectdir);
505 FREE_AND_NULL(o->alternate_db);
506
507 oidmap_free(o->replace_map, 1);
@@ -511,8 +511,9 @@ void raw_object_store_clear(struct raw_object_store *o)
511 o->commit_graph = NULL;
512 o->commit_graph_attempted = 0;
513
514 - free_alt_odbs(o);
515 - o->alt_odb_tail = NULL;
514 + free_object_directories(o);
515 + o->odb_tail = NULL;
516 + o->loaded_alternates = 0;
517
518 INIT_LIST_HEAD(&o->packed_git_mru);
519 close_all_packs(o);
packfile.c
+5 -5
@@ -970,12 +970,12 @@ static void prepare_packed_git(struct repository *r)
970
971 if (r->objects->packed_git_initialized)
972 return;
973 - prepare_multi_pack_index_one(r, r->objects->objectdir, 1);
974 - prepare_packed_git_one(r, r->objects->objectdir, 1);
973 +
974 prepare_alt_odb(r);
976 - for (odb = r->objects->alt_odb_list; odb; odb = odb->next) {
977 - prepare_multi_pack_index_one(r, odb->path, 0);
978 - prepare_packed_git_one(r, odb->path, 0);
975 + for (odb = r->objects->odb; odb; odb = odb->next) {
976 + int local = (odb == r->objects->odb);
977 + prepare_multi_pack_index_one(r, odb->path, local);
978 + prepare_packed_git_one(r, odb->path, local);
979 }
980 rearrange_packed_git(r);
981
path.c
+1 -1
@@ -383,7 +383,7 @@ static void adjust_git_path(const struct repository *repo,
383 strbuf_splice(buf, 0, buf->len,
384 repo->index_file, strlen(repo->index_file));
385 else if (dir_prefix(base, "objects"))
386 - replace_dir(buf, git_dir_len + 7, repo->objects->objectdir);
386 + replace_dir(buf, git_dir_len + 7, repo->objects->odb->path);
387 else if (git_hooks_path && dir_prefix(base, "hooks"))
388 replace_dir(buf, git_dir_len + 5, git_hooks_path);
389 else if (repo->different_commondir)
repository.c
+7 -1
@@ -63,8 +63,14 @@ void repo_set_gitdir(struct repository *repo,
63 free(old_gitdir);
64
65 repo_set_commondir(repo, o->commondir);
66 - expand_base_dir(&repo->objects->objectdir, o->object_dir,
66 +
67 + if (!repo->objects->odb) {
68 + repo->objects->odb = xcalloc(1, sizeof(*repo->objects->odb));
69 + repo->objects->odb_tail = &repo->objects->odb->next;
70 + }
71 + expand_base_dir(&repo->objects->odb->path, o->object_dir,
72 repo->commondir, "objects");
73 +
74 free(repo->objects->alternate_db);
75 repo->objects->alternate_db = xstrdup_or_null(o->alternate_db);
76 expand_base_dir(&repo->graft_file, o->graft_file,
sha1-file.c
+44 -78
@@ -346,11 +346,12 @@ static void fill_sha1_path(struct strbuf *buf, const unsigned char *sha1)
346 }
347 }
348
349 -static const char *odb_loose_path(const char *path, struct strbuf *buf,
349 +static const char *odb_loose_path(struct object_directory *odb,
350 + struct strbuf *buf,
351 const unsigned char *sha1)
352 {
353 strbuf_reset(buf);
353 - strbuf_addstr(buf, path);
354 + strbuf_addstr(buf, odb->path);
355 strbuf_addch(buf, '/');
356 fill_sha1_path(buf, sha1);
357 return buf->buf;
@@ -359,7 +360,7 @@ static const char *odb_loose_path(const char *path, struct strbuf *buf,
360 const char *loose_object_path(struct repository *r, struct strbuf *buf,
361 const unsigned char *sha1)
362 {
362 - return odb_loose_path(r->objects->objectdir, buf, sha1);
363 + return odb_loose_path(r->objects->odb, buf, sha1);
364 }
365
366 /*
@@ -383,7 +384,7 @@ static int alt_odb_usable(struct raw_object_store *o,
384 * Prevent the common mistake of listing the same
385 * thing twice, or object directory itself.
386 */
386 - for (odb = o->alt_odb_list; odb; odb = odb->next) {
387 + for (odb = o->odb; odb; odb = odb->next) {
388 if (!fspathcmp(path->buf, odb->path))
389 return 0;
390 }
@@ -442,11 +443,12 @@ static int link_alt_odb_entry(struct repository *r, const char *entry,
443 return -1;
444 }
445
445 - ent = alloc_alt_odb(pathbuf.buf);
446 + ent = xcalloc(1, sizeof(*ent));
447 + ent->path = xstrdup(pathbuf.buf);
448
449 /* add the alternate entry */
448 - *r->objects->alt_odb_tail = ent;
449 - r->objects->alt_odb_tail = &(ent->next);
450 + *r->objects->odb_tail = ent;
451 + r->objects->odb_tail = &(ent->next);
452 ent->next = NULL;
453
454 /* recursively add alternates */
@@ -500,7 +502,7 @@ static void link_alt_odb_entries(struct repository *r, const char *alt,
502 return;
503 }
504
503 - strbuf_add_absolute_path(&objdirbuf, r->objects->objectdir);
505 + strbuf_add_absolute_path(&objdirbuf, r->objects->odb->path);
506 if (strbuf_normalize_path(&objdirbuf) < 0)
507 die(_("unable to normalize object directory: %s"),
508 objdirbuf.buf);
@@ -535,15 +537,6 @@ static void read_info_alternates(struct repository *r,
537 free(path);
538 }
539
538 -struct object_directory *alloc_alt_odb(const char *dir)
539 -{
540 - struct object_directory *ent;
541 -
542 - FLEX_ALLOC_STR(ent, path, dir);
543 -
544 - return ent;
545 -}
546 -
540 void add_to_alternates_file(const char *reference)
541 {
542 struct lock_file lock = LOCK_INIT;
@@ -580,7 +573,7 @@ void add_to_alternates_file(const char *reference)
573 fprintf_or_die(out, "%s\n", reference);
574 if (commit_lock_file(&lock))
575 die_errno(_("unable to move new alternates file into place"));
583 - if (the_repository->objects->alt_odb_tail)
576 + if (the_repository->objects->loaded_alternates)
577 link_alt_odb_entries(the_repository, reference,
578 '\n', NULL, 0);
579 }
@@ -680,7 +673,7 @@ int foreach_alt_odb(alt_odb_fn fn, void *cb)
673 int r = 0;
674
675 prepare_alt_odb(the_repository);
683 - for (ent = the_repository->objects->alt_odb_list; ent; ent = ent->next) {
676 + for (ent = the_repository->objects->odb->next; ent; ent = ent->next) {
677 r = fn(ent, cb);
678 if (r)
679 break;
@@ -690,13 +683,13 @@ int foreach_alt_odb(alt_odb_fn fn, void *cb)
683
684 void prepare_alt_odb(struct repository *r)
685 {
693 - if (r->objects->alt_odb_tail)
686 + if (r->objects->loaded_alternates)
687 return;
688
696 - r->objects->alt_odb_tail = &r->objects->alt_odb_list;
689 link_alt_odb_entries(r, r->objects->alternate_db, PATH_SEP, NULL, 0);
690
699 - read_info_alternates(r, r->objects->objectdir, 0);
691 + read_info_alternates(r, r->objects->odb->path, 0);
692 + r->objects->loaded_alternates = 1;
693 }
694
695 /* Returns 1 if we have successfully freshened the file, 0 otherwise. */
@@ -723,24 +716,27 @@ int check_and_freshen_file(const char *fn, int freshen)
716 return 1;
717 }
718
726 -static int check_and_freshen_local(const struct object_id *oid, int freshen)
719 +static int check_and_freshen_odb(struct object_directory *odb,
720 + const struct object_id *oid,
721 + int freshen)
722 {
728 - static struct strbuf buf = STRBUF_INIT;
729 -
730 - loose_object_path(the_repository, &buf, oid->hash);
723 + static struct strbuf path = STRBUF_INIT;
724 + odb_loose_path(odb, &path, oid->hash);
725 + return check_and_freshen_file(path.buf, freshen);
726 +}
727
732 - return check_and_freshen_file(buf.buf, freshen);
728 +static int check_and_freshen_local(const struct object_id *oid, int freshen)
729 +{
730 + return check_and_freshen_odb(the_repository->objects->odb, oid, freshen);
731 }
732
733 static int check_and_freshen_nonlocal(const struct object_id *oid, int freshen)
734 {
735 struct object_directory *odb;
738 - static struct strbuf path = STRBUF_INIT;
736
737 prepare_alt_odb(the_repository);
741 - for (odb = the_repository->objects->alt_odb_list; odb; odb = odb->next) {
742 - odb_loose_path(odb->path, &path, oid->hash);
743 - if (check_and_freshen_file(path.buf, freshen))
738 + for (odb = the_repository->objects->odb->next; odb; odb = odb->next) {
739 + if (check_and_freshen_odb(odb, oid, freshen))
740 return 1;
741 }
742 return 0;
@@ -889,14 +885,9 @@ static int stat_sha1_file(struct repository *r, const unsigned char *sha1,
885 struct object_directory *odb;
886 static struct strbuf buf = STRBUF_INIT;
887
892 - *path = loose_object_path(r, &buf, sha1);
893 - if (!lstat(*path, st))
894 - return 0;
895 -
888 prepare_alt_odb(r);
897 - errno = ENOENT;
898 - for (odb = r->objects->alt_odb_list; odb; odb = odb->next) {
899 - *path = odb_loose_path(odb->path, &buf, sha1);
889 + for (odb = r->objects->odb; odb; odb = odb->next) {
890 + *path = odb_loose_path(odb, &buf, sha1);
891 if (!lstat(*path, st))
892 return 0;
893 }
@@ -913,21 +904,16 @@ static int open_sha1_file(struct repository *r,
904 {
905 int fd;
906 struct object_directory *odb;
916 - int most_interesting_errno;
907 + int most_interesting_errno = ENOENT;
908 static struct strbuf buf = STRBUF_INIT;
909
919 - *path = loose_object_path(r, &buf, sha1);
920 - fd = git_open(*path);
921 - if (fd >= 0)
922 - return fd;
923 - most_interesting_errno = errno;
924 -
910 prepare_alt_odb(r);
926 - for (odb = r->objects->alt_odb_list; odb; odb = odb->next) {
927 - *path = odb_loose_path(odb->path, &buf, sha1);
911 + for (odb = r->objects->odb; odb; odb = odb->next) {
912 + *path = odb_loose_path(odb, &buf, sha1);
913 fd = git_open(*path);
914 if (fd >= 0)
915 return fd;
916 +
917 if (most_interesting_errno == ENOENT)
918 most_interesting_errno = errno;
919 }
@@ -2120,43 +2106,23 @@ int for_each_loose_file_in_objdir(const char *path,
2106 return r;
2107 }
2108
2123 -struct loose_alt_odb_data {
2124 - each_loose_object_fn *cb;
2125 - void *data;
2126 -};
2127 -
2128 -static int loose_from_alt_odb(struct object_directory *odb,
2129 - void *vdata)
2130 -{
2131 - struct loose_alt_odb_data *data = vdata;
2132 - struct strbuf buf = STRBUF_INIT;
2133 - int r;
2134 -
2135 - strbuf_addstr(&buf, odb->path);
2136 - r = for_each_loose_file_in_objdir_buf(&buf,
2137 - data->cb, NULL, NULL,
2138 - data->data);
2139 - strbuf_release(&buf);
2140 - return r;
2141 -}
2142 -
2109 int for_each_loose_object(each_loose_object_fn cb, void *data,
2110 enum for_each_object_flags flags)
2111 {
2146 - struct loose_alt_odb_data alt;
2147 - int r;
2112 + struct object_directory *odb;
2113
2149 - r = for_each_loose_file_in_objdir(get_object_directory(),
2150 - cb, NULL, NULL, data);
2151 - if (r)
2152 - return r;
2114 + prepare_alt_odb(the_repository);
2115 + for (odb = the_repository->objects->odb; odb; odb = odb->next) {
2116 + int r = for_each_loose_file_in_objdir(odb->path, cb, NULL,
2117 + NULL, data);
2118 + if (r)
2119 + return r;
2120
2154 - if (flags & FOR_EACH_OBJECT_LOCAL_ONLY)
2155 - return 0;
2121 + if (flags & FOR_EACH_OBJECT_LOCAL_ONLY)
2122 + break;
2123 + }
2124
2157 - alt.cb = cb;
2158 - alt.data = data;
2159 - return foreach_alt_odb(loose_from_alt_odb, &alt);
2125 + return 0;
2126 }
2127
2128 static int check_stream_sha1(git_zstream *stream,
sha1-name.c
+3 -14
@@ -96,22 +96,11 @@ static void find_short_object_filename(struct disambiguate_state *ds)
96 {
97 int subdir_nr = ds->bin_pfx.hash[0];
98 struct object_directory *odb;
99 - static struct object_directory *fakeent;
99 struct strbuf buf = STRBUF_INIT;
100
102 - if (!fakeent) {
103 - /*
104 - * Create a "fake" alternate object database that
105 - * points to our own object database, to make it
106 - * easier to get a temporary working space in
107 - * alt->name/alt->base while iterating over the
108 - * object databases including our own.
109 - */
110 - fakeent = alloc_alt_odb(get_object_directory());
111 - }
112 - fakeent->next = the_repository->objects->alt_odb_list;
113 -
114 - for (odb = fakeent; odb && !ds->ambiguous; odb = odb->next) {
101 + for (odb = the_repository->objects->odb;
102 + odb && !ds->ambiguous;
103 + odb = odb->next) {
104 int pos;
105
106 if (!odb->loose_objects_subdir_seen[subdir_nr]) {