Merge branch 'en/fetch-negotiation-default-fix'

Interaction between fetch.negotiationAlgorithm and
feature.experimental configuration variables has been corrected.

* en/fetch-negotiation-default-fix:
  repo-settings: rename the traditional default fetch.negotiationAlgorithm
  repo-settings: fix error handling for unknown values
  repo-settings: fix checking for fetch.negotiationAlgorithm=default

diff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt
index 63748c02b72afe5211a710e963c2be502888545e..cd65d236b43ffc3a7c08fa9df480fcbe6c82e68c 100644 (file)
--- a/Documentation/config/fetch.txt
+++ b/Documentation/config/fetch.txt
@@ -56,18 +56,19 @@ fetch.output::
        OUTPUT in linkgit:git-fetch[1] for detail.
 
 fetch.negotiationAlgorithm::
-       Control how information about the commits in the local repository is
-       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; 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".
-       Unknown values will cause 'git fetch' to error out.
+       Control how information about the commits in the local repository
+       is sent when negotiating the contents of the packfile to be sent by
+       the server.  Set to "consecutive" to use an algorithm that walks
+       over consecutive commits checking each one.  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; 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.  Set to "default" to override settings made
+       previously and use the default behaviour.  The default is normally
+       "consecutive", but if `feature.experimental` is true, then the
+       default is "skipping".  Unknown values will cause 'git fetch' to
+       error out.
 +
 See also the `--negotiate-only` and `--negotiation-tip` options to
 linkgit:git-fetch[1].

diff --git a/fetch-negotiator.c b/fetch-negotiator.c
index 273390229fe4c442915e911b421339e2391fc37c..874797d767bb1abd8144be9412e7e258f3d8ad9d 100644 (file)
--- a/fetch-negotiator.c
+++ b/fetch-negotiator.c
@@ -18,7 +18,7 @@ void fetch_negotiator_init(struct repository *r,
                noop_negotiator_init(negotiator);
                return;
 
-       case FETCH_NEGOTIATION_DEFAULT:
+       case FETCH_NEGOTIATION_CONSECUTIVE:
                default_negotiator_init(negotiator);
                return;
        }

diff --git a/repo-settings.c b/repo-settings.c
index 00ca5571a1ab77e8a1b2d505d59e6ff59d629dd9..b4fbd16cdcc251386a1b77a6920fdbad923cc1ef 100644 (file)
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -26,7 +26,7 @@ void prepare_repo_settings(struct repository *r)
        /* Defaults */
        r->settings.index_version = -1;
        r->settings.core_untracked_cache = UNTRACKED_CACHE_KEEP;
-       r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_DEFAULT;
+       r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_CONSECUTIVE;
 
        /* Booleans config or default, cascades to other settings */
        repo_cfg_bool(r, "feature.manyfiles", &manyfiles, 0);
@@ -81,10 +81,17 @@ void prepare_repo_settings(struct repository *r)
        }
 
        if (!repo_config_get_string(r, "fetch.negotiationalgorithm", &strval)) {
+               int fetch_default = r->settings.fetch_negotiation_algorithm;
                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 if (!strcasecmp(strval, "consecutive"))
+                       r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_CONSECUTIVE;
+               else if (!strcasecmp(strval, "default"))
+                       r->settings.fetch_negotiation_algorithm = fetch_default;
+               else
+                       die("unknown fetch negotiation algorithm '%s'", strval);
        }
 
        /*

diff --git a/repository.h b/repository.h
index 2b5cf97f31e88934d9b23d96a97cffc60a9606fd..ca837cb9e914aaaf85c39f323ae671ab4eb8c8cb 100644 (file)
--- a/repository.h
+++ b/repository.h
@@ -20,7 +20,7 @@ enum untracked_cache_setting {
 };
 
 enum fetch_negotiation_setting {
-       FETCH_NEGOTIATION_DEFAULT,
+       FETCH_NEGOTIATION_CONSECUTIVE,
        FETCH_NEGOTIATION_SKIPPING,
        FETCH_NEGOTIATION_NOOP,
 };

diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index f0dc4e696860a4435a1be1182457325290bb1eb4..ee6d2dde9f35677fb9c83a228839bb7ad405a4fa 100755 (executable)
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -927,7 +927,8 @@ test_expect_success 'fetching deepen' '
        )
 '
 
-test_expect_success 'use ref advertisement to prune "have" lines sent' '
+test_negotiation_algorithm_default () {
+       test_when_finished rm -rf clientv0 clientv2 &&
        rm -rf server client &&
        git init server &&
        test_commit -C server both_have_1 &&
@@ -946,7 +947,7 @@ test_expect_success 'use ref advertisement to prune "have" lines sent' '
        rm -f trace &&
        cp -r client clientv0 &&
        GIT_TRACE_PACKET="$(pwd)/trace" git -C clientv0 \
-               fetch origin server_has both_have_2 &&
+               "$@" fetch origin server_has both_have_2 &&
        grep "have $(git -C client rev-parse client_has)" trace &&
        grep "have $(git -C client rev-parse both_have_2)" trace &&
        ! grep "have $(git -C client rev-parse both_have_2^)" trace &&
@@ -954,10 +955,27 @@ test_expect_success 'use ref advertisement to prune "have" lines sent' '
        rm -f trace &&
        cp -r client clientv2 &&
        GIT_TRACE_PACKET="$(pwd)/trace" git -C clientv2 -c protocol.version=2 \
-               fetch origin server_has both_have_2 &&
+               "$@" fetch origin server_has both_have_2 &&
        grep "have $(git -C client rev-parse client_has)" trace &&
        grep "have $(git -C client rev-parse both_have_2)" trace &&
        ! grep "have $(git -C client rev-parse both_have_2^)" trace
+}
+
+test_expect_success 'use ref advertisement to prune "have" lines sent' '
+       test_negotiation_algorithm_default
+'
+
+test_expect_success 'same as last but with config overrides' '
+       test_negotiation_algorithm_default \
+               -c feature.experimental=true \
+               -c fetch.negotiationAlgorithm=consecutive
+'
+
+test_expect_success 'ensure bogus fetch.negotiationAlgorithm yields error' '
+       test_when_finished rm -rf clientv0 &&
+       cp -r client clientv0 &&
+       test_must_fail git -C clientv0 --fetch.negotiationAlgorithm=bogus \
+                      fetch origin server_has both_have_2
 '
 
 test_expect_success 'filtering by size' '