From patchwork Tue Aug 18 04:01:31 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Tan X-Patchwork-Id: 11719843 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 4FE33138C for ; Tue, 18 Aug 2020 04:02:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 33C922076D for ; Tue, 18 Aug 2020 04:02:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="JtV2vixi" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726145AbgHREBx (ORCPT ); Tue, 18 Aug 2020 00:01:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37702 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725554AbgHREBw (ORCPT ); Tue, 18 Aug 2020 00:01:52 -0400 Received: from mail-pg1-x54a.google.com (mail-pg1-x54a.google.com [IPv6:2607:f8b0:4864:20::54a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 98575C061389 for ; Mon, 17 Aug 2020 21:01:52 -0700 (PDT) Received: by mail-pg1-x54a.google.com with SMTP id 36so11381915pgz.18 for ; Mon, 17 Aug 2020 21:01:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=qi+9PZY30isaq82on5tkbS94W0BWjhHFDQvwqi9G7NQ=; b=JtV2vixigVjiqmLWICiego7zOFEFugGBYCLhGQL2zx/F27DbEKvghby65qGEmG7OyJ PSzEKAs/VGe1AY/frdmWstJojxOILiahSXahB8mbzo/tMMrpk6C0m42JRwwH5AlPnnsu 2e6L43P2G6QArEjZSmss84PXAQ5VYcfQCjUk3IhGiAH4bJ06feip5ini4/iF/ZYAsCZd qPDesuH/30ya+0ctXWGieIYAtZDo1MNfC9zni6o3tGYdm2oiSvt6VKywoIx1zuBF7OHK rhCd4KBHhcuQKAh5EC3k5TPj1xdO4zL7/TnVeJH9gHUp8QAq9CkP9HqaZ+UTAxSj5faI VCNw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=qi+9PZY30isaq82on5tkbS94W0BWjhHFDQvwqi9G7NQ=; b=CK9Ee2vHDWAU9SqgLTDfhMmxnOgEJDUu9UMTmgohYywRqPn4VZeCYCUI8Yda7i5hB0 ICksTjac0+qMZs8nIulRtrFLNeYBEZTvfwC/Tw8xbDyTOEBcngMTsFeLbfNpHEthNDgw NhTRewQ0ZnM2cM04pWjktrGy8qd9HLTuHMuGkJ9JfNVkKo9fHQxu90435Vn+kzGWoicA w27UdM7Ht8QohxEyXFvnP/+NeMiWbK/zxykA1VqxzMkd30i+J0Qf2fY/pRDzxHYv1A64 bL64vQfZEfBnbQ3v1q71DhC3NAkgwtOvkpUVCvgOEZWCj64bKK7N46F4yBDnG4oGrBon Uk2w== X-Gm-Message-State: AOAM5305YP7daATn+NMpgCptaXM1IXx/jsQEKFuKOUYXfvowA42Aeyrn K07PH6n6CXMFgpzjsB8cVO7c3u7pbVICQ4s32Z4F7mzk7j9+eIV1LcbSaWIpE5JHgwM/qBKVCKY +n/HI4E3YscxfJhba5znAbOtyuSbRJHpE1aNng1SOE9piYH+8vu5vo5l4x7TtHN94JIGNUslhyH Dh X-Google-Smtp-Source: ABdhPJwJ3pQ6Sx8z31YDMflUHDq7i0W4V5yhCj9Rk4EhbJM4ssio0l9eC5Gm4S2hLua6FPc0nbUUxecwN6/Fh0UbhaiV X-Received: by 2002:a17:90a:e508:: with SMTP id t8mr15110230pjy.32.1597723311795; Mon, 17 Aug 2020 21:01:51 -0700 (PDT) Date: Mon, 17 Aug 2020 21:01:31 -0700 In-Reply-To: Message-Id: <5b3b49d1236756dc65479f7e6c062479f2e19b3c.1597722941.git.jonathantanmy@google.com> Mime-Version: 1.0 References: <20200724223844.2723397-1-jonathantanmy@google.com> X-Mailer: git-send-email 2.28.0.220.ged08abb693-goog Subject: [PATCH v3 1/7] negotiator/noop: add noop fetch negotiator From: Jonathan Tan To: git@vger.kernel.org Cc: Jonathan Tan , stolee@gmail.com, gitster@pobox.com Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Add a noop fetch negotiator. This is introduced to allow partial clones to skip the unneeded negotiation step when fetching missing objects using a "git fetch" subprocess. (The implementation of spawning a "git fetch" subprocess will be done in a subsequent patch.) But this can also be useful for end users, e.g. as a blunt fix for object corruption. Signed-off-by: Jonathan Tan --- Documentation/config/fetch.txt | 5 +++- Makefile | 1 + fetch-negotiator.c | 5 ++++ negotiator/noop.c | 44 ++++++++++++++++++++++++++++++++ negotiator/noop.h | 8 ++++++ repo-settings.c | 2 ++ repository.h | 1 + t/t5554-noop-fetch-negotiator.sh | 22 ++++++++++++++++ 8 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 negotiator/noop.c create mode 100644 negotiator/noop.h create mode 100755 t/t5554-noop-fetch-negotiator.sh diff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt index 0aaa05e8c0..19383ddf7b 100644 --- a/Documentation/config/fetch.txt +++ b/Documentation/config/fetch.txt @@ -60,7 +60,10 @@ fetch.negotiationAlgorithm:: sent when negotiating the contents of the packfile to be sent by the server. Set to "skipping" to use an algorithm that skips commits in an effort to converge faster, but may result in a larger-than-necessary - packfile; The default is "default" which instructs Git to use the default algorithm + packfile; or set to "noop" to not send any information at all, which + will almost certainly result in a larger-than-necessary packfile, but + will skip the negotiation step. + The default is "default" which instructs Git to use the default algorithm that never skips commits (unless the server has acknowledged it or one of its descendants). If `feature.experimental` is enabled, then this setting defaults to "skipping". diff --git a/Makefile b/Makefile index 372139f1f2..94a8fb54d1 100644 --- a/Makefile +++ b/Makefile @@ -917,6 +917,7 @@ LIB_OBJS += mergesort.o LIB_OBJS += midx.o LIB_OBJS += name-hash.o LIB_OBJS += negotiator/default.o +LIB_OBJS += negotiator/noop.o LIB_OBJS += negotiator/skipping.o LIB_OBJS += notes-cache.o LIB_OBJS += notes-merge.o diff --git a/fetch-negotiator.c b/fetch-negotiator.c index 0a1357dc9d..57ed5784e1 100644 --- a/fetch-negotiator.c +++ b/fetch-negotiator.c @@ -2,6 +2,7 @@ #include "fetch-negotiator.h" #include "negotiator/default.h" #include "negotiator/skipping.h" +#include "negotiator/noop.h" #include "repository.h" void fetch_negotiator_init(struct repository *r, @@ -13,6 +14,10 @@ void fetch_negotiator_init(struct repository *r, skipping_negotiator_init(negotiator); return; + case FETCH_NEGOTIATION_NOOP: + noop_negotiator_init(negotiator); + return; + case FETCH_NEGOTIATION_DEFAULT: default: default_negotiator_init(negotiator); diff --git a/negotiator/noop.c b/negotiator/noop.c new file mode 100644 index 0000000000..60569b8350 --- /dev/null +++ b/negotiator/noop.c @@ -0,0 +1,44 @@ +#include "cache.h" +#include "noop.h" +#include "../commit.h" +#include "../fetch-negotiator.h" + +static void known_common(struct fetch_negotiator *n, struct commit *c) +{ + /* do nothing */ +} + +static void add_tip(struct fetch_negotiator *n, struct commit *c) +{ + /* do nothing */ +} + +static const struct object_id *next(struct fetch_negotiator *n) +{ + return NULL; +} + +static int ack(struct fetch_negotiator *n, struct commit *c) +{ + /* + * This negotiator does not emit any commits, so there is no commit to + * be acknowledged. If there is any ack, there is a bug. + */ + BUG("ack with noop negotiator, which does not emit any commits"); + return 0; +} + +static void release(struct fetch_negotiator *n) +{ + /* nothing to release */ +} + +void noop_negotiator_init(struct fetch_negotiator *negotiator) +{ + negotiator->known_common = known_common; + negotiator->add_tip = add_tip; + negotiator->next = next; + negotiator->ack = ack; + negotiator->release = release; + negotiator->data = NULL; +} diff --git a/negotiator/noop.h b/negotiator/noop.h new file mode 100644 index 0000000000..2b4ec5d07a --- /dev/null +++ b/negotiator/noop.h @@ -0,0 +1,8 @@ +#ifndef NEGOTIATOR_NOOP_H +#define NEGOTIATOR_NOOP_H + +struct fetch_negotiator; + +void noop_negotiator_init(struct fetch_negotiator *negotiator); + +#endif diff --git a/repo-settings.c b/repo-settings.c index 0918408b34..aa61a35338 100644 --- a/repo-settings.c +++ b/repo-settings.c @@ -39,6 +39,8 @@ void prepare_repo_settings(struct repository *r) if (!repo_config_get_string(r, "fetch.negotiationalgorithm", &strval)) { if (!strcasecmp(strval, "skipping")) r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_SKIPPING; + else if (!strcasecmp(strval, "noop")) + r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_NOOP; else r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_DEFAULT; } diff --git a/repository.h b/repository.h index 3c1f7d54bd..628c834367 100644 --- a/repository.h +++ b/repository.h @@ -23,6 +23,7 @@ enum fetch_negotiation_setting { FETCH_NEGOTIATION_NONE = 0, FETCH_NEGOTIATION_DEFAULT = 1, FETCH_NEGOTIATION_SKIPPING = 2, + FETCH_NEGOTIATION_NOOP = 3, }; struct repo_settings { diff --git a/t/t5554-noop-fetch-negotiator.sh b/t/t5554-noop-fetch-negotiator.sh new file mode 100755 index 0000000000..2ac7b5859e --- /dev/null +++ b/t/t5554-noop-fetch-negotiator.sh @@ -0,0 +1,22 @@ +#!/bin/sh + +test_description='test noop fetch negotiator' +. ./test-lib.sh + +test_expect_success 'noop negotiator does not emit any "have"' ' + rm -f trace && + + test_create_repo server && + test_commit -C server to_fetch && + + test_create_repo client && + test_commit -C client we_have && + + test_config -C client fetch.negotiationalgorithm noop && + GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch "$(pwd)/server" && + + ! grep "fetch> have" trace && + grep "fetch> done" trace +' + +test_done From patchwork Tue Aug 18 04:01:33 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Tan X-Patchwork-Id: 11719847 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id B48F015E4 for ; Tue, 18 Aug 2020 04:02:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 899982072A for ; Tue, 18 Aug 2020 04:02:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="W/IU4H3w" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726353AbgHREB6 (ORCPT ); Tue, 18 Aug 2020 00:01:58 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37716 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725554AbgHREBz (ORCPT ); Tue, 18 Aug 2020 00:01:55 -0400 Received: from mail-yb1-xb4a.google.com (mail-yb1-xb4a.google.com [IPv6:2607:f8b0:4864:20::b4a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C57DAC061389 for ; Mon, 17 Aug 2020 21:01:55 -0700 (PDT) Received: by mail-yb1-xb4a.google.com with SMTP id n19so8829159ybf.0 for ; Mon, 17 Aug 2020 21:01:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=VmRHp8hAbHUsjPYlGKbIckUXMeT/vlR+MBAv3XSf4Rc=; b=W/IU4H3wXyXmpzLyoWlk7DqFq6c0XNqc/mjjzVkds8pgxZRZ52QGEvIly3QAvs4c31 9cbUQTxqvhzAIsYx6Uwz7yHlqigLunE5ktZVp6yO8E3gPTLZNqpSdSODD7aBtXrQvEqx NSG99OsTDBLZqhLp+VuLiCMF2CXJMuN6ZqW0rZ14l35kaM4fpgoECyx+n+XTBnok2cte SYGv/+jR/P/fQILBVNWZa10JbUu9mswNjFWfWTiKYyT/chbkyIndhe2Pk6mE/085u6fG hEa5PYHfGHbBZUyqElsGiHjWseqH5O5fgVT2Y393XbVpnm2nDh2gSzbLnLDHEIWaVqhN k08Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=VmRHp8hAbHUsjPYlGKbIckUXMeT/vlR+MBAv3XSf4Rc=; b=nYuq6ONYZ2jSwvoglCqZ+vnM1xLIq5uQU4ehctwjVPbhGFu/4Vhk6QVA1KNDIZR3fZ tqGE50HqDCwbM9SuATOh6oBDTvWqeiDYXNnR8p/GlvPKwhI3TB6HSmiRY4E0YMjGJc97 3fea987ZUXHKJiDbhi3KEbqCERFUypXVP/26MT4PATt6B608WzFn6vhHmWMTlN3hPLIH 8HBGkvNGevhWMySy5G/0yyckkCABNEh4mbUswq0IWSar07GHvLYNpIMpb+Si3a5GoTve GTbN4o/dMxCl+1ttlWjrhJwdjJBrYbHZ5wLYcAieRh4d3OWQeaWxCLFFVfvoSaoq6KFn /osw== X-Gm-Message-State: AOAM5302pIJjHI/13hiKvf8T0X+/Ipbzy1dVWiBWC9g+kn01tM7ut5a0 6+6xQvEXD/dsI+BNnPRqQBXO/KgWntszawW+Vm2IcmuyrTTVXJU9rOHDc/jRDG0dlhY5/gq/qMK aOomNhxzOS890sLGBpmDrd4LL7xgZyRQJ74sTm5JGcDBLtajPOiUxjHLsyrzvqV0qkA0NcZ+ci4 1S X-Google-Smtp-Source: ABdhPJzS/mhcK5geskCtap7s5sz6uHidFpKiXwNDe17zs7HgP5IWP0J8bSxQWJcCvkWhq5dx3k95FPmRZgR+LDCclImC X-Received: by 2002:a25:7491:: with SMTP id p139mr26311547ybc.293.1597723314927; Mon, 17 Aug 2020 21:01:54 -0700 (PDT) Date: Mon, 17 Aug 2020 21:01:33 -0700 In-Reply-To: Message-Id: Mime-Version: 1.0 References: <20200724223844.2723397-1-jonathantanmy@google.com> X-Mailer: git-send-email 2.28.0.220.ged08abb693-goog Subject: [PATCH v3 3/7] fetch: avoid reading submodule config until needed From: Jonathan Tan To: git@vger.kernel.org Cc: Jonathan Tan , stolee@gmail.com, gitster@pobox.com Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In "fetch", there are two parameters submodule_fetch_jobs_config and recurse_submodules that can be set in a variety of ways: through .gitmodules, through .git/config, and through the command line. Currently "fetch" handles this by first reading .gitmodules, then reading .git/config (allowing it to overwrite existing values), then reading the command line (allowing it to overwrite existing values). Notice that we can avoid reading .gitmodules if .git/config and/or the command line already provides us with what we need. In addition, if recurse_submodules is found to be "no", we do not need the value of submodule_fetch_jobs_config. Avoiding reading .gitmodules is especially important when we use "git fetch" to perform lazy fetches in a partial clone because the .gitmodules file itself might need to be lazy fetched (and otherwise causing an infinite loop). In light of all this, avoid reading .gitmodules until necessary. When reading it, we may only need one of the two parameters it provides, so teach fetch_config_from_gitmodules() to support NULL arguments. With this patch, users (including Git itself when invoking "git fetch" to lazy-fetch) will be able to guarantee avoiding reading .gitmodules by passing --recurse-submodules=no. Signed-off-by: Jonathan Tan --- builtin/fetch.c | 10 ++++++++-- submodule-config.c | 8 ++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index a5498646bf..29db219c68 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1786,12 +1786,18 @@ int cmd_fetch(int argc, const char **argv, const char *prefix) free(anon); } - fetch_config_from_gitmodules(&submodule_fetch_jobs_config, - &recurse_submodules); git_config(git_fetch_config, NULL); argc = parse_options(argc, argv, prefix, builtin_fetch_options, builtin_fetch_usage, 0); + if (recurse_submodules != RECURSE_SUBMODULES_OFF) { + int *sfjc = submodule_fetch_jobs_config == -1 + ? &submodule_fetch_jobs_config : NULL; + int *rs = recurse_submodules == RECURSE_SUBMODULES_DEFAULT + ? &recurse_submodules : NULL; + + fetch_config_from_gitmodules(sfjc, rs); + } if (deepen_relative) { if (deepen_relative < 0) diff --git a/submodule-config.c b/submodule-config.c index e175dfbc38..c569e22aa3 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -777,10 +777,14 @@ static int gitmodules_fetch_config(const char *var, const char *value, void *cb) { struct fetch_config *config = cb; if (!strcmp(var, "submodule.fetchjobs")) { - *(config->max_children) = parse_submodule_fetchjobs(var, value); + if (config->max_children) + *(config->max_children) = + parse_submodule_fetchjobs(var, value); return 0; } else if (!strcmp(var, "fetch.recursesubmodules")) { - *(config->recurse_submodules) = parse_fetch_recurse_submodules_arg(var, value); + if (config->recurse_submodules) + *(config->recurse_submodules) = + parse_fetch_recurse_submodules_arg(var, value); return 0; } From patchwork Tue Aug 18 04:01:34 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Tan X-Patchwork-Id: 11719849 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id E2CA017D4 for ; Tue, 18 Aug 2020 04:02:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BB7F72072A for ; Tue, 18 Aug 2020 04:02:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="lfiVYwxY" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726381AbgHRECF (ORCPT ); Tue, 18 Aug 2020 00:02:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37724 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726318AbgHREB5 (ORCPT ); Tue, 18 Aug 2020 00:01:57 -0400 Received: from mail-pl1-x64a.google.com (mail-pl1-x64a.google.com [IPv6:2607:f8b0:4864:20::64a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 23C54C061389 for ; Mon, 17 Aug 2020 21:01:57 -0700 (PDT) Received: by mail-pl1-x64a.google.com with SMTP id v10so11415887plp.5 for ; Mon, 17 Aug 2020 21:01:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=5hNEHEENUI4eo6zzDwPOVmARFBBOlvUqOQ7IQvMHKrQ=; b=lfiVYwxYe1PRc7B1iOXBWwzucdS4uE/r/uNNOpIzxUaaIO0IhjYf93EsOrPBsnPzFT 8fdcT1eHncH/naZwOsqtcROUpEA8SDvASSPeq1YYXuoTzrcEllDoP1cNA/b01HDr/E0s 9mdiyfQR9vbzenMxQ6ZyetL4JFde+q6wcuIYrCV2pRo24OW8KkxThLboV6wnxsa1vQEr WspPKqqHS7BEPwYBrR1Oyn8A2EBhRP8OvdArbkvw/VUmF6irzvtp5OfRDs/j0YcabfRy kKtl+4Zq3+yvdYBy257C+G5sXehHkGCg89rHQGDfCiPTFc4fXd3bi83CsORZkv5lyHAg cmAQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=5hNEHEENUI4eo6zzDwPOVmARFBBOlvUqOQ7IQvMHKrQ=; b=G1qoY2JKqYF6SUpvWMSxnzDBJncIT8d6pULNdu7gPbeVzQUQLbpvuoslk5+qFlAdOJ UzPv9URpuc+HdnBW1E4ZERtLk1Zkyi9cJfHLHIIINHTqssLyTOHi5ruFpM4mmYjLXr75 sSQXa66trVYHvBguQwkzo0Rw5J7Jdl94QV5NsZYH8a0yGdDj9TcxQ19BqiE2t88S+qL9 7vkdH2VGmcxWe6BmmdtX0FdfMhVvjnPk7RWMEABfMtfwTP7jLMQ8I0YoKr46mLfKw3eL I9dQATUh3dwRH+ZL54rpfDWJ//sPCvwW3VN4t5NVc9vhIvyXWiqHATSoDn89NNP1yQln pcmA== X-Gm-Message-State: AOAM5336ak5T4eRqklV7KAW4rD/x1tyU76NgDVs/trgVUk+PoLC39FFR XWfnNYjkZENGtDkujaiOgZifrcf6e2H3d7ghV/lvwTdG/Ap5V67bvT8FvuCrneHI8xD4RumxZdH tA8FhCKOcRIiJIwEA8P305HfV+EBLcpHCDpw5TqZfY98sLNlRFsAD3wi90FSmyhmZAvFHNUn8yK 1k X-Google-Smtp-Source: ABdhPJwcm2JVKTXnON222Vz8sWummVhPxxVEcsXeETkCmB8kSLoE/CSMnYbSVrLp0AmvIIRp3X8eo0WpkfgB1Ncaar7u X-Received: by 2002:a17:90b:3509:: with SMTP id ls9mr14846478pjb.230.1597723316511; Mon, 17 Aug 2020 21:01:56 -0700 (PDT) Date: Mon, 17 Aug 2020 21:01:34 -0700 In-Reply-To: Message-Id: Mime-Version: 1.0 References: <20200724223844.2723397-1-jonathantanmy@google.com> X-Mailer: git-send-email 2.28.0.220.ged08abb693-goog Subject: [PATCH v3 4/7] fetch: only populate existing_refs if needed From: Jonathan Tan To: git@vger.kernel.org Cc: Jonathan Tan , stolee@gmail.com, gitster@pobox.com Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In "fetch", get_ref_map() iterates over all refs to populate "existing_refs" in order to populate peer_ref->old_oid in the returned refmap, even if the refmap has no peer_ref set - which is the case when only literal hashes (i.e. no refs by name) are fetched. Iterating over refs causes the targets of those refs to be checked for existence. Avoiding this is especially important when we use "git fetch" to perform lazy fetches in a partial clone because a target of such a ref may need to be itself lazy-fetched (and otherwise causing an infinite loop). Therefore, avoid populating "existing_refs" until necessary. With this patch, because Git lazy-fetches objects by literal hashes (to be done in a subsequent commit), it will then be able to guarantee avoiding reading targets of refs. Signed-off-by: Jonathan Tan --- builtin/fetch.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 29db219c68..6460ce3f4e 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -449,6 +449,7 @@ static struct ref *get_ref_map(struct remote *remote, struct ref *orefs = NULL, **oref_tail = &orefs; struct hashmap existing_refs; + int existing_refs_populated = 0; if (rs->nr) { struct refspec *fetch_refspec; @@ -542,15 +543,18 @@ static struct ref *get_ref_map(struct remote *remote, ref_map = ref_remove_duplicates(ref_map); - refname_hash_init(&existing_refs); - for_each_ref(add_one_refname, &existing_refs); - for (rm = ref_map; rm; rm = rm->next) { if (rm->peer_ref) { const char *refname = rm->peer_ref->name; struct refname_hash_entry *peer_item; unsigned int hash = strhash(refname); + if (!existing_refs_populated) { + refname_hash_init(&existing_refs); + for_each_ref(add_one_refname, &existing_refs); + existing_refs_populated = 1; + } + peer_item = hashmap_get_entry_from_hash(&existing_refs, hash, refname, struct refname_hash_entry, ent); @@ -560,7 +564,8 @@ static struct ref *get_ref_map(struct remote *remote, } } } - hashmap_free_entries(&existing_refs, struct refname_hash_entry, ent); + if (existing_refs_populated) + hashmap_free_entries(&existing_refs, struct refname_hash_entry, ent); return ref_map; } From patchwork Tue Aug 18 04:01:35 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Tan X-Patchwork-Id: 11719853 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 19BE3175D for ; Tue, 18 Aug 2020 04:02:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id EBF472072A for ; Tue, 18 Aug 2020 04:02:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="JfGZtUM0" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726398AbgHRECH (ORCPT ); Tue, 18 Aug 2020 00:02:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37726 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726367AbgHREB7 (ORCPT ); Tue, 18 Aug 2020 00:01:59 -0400 Received: from mail-pf1-x449.google.com (mail-pf1-x449.google.com [IPv6:2607:f8b0:4864:20::449]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B6C0FC061342 for ; Mon, 17 Aug 2020 21:01:58 -0700 (PDT) Received: by mail-pf1-x449.google.com with SMTP id k12so12083343pfu.19 for ; Mon, 17 Aug 2020 21:01:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=7kwDGmF+PzWiqEmYDOFO62Dh/L031HUwJTUuZ7F64qc=; b=JfGZtUM0z7AXBDo9gwtqGyGrv6q2iI+X1oMe+boid1SSgphODFXuH1US7OIIIBkRfZ juHY3dcZYGwzQQUn+Jc7gyMkkgGmQTeF3Kdh2Rn5/HuiFZCTYxNlx0q7nL1OmQu6wQ06 drit59d2GS3DxzcS+EmT7PdrPBJe82JRfNFTb/x5zj1Q7dSuoNI7Y/YkQz5ZxWSppohl bu6l6GP8argQeMLmxU13MISFGp1oAAtstwSdh4tOyMiSQ0q174tGLbUAgT7JoWDo47Gf nQx3000fFcEqhnOCO8PfA5rprCBtYRrRDRCMzuijDycKMQJGNLxmakh0fv2WDy0n0Nwn lcuA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=7kwDGmF+PzWiqEmYDOFO62Dh/L031HUwJTUuZ7F64qc=; b=ttgRcflnKGd2TALEsjzl5U/Qlr9VaVrlXrBthaXwz/k0BOJy6q5qwglNBM8wQN5YNG BET4e7HxoY/lLN7GDSiM3E8PdzScZK8rtaRXe96P0fzkPjRE9BiQ42a/j77s8/2tkuAq Dche6OTeoUaFdZQXjUmksXVYFswkIWolAhmYlv0fFpYV+HYUMUKf3STRTPbLrzWIMGE5 q0kaVGI4wA1eo++sOzeDy4JPA5E8zRoKu5eEChBjHmtfzL2fv7uSdNSJw3g4jxO6wOXG R7b3kQgp54jFm8yzkWF2IXS5JkMotdGEYy+l6C8SaHK3O8qwsRSjC9WzB1I4lHLI/QLe sEEA== X-Gm-Message-State: AOAM530Vn/yo39nA9gPfOueFeyJz0XtnDHMs8xeyv9MfAeDaWWSw6b1E rBID9AP6wcz3kpt+qFO1x/a377IFJ3c41QIFXx4PAJ5xItPfWW0Ble579FWx2kBnBE5mw/yhxgz PMqJBL82oxcIzUz2l/bgr2p9PC5U8brghHwCJPfByTWs9lAjRGu/kUEO9kEaEBtiJgdItQ5DuB8 Ht X-Google-Smtp-Source: ABdhPJzIiHsU1MjukbvYGtOA2pcg6NgZA9roZdGDutRQ7+aYm3ASOLOkEOWzcESFi7Zw7aOYhvd0IJP4yUG144QO1mWL X-Received: by 2002:a17:90a:9405:: with SMTP id r5mr15405927pjo.74.1597723318130; Mon, 17 Aug 2020 21:01:58 -0700 (PDT) Date: Mon, 17 Aug 2020 21:01:35 -0700 In-Reply-To: Message-Id: Mime-Version: 1.0 References: <20200724223844.2723397-1-jonathantanmy@google.com> X-Mailer: git-send-email 2.28.0.220.ged08abb693-goog Subject: [PATCH v3 5/7] fetch-pack: do not lazy-fetch during ref iteration From: Jonathan Tan To: git@vger.kernel.org Cc: Jonathan Tan , stolee@gmail.com, gitster@pobox.com Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In order to determine negotiation tips, "fetch-pack" iterates over all refs and dereferences all annotated tags found. This causes the existence of targets of refs and annotated tags to be checked. Avoiding this is especially important when we use "git fetch" (which invokes "fetch-pack") to perform lazy fetches in a partial clone because a target of such a ref or annotated tag may need to be itself lazy-fetched (and otherwise causing an infinite loop). Therefore, teach "fetch-pack" not to lazy fetch whenever iterating over refs. This is done by using the raw form of ref iteration and by dereferencing tags ourselves. Signed-off-by: Jonathan Tan --- fetch-pack.c | 79 ++++++++++++++++++++++------------------ t/t5616-partial-clone.sh | 20 ++++++++++ 2 files changed, 64 insertions(+), 35 deletions(-) diff --git a/fetch-pack.c b/fetch-pack.c index 80fb3bd899..707bbc31fd 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -108,24 +108,48 @@ static void for_each_cached_alternate(struct fetch_negotiator *negotiator, cb(negotiator, cache.items[i]); } +static struct commit *deref_without_lazy_fetch(const struct object_id *oid, + int mark_tags_complete) +{ + enum object_type type; + struct object_info info = { .typep = &type }; + + while (1) { + if (oid_object_info_extended(the_repository, oid, &info, + OBJECT_INFO_SKIP_FETCH_OBJECT | OBJECT_INFO_QUICK)) + return NULL; + if (type == OBJ_TAG) { + struct tag *tag = (struct tag *) + parse_object(the_repository, oid); + + if (!tag->tagged) + return NULL; + if (mark_tags_complete) + tag->object.flags |= COMPLETE; + oid = &tag->tagged->oid; + } else { + break; + } + } + if (type == OBJ_COMMIT) + return (struct commit *) parse_object(the_repository, oid); + return NULL; +} + static int rev_list_insert_ref(struct fetch_negotiator *negotiator, - const char *refname, const struct object_id *oid) { - struct object *o = deref_tag(the_repository, - parse_object(the_repository, oid), - refname, 0); - - if (o && o->type == OBJ_COMMIT) - negotiator->add_tip(negotiator, (struct commit *)o); + struct commit *c = deref_without_lazy_fetch(oid, 0); + if (c) + negotiator->add_tip(negotiator, c); return 0; } static int rev_list_insert_ref_oid(const char *refname, const struct object_id *oid, int flag, void *cb_data) { - return rev_list_insert_ref(cb_data, refname, oid); + return rev_list_insert_ref(cb_data, oid); } enum ack_type { @@ -201,7 +225,7 @@ static void send_request(struct fetch_pack_args *args, static void insert_one_alternate_object(struct fetch_negotiator *negotiator, struct object *obj) { - rev_list_insert_ref(negotiator, NULL, &obj->oid); + rev_list_insert_ref(negotiator, &obj->oid); } #define INITIAL_FLUSH 16 @@ -230,13 +254,12 @@ static void mark_tips(struct fetch_negotiator *negotiator, int i; if (!negotiation_tips) { - for_each_ref(rev_list_insert_ref_oid, negotiator); + for_each_rawref(rev_list_insert_ref_oid, negotiator); return; } for (i = 0; i < negotiation_tips->nr; i++) - rev_list_insert_ref(negotiator, NULL, - &negotiation_tips->oid[i]); + rev_list_insert_ref(negotiator, &negotiation_tips->oid[i]); return; } @@ -503,21 +526,11 @@ static struct commit_list *complete; static int mark_complete(const struct object_id *oid) { - struct object *o = parse_object(the_repository, oid); - - while (o && o->type == OBJ_TAG) { - struct tag *t = (struct tag *) o; - if (!t->tagged) - break; /* broken repository */ - o->flags |= COMPLETE; - o = parse_object(the_repository, &t->tagged->oid); - } - if (o && o->type == OBJ_COMMIT) { - struct commit *commit = (struct commit *)o; - if (!(commit->object.flags & COMPLETE)) { - commit->object.flags |= COMPLETE; - commit_list_insert(commit, &complete); - } + struct commit *commit = deref_without_lazy_fetch(oid, 1); + + if (commit && !(commit->object.flags & COMPLETE)) { + commit->object.flags |= COMPLETE; + commit_list_insert(commit, &complete); } return 0; } @@ -702,7 +715,7 @@ static void mark_complete_and_common_ref(struct fetch_negotiator *negotiator, */ trace2_region_enter("fetch-pack", "mark_complete_local_refs", NULL); if (!args->deepen) { - for_each_ref(mark_complete_oid, NULL); + for_each_rawref(mark_complete_oid, NULL); for_each_cached_alternate(NULL, mark_alternate_complete); commit_list_sort_by_date(&complete); if (cutoff) @@ -716,16 +729,12 @@ static void mark_complete_and_common_ref(struct fetch_negotiator *negotiator, */ trace2_region_enter("fetch-pack", "mark_common_remote_refs", NULL); for (ref = *refs; ref; ref = ref->next) { - struct object *o = deref_tag(the_repository, - lookup_object(the_repository, - &ref->old_oid), - NULL, 0); + struct commit *c = deref_without_lazy_fetch(&ref->old_oid, 0); - if (!o || o->type != OBJ_COMMIT || !(o->flags & COMPLETE)) + if (!c || !(c->object.flags & COMPLETE)) continue; - negotiator->known_common(negotiator, - (struct commit *)o); + negotiator->known_common(negotiator, c); } trace2_region_leave("fetch-pack", "mark_common_remote_refs", NULL); diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh index 8a27452a51..e53492d595 100755 --- a/t/t5616-partial-clone.sh +++ b/t/t5616-partial-clone.sh @@ -384,6 +384,26 @@ test_expect_success 'fetch lazy-fetches only to resolve deltas, protocol v2' ' grep "want $(cat hash)" trace ' +test_expect_success 'fetch does not lazy-fetch missing targets of its refs' ' + rm -rf server client trace && + + test_create_repo server && + test_config -C server uploadpack.allowfilter 1 && + test_config -C server uploadpack.allowanysha1inwant 1 && + test_commit -C server foo && + + git clone --filter=blob:none "file://$(pwd)/server" client && + # Make all refs point to nothing by deleting all objects. + rm client/.git/objects/pack/* && + + test_commit -C server bar && + GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \ + --no-tags --recurse-submodules=no \ + origin refs/tags/bar && + FOO_HASH=$(git -C server rev-parse foo) && + ! grep "want $FOO_HASH" trace +' + # The following two tests must be in this order. It is important that # the srv.bare repository did not have tags during clone, but has tags # in the fetch. From patchwork Tue Aug 18 04:01:36 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Tan X-Patchwork-Id: 11719851 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 3F4E61894 for ; Tue, 18 Aug 2020 04:02:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 20C512072A for ; Tue, 18 Aug 2020 04:02:17 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="wHDLiKlw" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726422AbgHRECJ (ORCPT ); Tue, 18 Aug 2020 00:02:09 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37738 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725554AbgHRECB (ORCPT ); Tue, 18 Aug 2020 00:02:01 -0400 Received: from mail-yb1-xb49.google.com (mail-yb1-xb49.google.com [IPv6:2607:f8b0:4864:20::b49]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D439EC061343 for ; Mon, 17 Aug 2020 21:02:00 -0700 (PDT) Received: by mail-yb1-xb49.google.com with SMTP id e12so20765658ybc.18 for ; Mon, 17 Aug 2020 21:02:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=RDeUk6T5Cdlun3NuSjpMkJH6p/HD+IaMGum9baql2Gk=; b=wHDLiKlwis932n/K2rXQuRg3YoZSBgGtclTZlAKyAcesrWvulce4ZRU7bebwvGyDoy XXteqizC4FS9Hi5EDr606kArSjnEAKsseAuR6LxjkJWbRp/z4mx5a+IjTpLhV1o7s9a/ rgPeLKj2GXBR6Hf/G31goZTOakQVr//XwlM+rhgpMPkkrVpWGh2IuLJRbrfGQqCErXY4 d2E0QzdV9Flett4v0VXofDyguSSMvGnxi8vL9Bab9zEOM5T7g2KE8QBL3S9xcZZrR5C+ J96s9v3YaUpbdaxkxRjBzSWVY9Y8FfcGhDGwklRGT4oOMezWOTPTG7eh/3TCQGCU53Y7 Opxg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=RDeUk6T5Cdlun3NuSjpMkJH6p/HD+IaMGum9baql2Gk=; b=pzpbv9fQXdy3S/JQiUrM4X69jwbWjmFu+Mi/yXbykns0JWBOGrt8SdC8nOMy/+8aFx elN2IEFQj5b7jrZp78CPRv92mVZfT11Q8vITPhJtNyC+1GduLw9rfJI2mKPKIOja9aaL A3b8eQRtLx0rjZUvmNy5bPa4yQyZVc+mIYFBMP80GGNwPrFZUW3s3wmg+OMVvihDEHeC 3soVDSTAhhkPzLEjQqwFJSSk+DjzL2Er2nHd0zIXhGwfsW5w+kWdKs2KxCCED01a8MDR Vm5wD+/yq6WfvhApEJnZYA6I1hRiezkMbCpr/2SnV60gOnnlYmXuqWhiL/EpUpHV4Knf +mWQ== X-Gm-Message-State: AOAM530WApLsYsuIKGpghRoWsoym0Gqc0c7fybQQWb+hFYpl2JE+GZrq DpTviAD6wkH/rmctCpmiUZGrIE4DjslgHrQTX/L4931MFEZyuQ6OZ1JgaZb3TQUMV0TnROFk5cu P9SULMHApnFP66j8stZ+u9vcASq3KmYOTTmLSbv/Ugiec9hG5fxr6d76Rh/mUkdmGVUz68OBVn0 G8 X-Google-Smtp-Source: ABdhPJzfTtRIkkyhGL5MmYSajXJYEWctwevteVJTbg2tA0OqPTzIgvA9V709ZKIl7T7nj98cw5cyuNtCYkZN90Iexckb X-Received: by 2002:a25:187:: with SMTP id 129mr1513722ybb.448.1597723319942; Mon, 17 Aug 2020 21:01:59 -0700 (PDT) Date: Mon, 17 Aug 2020 21:01:36 -0700 In-Reply-To: Message-Id: <4e72ca62583a2938509511818ce6bef8af24c271.1597722942.git.jonathantanmy@google.com> Mime-Version: 1.0 References: <20200724223844.2723397-1-jonathantanmy@google.com> X-Mailer: git-send-email 2.28.0.220.ged08abb693-goog Subject: [PATCH v3 6/7] promisor-remote: lazy-fetch objects in subprocess From: Jonathan Tan To: git@vger.kernel.org Cc: Jonathan Tan , stolee@gmail.com, gitster@pobox.com Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Teach Git to lazy-fetch missing objects in a subprocess instead of doing it in-process. This allows any fatal errors that occur during the fetch to be isolated and converted into an error return value, instead of causing the current command being run to terminate. Signed-off-by: Jonathan Tan --- Documentation/technical/partial-clone.txt | 13 ++----- promisor-remote.c | 46 +++++++++++------------ t/t0410-partial-clone.sh | 2 +- t/t4067-diff-partial-clone.sh | 8 ++-- t/t5601-clone.sh | 2 +- 5 files changed, 30 insertions(+), 41 deletions(-) diff --git a/Documentation/technical/partial-clone.txt b/Documentation/technical/partial-clone.txt index b9e17e7a28..0780d30cac 100644 --- a/Documentation/technical/partial-clone.txt +++ b/Documentation/technical/partial-clone.txt @@ -171,20 +171,13 @@ additional flag. Fetching Missing Objects ------------------------ -- Fetching of objects is done using the existing transport mechanism using - transport_fetch_refs(), setting a new transport option - TRANS_OPT_NO_DEPENDENTS to indicate that only the objects themselves are - desired, not any object that they refer to. -+ -Because some transports invoke fetch_pack() in the same process, fetch_pack() -has been updated to not use any object flags when the corresponding argument -(no_dependents) is set. +- Fetching of objects is done by invoking a "git fetch" subprocess. - The local repository sends a request with the hashes of all requested - objects as "want" lines, and does not perform any packfile negotiation. + objects, and does not perform any packfile negotiation. It then receives a packfile. -- Because we are reusing the existing fetch-pack mechanism, fetching +- Because we are reusing the existing fetch mechanism, fetching currently fetches all objects referred to by the requested objects, even though they are not necessary. diff --git a/promisor-remote.c b/promisor-remote.c index baaea12fd6..6e647610e9 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -3,6 +3,7 @@ #include "promisor-remote.h" #include "config.h" #include "transport.h" +#include "argv-array.h" static char *repository_format_partial_clone; static const char *core_partial_clone_filter_default; @@ -12,39 +13,34 @@ void set_repository_format_partial_clone(char *partial_clone) repository_format_partial_clone = xstrdup_or_null(partial_clone); } -static int fetch_refs(const char *remote_name, struct ref *ref) -{ - struct remote *remote; - struct transport *transport; - int res; - - remote = remote_get(remote_name); - if (!remote->url[0]) - die(_("Remote with no URL")); - transport = transport_get(remote, remote->url[0]); - - transport_set_option(transport, TRANS_OPT_FROM_PROMISOR, "1"); - transport_set_option(transport, TRANS_OPT_NO_DEPENDENTS, "1"); - res = transport_fetch_refs(transport, ref); - - return res; -} - static int fetch_objects(const char *remote_name, const struct object_id *oids, int oid_nr) { - struct ref *ref = NULL; + struct child_process child = CHILD_PROCESS_INIT; int i; + FILE *child_in; + + child.git_cmd = 1; + child.in = -1; + argv_array_pushl(&child.args, "-c", "fetch.negotiationAlgorithm=null", + "fetch", remote_name, "--no-tags", + "--no-write-fetch-head", "--recurse-submodules=no", + "--filter=blob:none", "--stdin", NULL); + if (start_command(&child)) + die(_("promisor-remote: unable to fork off fetch subprocess")); + child_in = xfdopen(child.in, "w"); for (i = 0; i < oid_nr; i++) { - struct ref *new_ref = alloc_ref(oid_to_hex(&oids[i])); - oidcpy(&new_ref->old_oid, &oids[i]); - new_ref->exact_oid = 1; - new_ref->next = ref; - ref = new_ref; + if (fputs(oid_to_hex(&oids[i]), child_in) < 0) + die_errno(_("promisor-remote: could not write to fetch subprocess")); + if (fputc('\n', child_in) < 0) + die_errno(_("promisor-remote: could not write to fetch subprocess")); } - return fetch_refs(remote_name, ref); + + if (fclose(child_in) < 0) + die_errno(_("promisor-remote: could not close stdin to fetch subprocess")); + return finish_command(&child) ? -1 : 0; } static struct promisor_remote *promisors; diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh index 463dc3a8be..3e454f934e 100755 --- a/t/t0410-partial-clone.sh +++ b/t/t0410-partial-clone.sh @@ -203,7 +203,7 @@ test_expect_success 'fetching of missing objects works with ref-in-want enabled' rm -rf repo/.git/objects/* && rm -f trace && GIT_TRACE_PACKET="$(pwd)/trace" git -C repo cat-file -p "$HASH" && - grep "git< fetch=.*ref-in-want" trace + grep "fetch< fetch=.*ref-in-want" trace ' test_expect_success 'fetching of missing objects from another promisor remote' ' diff --git a/t/t4067-diff-partial-clone.sh b/t/t4067-diff-partial-clone.sh index ef8e0e9cb0..804f2a82e8 100755 --- a/t/t4067-diff-partial-clone.sh +++ b/t/t4067-diff-partial-clone.sh @@ -20,7 +20,7 @@ test_expect_success 'git show batches blobs' ' # Ensure that there is exactly 1 negotiation by checking that there is # only 1 "done" line sent. ("done" marks the end of negotiation.) GIT_TRACE_PACKET="$(pwd)/trace" git -C client show HEAD && - grep "git> done" trace >done_lines && + grep "fetch> done" trace >done_lines && test_line_count = 1 done_lines ' @@ -44,7 +44,7 @@ test_expect_success 'diff batches blobs' ' # Ensure that there is exactly 1 negotiation by checking that there is # only 1 "done" line sent. ("done" marks the end of negotiation.) GIT_TRACE_PACKET="$(pwd)/trace" git -C client diff HEAD^ HEAD && - grep "git> done" trace >done_lines && + grep "fetch> done" trace >done_lines && test_line_count = 1 done_lines ' @@ -127,7 +127,7 @@ test_expect_success 'diff with rename detection batches blobs' ' # only 1 "done" line sent. ("done" marks the end of negotiation.) GIT_TRACE_PACKET="$(pwd)/trace" git -C client diff --raw -M HEAD^ HEAD >out && grep ":100644 100644.*R[0-9][0-9][0-9].*b.*c" out && - grep "git> done" trace >done_lines && + grep "fetch> done" trace >done_lines && test_line_count = 1 done_lines ' @@ -175,7 +175,7 @@ test_expect_success 'diff --break-rewrites fetches only if necessary, and batche # by checking that there is only 1 "done" line sent. ("done" marks the # end of negotiation.) GIT_TRACE_PACKET="$(pwd)/trace" git -C client diff --break-rewrites --raw -M HEAD^ HEAD && - grep "git> done" trace >done_lines && + grep "fetch> done" trace >done_lines && test_line_count = 1 done_lines ' diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index 84ea2a3eb7..f82b0dbb5a 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -702,7 +702,7 @@ test_expect_success 'batch missing blob request during checkout' ' # Ensure that there is only one negotiation by checking that there is # only "done" line sent. ("done" marks the end of negotiation.) GIT_TRACE_PACKET="$(pwd)/trace" git -C client checkout HEAD^ && - grep "git> done" trace >done_lines && + grep "fetch> done" trace >done_lines && test_line_count = 1 done_lines ' From patchwork Tue Aug 18 04:01:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Tan X-Patchwork-Id: 11719855 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 60CED1805 for ; Tue, 18 Aug 2020 04:02:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 44C922072A for ; Tue, 18 Aug 2020 04:02:17 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="Rt+nJC1S" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726435AbgHRECK (ORCPT ); Tue, 18 Aug 2020 00:02:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37740 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726370AbgHRECC (ORCPT ); Tue, 18 Aug 2020 00:02:02 -0400 Received: from mail-pg1-x54a.google.com (mail-pg1-x54a.google.com [IPv6:2607:f8b0:4864:20::54a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 402BCC061344 for ; Mon, 17 Aug 2020 21:02:02 -0700 (PDT) Received: by mail-pg1-x54a.google.com with SMTP id x3so11418204pga.4 for ; Mon, 17 Aug 2020 21:02:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=XMvRLHX//Q6xP55ShLnMeec/Aq4eZwCgRc6ZtZ1n7AM=; b=Rt+nJC1SHExZkvoXwNDGKh6MsvWo+PDNjeMCJ3MeWaiMnP86KwwQxQlgNCFxb2wRp1 QmGTsIfoNC2PhaMzXcHS9LeeZWMCMnWblt8L2oqqxXJcyus1NRumFiExTccDc7q0b6cA vfpuXv8O50wUC3dzAhf0sEj9Nsdue7yIEtzLb38wdYWmN3x9tTESBzLNyrl1GI/znqvn t8UZIHG1B2PkeotmQhtVyBBYlB95kGdwZyMk3agTV/rxJOhYzxg6Sve270JekLi/pW9e unlxk8INX8VP0aKbFtcLCS7BOL076kahiRbh4QVv/2UQVGxJoP1Vn8UPBObZgtdzsfhe Iyzg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=XMvRLHX//Q6xP55ShLnMeec/Aq4eZwCgRc6ZtZ1n7AM=; b=KBMh8Vpb4wQcvDqgMTz3stk0r4OQ5aGzYBkwEUnkRlGh2mhA7xqKfL2EJiJJU6W27j 35QviOHynKfpcmkJeMO1UtTG5GG6O2A8YwtsKwvgLmA82eFNN0gBljRzEG9PvhWtmKlo mYo7xDUDGbSZ+zdjy8r4RT4n5mU0zOQZyibHsZXuojwVigye6mwaTTrBb22JG+vAw5i6 u29MaY/cB594Cpl2rkGvLkNmkCDOwbH6/NtQNV+OWdmLD3py+1iVpgg0yNDOcgOASJOW t8Oujligp7iARCnEWIZ22EqM5xcgl73mtmC5HLTHJmK8pzIjvxXE8ldpmLGLiCSQR2xV o5fQ== X-Gm-Message-State: AOAM530558i/beZwUpCYo9oHafkrtqHpIwZZ0pUBUhpWJ6kpITTmQEgv FG7MJZc/Sg6kn6rk/m0UcGB6VSwbeU+3UvuxmQqudQlA1sFN6E96DEYYFP18iNWGGi7b+VI0hIh 2U3x9S4lMYGnMHOrcrcOG/NWwGBtz7y/Otq6aTWjiUR1ElsqnaTSSE9rkV51Brdx/gje977h927 sb X-Google-Smtp-Source: ABdhPJy6NVadq3pQZk4SpndqmusFmsM0QWk+ypkUo4jKpdIJc4sEG9AMG+jYd4bUesKZyfwrOkl12kF0peAyOEx6LYWn X-Received: by 2002:a17:90b:4a07:: with SMTP id kk7mr15705209pjb.125.1597723321610; Mon, 17 Aug 2020 21:02:01 -0700 (PDT) Date: Mon, 17 Aug 2020 21:01:37 -0700 In-Reply-To: Message-Id: <3ff9d034e91f394457e1c1813437603e70741aa6.1597722942.git.jonathantanmy@google.com> Mime-Version: 1.0 References: <20200724223844.2723397-1-jonathantanmy@google.com> X-Mailer: git-send-email 2.28.0.220.ged08abb693-goog Subject: [PATCH v3 7/7] fetch-pack: remove no_dependents code From: Jonathan Tan To: git@vger.kernel.org Cc: Jonathan Tan , stolee@gmail.com, gitster@pobox.com Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Now that Git has switched to using a subprocess to lazy-fetch missing objects, remove the no_dependents code as it is no longer used. Signed-off-by: Jonathan Tan --- builtin/fetch-pack.c | 4 -- fetch-pack.c | 110 ++++++++++++------------------------------- fetch-pack.h | 14 ------ remote-curl.c | 6 --- transport.c | 4 -- transport.h | 7 --- 6 files changed, 30 insertions(+), 115 deletions(-) diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index bbb5c96167..58b7c1fbdc 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -153,10 +153,6 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) args.from_promisor = 1; continue; } - if (!strcmp("--no-dependents", arg)) { - args.no_dependents = 1; - continue; - } if (skip_prefix(arg, ("--" CL_ARG__FILTER "="), &arg)) { parse_list_objects_filter(&args.filter_options, arg); continue; diff --git a/fetch-pack.c b/fetch-pack.c index 707bbc31fd..3212957dae 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -285,10 +285,8 @@ static int find_common(struct fetch_negotiator *negotiator, PACKET_READ_CHOMP_NEWLINE | PACKET_READ_DIE_ON_ERR_PACKET); - if (!args->no_dependents) { - mark_tips(negotiator, args->negotiation_tips); - for_each_cached_alternate(negotiator, insert_one_alternate_object); - } + mark_tips(negotiator, args->negotiation_tips); + for_each_cached_alternate(negotiator, insert_one_alternate_object); fetching = 0; for ( ; refs ; refs = refs->next) { @@ -305,12 +303,8 @@ static int find_common(struct fetch_negotiator *negotiator, * We use lookup_object here because we are only * interested in the case we *know* the object is * reachable and we have already scanned it. - * - * Do this only if args->no_dependents is false (if it is true, - * we cannot trust the object flags). */ - if (!args->no_dependents && - ((o = lookup_object(the_repository, remote)) != NULL) && + if (((o = lookup_object(the_repository, remote)) != NULL) && (o->flags & COMPLETE)) { continue; } @@ -410,8 +404,6 @@ static int find_common(struct fetch_negotiator *negotiator, trace2_region_enter("fetch-pack", "negotiation_v0_v1", the_repository); flushes = 0; retval = -1; - if (args->no_dependents) - goto done; while ((oid = negotiator->next(negotiator))) { packet_buf_write(&req_buf, "have %s\n", oid_to_hex(oid)); print_verbose(args, "have %s", oid_to_hex(oid)); @@ -666,9 +658,7 @@ struct loose_object_iter { /* * 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 - * COMMON_REF (otherwise, we are not planning to participate in negotiation, and - * thus do not need COMMON_REF marks). + * COMPLETE. * * The cutoff time for recency is determined by this heuristic: it is the * earliest commit time of the objects in refs that are commits and that we know @@ -969,12 +959,8 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args, struct fetch_negotiator negotiator_alloc; struct fetch_negotiator *negotiator; - if (args->no_dependents) { - negotiator = NULL; - } else { - negotiator = &negotiator_alloc; - fetch_negotiator_init(r, negotiator); - } + negotiator = &negotiator_alloc; + fetch_negotiator_init(r, negotiator); sort_ref_list(&ref, ref_compare_name); QSORT(sought, nr_sought, cmp_ref_by_name); @@ -1062,15 +1048,11 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args, if (!server_supports_hash(the_hash_algo->name, NULL)) die(_("Server does not support this repository's object format")); - if (!args->no_dependents) { - mark_complete_and_common_ref(negotiator, args, &ref); - filter_refs(args, &ref, sought, nr_sought); - if (everything_local(args, &ref)) { - packet_flush(fd[1]); - goto all_done; - } - } else { - filter_refs(args, &ref, sought, nr_sought); + mark_complete_and_common_ref(negotiator, args, &ref); + filter_refs(args, &ref, sought, nr_sought); + if (everything_local(args, &ref)) { + packet_flush(fd[1]); + goto all_done; } if (find_common(negotiator, args, fd, &oid, ref) < 0) if (!args->keep_pack) @@ -1119,7 +1101,7 @@ static void add_shallow_requests(struct strbuf *req_buf, packet_buf_write(req_buf, "deepen-relative\n"); } -static void add_wants(int no_dependents, const struct ref *wants, struct strbuf *req_buf) +static void add_wants(const struct ref *wants, struct strbuf *req_buf) { int use_ref_in_want = server_supports_feature("fetch", "ref-in-want", 0); @@ -1136,12 +1118,8 @@ static void add_wants(int no_dependents, const struct ref *wants, struct strbuf * We use lookup_object here because we are only * interested in the case we *know* the object is * reachable and we have already scanned it. - * - * Do this only if args->no_dependents is false (if it is true, - * we cannot trust the object flags). */ - if (!no_dependents && - ((o = lookup_object(the_repository, remote)) != NULL) && + if (((o = lookup_object(the_repository, remote)) != NULL) && (o->flags & COMPLETE)) { continue; } @@ -1275,19 +1253,14 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out, } /* add wants */ - add_wants(args->no_dependents, wants, &req_buf); + add_wants(wants, &req_buf); - if (args->no_dependents) { - packet_buf_write(&req_buf, "done"); - ret = 1; - } else { - /* Add all of the common commits we've found in previous rounds */ - add_common(&req_buf, common); + /* Add all of the common commits we've found in previous rounds */ + add_common(&req_buf, common); - /* Add initial haves */ - ret = add_haves(negotiator, seen_ack, &req_buf, - haves_to_send, in_vain); - } + /* Add initial haves */ + ret = add_haves(negotiator, seen_ack, &req_buf, + haves_to_send, in_vain); /* Send request */ packet_buf_flush(&req_buf); @@ -1547,12 +1520,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, struct string_list packfile_uris = STRING_LIST_INIT_DUP; int i; - if (args->no_dependents) { - negotiator = NULL; - } else { - negotiator = &negotiator_alloc; - fetch_negotiator_init(r, negotiator); - } + negotiator = &negotiator_alloc; + fetch_negotiator_init(r, negotiator); packet_reader_init(&reader, fd[0], NULL, 0, PACKET_READ_CHOMP_NEWLINE | @@ -1576,21 +1545,16 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, args->deepen = 1; /* Filter 'ref' by 'sought' and those that aren't local */ - if (!args->no_dependents) { - mark_complete_and_common_ref(negotiator, args, &ref); - filter_refs(args, &ref, sought, nr_sought); - if (everything_local(args, &ref)) - state = FETCH_DONE; - else - state = FETCH_SEND_REQUEST; - - mark_tips(negotiator, args->negotiation_tips); - for_each_cached_alternate(negotiator, - insert_one_alternate_object); - } else { - filter_refs(args, &ref, sought, nr_sought); + mark_complete_and_common_ref(negotiator, args, &ref); + filter_refs(args, &ref, sought, nr_sought); + if (everything_local(args, &ref)) + state = FETCH_DONE; + else state = FETCH_SEND_REQUEST; - } + + mark_tips(negotiator, args->negotiation_tips); + for_each_cached_alternate(negotiator, + insert_one_alternate_object); break; case FETCH_SEND_REQUEST: if (!negotiation_started) { @@ -1911,20 +1875,6 @@ struct ref *fetch_pack(struct fetch_pack_args *args, if (nr_sought) nr_sought = remove_duplicates_in_refs(sought, nr_sought); - if (args->no_dependents && !args->filter_options.choice) { - /* - * The protocol does not support requesting that only the - * wanted objects be sent, so approximate this by setting a - * "blob:none" filter if no filter is already set. This works - * for all object types: note that wanted blobs will still be - * sent because they are directly specified as a "want". - * - * NEEDSWORK: Add an option in the protocol to request that - * only the wanted objects be sent, and implement it. - */ - parse_list_objects_filter(&args->filter_options, "blob:none"); - } - if (version != protocol_v2 && !ref) { packet_flush(fd[1]); die(_("no matching remote head")); diff --git a/fetch-pack.h b/fetch-pack.h index 85d1e39fe7..bbe2938059 100644 --- a/fetch-pack.h +++ b/fetch-pack.h @@ -42,20 +42,6 @@ struct fetch_pack_args { unsigned deepen:1; unsigned from_promisor:1; - /* - * Attempt to fetch only the wanted objects, and not any objects - * referred to by them. Due to protocol limitations, extraneous - * objects may still be included. (When fetching non-blob - * objects, only blobs are excluded; when fetching a blob, the - * blob itself will still be sent. The client does not need to - * know whether a wanted object is a blob or not.) - * - * If 1, fetch_pack() will also not modify any object flags. - * This allows fetch_pack() to safely be called by any function, - * regardless of which object flags it uses (if any). - */ - unsigned no_dependents:1; - /* * Because fetch_pack() overwrites the shallow file upon a * successful deepening non-clone fetch, if this struct diff --git a/remote-curl.c b/remote-curl.c index 5cbc6e5002..a0c81a64bc 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -40,7 +40,6 @@ struct options { push_cert : 2, deepen_relative : 1, from_promisor : 1, - no_dependents : 1, atomic : 1, object_format : 1; const struct git_hash_algo *hash_algo; @@ -186,9 +185,6 @@ static int set_option(const char *name, const char *value) } else if (!strcmp(name, "from-promisor")) { options.from_promisor = 1; return 0; - } else if (!strcmp(name, "no-dependents")) { - options.no_dependents = 1; - return 0; } else if (!strcmp(name, "filter")) { options.filter = xstrdup(value); return 0; @@ -1171,8 +1167,6 @@ static int fetch_git(struct discovery *heads, argv_array_push(&args, "--deepen-relative"); if (options.from_promisor) argv_array_push(&args, "--from-promisor"); - if (options.no_dependents) - argv_array_push(&args, "--no-dependents"); if (options.filter) argv_array_pushf(&args, "--filter=%s", options.filter); argv_array_push(&args, url.buf); diff --git a/transport.c b/transport.c index b41386eccb..32e1f21f0c 100644 --- a/transport.c +++ b/transport.c @@ -232,9 +232,6 @@ static int set_git_option(struct git_transport_options *opts, } else if (!strcmp(name, TRANS_OPT_FROM_PROMISOR)) { opts->from_promisor = !!value; return 0; - } else if (!strcmp(name, TRANS_OPT_NO_DEPENDENTS)) { - opts->no_dependents = !!value; - return 0; } else if (!strcmp(name, TRANS_OPT_LIST_OBJECTS_FILTER)) { list_objects_filter_die_if_populated(&opts->filter_options); parse_list_objects_filter(&opts->filter_options, value); @@ -359,7 +356,6 @@ static int fetch_refs_via_pack(struct transport *transport, args.cloning = transport->cloning; args.update_shallow = data->options.update_shallow; args.from_promisor = data->options.from_promisor; - args.no_dependents = data->options.no_dependents; args.filter_options = data->options.filter_options; args.stateless_rpc = transport->stateless_rpc; args.server_options = transport->server_options; diff --git a/transport.h b/transport.h index b3c30133ea..7aa1f33145 100644 --- a/transport.h +++ b/transport.h @@ -16,7 +16,6 @@ struct git_transport_options { unsigned update_shallow : 1; unsigned deepen_relative : 1; unsigned from_promisor : 1; - unsigned no_dependents : 1; /* * If this transport supports connect or stateless-connect, @@ -201,12 +200,6 @@ void transport_check_allowed(const char *type); /* Indicate that these objects are being fetched by a promisor */ #define TRANS_OPT_FROM_PROMISOR "from-promisor" -/* - * Indicate that only the objects wanted need to be fetched, not their - * dependents - */ -#define TRANS_OPT_NO_DEPENDENTS "no-dependents" - /* Filter objects for partial clone and fetch */ #define TRANS_OPT_LIST_OBJECTS_FILTER "filter"