From 7a07ec2f4707e072f412f6d95c88b0f57719e7c1 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Thu, 21 Apr 2022 10:14:31 -0400 Subject: [PATCH 1/3] midx: use real paths in lookup_multi_pack_index() This helper looks for a parsed multi-pack-index whose object directory matches the given object_dir. Before going into the loop over the parsed multi-pack-indexes, it calls find_odb() to ensure that the given object_dir is actually a known object directory. However, find_odb() uses real-path manipulations to compare the input to the alternate directories. This same real-path comparison is not used in the loop, leading to potential issues with the strcmp(). Update the method to use the real-path values instead. Signed-off-by: Derrick Stolee --- midx.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/midx.c b/midx.c index 107365d2114ce2..3db0e47735f600 100644 --- a/midx.c +++ b/midx.c @@ -1132,17 +1132,26 @@ static int write_midx_bitmap(char *midx_name, unsigned char *midx_hash, static struct multi_pack_index *lookup_multi_pack_index(struct repository *r, const char *object_dir) { + struct multi_pack_index *result = NULL; struct multi_pack_index *cur; + char *obj_dir_real = real_pathdup(object_dir, 1); + struct strbuf cur_path_real = STRBUF_INIT; /* Ensure the given object_dir is local, or a known alternate. */ - find_odb(r, object_dir); + find_odb(r, obj_dir_real); for (cur = get_multi_pack_index(r); cur; cur = cur->next) { - if (!strcmp(object_dir, cur->object_dir)) - return cur; + strbuf_realpath(&cur_path_real, cur->object_dir, 1); + if (!strcmp(obj_dir_real, cur_path_real.buf)) { + result = cur; + goto cleanup; + } } - return NULL; +cleanup: + free(obj_dir_real); + strbuf_release(&cur_path_real); + return result; } static int write_midx_internal(const char *object_dir, From 444e0640244fd1048b228ed90223d59d47ca4c24 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Thu, 21 Apr 2022 10:17:00 -0400 Subject: [PATCH 2/3] multi-pack-index: use --object-dir real path The --object-dir argument to 'git multi-pack-index' allows a user to specify an alternate to use instead of the local $GITDIR. This is used by third-party tools like VFS for Git to maintain the pack-files in a "shared object cache" used by multiple clones. On Windows, the user can specify a path using a Windows-style file path with backslashes such as "C:\Path\To\ObjectDir". This same path style is used in the .git/objects/info/alternates file, so it already matches the path of that alternate. However, find_odb() converts these paths to real-paths for the comparison, which use forward slashes. As of the previous change, lookup_multi_pack_index() uses real-paths, so it correctly finds the target multi-pack-index when given these paths. Some commands such as 'git multi-pack-index repack' call child processes using the object_dir value, so it can be helpful to convert the path to the real-path before sending it to those locations. Add a callback to convert the real path immediately upon parsing the argument. We need to be careful that we don't store the exact value out of get_object_directory() and free it, or we could corrupt a later use of the_repository->objects->odb->path. We don't use get_object_directory() for the initial instantiation in cmd_multi_pack_index() because we need 'git multi-pack-index -h' to work without a Git repository. Signed-off-by: Derrick Stolee --- builtin/multi-pack-index.c | 45 ++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c index 4480ba398277d4..5edbb7fe86e81f 100644 --- a/builtin/multi-pack-index.c +++ b/builtin/multi-pack-index.c @@ -44,7 +44,7 @@ static char const * const builtin_multi_pack_index_usage[] = { }; static struct opts_multi_pack_index { - const char *object_dir; + char *object_dir; const char *preferred_pack; const char *refs_snapshot; unsigned long batch_size; @@ -52,9 +52,23 @@ static struct opts_multi_pack_index { int stdin_packs; } opts; + +static int parse_object_dir(const struct option *opt, const char *arg, + int unset) +{ + free(opts.object_dir); + if (unset) + opts.object_dir = xstrdup(get_object_directory()); + else + opts.object_dir = real_pathdup(arg, 1); + return 0; +} + static struct option common_opts[] = { - OPT_FILENAME(0, "object-dir", &opts.object_dir, - N_("object directory containing set of packfile and pack-index pairs")), + OPT_CALLBACK(0, "object-dir", &opts.object_dir, + N_("directory"), + N_("object directory containing set of packfile and pack-index pairs"), + parse_object_dir), OPT_END(), }; @@ -232,31 +246,40 @@ static int cmd_multi_pack_index_repack(int argc, const char **argv) int cmd_multi_pack_index(int argc, const char **argv, const char *prefix) { + int res; struct option *builtin_multi_pack_index_options = common_opts; git_config(git_default_config, NULL); + if (the_repository && + the_repository->objects && + the_repository->objects->odb) + opts.object_dir = xstrdup(the_repository->objects->odb->path); + argc = parse_options(argc, argv, prefix, builtin_multi_pack_index_options, builtin_multi_pack_index_usage, PARSE_OPT_STOP_AT_NON_OPTION); - if (!opts.object_dir) - opts.object_dir = get_object_directory(); - if (!argc) goto usage; if (!strcmp(argv[0], "repack")) - return cmd_multi_pack_index_repack(argc, argv); + res = cmd_multi_pack_index_repack(argc, argv); else if (!strcmp(argv[0], "write")) - return cmd_multi_pack_index_write(argc, argv); + res = cmd_multi_pack_index_write(argc, argv); else if (!strcmp(argv[0], "verify")) - return cmd_multi_pack_index_verify(argc, argv); + res = cmd_multi_pack_index_verify(argc, argv); else if (!strcmp(argv[0], "expire")) - return cmd_multi_pack_index_expire(argc, argv); + res = cmd_multi_pack_index_expire(argc, argv); + else { + error(_("unrecognized subcommand: %s"), argv[0]); + goto usage; + } + + free(opts.object_dir); + return res; - error(_("unrecognized subcommand: %s"), argv[0]); usage: usage_with_options(builtin_multi_pack_index_usage, builtin_multi_pack_index_options); From e974d8f2ce5665dcbe5e11d5267894059e04c09c Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Thu, 21 Apr 2022 15:41:46 -0400 Subject: [PATCH 3/3] cache: use const char * for get_object_directory() The get_object_directory() method returns the exact string stored at the_repository->objects->odb->path. The return type of "char *" implies that the caller must keep track of the buffer and free() it when complete. This causes significant problems later when the ODB is accessed. Use "const char *" as the return type to avoid this confusion. There are no current callers that care about the non-const definition. Signed-off-by: Derrick Stolee --- cache.h | 2 +- environment.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cache.h b/cache.h index 4b666b2848003d..ae744997357c78 100644 --- a/cache.h +++ b/cache.h @@ -566,7 +566,7 @@ extern char *git_work_tree_cfg; int is_inside_work_tree(void); const char *get_git_dir(void); const char *get_git_common_dir(void); -char *get_object_directory(void); +const char *get_object_directory(void); char *get_index_file(void); char *get_graft_file(struct repository *r); void set_git_dir(const char *path, int make_realpath); diff --git a/environment.c b/environment.c index 5bff1b386fd882..b3296ce7d15140 100644 --- a/environment.c +++ b/environment.c @@ -273,7 +273,7 @@ const char *get_git_work_tree(void) return the_repository->worktree; } -char *get_object_directory(void) +const char *get_object_directory(void) { if (!the_repository->objects->odb) BUG("git environment hasn't been setup");