Skip to content

Commit 4b7e17a

Browse files
dschodscho
authored
Work around t5799 failure with curl v8.16.0 (#801)
I investigated a couple of new test failures over in #799; Essentially, t5799.13(curl-connect: no server) no longer received the expected `CURLE_COULDNT_CONNECT` immediately, but instead received `CURLE_OPERATION_TIMEDOUT` after 5 minutes (!), and due to internal details that problem was repeated 4 times (!!!). Turns out that the regression was _not_ introduced by the patches in that PR, but by the independent Git for Windows SDK update of cURL to v8.16.0. I integrated a fix into the Git for Windows SDK in git-for-windows/MINGW-packages#163, but since Microsoft Git also targets Linux and macOS (where we do not control the cURL version), here is a work-around. This PR also adds some clean-ups for the `t5799-gvfs-helper.sh` test script in general, e.g. to improve future debug'ability.
2 parents 5707d2c + b79ae3c commit 4b7e17a

File tree

2 files changed

+81
-21
lines changed

2 files changed

+81
-21
lines changed

‎gvfs-helper.c‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,7 @@
254254
#include "wrapper.h"
255255
#include "packfile.h"
256256
#include "date.h"
257+
#include "versioncmp.h"
257258

258259
#define TR2_CAT "gvfs-helper"
259260

@@ -286,6 +287,11 @@ static const char *const server_usage[] = {
286287
NULL
287288
};
288289

290+
static const char *const curl_version_usage[] = {
291+
N_("git gvfs-helper [<main_options>] curl-version [<operator> <version>]"),
292+
NULL
293+
};
294+
289295
/*
290296
* "commitDepth" field in gvfs protocol
291297
*/
@@ -375,6 +381,7 @@ static struct gh__global {
375381
int cache_server_is_initialized; /* did sub-command look for one */
376382
int main_creds_need_approval; /* try to only approve them once */
377383

384+
unsigned long connect_timeout_ms;
378385
} gh__global;
379386

380387
enum gh__server_type {
@@ -2964,6 +2971,10 @@ static void do_req(const char *url_base,
29642971
curl_easy_setopt(slot->curl, CURLOPT_NOPROGRESS, 1L);
29652972
}
29662973

2974+
if (gh__global.connect_timeout_ms)
2975+
curl_easy_setopt(slot->curl, CURLOPT_CONNECTTIMEOUT_MS,
2976+
gh__global.connect_timeout_ms);
2977+
29672978
gh__run_one_slot(slot, params, status);
29682979
strbuf_release(&rest_url);
29692980
}
@@ -3672,6 +3683,9 @@ static enum gh__error_code do_sub_cmd__get(int argc, const char **argv)
36723683
static struct option get_options[] = {
36733684
OPT_INTEGER('r', "max-retries", &gh__cmd_opts.max_retries,
36743685
N_("retries for transient network errors")),
3686+
OPT_UNSIGNED(0, "connect-timeout-ms",
3687+
&gh__global.connect_timeout_ms,
3688+
N_("try to connect only for this many milliseconds")),
36753689
OPT_END(),
36763690
};
36773691

@@ -4148,6 +4162,37 @@ static enum gh__error_code do_sub_cmd__server(int argc, const char **argv)
41484162
return ec;
41494163
}
41504164

