This regression is not new in v2.35; it first appeared in v2.34. So, not urgent. Changes since v3: * 'consecutive' is used as the traditional default. 'default' means either 'consecutive' or 'skipping' depending on feature.experimental. Changes since v2: * Also fix the fact that fetch.negotationAlgorithm=$BOGUS_VALUE no longer errors out (yet another regression, this one dating back to v2.24.0), and add a test to make sure we don't regress it again. * Add 'consecutive' as a synonym for 'default', and remove 'default' from the documentation to guide people towards using 'consecutive' when they want the classic behavior. Changes since v1: * Put the common code in two testcases into a function, and then just invoked it from each Elijah Newren (3): repo-settings: fix checking for fetch.negotiationAlgorithm=default repo-settings: fix error handling for unknown values repo-settings: rename the traditional default fetch.negotiationAlgorithm Documentation/config/fetch.txt | 25 +++++++++++++------------ fetch-negotiator.c | 2 +- repo-settings.c | 9 ++++++++- repository.h | 2 +- t/t5500-fetch-pack.sh | 24 +++++++++++++++++++++--- 5 files changed, 44 insertions(+), 18 deletions(-) base-commit: 89bece5c8c96f0b962cfc89e63f82d603fd60bed Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1131%2Fnewren%2Ffix-fetch-negotiation-algorithm-equals-default-v4 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1131/newren/fix-fetch-negotiation-algorithm-equals-default-v4 Pull-Request: https://github.com/gitgitgadget/git/pull/1131 Range-diff vs v3: 1: df0ec5ffe98 = 1: df0ec5ffe98 repo-settings: fix checking for fetch.negotiationAlgorithm=default 2: 23f692b81be = 2: 23f692b81be repo-settings: fix error handling for unknown values 3: 7b28c527a90 ! 3: 7500a4d2e44 repo-settings: name the default fetch.negotiationAlgorithm 'consecutive' @@ Metadata Author: Elijah Newren <newren@xxxxxxxxx> ## Commit message ## - repo-settings: name the default fetch.negotiationAlgorithm 'consecutive' + repo-settings: rename the traditional default fetch.negotiationAlgorithm - Give the default fetch.negotiationAlgorithm the name 'consecutive' and - update the documentation accordingly. Since there may be some users - using the name 'default' for this behavior, retain that name for now. - We do not want to use that name indefinitely, though, because if - 'skipping' becomes the default, then the "default" behavior will not be - the default behavior, which would be confusing. + Give the traditional default fetch.negotiationAlgorithm the name + 'consecutive'. Also allow a choice of 'default' to have Git decide + between the choices (currently, picking 'skipping' if + feature.experimental is true and 'consecutive' otherwise). Update the + documentation accordingly. Signed-off-by: Elijah Newren <newren@xxxxxxxxx> @@ Documentation/config/fetch.txt: fetch.output:: - 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 @@ Documentation/config/fetch.txt: fetch.output:: + 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 normally "consecutive", but -+ if `feature.experimental` is true, then the default is "skipping". - Unknown values will cause 'git fetch' to error out. ++ 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]. ## fetch-negotiator.c ## @@ fetch-negotiator.c: void fetch_negotiator_init(struct repository *r, @@ repo-settings.c: void prepare_repo_settings(struct repository *r) /* Booleans config or default, cascades to other settings */ repo_cfg_bool(r, "feature.manyfiles", &manyfiles, 0); @@ repo-settings.c: 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, "default")) -- r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_DEFAULT; -+ else if (!strcasecmp(strval, "consecutive") || -+ !strcasecmp(strval, "default")) ++ else if (!strcasecmp(strval, "consecutive")) + r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_CONSECUTIVE; + else if (!strcasecmp(strval, "default")) +- r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_DEFAULT; ++ r->settings.fetch_negotiation_algorithm = fetch_default; else die("unknown fetch negotiation algorithm '%s'", strval); } -- gitgitgadget