From patchwork Mon Nov 12 14:46:54 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 10678755 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 71633109C for ; Mon, 12 Nov 2018 14:46:58 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5D58E29C64 for ; Mon, 12 Nov 2018 14:46:58 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 5146929CD4; Mon, 12 Nov 2018 14:46:58 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E689229C64 for ; Mon, 12 Nov 2018 14:46:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727842AbeKMAka (ORCPT ); Mon, 12 Nov 2018 19:40:30 -0500 Received: from cloud.peff.net ([104.130.231.41]:35658 "HELO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1726385AbeKMAka (ORCPT ); Mon, 12 Nov 2018 19:40:30 -0500 Received: (qmail 29222 invoked by uid 109); 12 Nov 2018 14:46:56 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with SMTP; Mon, 12 Nov 2018 14:46:56 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 11189 invoked by uid 111); 12 Nov 2018 14:46:16 -0000 Received: from sigill.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.7) by peff.net (qpsmtpd/0.94) with (ECDHE-RSA-AES256-GCM-SHA384 encrypted) SMTP; Mon, 12 Nov 2018 09:46:16 -0500 Authentication-Results: peff.net; auth=none Received: by sigill.intra.peff.net (sSMTP sendmail emulation); Mon, 12 Nov 2018 09:46:54 -0500 Date: Mon, 12 Nov 2018 09:46:54 -0500 From: Jeff King To: Geert Jansen Cc: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Junio C Hamano , "git@vger.kernel.org" , =?utf-8?b?UmVuw6k=?= Scharfe , Takuto Ikuta Subject: [PATCH 1/9] fsck: do not reuse child_process structs Message-ID: <20181112144654.GA7400@sigill.intra.peff.net> References: <20181112144627.GA2478@sigill.intra.peff.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20181112144627.GA2478@sigill.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The run-command API makes no promises about what is left in a struct child_process after a command finishes, and it's not safe to simply reuse it again for a similar command. In particular: - if you use child->args or child->env_array, they are cleared after finish_command() - likewise, start_command() may point child->argv at child->args->argv; reusing that would lead to accessing freed memory - the in/out/err may hold pipe descriptors from the previous run These two calls are _probably_ OK because they do not use any of those features. But it's only by chance, and may break in the future; let's reinitialize our struct for each program we run. Signed-off-by: Jeff King --- builtin/fsck.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/builtin/fsck.c b/builtin/fsck.c index 06eb421720..b10f2b154c 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -841,6 +841,9 @@ int cmd_fsck(int argc, const char **argv, const char *prefix) prepare_alt_odb(the_repository); for (alt = the_repository->objects->alt_odb_list; alt; alt = alt->next) { + child_process_init(&commit_graph_verify); + commit_graph_verify.argv = verify_argv; + commit_graph_verify.git_cmd = 1; verify_argv[2] = "--object-dir"; verify_argv[3] = alt->path; if (run_command(&commit_graph_verify)) @@ -859,6 +862,9 @@ int cmd_fsck(int argc, const char **argv, const char *prefix) prepare_alt_odb(the_repository); for (alt = the_repository->objects->alt_odb_list; alt; alt = alt->next) { + child_process_init(&midx_verify); + midx_verify.argv = midx_argv; + midx_verify.git_cmd = 1; midx_argv[2] = "--object-dir"; midx_argv[3] = alt->path; if (run_command(&midx_verify)) From patchwork Mon Nov 12 14:47:04 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 10678757 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 9D457109C for ; Mon, 12 Nov 2018 14:47:07 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 89F2F29C64 for ; Mon, 12 Nov 2018 14:47:07 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7C1D829CD4; Mon, 12 Nov 2018 14:47:07 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1FEC529C64 for ; Mon, 12 Nov 2018 14:47:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728154AbeKMAkk (ORCPT ); Mon, 12 Nov 2018 19:40:40 -0500 Received: from cloud.peff.net ([104.130.231.41]:35668 "HELO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1726385AbeKMAkk (ORCPT ); Mon, 12 Nov 2018 19:40:40 -0500 Received: (qmail 29239 invoked by uid 109); 12 Nov 2018 14:47:05 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with SMTP; Mon, 12 Nov 2018 14:47:05 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 11210 invoked by uid 111); 12 Nov 2018 14:46:25 -0000 Received: from sigill.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.7) by peff.net (qpsmtpd/0.94) with (ECDHE-RSA-AES256-GCM-SHA384 encrypted) SMTP; Mon, 12 Nov 2018 09:46:25 -0500 Authentication-Results: peff.net; auth=none Received: by sigill.intra.peff.net (sSMTP sendmail emulation); Mon, 12 Nov 2018 09:47:04 -0500 Date: Mon, 12 Nov 2018 09:47:04 -0500 From: Jeff King To: Geert Jansen Cc: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Junio C Hamano , "git@vger.kernel.org" , =?utf-8?b?UmVuw6k=?= Scharfe , Takuto Ikuta Subject: [PATCH 2/9] submodule--helper: prefer strip_suffix() to ends_with() Message-ID: <20181112144703.GB7400@sigill.intra.peff.net> References: <20181112144627.GA2478@sigill.intra.peff.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20181112144627.GA2478@sigill.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Using strip_suffix() lets us avoid repeating ourselves. It also makes the handling of "/" a bit less subtle (we strip one less character than we matched in order to leave it in place, but we can just as easily include the "/" when we add more path components). Signed-off-by: Jeff King --- builtin/submodule--helper.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 676175b9be..28b9449e82 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -1268,16 +1268,17 @@ static int add_possible_reference_from_superproject( struct alternate_object_database *alt, void *sas_cb) { struct submodule_alternate_setup *sas = sas_cb; + size_t len; /* * If the alternate object store is another repository, try the * standard layout with .git/(modules/)+/objects */ - if (ends_with(alt->path, "/objects")) { + if (strip_suffix(alt->path, "/objects", &len)) { char *sm_alternate; struct strbuf sb = STRBUF_INIT; struct strbuf err = STRBUF_INIT; - strbuf_add(&sb, alt->path, strlen(alt->path) - strlen("objects")); + strbuf_add(&sb, alt->path, len); /* * We need to end the new path with '/' to mark it as a dir, @@ -1285,7 +1286,7 @@ static int add_possible_reference_from_superproject( * as the last part of a missing submodule reference would * be taken as a file name. */ - strbuf_addf(&sb, "modules/%s/", sas->submodule_name); + strbuf_addf(&sb, "/modules/%s/", sas->submodule_name); sm_alternate = compute_alternate_path(sb.buf, &err); if (sm_alternate) { From patchwork Mon Nov 12 14:48:47 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 10678759 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 9F4C5109C for ; Mon, 12 Nov 2018 14:49:00 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 16CC329913 for ; Mon, 12 Nov 2018 14:48:55 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0AA9129917; Mon, 12 Nov 2018 14:48:55 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EF32C29913 for ; Mon, 12 Nov 2018 14:48:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727185AbeKMAm0 (ORCPT ); Mon, 12 Nov 2018 19:42:26 -0500 Received: from cloud.peff.net ([104.130.231.41]:35690 "HELO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1726385AbeKMAm0 (ORCPT ); Mon, 12 Nov 2018 19:42:26 -0500 Received: (qmail 29335 invoked by uid 109); 12 Nov 2018 14:48:49 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with SMTP; Mon, 12 Nov 2018 14:48:49 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 11232 invoked by uid 111); 12 Nov 2018 14:48:08 -0000 Received: from sigill.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.7) by peff.net (qpsmtpd/0.94) with (ECDHE-RSA-AES256-GCM-SHA384 encrypted) SMTP; Mon, 12 Nov 2018 09:48:08 -0500 Authentication-Results: peff.net; auth=none Received: by sigill.intra.peff.net (sSMTP sendmail emulation); Mon, 12 Nov 2018 09:48:47 -0500 Date: Mon, 12 Nov 2018 09:48:47 -0500 From: Jeff King To: Geert Jansen Cc: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Junio C Hamano , "git@vger.kernel.org" , =?utf-8?b?UmVuw6k=?= Scharfe , Takuto Ikuta Subject: [PATCH 3/9] rename "alternate_object_database" to "object_directory" Message-ID: <20181112144846.GC7400@sigill.intra.peff.net> References: <20181112144627.GA2478@sigill.intra.peff.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20181112144627.GA2478@sigill.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP In preparation for unifying the handling of alt odb's and the normal repo object directory, let's use a more neutral name. This patch is purely mechanical, swapping the type name, and converting any variables named "alt" to "odb". There should be no functional change, but it will reduce the noise in subsequent diffs. Signed-off-by: Jeff King --- I waffled on calling this object_database instead of object_directory. But really, it is very specifically about the directory (packed storage, including packs from alternates, is handled elsewhere). builtin/count-objects.c | 4 ++-- builtin/fsck.c | 16 ++++++------- builtin/submodule--helper.c | 6 ++--- commit-graph.c | 10 ++++---- object-store.h | 14 +++++------ object.c | 10 ++++---- packfile.c | 8 +++---- sha1-file.c | 48 ++++++++++++++++++------------------- sha1-name.c | 20 ++++++++-------- transport.c | 2 +- 10 files changed, 69 insertions(+), 69 deletions(-) diff --git a/builtin/count-objects.c b/builtin/count-objects.c index a7cad052c6..3fae474f6f 100644 --- a/builtin/count-objects.c +++ b/builtin/count-objects.c @@ -78,10 +78,10 @@ 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) +static int print_alternate(struct object_directory *odb, void *data) { printf("alternate: "); - quote_c_style(alt->path, NULL, stdout, 0); + quote_c_style(odb->path, NULL, stdout, 0); putchar('\n'); return 0; } diff --git a/builtin/fsck.c b/builtin/fsck.c index b10f2b154c..55153cf92a 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -688,7 +688,7 @@ static struct option fsck_opts[] = { int cmd_fsck(int argc, const char **argv, const char *prefix) { int i; - struct alternate_object_database *alt; + struct object_directory *odb; /* fsck knows how to handle missing promisor objects */ fetch_if_missing = 0; @@ -725,14 +725,14 @@ int cmd_fsck(int argc, const char **argv, const char *prefix) for_each_loose_object(mark_loose_for_connectivity, NULL, 0); for_each_packed_object(mark_packed_for_connectivity, NULL, 0); } else { - struct alternate_object_database *alt_odb_list; + struct object_directory *alt_odb_list; fsck_object_dir(get_object_directory()); prepare_alt_odb(the_repository); alt_odb_list = the_repository->objects->alt_odb_list; - for (alt = alt_odb_list; alt; alt = alt->next) - fsck_object_dir(alt->path); + for (odb = alt_odb_list; odb; odb = odb->next) + fsck_object_dir(odb->path); if (check_full) { struct packed_git *p; @@ -840,12 +840,12 @@ int cmd_fsck(int argc, const char **argv, const char *prefix) errors_found |= ERROR_COMMIT_GRAPH; prepare_alt_odb(the_repository); - for (alt = the_repository->objects->alt_odb_list; alt; alt = alt->next) { + for (odb = the_repository->objects->alt_odb_list; odb; odb = odb->next) { child_process_init(&commit_graph_verify); commit_graph_verify.argv = verify_argv; commit_graph_verify.git_cmd = 1; verify_argv[2] = "--object-dir"; - verify_argv[3] = alt->path; + verify_argv[3] = odb->path; if (run_command(&commit_graph_verify)) errors_found |= ERROR_COMMIT_GRAPH; } @@ -861,12 +861,12 @@ int cmd_fsck(int argc, const char **argv, const char *prefix) errors_found |= ERROR_COMMIT_GRAPH; prepare_alt_odb(the_repository); - for (alt = the_repository->objects->alt_odb_list; alt; alt = alt->next) { + for (odb = the_repository->objects->alt_odb_list; odb; odb = odb->next) { child_process_init(&midx_verify); midx_verify.argv = midx_argv; midx_verify.git_cmd = 1; midx_argv[2] = "--object-dir"; - midx_argv[3] = alt->path; + midx_argv[3] = odb->path; if (run_command(&midx_verify)) errors_found |= ERROR_COMMIT_GRAPH; } diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 28b9449e82..3ae451bc46 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -1265,7 +1265,7 @@ struct submodule_alternate_setup { SUBMODULE_ALTERNATE_ERROR_IGNORE, NULL } static int add_possible_reference_from_superproject( - struct alternate_object_database *alt, void *sas_cb) + struct object_directory *odb, void *sas_cb) { struct submodule_alternate_setup *sas = sas_cb; size_t len; @@ -1274,11 +1274,11 @@ static int add_possible_reference_from_superproject( * If the alternate object store is another repository, try the * standard layout with .git/(modules/)+/objects */ - if (strip_suffix(alt->path, "/objects", &len)) { + if (strip_suffix(odb->path, "/objects", &len)) { char *sm_alternate; struct strbuf sb = STRBUF_INIT; struct strbuf err = STRBUF_INIT; - strbuf_add(&sb, alt->path, len); + strbuf_add(&sb, odb->path, len); /* * We need to end the new path with '/' to mark it as a dir, diff --git a/commit-graph.c b/commit-graph.c index 40c855f185..5dd3f5b15c 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -230,7 +230,7 @@ static void prepare_commit_graph_one(struct repository *r, const char *obj_dir) */ static int prepare_commit_graph(struct repository *r) { - struct alternate_object_database *alt; + struct object_directory *odb; char *obj_dir; int config_value; @@ -255,10 +255,10 @@ static int prepare_commit_graph(struct repository *r) obj_dir = r->objects->objectdir; prepare_commit_graph_one(r, obj_dir); prepare_alt_odb(r); - for (alt = r->objects->alt_odb_list; - !r->objects->commit_graph && alt; - alt = alt->next) - prepare_commit_graph_one(r, alt->path); + for (odb = r->objects->alt_odb_list; + !r->objects->commit_graph && odb; + odb = odb->next) + prepare_commit_graph_one(r, odb->path); return !!r->objects->commit_graph; } diff --git a/object-store.h b/object-store.h index 63b7605a3e..122d5f75e2 100644 --- a/object-store.h +++ b/object-store.h @@ -7,8 +7,8 @@ #include "sha1-array.h" #include "strbuf.h" -struct alternate_object_database { - struct alternate_object_database *next; +struct object_directory { + struct object_directory *next; /* see alt_scratch_buf() */ struct strbuf scratch; @@ -32,14 +32,14 @@ struct alternate_object_database { }; void prepare_alt_odb(struct repository *r); char *compute_alternate_path(const char *path, struct strbuf *err); -typedef int alt_odb_fn(struct alternate_object_database *, void *); +typedef int alt_odb_fn(struct object_directory *, void *); int foreach_alt_odb(alt_odb_fn, void*); /* * Allocate a "struct alternate_object_database" but do _not_ actually * add it to the list of alternates. */ -struct alternate_object_database *alloc_alt_odb(const char *dir); +struct object_directory *alloc_alt_odb(const char *dir); /* * Add the directory to the on-disk alternates file; the new entry will also @@ -60,7 +60,7 @@ void add_to_alternates_memory(const char *dir); * alternate. Always use this over direct access to alt->scratch, as it * cleans up any previous use of the scratch buffer. */ -struct strbuf *alt_scratch_buf(struct alternate_object_database *alt); +struct strbuf *alt_scratch_buf(struct object_directory *odb); struct packed_git { struct packed_git *next; @@ -100,8 +100,8 @@ struct raw_object_store { /* Path to extra alternate object database if not NULL */ char *alternate_db; - struct alternate_object_database *alt_odb_list; - struct alternate_object_database **alt_odb_tail; + struct object_directory *alt_odb_list; + struct object_directory **alt_odb_tail; /* * Objects that should be substituted by other objects diff --git a/object.c b/object.c index e54160550c..6af8e908bb 100644 --- a/object.c +++ b/object.c @@ -482,17 +482,17 @@ struct raw_object_store *raw_object_store_new(void) return o; } -static void free_alt_odb(struct alternate_object_database *alt) +static void free_alt_odb(struct object_directory *odb) { - strbuf_release(&alt->scratch); - oid_array_clear(&alt->loose_objects_cache); - free(alt); + strbuf_release(&odb->scratch); + oid_array_clear(&odb->loose_objects_cache); + free(odb); } static void free_alt_odbs(struct raw_object_store *o) { while (o->alt_odb_list) { - struct alternate_object_database *next; + struct object_directory *next; next = o->alt_odb_list->next; free_alt_odb(o->alt_odb_list); diff --git a/packfile.c b/packfile.c index f2850a00b5..d6d511cfd2 100644 --- a/packfile.c +++ b/packfile.c @@ -966,16 +966,16 @@ static void prepare_packed_git_mru(struct repository *r) static void prepare_packed_git(struct repository *r) { - struct alternate_object_database *alt; + struct object_directory *odb; if (r->objects->packed_git_initialized) return; prepare_multi_pack_index_one(r, r->objects->objectdir, 1); prepare_packed_git_one(r, r->objects->objectdir, 1); prepare_alt_odb(r); - for (alt = r->objects->alt_odb_list; alt; alt = alt->next) { - prepare_multi_pack_index_one(r, alt->path, 0); - prepare_packed_git_one(r, alt->path, 0); + for (odb = r->objects->alt_odb_list; odb; odb = odb->next) { + prepare_multi_pack_index_one(r, odb->path, 0); + prepare_packed_git_one(r, odb->path, 0); } rearrange_packed_git(r); diff --git a/sha1-file.c b/sha1-file.c index dd0b6aa873..a3cc650a0a 100644 --- a/sha1-file.c +++ b/sha1-file.c @@ -353,16 +353,16 @@ void sha1_file_name(struct repository *r, struct strbuf *buf, const unsigned cha fill_sha1_path(buf, sha1); } -struct strbuf *alt_scratch_buf(struct alternate_object_database *alt) +struct strbuf *alt_scratch_buf(struct object_directory *odb) { - strbuf_setlen(&alt->scratch, alt->base_len); - return &alt->scratch; + strbuf_setlen(&odb->scratch, odb->base_len); + return &odb->scratch; } -static const char *alt_sha1_path(struct alternate_object_database *alt, +static const char *alt_sha1_path(struct object_directory *odb, const unsigned char *sha1) { - struct strbuf *buf = alt_scratch_buf(alt); + struct strbuf *buf = alt_scratch_buf(odb); fill_sha1_path(buf, sha1); return buf->buf; } @@ -374,7 +374,7 @@ static int alt_odb_usable(struct raw_object_store *o, struct strbuf *path, const char *normalized_objdir) { - struct alternate_object_database *alt; + struct object_directory *odb; /* Detect cases where alternate disappeared */ if (!is_directory(path->buf)) { @@ -388,8 +388,8 @@ static int alt_odb_usable(struct raw_object_store *o, * Prevent the common mistake of listing the same * thing twice, or object directory itself. */ - for (alt = o->alt_odb_list; alt; alt = alt->next) { - if (!fspathcmp(path->buf, alt->path)) + for (odb = o->alt_odb_list; odb; odb = odb->next) { + if (!fspathcmp(path->buf, odb->path)) return 0; } if (!fspathcmp(path->buf, normalized_objdir)) @@ -402,7 +402,7 @@ static int alt_odb_usable(struct raw_object_store *o, * Prepare alternate object database registry. * * The variable alt_odb_list points at the list of struct - * alternate_object_database. The elements on this list come from + * object_directory. The elements on this list come from * non-empty elements from colon separated ALTERNATE_DB_ENVIRONMENT * environment variable, and $GIT_OBJECT_DIRECTORY/info/alternates, * whose contents is similar to that environment variable but can be @@ -419,7 +419,7 @@ static void read_info_alternates(struct repository *r, static int link_alt_odb_entry(struct repository *r, const char *entry, const char *relative_base, int depth, const char *normalized_objdir) { - struct alternate_object_database *ent; + struct object_directory *ent; struct strbuf pathbuf = STRBUF_INIT; if (!is_absolute_path(entry) && relative_base) { @@ -540,9 +540,9 @@ static void read_info_alternates(struct repository *r, free(path); } -struct alternate_object_database *alloc_alt_odb(const char *dir) +struct object_directory *alloc_alt_odb(const char *dir) { - struct alternate_object_database *ent; + struct object_directory *ent; FLEX_ALLOC_STR(ent, path, dir); strbuf_init(&ent->scratch, 0); @@ -684,7 +684,7 @@ char *compute_alternate_path(const char *path, struct strbuf *err) int foreach_alt_odb(alt_odb_fn fn, void *cb) { - struct alternate_object_database *ent; + struct object_directory *ent; int r = 0; prepare_alt_odb(the_repository); @@ -743,10 +743,10 @@ static int check_and_freshen_local(const struct object_id *oid, int freshen) static int check_and_freshen_nonlocal(const struct object_id *oid, int freshen) { - struct alternate_object_database *alt; + struct object_directory *odb; prepare_alt_odb(the_repository); - for (alt = the_repository->objects->alt_odb_list; alt; alt = alt->next) { - const char *path = alt_sha1_path(alt, oid->hash); + for (odb = the_repository->objects->alt_odb_list; odb; odb = odb->next) { + const char *path = alt_sha1_path(odb, oid->hash); if (check_and_freshen_file(path, freshen)) return 1; } @@ -893,7 +893,7 @@ int git_open_cloexec(const char *name, int flags) static int stat_sha1_file(struct repository *r, const unsigned char *sha1, struct stat *st, const char **path) { - struct alternate_object_database *alt; + struct object_directory *odb; static struct strbuf buf = STRBUF_INIT; strbuf_reset(&buf); @@ -905,8 +905,8 @@ static int stat_sha1_file(struct repository *r, const unsigned char *sha1, prepare_alt_odb(r); errno = ENOENT; - for (alt = r->objects->alt_odb_list; alt; alt = alt->next) { - *path = alt_sha1_path(alt, sha1); + for (odb = r->objects->alt_odb_list; odb; odb = odb->next) { + *path = alt_sha1_path(odb, sha1); if (!lstat(*path, st)) return 0; } @@ -922,7 +922,7 @@ static int open_sha1_file(struct repository *r, const unsigned char *sha1, const char **path) { int fd; - struct alternate_object_database *alt; + struct object_directory *odb; int most_interesting_errno; static struct strbuf buf = STRBUF_INIT; @@ -936,8 +936,8 @@ static int open_sha1_file(struct repository *r, most_interesting_errno = errno; prepare_alt_odb(r); - for (alt = r->objects->alt_odb_list; alt; alt = alt->next) { - *path = alt_sha1_path(alt, sha1); + for (odb = r->objects->alt_odb_list; odb; odb = odb->next) { + *path = alt_sha1_path(odb, sha1); fd = git_open(*path); if (fd >= 0) return fd; @@ -2139,14 +2139,14 @@ struct loose_alt_odb_data { void *data; }; -static int loose_from_alt_odb(struct alternate_object_database *alt, +static int loose_from_alt_odb(struct object_directory *odb, void *vdata) { struct loose_alt_odb_data *data = vdata; struct strbuf buf = STRBUF_INIT; int r; - strbuf_addstr(&buf, alt->path); + strbuf_addstr(&buf, odb->path); r = for_each_loose_file_in_objdir_buf(&buf, data->cb, NULL, NULL, data->data); diff --git a/sha1-name.c b/sha1-name.c index faa60f69e3..2594aa79f8 100644 --- a/sha1-name.c +++ b/sha1-name.c @@ -95,8 +95,8 @@ static int match_sha(unsigned, const unsigned char *, const unsigned char *); static void find_short_object_filename(struct disambiguate_state *ds) { int subdir_nr = ds->bin_pfx.hash[0]; - struct alternate_object_database *alt; - static struct alternate_object_database *fakeent; + struct object_directory *odb; + static struct object_directory *fakeent; if (!fakeent) { /* @@ -110,24 +110,24 @@ static void find_short_object_filename(struct disambiguate_state *ds) } fakeent->next = the_repository->objects->alt_odb_list; - for (alt = fakeent; alt && !ds->ambiguous; alt = alt->next) { + for (odb = fakeent; odb && !ds->ambiguous; odb = odb->next) { int pos; - if (!alt->loose_objects_subdir_seen[subdir_nr]) { - struct strbuf *buf = alt_scratch_buf(alt); + if (!odb->loose_objects_subdir_seen[subdir_nr]) { + struct strbuf *buf = alt_scratch_buf(odb); for_each_file_in_obj_subdir(subdir_nr, buf, append_loose_object, NULL, NULL, - &alt->loose_objects_cache); - alt->loose_objects_subdir_seen[subdir_nr] = 1; + &odb->loose_objects_cache); + odb->loose_objects_subdir_seen[subdir_nr] = 1; } - pos = oid_array_lookup(&alt->loose_objects_cache, &ds->bin_pfx); + pos = oid_array_lookup(&odb->loose_objects_cache, &ds->bin_pfx); if (pos < 0) pos = -1 - pos; - while (!ds->ambiguous && pos < alt->loose_objects_cache.nr) { + while (!ds->ambiguous && pos < odb->loose_objects_cache.nr) { const struct object_id *oid; - oid = alt->loose_objects_cache.oid + pos; + oid = odb->loose_objects_cache.oid + pos; if (!match_sha(ds->len, ds->bin_pfx.hash, oid->hash)) break; update_candidates(ds, oid); diff --git a/transport.c b/transport.c index 5a74b609ff..040e92c134 100644 --- a/transport.c +++ b/transport.c @@ -1433,7 +1433,7 @@ struct alternate_refs_data { void *data; }; -static int refs_from_alternate_cb(struct alternate_object_database *e, +static int refs_from_alternate_cb(struct object_directory *e, void *data) { struct strbuf path = STRBUF_INIT; From patchwork Mon Nov 12 14:48:56 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 10678761 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id D6E2F14BA for ; Mon, 12 Nov 2018 14:49:00 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 90D3529945 for ; Mon, 12 Nov 2018 14:49:00 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8582329944; Mon, 12 Nov 2018 14:49:00 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0449229914 for ; Mon, 12 Nov 2018 14:49:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727247AbeKMAmd (ORCPT ); Mon, 12 Nov 2018 19:42:33 -0500 Received: from cloud.peff.net ([104.130.231.41]:35706 "HELO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1726385AbeKMAmd (ORCPT ); Mon, 12 Nov 2018 19:42:33 -0500 Received: (qmail 29352 invoked by uid 109); 12 Nov 2018 14:48:58 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with SMTP; Mon, 12 Nov 2018 14:48:58 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 11253 invoked by uid 111); 12 Nov 2018 14:48:18 -0000 Received: from sigill.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.7) by peff.net (qpsmtpd/0.94) with (ECDHE-RSA-AES256-GCM-SHA384 encrypted) SMTP; Mon, 12 Nov 2018 09:48:18 -0500 Authentication-Results: peff.net; auth=none Received: by sigill.intra.peff.net (sSMTP sendmail emulation); Mon, 12 Nov 2018 09:48:56 -0500 Date: Mon, 12 Nov 2018 09:48:56 -0500 From: Jeff King To: Geert Jansen Cc: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Junio C Hamano , "git@vger.kernel.org" , =?utf-8?b?UmVuw6k=?= Scharfe , Takuto Ikuta Subject: [PATCH 4/9] sha1_file_name(): overwrite buffer instead of appending Message-ID: <20181112144856.GD7400@sigill.intra.peff.net> References: <20181112144627.GA2478@sigill.intra.peff.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20181112144627.GA2478@sigill.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The sha1_file_name() function is used to generate the path to a loose object in the object directory. It doesn't make much sense for it to append, since the the path we write may be absolute (i.e., you cannot reliably build up a path with it). Because many callers use it with a static buffer, they have to strbuf_reset() manually before each call (and the other callers always use an empty buffer, so they don't care either way). Let's handle this automatically. Since we're changing the semantics, let's take the opportunity to give it a more hash-neutral name (which will also catch any callers from topics in flight). Signed-off-by: Jeff King --- http-walker.c | 2 +- http.c | 4 ++-- object-store.h | 2 +- sha1-file.c | 18 ++++++++---------- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/http-walker.c b/http-walker.c index b3334bf657..0a392c85b6 100644 --- a/http-walker.c +++ b/http-walker.c @@ -547,7 +547,7 @@ static int fetch_object(struct walker *walker, unsigned char *sha1) ret = error("File %s has bad hash", hex); } else if (req->rename < 0) { struct strbuf buf = STRBUF_INIT; - sha1_file_name(the_repository, &buf, req->sha1); + loose_object_path(the_repository, &buf, req->sha1); ret = error("unable to write sha1 filename %s", buf.buf); strbuf_release(&buf); } diff --git a/http.c b/http.c index 3dc8c560d6..46c2e7a275 100644 --- a/http.c +++ b/http.c @@ -2314,7 +2314,7 @@ struct http_object_request *new_http_object_request(const char *base_url, hashcpy(freq->sha1, sha1); freq->localfile = -1; - sha1_file_name(the_repository, &filename, sha1); + loose_object_path(the_repository, &filename, sha1); strbuf_addf(&freq->tmpfile, "%s.temp", filename.buf); strbuf_addf(&prevfile, "%s.prev", filename.buf); @@ -2465,7 +2465,7 @@ int finish_http_object_request(struct http_object_request *freq) unlink_or_warn(freq->tmpfile.buf); return -1; } - sha1_file_name(the_repository, &filename, freq->sha1); + loose_object_path(the_repository, &filename, freq->sha1); freq->rename = finalize_object_file(freq->tmpfile.buf, filename.buf); strbuf_release(&filename); diff --git a/object-store.h b/object-store.h index 122d5f75e2..fefa17e380 100644 --- a/object-store.h +++ b/object-store.h @@ -157,7 +157,7 @@ void raw_object_store_clear(struct raw_object_store *o); * Put in `buf` the name of the file in the local object database that * would be used to store a loose object with the specified sha1. */ -void sha1_file_name(struct repository *r, struct strbuf *buf, const unsigned char *sha1); +void loose_object_path(struct repository *r, struct strbuf *buf, const unsigned char *sha1); void *map_sha1_file(struct repository *r, const unsigned char *sha1, unsigned long *size); diff --git a/sha1-file.c b/sha1-file.c index a3cc650a0a..478eac326b 100644 --- a/sha1-file.c +++ b/sha1-file.c @@ -346,8 +346,10 @@ static void fill_sha1_path(struct strbuf *buf, const unsigned char *sha1) } } -void sha1_file_name(struct repository *r, struct strbuf *buf, const unsigned char *sha1) +void loose_object_path(struct repository *r, struct strbuf *buf, + const unsigned char *sha1) { + strbuf_reset(buf); strbuf_addstr(buf, r->objects->objectdir); strbuf_addch(buf, '/'); fill_sha1_path(buf, sha1); @@ -735,8 +737,7 @@ static int check_and_freshen_local(const struct object_id *oid, int freshen) { static struct strbuf buf = STRBUF_INIT; - strbuf_reset(&buf); - sha1_file_name(the_repository, &buf, oid->hash); + loose_object_path(the_repository, &buf, oid->hash); return check_and_freshen_file(buf.buf, freshen); } @@ -888,7 +889,7 @@ int git_open_cloexec(const char *name, int flags) * * The "path" out-parameter will give the path of the object we found (if any). * Note that it may point to static storage and is only valid until another - * call to sha1_file_name(), etc. + * call to loose_object_path(), etc. */ static int stat_sha1_file(struct repository *r, const unsigned char *sha1, struct stat *st, const char **path) @@ -896,8 +897,7 @@ static int stat_sha1_file(struct repository *r, const unsigned char *sha1, struct object_directory *odb; static struct strbuf buf = STRBUF_INIT; - strbuf_reset(&buf); - sha1_file_name(r, &buf, sha1); + loose_object_path(r, &buf, sha1); *path = buf.buf; if (!lstat(*path, st)) @@ -926,8 +926,7 @@ static int open_sha1_file(struct repository *r, int most_interesting_errno; static struct strbuf buf = STRBUF_INIT; - strbuf_reset(&buf); - sha1_file_name(r, &buf, sha1); + loose_object_path(r, &buf, sha1); *path = buf.buf; fd = git_open(*path); @@ -1626,8 +1625,7 @@ static int write_loose_object(const struct object_id *oid, char *hdr, static struct strbuf tmp_file = STRBUF_INIT; static struct strbuf filename = STRBUF_INIT; - strbuf_reset(&filename); - sha1_file_name(the_repository, &filename, oid->hash); + loose_object_path(the_repository, &filename, oid->hash); fd = create_tmpfile(&tmp_file, filename.buf); if (fd < 0) { From patchwork Mon Nov 12 14:49:35 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 10678763 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 137C61759 for ; Mon, 12 Nov 2018 14:49:42 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id F2FCC29DC6 for ; Mon, 12 Nov 2018 14:49:41 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E6C1E29DDF; Mon, 12 Nov 2018 14:49:41 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 44FF529DD1 for ; Mon, 12 Nov 2018 14:49:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728149AbeKMAnO (ORCPT ); Mon, 12 Nov 2018 19:43:14 -0500 Received: from cloud.peff.net ([104.130.231.41]:35724 "HELO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1726385AbeKMAnO (ORCPT ); Mon, 12 Nov 2018 19:43:14 -0500 Received: (qmail 29405 invoked by uid 109); 12 Nov 2018 14:49:37 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with SMTP; Mon, 12 Nov 2018 14:49:37 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 11274 invoked by uid 111); 12 Nov 2018 14:48:57 -0000 Received: from sigill.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.7) by peff.net (qpsmtpd/0.94) with (ECDHE-RSA-AES256-GCM-SHA384 encrypted) SMTP; Mon, 12 Nov 2018 09:48:57 -0500 Authentication-Results: peff.net; auth=none Received: by sigill.intra.peff.net (sSMTP sendmail emulation); Mon, 12 Nov 2018 09:49:35 -0500 Date: Mon, 12 Nov 2018 09:49:35 -0500 From: Jeff King To: Geert Jansen Cc: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Junio C Hamano , "git@vger.kernel.org" , =?utf-8?b?UmVuw6k=?= Scharfe , Takuto Ikuta Subject: [PATCH 5/9] handle alternates paths the same as the main object dir Message-ID: <20181112144935.GE7400@sigill.intra.peff.net> References: <20181112144627.GA2478@sigill.intra.peff.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20181112144627.GA2478@sigill.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP When we generate loose file paths for the main object directory, the caller provides a buffer to loose_object_path (formerly sha1_file_name). The callers generally keep their own static buffer to avoid excessive reallocations. But for alternate directories, each struct carries its own scratch buffer. This is needlessly different; let's unify them. We could go either direction here, but this patch moves the alternates struct over to the main directory style (rather than vice-versa). Technically the alternates style is more efficient, as it avoids rewriting the object directory name on each call. But this is unlikely to matter in practice, as we avoid reallocations either way (and nobody has ever noticed or complained that the main object directory is copying a few extra bytes before making a much more expensive system call). And this has the advantage that the reusable buffers are tied to particular calls, which makes the invalidation rules simpler (for example, the return value from stat_sha1_file() used to be invalidated by basically any other object call, but now it is affected only by other calls to stat_sha1_file()). We do steal the trick from alt_sha1_path() of returning a pointer to the filled buffer, which makes a few conversions more convenient. Signed-off-by: Jeff King --- object-store.h | 14 +------------- object.c | 1 - sha1-file.c | 44 ++++++++++++++++---------------------------- sha1-name.c | 8 ++++++-- 4 files changed, 23 insertions(+), 44 deletions(-) diff --git a/object-store.h b/object-store.h index fefa17e380..b2fa0d0df0 100644 --- a/object-store.h +++ b/object-store.h @@ -10,10 +10,6 @@ struct object_directory { struct object_directory *next; - /* see alt_scratch_buf() */ - struct strbuf scratch; - size_t base_len; - /* * Used to store the results of readdir(3) calls when searching * for unique abbreviated hashes. This cache is never @@ -54,14 +50,6 @@ void add_to_alternates_file(const char *dir); */ void add_to_alternates_memory(const char *dir); -/* - * Returns a scratch strbuf pre-filled with the alternate object directory, - * including a trailing slash, which can be used to access paths in the - * alternate. Always use this over direct access to alt->scratch, as it - * cleans up any previous use of the scratch buffer. - */ -struct strbuf *alt_scratch_buf(struct object_directory *odb); - struct packed_git { struct packed_git *next; struct list_head mru; @@ -157,7 +145,7 @@ void raw_object_store_clear(struct raw_object_store *o); * Put in `buf` the name of the file in the local object database that * would be used to store a loose object with the specified sha1. */ -void loose_object_path(struct repository *r, struct strbuf *buf, const unsigned char *sha1); +const char *loose_object_path(struct repository *r, struct strbuf *buf, const unsigned char *sha1); void *map_sha1_file(struct repository *r, const unsigned char *sha1, unsigned long *size); diff --git a/object.c b/object.c index 6af8e908bb..dd485ac629 100644 --- a/object.c +++ b/object.c @@ -484,7 +484,6 @@ struct raw_object_store *raw_object_store_new(void) static void free_alt_odb(struct object_directory *odb) { - strbuf_release(&odb->scratch); oid_array_clear(&odb->loose_objects_cache); free(odb); } diff --git a/sha1-file.c b/sha1-file.c index 478eac326b..15db6b61a9 100644 --- a/sha1-file.c +++ b/sha1-file.c @@ -346,27 +346,20 @@ static void fill_sha1_path(struct strbuf *buf, const unsigned char *sha1) } } -void loose_object_path(struct repository *r, struct strbuf *buf, - const unsigned char *sha1) +static const char *odb_loose_path(const char *path, struct strbuf *buf, + const unsigned char *sha1) { strbuf_reset(buf); - strbuf_addstr(buf, r->objects->objectdir); + strbuf_addstr(buf, path); strbuf_addch(buf, '/'); fill_sha1_path(buf, sha1); + return buf->buf; } -struct strbuf *alt_scratch_buf(struct object_directory *odb) +const char *loose_object_path(struct repository *r, struct strbuf *buf, + const unsigned char *sha1) { - strbuf_setlen(&odb->scratch, odb->base_len); - return &odb->scratch; -} - -static const char *alt_sha1_path(struct object_directory *odb, - const unsigned char *sha1) -{ - struct strbuf *buf = alt_scratch_buf(odb); - fill_sha1_path(buf, sha1); - return buf->buf; + return odb_loose_path(r->objects->objectdir, buf, sha1); } /* @@ -547,9 +540,6 @@ struct object_directory *alloc_alt_odb(const char *dir) struct object_directory *ent; FLEX_ALLOC_STR(ent, path, dir); - strbuf_init(&ent->scratch, 0); - strbuf_addf(&ent->scratch, "%s/", dir); - ent->base_len = ent->scratch.len; return ent; } @@ -745,10 +735,12 @@ static int check_and_freshen_local(const struct object_id *oid, int freshen) static int check_and_freshen_nonlocal(const struct object_id *oid, int freshen) { struct object_directory *odb; + static struct strbuf path = STRBUF_INIT; + prepare_alt_odb(the_repository); for (odb = the_repository->objects->alt_odb_list; odb; odb = odb->next) { - const char *path = alt_sha1_path(odb, oid->hash); - if (check_and_freshen_file(path, freshen)) + odb_loose_path(odb->path, &path, oid->hash); + if (check_and_freshen_file(path.buf, freshen)) return 1; } return 0; @@ -889,7 +881,7 @@ int git_open_cloexec(const char *name, int flags) * * The "path" out-parameter will give the path of the object we found (if any). * Note that it may point to static storage and is only valid until another - * call to loose_object_path(), etc. + * call to stat_sha1_file(). */ static int stat_sha1_file(struct repository *r, const unsigned char *sha1, struct stat *st, const char **path) @@ -897,16 +889,14 @@ static int stat_sha1_file(struct repository *r, const unsigned char *sha1, struct object_directory *odb; static struct strbuf buf = STRBUF_INIT; - loose_object_path(r, &buf, sha1); - *path = buf.buf; - + *path = loose_object_path(r, &buf, sha1); if (!lstat(*path, st)) return 0; prepare_alt_odb(r); errno = ENOENT; for (odb = r->objects->alt_odb_list; odb; odb = odb->next) { - *path = alt_sha1_path(odb, sha1); + *path = odb_loose_path(odb->path, &buf, sha1); if (!lstat(*path, st)) return 0; } @@ -926,9 +916,7 @@ static int open_sha1_file(struct repository *r, int most_interesting_errno; static struct strbuf buf = STRBUF_INIT; - loose_object_path(r, &buf, sha1); - *path = buf.buf; - + *path = loose_object_path(r, &buf, sha1); fd = git_open(*path); if (fd >= 0) return fd; @@ -936,7 +924,7 @@ static int open_sha1_file(struct repository *r, prepare_alt_odb(r); for (odb = r->objects->alt_odb_list; odb; odb = odb->next) { - *path = alt_sha1_path(odb, sha1); + *path = odb_loose_path(odb->path, &buf, sha1); fd = git_open(*path); if (fd >= 0) return fd; diff --git a/sha1-name.c b/sha1-name.c index 2594aa79f8..96a8e71482 100644 --- a/sha1-name.c +++ b/sha1-name.c @@ -97,6 +97,7 @@ static void find_short_object_filename(struct disambiguate_state *ds) int subdir_nr = ds->bin_pfx.hash[0]; struct object_directory *odb; static struct object_directory *fakeent; + struct strbuf buf = STRBUF_INIT; if (!fakeent) { /* @@ -114,8 +115,9 @@ static void find_short_object_filename(struct disambiguate_state *ds) int pos; if (!odb->loose_objects_subdir_seen[subdir_nr]) { - struct strbuf *buf = alt_scratch_buf(odb); - for_each_file_in_obj_subdir(subdir_nr, buf, + strbuf_reset(&buf); + strbuf_addstr(&buf, odb->path); + for_each_file_in_obj_subdir(subdir_nr, &buf, append_loose_object, NULL, NULL, &odb->loose_objects_cache); @@ -134,6 +136,8 @@ static void find_short_object_filename(struct disambiguate_state *ds) pos++; } } + + strbuf_release(&buf); } static int match_sha(unsigned len, const unsigned char *a, const unsigned char *b) From patchwork Mon Nov 12 14:50:39 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 10678765 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id D986714BA for ; Mon, 12 Nov 2018 14:50:45 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C16A929DE2 for ; Mon, 12 Nov 2018 14:50:45 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B442029E09; Mon, 12 Nov 2018 14:50:45 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1CFB329DE2 for ; Mon, 12 Nov 2018 14:50:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727033AbeKMAoR (ORCPT ); Mon, 12 Nov 2018 19:44:17 -0500 Received: from cloud.peff.net ([104.130.231.41]:35734 "HELO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1726385AbeKMAoR (ORCPT ); Mon, 12 Nov 2018 19:44:17 -0500 Received: (qmail 29470 invoked by uid 109); 12 Nov 2018 14:50:41 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with SMTP; Mon, 12 Nov 2018 14:50:41 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 11300 invoked by uid 111); 12 Nov 2018 14:50:01 -0000 Received: from sigill.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.7) by peff.net (qpsmtpd/0.94) with (ECDHE-RSA-AES256-GCM-SHA384 encrypted) SMTP; Mon, 12 Nov 2018 09:50:00 -0500 Authentication-Results: peff.net; auth=none Received: by sigill.intra.peff.net (sSMTP sendmail emulation); Mon, 12 Nov 2018 09:50:39 -0500 Date: Mon, 12 Nov 2018 09:50:39 -0500 From: Jeff King To: Geert Jansen Cc: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Junio C Hamano , "git@vger.kernel.org" , =?utf-8?b?UmVuw6k=?= Scharfe , Takuto Ikuta Subject: [PATCH 6/9] sha1-file: use an object_directory for the main object dir Message-ID: <20181112145039.GF7400@sigill.intra.peff.net> References: <20181112144627.GA2478@sigill.intra.peff.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20181112144627.GA2478@sigill.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 --- If the "the first one is the main store, the rest are alternates" bit is too subtle, we could mark each "struct object_directory" with a bit for "is_local". builtin/fsck.c | 21 ++------- builtin/grep.c | 2 +- commit-graph.c | 5 +- environment.c | 4 +- object-store.h | 27 ++++++----- object.c | 19 ++++---- packfile.c | 10 ++-- path.c | 2 +- repository.c | 8 +++- sha1-file.c | 122 ++++++++++++++++++------------------------------- sha1-name.c | 17 ++----- 11 files changed, 90 insertions(+), 147 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index 55153cf92a..15338bd178 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -725,13 +725,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix) for_each_loose_object(mark_loose_for_connectivity, NULL, 0); for_each_packed_object(mark_packed_for_connectivity, NULL, 0); } else { - struct object_directory *alt_odb_list; - - fsck_object_dir(get_object_directory()); - prepare_alt_odb(the_repository); - alt_odb_list = the_repository->objects->alt_odb_list; - for (odb = alt_odb_list; odb; odb = odb->next) + for (odb = the_repository->objects->odb; odb; odb = odb->next) fsck_object_dir(odb->path); if (check_full) { @@ -834,13 +829,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix) struct child_process commit_graph_verify = CHILD_PROCESS_INIT; const char *verify_argv[] = { "commit-graph", "verify", NULL, NULL, NULL }; - commit_graph_verify.argv = verify_argv; - commit_graph_verify.git_cmd = 1; - if (run_command(&commit_graph_verify)) - errors_found |= ERROR_COMMIT_GRAPH; - prepare_alt_odb(the_repository); - for (odb = the_repository->objects->alt_odb_list; odb; odb = odb->next) { + for (odb = the_repository->objects->odb; odb; odb = odb->next) { child_process_init(&commit_graph_verify); commit_graph_verify.argv = verify_argv; commit_graph_verify.git_cmd = 1; @@ -855,13 +845,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix) struct child_process midx_verify = CHILD_PROCESS_INIT; const char *midx_argv[] = { "multi-pack-index", "verify", NULL, NULL, NULL }; - midx_verify.argv = midx_argv; - midx_verify.git_cmd = 1; - if (run_command(&midx_verify)) - errors_found |= ERROR_COMMIT_GRAPH; - prepare_alt_odb(the_repository); - for (odb = the_repository->objects->alt_odb_list; odb; odb = odb->next) { + for (odb = the_repository->objects->odb; odb; odb = odb->next) { child_process_init(&midx_verify); midx_verify.argv = midx_argv; midx_verify.git_cmd = 1; diff --git a/builtin/grep.c b/builtin/grep.c index d8508ddf79..714c8d91ba 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -441,7 +441,7 @@ static int grep_submodule(struct grep_opt *opt, struct repository *superproject, * object. */ grep_read_lock(); - add_to_alternates_memory(submodule.objects->objectdir); + add_to_alternates_memory(submodule.objects->odb->path); grep_read_unlock(); if (oid) { diff --git a/commit-graph.c b/commit-graph.c index 5dd3f5b15c..99163c244b 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -231,7 +231,6 @@ static void prepare_commit_graph_one(struct repository *r, const char *obj_dir) static int prepare_commit_graph(struct repository *r) { struct object_directory *odb; - char *obj_dir; int config_value; if (r->objects->commit_graph_attempted) @@ -252,10 +251,8 @@ static int prepare_commit_graph(struct repository *r) if (!commit_graph_compatible(r)) return 0; - obj_dir = r->objects->objectdir; - prepare_commit_graph_one(r, obj_dir); prepare_alt_odb(r); - for (odb = r->objects->alt_odb_list; + for (odb = r->objects->odb; !r->objects->commit_graph && odb; odb = odb->next) prepare_commit_graph_one(r, odb->path); diff --git a/environment.c b/environment.c index 3f3c8746c2..441ce56690 100644 --- a/environment.c +++ b/environment.c @@ -274,9 +274,9 @@ const char *get_git_work_tree(void) char *get_object_directory(void) { - if (!the_repository->objects->objectdir) + if (!the_repository->objects->odb) BUG("git environment hasn't been setup"); - return the_repository->objects->objectdir; + return the_repository->objects->odb->path; } int odb_mkstemp(struct strbuf *temp_filename, const char *pattern) diff --git a/object-store.h b/object-store.h index b2fa0d0df0..30faf7b391 100644 --- a/object-store.h +++ b/object-store.h @@ -24,19 +24,14 @@ struct object_directory { * Path to the alternative object store. If this is a relative path, * it is relative to the current working directory. */ - char path[FLEX_ARRAY]; + char *path; }; + void prepare_alt_odb(struct repository *r); char *compute_alternate_path(const char *path, struct strbuf *err); typedef int alt_odb_fn(struct object_directory *, void *); int foreach_alt_odb(alt_odb_fn, void*); -/* - * Allocate a "struct alternate_object_database" but do _not_ actually - * add it to the list of alternates. - */ -struct object_directory *alloc_alt_odb(const char *dir); - /* * Add the directory to the on-disk alternates file; the new entry will also * take effect in the current process. @@ -80,17 +75,21 @@ struct multi_pack_index; struct raw_object_store { /* - * Path to the repository's object store. - * Cannot be NULL after initialization. + * Set of all object directories; the main directory is first (and + * cannot be NULL after initialization). Subsequent directories are + * alternates. */ - char *objectdir; + struct object_directory *odb; + struct object_directory **odb_tail; + int loaded_alternates; - /* Path to extra alternate object database if not NULL */ + /* + * A list of alternate object directories loaded from the environment; + * this should not generally need to be accessed directly, but will + * populate the "odb" list when prepare_alt_odb() is run. + */ char *alternate_db; - struct object_directory *alt_odb_list; - struct object_directory **alt_odb_tail; - /* * Objects that should be substituted by other objects * (see git-replace(1)). diff --git a/object.c b/object.c index dd485ac629..79d636091c 100644 --- a/object.c +++ b/object.c @@ -482,26 +482,26 @@ struct raw_object_store *raw_object_store_new(void) return o; } -static void free_alt_odb(struct object_directory *odb) +static void free_object_directory(struct object_directory *odb) { + free(odb->path); oid_array_clear(&odb->loose_objects_cache); free(odb); } -static void free_alt_odbs(struct raw_object_store *o) +static void free_object_directories(struct raw_object_store *o) { - while (o->alt_odb_list) { + while (o->odb) { struct object_directory *next; - next = o->alt_odb_list->next; - free_alt_odb(o->alt_odb_list); - o->alt_odb_list = next; + next = o->odb->next; + free_object_directory(o->odb); + o->odb = next; } } void raw_object_store_clear(struct raw_object_store *o) { - FREE_AND_NULL(o->objectdir); FREE_AND_NULL(o->alternate_db); oidmap_free(o->replace_map, 1); @@ -511,8 +511,9 @@ void raw_object_store_clear(struct raw_object_store *o) o->commit_graph = NULL; o->commit_graph_attempted = 0; - free_alt_odbs(o); - o->alt_odb_tail = NULL; + free_object_directories(o); + o->odb_tail = NULL; + o->loaded_alternates = 0; INIT_LIST_HEAD(&o->packed_git_mru); close_all_packs(o); diff --git a/packfile.c b/packfile.c index d6d511cfd2..1eda33247f 100644 --- a/packfile.c +++ b/packfile.c @@ -970,12 +970,12 @@ static void prepare_packed_git(struct repository *r) if (r->objects->packed_git_initialized) return; - prepare_multi_pack_index_one(r, r->objects->objectdir, 1); - prepare_packed_git_one(r, r->objects->objectdir, 1); + prepare_alt_odb(r); - for (odb = r->objects->alt_odb_list; odb; odb = odb->next) { - prepare_multi_pack_index_one(r, odb->path, 0); - prepare_packed_git_one(r, odb->path, 0); + for (odb = r->objects->odb; odb; odb = odb->next) { + int local = (odb == r->objects->odb); + prepare_multi_pack_index_one(r, odb->path, local); + prepare_packed_git_one(r, odb->path, local); } rearrange_packed_git(r); diff --git a/path.c b/path.c index ba06ec5b2d..e8609cf56d 100644 --- a/path.c +++ b/path.c @@ -383,7 +383,7 @@ static void adjust_git_path(const struct repository *repo, strbuf_splice(buf, 0, buf->len, repo->index_file, strlen(repo->index_file)); else if (dir_prefix(base, "objects")) - replace_dir(buf, git_dir_len + 7, repo->objects->objectdir); + replace_dir(buf, git_dir_len + 7, repo->objects->odb->path); else if (git_hooks_path && dir_prefix(base, "hooks")) replace_dir(buf, git_dir_len + 5, git_hooks_path); else if (repo->different_commondir) diff --git a/repository.c b/repository.c index 5dd1486718..7b02e1dffa 100644 --- a/repository.c +++ b/repository.c @@ -63,8 +63,14 @@ void repo_set_gitdir(struct repository *repo, free(old_gitdir); repo_set_commondir(repo, o->commondir); - expand_base_dir(&repo->objects->objectdir, o->object_dir, + + if (!repo->objects->odb) { + repo->objects->odb = xcalloc(1, sizeof(*repo->objects->odb)); + repo->objects->odb_tail = &repo->objects->odb->next; + } + expand_base_dir(&repo->objects->odb->path, o->object_dir, repo->commondir, "objects"); + free(repo->objects->alternate_db); repo->objects->alternate_db = xstrdup_or_null(o->alternate_db); expand_base_dir(&repo->graft_file, o->graft_file, diff --git a/sha1-file.c b/sha1-file.c index 15db6b61a9..503262edd2 100644 --- a/sha1-file.c +++ b/sha1-file.c @@ -346,11 +346,12 @@ static void fill_sha1_path(struct strbuf *buf, const unsigned char *sha1) } } -static const char *odb_loose_path(const char *path, struct strbuf *buf, +static const char *odb_loose_path(struct object_directory *odb, + struct strbuf *buf, const unsigned char *sha1) { strbuf_reset(buf); - strbuf_addstr(buf, path); + strbuf_addstr(buf, odb->path); strbuf_addch(buf, '/'); fill_sha1_path(buf, sha1); return buf->buf; @@ -359,7 +360,7 @@ static const char *odb_loose_path(const char *path, struct strbuf *buf, const char *loose_object_path(struct repository *r, struct strbuf *buf, const unsigned char *sha1) { - return odb_loose_path(r->objects->objectdir, buf, sha1); + return odb_loose_path(r->objects->odb, buf, sha1); } /* @@ -383,7 +384,7 @@ static int alt_odb_usable(struct raw_object_store *o, * Prevent the common mistake of listing the same * thing twice, or object directory itself. */ - for (odb = o->alt_odb_list; odb; odb = odb->next) { + for (odb = o->odb; odb; odb = odb->next) { if (!fspathcmp(path->buf, odb->path)) return 0; } @@ -442,11 +443,12 @@ static int link_alt_odb_entry(struct repository *r, const char *entry, return -1; } - ent = alloc_alt_odb(pathbuf.buf); + ent = xcalloc(1, sizeof(*ent)); + ent->path = xstrdup(pathbuf.buf); /* add the alternate entry */ - *r->objects->alt_odb_tail = ent; - r->objects->alt_odb_tail = &(ent->next); + *r->objects->odb_tail = ent; + r->objects->odb_tail = &(ent->next); ent->next = NULL; /* recursively add alternates */ @@ -500,7 +502,7 @@ static void link_alt_odb_entries(struct repository *r, const char *alt, return; } - strbuf_add_absolute_path(&objdirbuf, r->objects->objectdir); + strbuf_add_absolute_path(&objdirbuf, r->objects->odb->path); if (strbuf_normalize_path(&objdirbuf) < 0) die(_("unable to normalize object directory: %s"), objdirbuf.buf); @@ -535,15 +537,6 @@ static void read_info_alternates(struct repository *r, free(path); } -struct object_directory *alloc_alt_odb(const char *dir) -{ - struct object_directory *ent; - - FLEX_ALLOC_STR(ent, path, dir); - - return ent; -} - void add_to_alternates_file(const char *reference) { struct lock_file lock = LOCK_INIT; @@ -580,7 +573,7 @@ void add_to_alternates_file(const char *reference) fprintf_or_die(out, "%s\n", reference); if (commit_lock_file(&lock)) die_errno(_("unable to move new alternates file into place")); - if (the_repository->objects->alt_odb_tail) + if (the_repository->objects->loaded_alternates) link_alt_odb_entries(the_repository, reference, '\n', NULL, 0); } @@ -680,7 +673,7 @@ int foreach_alt_odb(alt_odb_fn fn, void *cb) int r = 0; prepare_alt_odb(the_repository); - for (ent = the_repository->objects->alt_odb_list; ent; ent = ent->next) { + for (ent = the_repository->objects->odb->next; ent; ent = ent->next) { r = fn(ent, cb); if (r) break; @@ -690,13 +683,13 @@ int foreach_alt_odb(alt_odb_fn fn, void *cb) void prepare_alt_odb(struct repository *r) { - if (r->objects->alt_odb_tail) + if (r->objects->loaded_alternates) return; - r->objects->alt_odb_tail = &r->objects->alt_odb_list; link_alt_odb_entries(r, r->objects->alternate_db, PATH_SEP, NULL, 0); - read_info_alternates(r, r->objects->objectdir, 0); + read_info_alternates(r, r->objects->odb->path, 0); + r->objects->loaded_alternates = 1; } /* 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) return 1; } -static int check_and_freshen_local(const struct object_id *oid, int freshen) +static int check_and_freshen_odb(struct object_directory *odb, + const struct object_id *oid, + int freshen) { - static struct strbuf buf = STRBUF_INIT; - - loose_object_path(the_repository, &buf, oid->hash); + static struct strbuf path = STRBUF_INIT; + odb_loose_path(odb, &path, oid->hash); + return check_and_freshen_file(path.buf, freshen); +} - return check_and_freshen_file(buf.buf, freshen); +static int check_and_freshen_local(const struct object_id *oid, int freshen) +{ + return check_and_freshen_odb(the_repository->objects->odb, oid, freshen); } static int check_and_freshen_nonlocal(const struct object_id *oid, int freshen) { struct object_directory *odb; - static struct strbuf path = STRBUF_INIT; prepare_alt_odb(the_repository); - for (odb = the_repository->objects->alt_odb_list; odb; odb = odb->next) { - odb_loose_path(odb->path, &path, oid->hash); - if (check_and_freshen_file(path.buf, freshen)) + for (odb = the_repository->objects->odb->next; odb; odb = odb->next) { + if (check_and_freshen_odb(odb, oid, freshen)) return 1; } return 0; @@ -889,14 +885,9 @@ static int stat_sha1_file(struct repository *r, const unsigned char *sha1, struct object_directory *odb; static struct strbuf buf = STRBUF_INIT; - *path = loose_object_path(r, &buf, sha1); - if (!lstat(*path, st)) - return 0; - prepare_alt_odb(r); - errno = ENOENT; - for (odb = r->objects->alt_odb_list; odb; odb = odb->next) { - *path = odb_loose_path(odb->path, &buf, sha1); + for (odb = r->objects->odb; odb; odb = odb->next) { + *path = odb_loose_path(odb, &buf, sha1); if (!lstat(*path, st)) return 0; } @@ -913,21 +904,16 @@ static int open_sha1_file(struct repository *r, { int fd; struct object_directory *odb; - int most_interesting_errno; + int most_interesting_errno = ENOENT; static struct strbuf buf = STRBUF_INIT; - *path = loose_object_path(r, &buf, sha1); - fd = git_open(*path); - if (fd >= 0) - return fd; - most_interesting_errno = errno; - prepare_alt_odb(r); - for (odb = r->objects->alt_odb_list; odb; odb = odb->next) { - *path = odb_loose_path(odb->path, &buf, sha1); + for (odb = r->objects->odb; odb; odb = odb->next) { + *path = odb_loose_path(odb, &buf, sha1); fd = git_open(*path); if (fd >= 0) return fd; + if (most_interesting_errno == ENOENT) most_interesting_errno = errno; } @@ -2120,43 +2106,23 @@ int for_each_loose_file_in_objdir(const char *path, return r; } -struct loose_alt_odb_data { - each_loose_object_fn *cb; - void *data; -}; - -static int loose_from_alt_odb(struct object_directory *odb, - void *vdata) -{ - struct loose_alt_odb_data *data = vdata; - struct strbuf buf = STRBUF_INIT; - int r; - - strbuf_addstr(&buf, odb->path); - r = for_each_loose_file_in_objdir_buf(&buf, - data->cb, NULL, NULL, - data->data); - strbuf_release(&buf); - return r; -} - int for_each_loose_object(each_loose_object_fn cb, void *data, enum for_each_object_flags flags) { - struct loose_alt_odb_data alt; - int r; + struct object_directory *odb; - r = for_each_loose_file_in_objdir(get_object_directory(), - cb, NULL, NULL, data); - if (r) - return r; + prepare_alt_odb(the_repository); + for (odb = the_repository->objects->odb; odb; odb = odb->next) { + int r = for_each_loose_file_in_objdir(odb->path, cb, NULL, + NULL, data); + if (r) + return r; - if (flags & FOR_EACH_OBJECT_LOCAL_ONLY) - return 0; + if (flags & FOR_EACH_OBJECT_LOCAL_ONLY) + break; + } - alt.cb = cb; - alt.data = data; - return foreach_alt_odb(loose_from_alt_odb, &alt); + return 0; } static int check_stream_sha1(git_zstream *stream, diff --git a/sha1-name.c b/sha1-name.c index 96a8e71482..358ca5e288 100644 --- a/sha1-name.c +++ b/sha1-name.c @@ -96,22 +96,11 @@ static void find_short_object_filename(struct disambiguate_state *ds) { int subdir_nr = ds->bin_pfx.hash[0]; struct object_directory *odb; - static struct object_directory *fakeent; struct strbuf buf = STRBUF_INIT; - if (!fakeent) { - /* - * Create a "fake" alternate object database that - * points to our own object database, to make it - * easier to get a temporary working space in - * alt->name/alt->base while iterating over the - * object databases including our own. - */ - fakeent = alloc_alt_odb(get_object_directory()); - } - fakeent->next = the_repository->objects->alt_odb_list; - - for (odb = fakeent; odb && !ds->ambiguous; odb = odb->next) { + for (odb = the_repository->objects->odb; + odb && !ds->ambiguous; + odb = odb->next) { int pos; if (!odb->loose_objects_subdir_seen[subdir_nr]) { From patchwork Mon Nov 12 14:50:56 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 10678767 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 3538D14BA for ; Mon, 12 Nov 2018 14:51:01 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 22E2D29E3E for ; Mon, 12 Nov 2018 14:51:01 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1778D29E22; Mon, 12 Nov 2018 14:51:01 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6BEA429E16 for ; Mon, 12 Nov 2018 14:51:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728196AbeKMAoe (ORCPT ); Mon, 12 Nov 2018 19:44:34 -0500 Received: from cloud.peff.net ([104.130.231.41]:35750 "HELO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1726443AbeKMAoe (ORCPT ); Mon, 12 Nov 2018 19:44:34 -0500 Received: (qmail 29487 invoked by uid 109); 12 Nov 2018 14:50:58 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with SMTP; Mon, 12 Nov 2018 14:50:58 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 11321 invoked by uid 111); 12 Nov 2018 14:50:18 -0000 Received: from sigill.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.7) by peff.net (qpsmtpd/0.94) with (ECDHE-RSA-AES256-GCM-SHA384 encrypted) SMTP; Mon, 12 Nov 2018 09:50:18 -0500 Authentication-Results: peff.net; auth=none Received: by sigill.intra.peff.net (sSMTP sendmail emulation); Mon, 12 Nov 2018 09:50:56 -0500 Date: Mon, 12 Nov 2018 09:50:56 -0500 From: Jeff King To: Geert Jansen Cc: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Junio C Hamano , "git@vger.kernel.org" , =?utf-8?b?UmVuw6k=?= Scharfe , Takuto Ikuta Subject: [PATCH 7/9] object-store: provide helpers for loose_objects_cache Message-ID: <20181112145056.GG7400@sigill.intra.peff.net> References: <20181112144627.GA2478@sigill.intra.peff.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20181112144627.GA2478@sigill.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Our object_directory struct has a loose objects cache that all users of the struct can see. But the only one that knows how to load the cache is find_short_object_filename(). Let's extract that logic in to a reusable function. While we're at it, let's also reset the cache when we re-read the object directories. This shouldn't have an impact on performance, as re-reads are meant to be rare (and are already expensive, so we avoid them with things like OBJECT_INFO_QUICK). Since the cache is already meant to be an approximation, it's tempting to skip even this bit of safety. But it's necessary to allow more code to use it. For instance, fetch-pack explicitly re-reads the object directory after performing its fetch, and would be confused if we didn't clear the cache. Signed-off-by: Jeff King --- object-store.h | 18 +++++++++++++----- packfile.c | 8 ++++++++ sha1-file.c | 26 ++++++++++++++++++++++++++ sha1-name.c | 21 +-------------------- 4 files changed, 48 insertions(+), 25 deletions(-) diff --git a/object-store.h b/object-store.h index 30faf7b391..bf1e0cb761 100644 --- a/object-store.h +++ b/object-store.h @@ -11,11 +11,12 @@ struct object_directory { struct object_directory *next; /* - * Used to store the results of readdir(3) calls when searching - * for unique abbreviated hashes. This cache is never - * invalidated, thus it's racy and not necessarily accurate. - * That's fine for its purpose; don't use it for tasks requiring - * greater accuracy! + * Used to store the results of readdir(3) calls when we are OK + * sacrificing accuracy due to races for speed. That includes + * our search for unique abbreviated hashes. Don't use it for tasks + * requiring greater accuracy! + * + * Be sure to call odb_load_loose_cache() before using. */ char loose_objects_subdir_seen[256]; struct oid_array loose_objects_cache; @@ -45,6 +46,13 @@ void add_to_alternates_file(const char *dir); */ void add_to_alternates_memory(const char *dir); +/* + * Populate an odb's loose object cache for one particular subdirectory (i.e., + * the one that corresponds to the first byte of objects you're interested in, + * from 0 to 255 inclusive). + */ +void odb_load_loose_cache(struct object_directory *odb, int subdir_nr); + struct packed_git { struct packed_git *next; struct list_head mru; diff --git a/packfile.c b/packfile.c index 1eda33247f..91fd40efb0 100644 --- a/packfile.c +++ b/packfile.c @@ -987,6 +987,14 @@ static void prepare_packed_git(struct repository *r) void reprepare_packed_git(struct repository *r) { + struct object_directory *odb; + + for (odb = r->objects->odb; odb; odb = odb->next) { + oid_array_clear(&odb->loose_objects_cache); + memset(&odb->loose_objects_subdir_seen, 0, + sizeof(odb->loose_objects_subdir_seen)); + } + r->objects->approximate_object_count_valid = 0; r->objects->packed_git_initialized = 0; prepare_packed_git(r); diff --git a/sha1-file.c b/sha1-file.c index 503262edd2..4aae716a37 100644 --- a/sha1-file.c +++ b/sha1-file.c @@ -2125,6 +2125,32 @@ int for_each_loose_object(each_loose_object_fn cb, void *data, return 0; } +static int append_loose_object(const struct object_id *oid, const char *path, + void *data) +{ + oid_array_append(data, oid); + return 0; +} + +void odb_load_loose_cache(struct object_directory *odb, int subdir_nr) +{ + struct strbuf buf = STRBUF_INIT; + + if (subdir_nr < 0 || + subdir_nr >= ARRAY_SIZE(odb->loose_objects_subdir_seen)) + BUG("subdir_nr out of range"); + + if (odb->loose_objects_subdir_seen[subdir_nr]) + return; + + strbuf_addstr(&buf, odb->path); + for_each_file_in_obj_subdir(subdir_nr, &buf, + append_loose_object, + NULL, NULL, + &odb->loose_objects_cache); + odb->loose_objects_subdir_seen[subdir_nr] = 1; +} + static int check_stream_sha1(git_zstream *stream, const char *hdr, unsigned long size, diff --git a/sha1-name.c b/sha1-name.c index 358ca5e288..b24502811b 100644 --- a/sha1-name.c +++ b/sha1-name.c @@ -83,36 +83,19 @@ static void update_candidates(struct disambiguate_state *ds, const struct object /* otherwise, current can be discarded and candidate is still good */ } -static int append_loose_object(const struct object_id *oid, const char *path, - void *data) -{ - oid_array_append(data, oid); - return 0; -} - static int match_sha(unsigned, const unsigned char *, const unsigned char *); static void find_short_object_filename(struct disambiguate_state *ds) { int subdir_nr = ds->bin_pfx.hash[0]; struct object_directory *odb; - struct strbuf buf = STRBUF_INIT; for (odb = the_repository->objects->odb; odb && !ds->ambiguous; odb = odb->next) { int pos; - if (!odb->loose_objects_subdir_seen[subdir_nr]) { - strbuf_reset(&buf); - strbuf_addstr(&buf, odb->path); - for_each_file_in_obj_subdir(subdir_nr, &buf, - append_loose_object, - NULL, NULL, - &odb->loose_objects_cache); - odb->loose_objects_subdir_seen[subdir_nr] = 1; - } - + odb_load_loose_cache(odb, subdir_nr); pos = oid_array_lookup(&odb->loose_objects_cache, &ds->bin_pfx); if (pos < 0) pos = -1 - pos; @@ -125,8 +108,6 @@ static void find_short_object_filename(struct disambiguate_state *ds) pos++; } } - - strbuf_release(&buf); } static int match_sha(unsigned len, const unsigned char *a, const unsigned char *b) From patchwork Mon Nov 12 14:54:42 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 10678769 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id C9BA713BF for ; Mon, 12 Nov 2018 14:54:47 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B4A6428FDD for ; Mon, 12 Nov 2018 14:54:47 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A797529017; Mon, 12 Nov 2018 14:54:47 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9410D28FDD for ; Mon, 12 Nov 2018 14:54:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729624AbeKMAsV (ORCPT ); Mon, 12 Nov 2018 19:48:21 -0500 Received: from cloud.peff.net ([104.130.231.41]:35774 "HELO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1726385AbeKMAsU (ORCPT ); Mon, 12 Nov 2018 19:48:20 -0500 Received: (qmail 29661 invoked by uid 109); 12 Nov 2018 14:54:44 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with SMTP; Mon, 12 Nov 2018 14:54:44 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 11426 invoked by uid 111); 12 Nov 2018 14:54:04 -0000 Received: from sigill.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.7) by peff.net (qpsmtpd/0.94) with (ECDHE-RSA-AES256-GCM-SHA384 encrypted) SMTP; Mon, 12 Nov 2018 09:54:04 -0500 Authentication-Results: peff.net; auth=none Received: by sigill.intra.peff.net (sSMTP sendmail emulation); Mon, 12 Nov 2018 09:54:42 -0500 Date: Mon, 12 Nov 2018 09:54:42 -0500 From: Jeff King To: Geert Jansen Cc: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Junio C Hamano , "git@vger.kernel.org" , =?utf-8?b?UmVuw6k=?= Scharfe , Takuto Ikuta Subject: [PATCH 8/9] sha1-file: use loose object cache for quick existence check Message-ID: <20181112145442.GH7400@sigill.intra.peff.net> References: <20181112144627.GA2478@sigill.intra.peff.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20181112144627.GA2478@sigill.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP In cases where we expect to ask has_sha1_file() about a lot of objects that we are not likely to have (e.g., during fetch negotiation), we already use OBJECT_INFO_QUICK to sacrifice accuracy (due to racing with a simultaneous write or repack) for speed (we avoid re-scanning the pack directory). However, even checking for loose objects can be expensive, as we will stat() each one. On many systems this cost isn't too noticeable, but stat() can be particularly slow on some operating systems, or due to network filesystems. Since the QUICK flag already tells us that we're OK with a slightly stale answer, we can use that as a cue to look in our in-memory cache of each object directory. That basically trades an in-memory binary search for a stat() call. Note that it is possible for this to actually be _slower_. We'll do a full readdir() to fill the cache, so if you have a very large number of loose objects and a very small number of lookups, that readdir() may end up more expensive. This shouldn't be a big deal in practice. If you have a large number of reachable loose objects, you'll already run into performance problems (which you should remedy by repacking). You may have unreachable objects which wouldn't otherwise impact performance. Usually these would go away with the prune step of "git gc", but they may be held for up to 2 weeks in the default configuration. So it comes down to how many such objects you might reasonably expect to have, how much slower is readdir() on N entries versus M stat() calls (and here we really care about the syscall backing readdir(), like getdents() on Linux, but I'll just call this readdir() below). If N is much smaller than M (a typical packed repo), we know this is a big win (few readdirs() followed by many uses of the resulting cache). When N and M are similar in size, it's also a win. We care about the latency of making a syscall, and readdir() should be giving us many values in a single call. How many? On Linux, running "strace -e getdents ls" shows a 32k buffer getting 512 entries per call (which is 64 bytes per entry; the name itself is 38 bytes, plus there are some other fields). So we can imagine that this is always a win as long as the number of loose objects in the repository is a factor of 500 less than the number of lookups you make. It's hard to auto-tune this because we don't generally know up front how many lookups we're going to do. But it's unlikely for this to perform significantly worse. Signed-off-by: Jeff King Reported-by: Ævar Arnfjörð Bjarmason Signed-off-by: Rene Scharfe --- There's some obvious hand-waving in the paragraphs above. I would love it if somebody with an NFS system could do some before/after timings with various numbers of loose objects, to get a sense of where the breakeven point is. My gut is that we do not need the complexity of a cache-size limit, nor of a config option to disable this. But it would be nice to have a real number where "reasonable" ends and "pathological" begins. :) object-store.h | 1 + sha1-file.c | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/object-store.h b/object-store.h index bf1e0cb761..60758efad8 100644 --- a/object-store.h +++ b/object-store.h @@ -13,6 +13,7 @@ struct object_directory { /* * Used to store the results of readdir(3) calls when we are OK * sacrificing accuracy due to races for speed. That includes + * object existence with OBJECT_INFO_QUICK, as well as * our search for unique abbreviated hashes. Don't use it for tasks * requiring greater accuracy! * diff --git a/sha1-file.c b/sha1-file.c index 4aae716a37..e53da0b701 100644 --- a/sha1-file.c +++ b/sha1-file.c @@ -921,6 +921,24 @@ static int open_sha1_file(struct repository *r, return -1; } +static int quick_has_loose(struct repository *r, + const unsigned char *sha1) +{ + int subdir_nr = sha1[0]; + struct object_id oid; + struct object_directory *odb; + + hashcpy(oid.hash, sha1); + + prepare_alt_odb(r); + for (odb = r->objects->odb; odb; odb = odb->next) { + odb_load_loose_cache(odb, subdir_nr); + if (oid_array_lookup(&odb->loose_objects_cache, &oid) >= 0) + return 1; + } + return 0; +} + /* * Map the loose object at "path" if it is not NULL, or the path found by * searching for a loose object named "sha1". @@ -1171,6 +1189,8 @@ static int sha1_loose_object_info(struct repository *r, if (!oi->typep && !oi->type_name && !oi->sizep && !oi->contentp) { const char *path; struct stat st; + if (!oi->disk_sizep && (flags & OBJECT_INFO_QUICK)) + return quick_has_loose(r, sha1) ? 0 : -1; if (stat_sha1_file(r, sha1, &st, &path) < 0) return -1; if (oi->disk_sizep) From patchwork Mon Nov 12 14:55:58 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 10678771 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 7824414BA for ; Mon, 12 Nov 2018 14:56:02 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 651D728FE2 for ; Mon, 12 Nov 2018 14:56:02 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 58C1729088; Mon, 12 Nov 2018 14:56:02 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id ECE2128FE2 for ; Mon, 12 Nov 2018 14:56:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727354AbeKMAtg (ORCPT ); Mon, 12 Nov 2018 19:49:36 -0500 Received: from cloud.peff.net ([104.130.231.41]:35802 "HELO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1726912AbeKMAtg (ORCPT ); Mon, 12 Nov 2018 19:49:36 -0500 Received: (qmail 29718 invoked by uid 109); 12 Nov 2018 14:56:00 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with SMTP; Mon, 12 Nov 2018 14:56:00 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 11482 invoked by uid 111); 12 Nov 2018 14:55:20 -0000 Received: from sigill.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.7) by peff.net (qpsmtpd/0.94) with (ECDHE-RSA-AES256-GCM-SHA384 encrypted) SMTP; Mon, 12 Nov 2018 09:55:20 -0500 Authentication-Results: peff.net; auth=none Received: by sigill.intra.peff.net (sSMTP sendmail emulation); Mon, 12 Nov 2018 09:55:58 -0500 Date: Mon, 12 Nov 2018 09:55:58 -0500 From: Jeff King To: Geert Jansen Cc: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Junio C Hamano , "git@vger.kernel.org" , =?utf-8?b?UmVuw6k=?= Scharfe , Takuto Ikuta Subject: [PATCH 9/9] fetch-pack: drop custom loose object cache Message-ID: <20181112145558.GI7400@sigill.intra.peff.net> References: <20181112144627.GA2478@sigill.intra.peff.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20181112144627.GA2478@sigill.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Commit 024aa4696c (fetch-pack.c: use oidset to check existence of loose object, 2018-03-14) added a cache to avoid calling stat() for a bunch of loose objects we don't have. Now that OBJECT_INFO_QUICK handles this caching itself, we can drop the custom solution. Note that this might perform slightly differently, as the original code stopped calling readdir() when we saw more loose objects than there were refs. So: 1. The old code might have spent work on readdir() to fill the cache, but then decided there were too many loose objects, wasting that effort. 2. The new code might spend a lot of time on readdir() if you have a lot of loose objects, even though there are very few objects to ask about. In practice it probably won't matter either way; see the previous commit for some discussion of the tradeoff. Signed-off-by: Jeff King --- fetch-pack.c | 39 ++------------------------------------- 1 file changed, 2 insertions(+), 37 deletions(-) diff --git a/fetch-pack.c b/fetch-pack.c index b3ed7121bc..25a88f4eb2 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -636,23 +636,6 @@ struct loose_object_iter { struct ref *refs; }; -/* - * If the number of refs is not larger than the number of loose objects, - * this function stops inserting. - */ -static int add_loose_objects_to_set(const struct object_id *oid, - const char *path, - void *data) -{ - struct loose_object_iter *iter = data; - oidset_insert(iter->loose_object_set, oid); - if (iter->refs == NULL) - return 1; - - iter->refs = iter->refs->next; - return 0; -} - /* * Mark recent commits available locally and reachable from a local ref as * COMPLETE. If args->no_dependents is false, also mark COMPLETE remote refs as @@ -670,30 +653,14 @@ static void mark_complete_and_common_ref(struct fetch_negotiator *negotiator, struct ref *ref; int old_save_commit_buffer = save_commit_buffer; timestamp_t cutoff = 0; - struct oidset loose_oid_set = OIDSET_INIT; - int use_oidset = 0; - struct loose_object_iter iter = {&loose_oid_set, *refs}; - - /* Enumerate all loose objects or know refs are not so many. */ - use_oidset = !for_each_loose_object(add_loose_objects_to_set, - &iter, 0); save_commit_buffer = 0; for (ref = *refs; ref; ref = ref->next) { struct object *o; - unsigned int flags = OBJECT_INFO_QUICK; - if (use_oidset && - !oidset_contains(&loose_oid_set, &ref->old_oid)) { - /* - * I know this does not exist in the loose form, - * so check if it exists in a non-loose form. - */ - flags |= OBJECT_INFO_IGNORE_LOOSE; - } - - if (!has_object_file_with_flags(&ref->old_oid, flags)) + if (!has_object_file_with_flags(&ref->old_oid, + OBJECT_INFO_QUICK)) continue; o = parse_object(the_repository, &ref->old_oid); if (!o) @@ -710,8 +677,6 @@ static void mark_complete_and_common_ref(struct fetch_negotiator *negotiator, } } - oidset_clear(&loose_oid_set); - if (!args->deepen) { for_each_ref(mark_complete_oid, NULL); for_each_cached_alternate(NULL, mark_alternate_complete);