4165+
static enum gh__error_code do_sub_cmd__curl_version(int argc, const char **argv)
4166+
{
4167+
static struct option curl_version_options[] = {
4168+
OPT_END(),
4169+
};
4170+
const char *current_version = curl_version_info(CURLVERSION_NOW)->version;
4171+
4172+
trace2_cmd_mode("curl-version");
4173+
4174+
if (argc > 1 && !strcmp(argv[1], "-h"))
4175+
usage_with_options(curl_version_usage, curl_version_options);
4176+
4177+
argc = parse_options(argc, argv, NULL,
4178+
curl_version_options, curl_version_usage, 0);
4179+
4180+
if (argc == 0)
4181+
printf("%s\n", current_version);
4182+
else if (argc != 2)
4183+
die("expected [<operator> <version>], but got %d parameters", argc);
4184+
else {
4185+
int cmp = versioncmp(current_version, argv[1]);
4186+
4187+
return (strchr(argv[0], '=') && !cmp) ||
4188+
(strchr(argv[0], '>') && cmp > 0) ||
4189+
(strchr(argv[0], '<') && cmp < 0) ?
4190+
GH__ERROR_CODE__OK : GH__ERROR_CODE__ERROR;
4191+
}
4192+
4193+
return GH__ERROR_CODE__OK;
4194+
}
4195+
41514196
static enum gh__error_code do_sub_cmd(int argc, const char **argv)
41524197
{
41534198
if (!strcmp(argv[0], "get"))
@@ -4172,6 +4217,9 @@ static enum gh__error_code do_sub_cmd(int argc, const char **argv)
41724217
if (!strcmp(argv[0], "server"))
41734218
return do_sub_cmd__server(argc, argv);
41744219

4220+
if (!strcmp(argv[0], "curl-version"))
4221+
return do_sub_cmd__curl_version(argc, argv);
4222+
41754223
return GH__ERROR_CODE__USAGE;
41764224
}
41774225

