From 597f9134ded20f882e2bf6bca8b0e1f03981b98d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 3 Oct 2016 16:35:51 -0400 Subject: alternates: use a separate scratch space The alternate_object_database struct uses a single buffer both for storing the path to the alternate, and as a scratch buffer for forming object names. This is efficient (since otherwise we'd end up storing the path twice), but it makes life hard for callers who just want to know the path to the alternate. They have to remember to stop reading after "alt->name - alt->base" bytes, and to subtract one for the trailing '/'. It would be much simpler if they could simply access a NUL-terminated path string. We could encapsulate this in a function which puts a NUL in the scratch buffer and returns the string, but that opens up questions about the lifetime of the result. The first time another caller uses the alternate, the scratch buffer may get other data tacked onto it. Let's instead just store the root path separately from the scratch buffer. There aren't enough alternates being stored for the duplicated data to matter for performance, and this keeps things simple and safe for the callers. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/fsck.c | 10 ++-------- builtin/submodule--helper.c | 11 +++-------- 2 files changed, 5 insertions(+), 16 deletions(-) (limited to 'builtin') diff --git a/builtin/fsck.c b/builtin/fsck.c index 055dfdcf9e..f01b81eebf 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -644,14 +644,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix) fsck_object_dir(get_object_directory()); prepare_alt_odb(); - for (alt = alt_odb_list; alt; alt = alt->next) { - /* directory name, minus trailing slash */ - size_t namelen = alt->name - alt->base - 1; - struct strbuf name = STRBUF_INIT; - strbuf_add(&name, alt->base, namelen); - fsck_object_dir(name.buf); - strbuf_release(&name); - } + for (alt = alt_odb_list; alt; alt = alt->next) + fsck_object_dir(alt->path); } if (check_full) { diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index e3fdc0aa78..fd72c90442 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -492,20 +492,16 @@ static int add_possible_reference_from_superproject( { struct submodule_alternate_setup *sas = sas_cb; - /* directory name, minus trailing slash */ - size_t namelen = alt->name - alt->base - 1; - struct strbuf name = STRBUF_INIT; - strbuf_add(&name, alt->base, namelen); - /* * If the alternate object store is another repository, try the * standard layout with .git/modules//objects */ - if (ends_with(name.buf, ".git/objects")) { + if (ends_with(alt->path, ".git/objects")) { char *sm_alternate; struct strbuf sb = STRBUF_INIT; struct strbuf err = STRBUF_INIT; - strbuf_add(&sb, name.buf, name.len - strlen("objects")); + strbuf_add(&sb, alt->path, strlen(alt->path) - strlen("objects")); + /* * We need to end the new path with '/' to mark it as a dir, * otherwise a submodule name containing '/' will be broken @@ -533,7 +529,6 @@ static int add_possible_reference_from_superproject( strbuf_release(&sb); } - strbuf_release(&name); return 0; } -- cgit v1.3 From 5fe849d651e259af58f29f9cfb1b1405154ffacc Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 3 Oct 2016 16:36:18 -0400 Subject: count-objects: report alternates via verbose mode There's no way to get the list of alternates that git computes internally; our tests only infer it based on which objects are available. In addition to testing, knowing this list may be helpful for somebody debugging their alternates setup. Let's add it to the "count-objects -v" output. We could give it a separate flag, but there's not really any need. "count-objects -v" is already a debugging catch-all for the object database, its output is easily extensible to new data items, and printing the alternates is not expensive (we already had to find them to count the objects). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/git-count-objects.txt | 5 +++++ builtin/count-objects.c | 10 ++++++++++ t/t5613-info-alternate.sh | 10 ++++++++++ 3 files changed, 25 insertions(+) (limited to 'builtin') diff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt index 2ff35683e5..cb9b4d2e46 100644 --- a/Documentation/git-count-objects.txt +++ b/Documentation/git-count-objects.txt @@ -38,6 +38,11 @@ objects nor valid packs + size-garbage: disk space consumed by garbage files, in KiB (unless -H is specified) ++ +alternate: absolute path of alternate object databases; may appear +multiple times, one line per path. Note that if the path contains +non-printable characters, it may be surrounded by double-quotes and +contain C-style backslashed escape sequences. -H:: --human-readable:: diff --git a/builtin/count-objects.c b/builtin/count-objects.c index ba9291944f..a700409bf5 100644 --- a/builtin/count-objects.c +++ b/builtin/count-objects.c @@ -8,6 +8,7 @@ #include "dir.h" #include "builtin.h" #include "parse-options.h" +#include "quote.h" static unsigned long garbage; static off_t size_garbage; @@ -73,6 +74,14 @@ static int count_cruft(const char *basename, const char *path, void *data) return 0; } +static int print_alternate(struct alternate_object_database *alt, void *data) +{ + printf("alternate: "); + quote_c_style(alt->path, NULL, stdout, 0); + putchar('\n'); + return 0; +} + static char const * const count_objects_usage[] = { N_("git count-objects [-v] [-H | --human-readable]"), NULL @@ -140,6 +149,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix) printf("prune-packable: %lu\n", packed_loose); printf("garbage: %lu\n", garbage); printf("size-garbage: %s\n", garbage_buf.buf); + foreach_alt_odb(print_alternate, NULL); strbuf_release(&loose_buf); strbuf_release(&pack_buf); strbuf_release(&garbage_buf); diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh index 62170b7659..e146a8def6 100755 --- a/t/t5613-info-alternate.sh +++ b/t/t5613-info-alternate.sh @@ -39,6 +39,16 @@ test_expect_success 'preparing third repository' ' ) ' +test_expect_success 'count-objects shows the alternates' ' + cat >expect <<-EOF && + alternate: $(pwd)/B/.git/objects + alternate: $(pwd)/A/.git/objects + EOF + git -C C count-objects -v >actual && + grep ^alternate: actual >actual.alternates && + test_cmp expect actual.alternates +' + # Note: These tests depend on the hard-coded value of 5 as the maximum depth # we will follow recursion. We start the depth at 0 and count links, not # repositories. This means that in a chain like: -- cgit v1.3 From ea0fc3b4176a424a2b20eb76a6a503dc4d59cebb Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 3 Oct 2016 16:36:26 -0400 Subject: alternates: use fspathcmp to detect duplicates On a case-insensitive filesystem, we should realize that "a/objects" and "A/objects" are the same path. We already use fspathcmp() to check against the main object directory, but until recently we couldn't use it for comparing against other alternates (because their paths were not NUL-terminated strings). But now we can, so let's do so. Note that we also need to adjust count-objects to load the config, so that it can see the setting of core.ignorecase (this is required by the test, but is also a general bugfix for users of count-objects). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/count-objects.c | 2 ++ sha1_file.c | 2 +- t/t5613-info-alternate.sh | 17 +++++++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) (limited to 'builtin') diff --git a/builtin/count-objects.c b/builtin/count-objects.c index a700409bf5..a04b4f2ef3 100644 --- a/builtin/count-objects.c +++ b/builtin/count-objects.c @@ -97,6 +97,8 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix) OPT_END(), }; + git_config(git_default_config, NULL); + argc = parse_options(argc, argv, prefix, opts, count_objects_usage, 0); /* we do not take arguments other than flags for now */ if (argc) diff --git a/sha1_file.c b/sha1_file.c index 9cad56f7b0..064651947d 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -260,7 +260,7 @@ static int alt_odb_usable(struct strbuf *path, const char *normalized_objdir) * thing twice, or object directory itself. */ for (alt = alt_odb_list; alt; alt = alt->next) { - if (!strcmp(path->buf, alt->path)) + if (!fspathcmp(path->buf, alt->path)) return 0; } if (!fspathcmp(path->buf, normalized_objdir)) diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh index 76f1a20e2c..895f46bb91 100755 --- a/t/t5613-info-alternate.sh +++ b/t/t5613-info-alternate.sh @@ -119,4 +119,21 @@ test_expect_success 'relative duplicates are eliminated' ' test_cmp expect actual.alternates ' +test_expect_success CASE_INSENSITIVE_FS 'dup finding can be case-insensitive' ' + git init --bare insensitive.git && + # the previous entry for "A" will have used uppercase + cat >insensitive.git/objects/info/alternates <<-\EOF && + ../../C/.git/objects + ../../a/.git/objects + EOF + cat >expect <<-EOF && + alternate: $(pwd)/C/.git/objects + alternate: $(pwd)/B/.git/objects + alternate: $(pwd)/A/.git/objects + EOF + git -C insensitive.git count-objects -v >actual && + grep ^alternate: actual >actual.alternates && + test_cmp expect actual.alternates +' + test_done -- cgit v1.3