From af3a67de014a145216ef06b588733cd0db002990 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Wed, 1 Aug 2018 15:18:34 +0000 Subject: [PATCH 1/2] negotiator: unknown fetch.negotiationAlgorithm should error out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change the handling of fetch.negotiationAlgorithm= to error out on unknown strings, i.e. everything except "default" or "skipping". This changes the behavior added in 42cc7485a2 ("negotiator/skipping: skip commits during fetch", 2018-07-16) which would ignore all unknown values and silently fall back to the "default" value. For a feature like this it's much better to produce an error than proceed. We don't want users to debug some amazingly slow fetch that should benefit from "skipping", only to find that they'd forgotten to deploy the new git version on that particular machine. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- Documentation/config.txt | 3 ++- fetch-negotiator.c | 12 +++++++++--- t/t5552-skipping-fetch-negotiator.sh | 23 +++++++++++++++++++++++ 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 53443fb9db..46e0400e24 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1496,9 +1496,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; any other value instructs Git to use the default algorithm + packfile; 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). + Unknown values will cause 'git fetch' to error out. format.attach:: Enable multipart/mixed attachments as the default for diff --git a/fetch-negotiator.c b/fetch-negotiator.c index 5d283049f4..d6d685cba0 100644 --- a/fetch-negotiator.c +++ b/fetch-negotiator.c @@ -6,9 +6,15 @@ void fetch_negotiator_init(struct fetch_negotiator *negotiator, const char *algorithm) { - if (algorithm && !strcmp(algorithm, "skipping")) { - skipping_negotiator_init(negotiator); - return; + if (algorithm) { + if (!strcmp(algorithm, "skipping")) { + skipping_negotiator_init(negotiator); + return; + } else if (!strcmp(algorithm, "default")) { + /* Fall through to default initialization */ + } else { + die("unknown fetch negotiation algorithm '%s'", algorithm); + } } default_negotiator_init(negotiator); } diff --git a/t/t5552-skipping-fetch-negotiator.sh b/t/t5552-skipping-fetch-negotiator.sh index 0a8e0e42ed..3b60bd44e0 100755 --- a/t/t5552-skipping-fetch-negotiator.sh +++ b/t/t5552-skipping-fetch-negotiator.sh @@ -47,6 +47,29 @@ test_expect_success 'commits with no parents are sent regardless of skip distanc have_not_sent c6 c4 c3 ' +test_expect_success 'unknown fetch.negotiationAlgorithm values error out' ' + rm -rf server client trace && + git init server && + test_commit -C server to_fetch && + + git init client && + test_commit -C client on_client && + git -C client checkout on_client && + + test_config -C client fetch.negotiationAlgorithm invalid && + test_must_fail git -C client fetch "$(pwd)/server" 2>err && + test_i18ngrep "unknown fetch negotiation algorithm" err && + + # Explicit "default" value + test_config -C client fetch.negotiationAlgorithm default && + git -C client -c fetch.negotiationAlgorithm=default fetch "$(pwd)/server" && + + # Implementation detail: If there is nothing to fetch, we will not error out + test_config -C client fetch.negotiationAlgorithm invalid && + git -C client fetch "$(pwd)/server" 2>err && + test_i18ngrep ! "unknown fetch negotiation algorithm" err +' + test_expect_success 'when two skips collide, favor the larger one' ' rm -rf server client trace && git init server && From 526608284a7600c5f0bcd6f5435ee680d79347ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Wed, 1 Aug 2018 15:18:35 +0000 Subject: [PATCH 2/2] fetch doc: cross-link two new negotiation options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Users interested in the fetch.negotiationAlgorithm variable added in 42cc7485a2 ("negotiator/skipping: skip commits during fetch", 2018-07-16) are probably interested in the related --negotiation-tip option added in 3390e42adb ("fetch-pack: support negotiation tip whitelist", 2018-07-02). Change the documentation for those two to reference one another to point readers in the right direction. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- Documentation/config.txt | 2 ++ Documentation/fetch-options.txt | 3 +++ 2 files changed, 5 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index 46e0400e24..3b7d3926d1 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1500,6 +1500,8 @@ fetch.negotiationAlgorithm:: that never skips commits (unless the server has acknowledged it or one of its descendants). Unknown values will cause 'git fetch' to error out. ++ +See also the `--negotiation-tip` option for linkgit:git-fetch[1]. format.attach:: Enable multipart/mixed attachments as the default for diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index 2d09f87b4b..8bc36af4b1 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -57,6 +57,9 @@ commits reachable from any of the given commits. The argument to this option may be a glob on ref names, a ref, or the (possibly abbreviated) SHA-1 of a commit. Specifying a glob is equivalent to specifying this option multiple times, one for each matching ref name. ++ +See also the `fetch.negotiationAlgorithm` configuration variable +documented in linkgit:git-config[1]. ifndef::git-pull[] --dry-run::