‎t/t5799-gvfs-helper.sh‎

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ verify_objects_in_shared_cache () {
368368
# See if any of the objects are missing from repo_t1.
369369
#
370370
git -C "$REPO_T1" cat-file --batch-check <"$1" >OUT.bc_actual || return 1
371-
grep -q " missing" OUT.bc_actual && return 1
371+
test_grep " missing" OUT.bc_actual && return 1
372372
#
373373
# See if any of the objects have different sizes or types than repo_src.
374374
#
@@ -540,7 +540,7 @@ test_expect_success 'basic: GET gvfs/config' '
540540
# The cache-server URL should be listed in the gvfs/config output.
541541
# We confirm this before assuming error-mode will work.
542542
#
543-
grep -q "$CACHE_URL" OUT.output
543+
test_grep "$CACHE_URL" OUT.output
544544
'
545545

546546
test_expect_success 'basic: GET cache-server multi-get error-mode' '
@@ -799,14 +799,14 @@ test_expect_success 'basic: PREFETCH up-to-date' '
799799
#################################################################
800800

801801
mayhem_observed__close__connections () {
802-
if $(grep -q "transient" OUT.stderr)
802+
if grep "transient" OUT.stderr
803803
then
804804
# Transient errors should retry.
805805
# 1 for initial request + 2 retries.
806806
#
807807
verify_connection_count 3
808808
return $?
809-
elif $(grep -q "hard_fail" OUT.stderr)
809+
elif grep "hard_fail" OUT.stderr
810810
then
811811
# Hard errors should not retry.
812812
#
@@ -837,10 +837,10 @@ mayhem_observed__close () {
837837
# going to verify the connection counts based upon what type of error
838838
# gvfs-helper claimed it to be.
839839
#
840-
if $(grep -q "error: get: (curl:18)" OUT.stderr) ||
841-
$(grep -q "error: get: (curl:52)" OUT.stderr) ||
842-
$(grep -q "error: get: (curl:55)" OUT.stderr) ||
843-
$(grep -q "error: get: (curl:56)" OUT.stderr)
840+
if grep "error: get: (curl:18)" OUT.stderr ||
841+
grep "error: get: (curl:52)" OUT.stderr ||
842+
grep "error: get: (curl:55)" OUT.stderr ||
843+
grep "error: get: (curl:56)" OUT.stderr
844844
then
845845
mayhem_observed__close__connections
846846
return $?
@@ -850,9 +850,25 @@ mayhem_observed__close () {
850850
fi
851851
}
852852

853+
test_lazy_prereq CURL_8_16_0 '
854+
git gvfs-helper curl-version = 8.16.0 ||
855+
test 8.15.0-DEV = "$(git gvfs-helper curl-version)"
856+
'
857+
853858
test_expect_success 'curl-error: no server' '
854859
test_when_finished "per_test_cleanup" &&
855860
861+
connect_timeout_ms= &&
862+
# CURLE_COULDNT_CONNECT 7
863+
regex="error: get: (curl:7)" &&
864+
if test_have_prereq CURL_8_16_0
865+
then
866+
connect_timeout_ms=--connect-timeout-ms=200 &&
867+
# CURLE_COULDNT_CONNECT 7
868+
# CURLE_OPERATION_TIMEDOUT 28
869+
regex="error: get: (curl:\(7\|28\))"
870+
fi &&
871+
856872
# Try to do a multi-get without a server.
857873
#
858874
# Use small max-retry value because of exponential backoff,
@@ -864,10 +880,9 @@ test_expect_success 'curl-error: no server' '
864880
--remote=origin \
865881
get \
866882
--max-retries=2 \
883+
$connect_timeout_ms \
867884
<"$OIDS_FILE" >OUT.output 2>OUT.stderr &&
868-
869-
# CURLE_COULDNT_CONNECT 7
870-
grep -q "error: get: (curl:7)" OUT.stderr
885+
test_grep "$regex" OUT.stderr
871886
'
872887

873888
test_expect_success 'curl-error: close socket while reading request' '
@@ -980,7 +995,7 @@ test_expect_success 'http-error: 503 Service Unavailable (with retry)' '
980995
981996
stop_gvfs_protocol_server &&
982997
983-
grep -q "error: get: (http:503)" OUT.stderr &&
998+
test_grep "error: get: (http:503)" OUT.stderr &&
984999
verify_connection_count 3
9851000
'
9861001

@@ -998,7 +1013,7 @@ test_expect_success 'http-error: 429 Service Unavailable (with retry)' '
9981013
9991014
stop_gvfs_protocol_server &&
10001015
1001-
grep -q "error: get: (http:429)" OUT.stderr &&
1016+
test_grep "error: get: (http:429)" OUT.stderr &&
10021017
verify_connection_count 3
10031018
'
10041019

@@ -1016,7 +1031,7 @@ test_expect_success 'http-error: 404 Not Found (no retry)' '
10161031
10171032
stop_gvfs_protocol_server &&
10181033
1019-
grep -q "error: get: (http:404)" OUT.stderr &&
1034+
test_grep "error: get: (http:404)" OUT.stderr &&
10201035
verify_connection_count 1
10211036
'
10221037

@@ -1082,7 +1097,7 @@ test_expect_success 'http-error: 503 Service Unavailable (with retry and fallbac
10821097
10831098
stop_gvfs_protocol_server &&
10841099
1085-
grep -q "error: get: (http:503)" OUT.stderr &&
1100+
test_grep "error: get: (http:503)" OUT.stderr &&
10861101
verify_connection_count 6
10871102
'
10881103

@@ -1113,7 +1128,7 @@ test_expect_success 'http-error: 503 Service Unavailable (with retry and no-fall
11131128
11141129
stop_gvfs_protocol_server &&
11151130
1116-
grep -q "error: get: (http:503)" OUT.stderr &&
1131+
test_grep "error: get: (http:503)" OUT.stderr &&
11171132
verify_connection_count 3
11181133
'
11191134

@@ -1123,10 +1138,7 @@ test_expect_success 'http-error: 503 Service Unavailable (with retry and no-fall
11231138
#################################################################
11241139

11251140
test_lazy_prereq CURL_7_75_OR_NEWER '
1126-
case "$(curl version | sed -n "1s/^curl \([^ ]*\).*/\1/p")" in
1127-
""|[0-6].*|7.[0-9]*.*|7.[1-6][0-9].*|7.7[0-4]*.*) return 1;;
1128-
*) return 0;;
1129-
esac
1141+
git gvfs-helper curl-version ">=" 7.75.0
11301142
'
11311143

11321144
test_expect_success 'HTTP GET Auth on Origin Server' '
@@ -1534,7 +1546,7 @@ test_expect_success 'prefetch corrupt pack without idx' '
15341546
# Verify corruption detected in pack when building
15351547
# local idx file for it.
15361548
1537-
grep -q "error: .* index-pack failed" <OUT.stderr
1549+
test_grep "error: .* index-pack failed" OUT.stderr
15381550
'
15391551

15401552
# Send corrupt PACK files with IDX files. Since the cache server

0 commit comments

Comments
 (0)