Skip to content

Commit f9f5716

Browse files
committed
ra_serf: Guard against SERF_VERSION_AT_LEAST(unreleased version) logic.
Before this change, our code contained several blocks of code conditional on SERF_VERSION_AT_LEAST(1, 4, 0). Since Serf 1.4.0 is unreleased, this code relies on development snapshots rather than a stable API. Essentially, that's a ticking timebomb. The code encodes assumptions about a development version of Serf but will automatically activate if a user builds against an officially released Serf 1.4.0+. More specifically: - The code may not compile, depending on the final API state in Serf 1.4.0+. - Even if it compiles, assumptions about Serf's behavior may no longer hold. - Even if the assumptions hold, these code paths may still not work as intended, because they are effectively hidden from our standard release testing and buildbots. To fix this issue, place such code under an additional guard (#ifdef SVN__SERF_EXPERIMENTAL) that is never defined in production builds. See [1] for additional details. * subversion/libsvn_ra_serf/serf.c (load_config): Guard code with #ifdef SVN__SERF_EXPERIMENTAL. * subversion/libsvn_ra_serf/update.c (get_best_connection): Guard code with #ifdef SVN__SERF_EXPERIMENTAL. * subversion/libsvn_ra_serf/util.c (conn_negotiate_protocol, conn_setup, svn_ra_serf__default_readline): Guard code with #ifdef SVN__SERF_EXPERIMENTAL. [1]: https://lists.apache.org/thread/7dpmb63wc1phqvgwvp9hnyrzfqpd2mz5 git-svn-id: https://svn.apache.org/repos/asf/subversion/trunk@1930808 13f79535-47bb-0310-9956-ffa450edef68
1 parent c2c3f37 commit f9f5716

File tree

3 files changed

+14
-2
lines changed

3 files changed

+14
-2
lines changed

subversion/libsvn_ra_serf/serf.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,9 +165,11 @@ load_config(svn_ra_serf__session_t *session,
165165
const char *exceptions;
166166
apr_port_t proxy_port;
167167
svn_tristate_t chunked_requests;
168+
#ifdef SVN__SERF_EXPERIMENTAL
168169
#if SERF_VERSION_AT_LEAST(1, 4, 0) && !defined(SVN_SERF_NO_LOGGING)
169170
apr_int64_t log_components;
170171
apr_int64_t log_level;
172+
#endif
171173
#endif
172174

173175
if (config_hash)
@@ -252,6 +254,7 @@ load_config(svn_ra_serf__session_t *session,
252254
SVN_CONFIG_OPTION_HTTP_CHUNKED_REQUESTS,
253255
"auto", svn_tristate_unknown));
254256

257+
#ifdef SVN__SERF_EXPERIMENTAL
255258
#if SERF_VERSION_AT_LEAST(1, 4, 0) && !defined(SVN_SERF_NO_LOGGING)
256259
SVN_ERR(svn_config_get_int64(config, &log_components,
257260
SVN_CONFIG_SECTION_GLOBAL,
@@ -261,6 +264,7 @@ load_config(svn_ra_serf__session_t *session,
261264
SVN_CONFIG_SECTION_GLOBAL,
262265
SVN_CONFIG_OPTION_SERF_LOG_LEVEL,
263266
SERF_LOG_INFO));
267+
#endif
264268
#endif
265269

266270
server_group = svn_auth_get_parameter(session->auth_baton,
@@ -319,6 +323,7 @@ load_config(svn_ra_serf__session_t *session,
319323
SVN_CONFIG_OPTION_HTTP_CHUNKED_REQUESTS,
320324
"auto", chunked_requests));
321325

326+
#ifdef SVN__SERF_EXPERIMENTAL
322327
#if SERF_VERSION_AT_LEAST(1, 4, 0) && !defined(SVN_SERF_NO_LOGGING)
323328
SVN_ERR(svn_config_get_int64(config, &log_components,
324329
server_group,
@@ -328,9 +333,11 @@ load_config(svn_ra_serf__session_t *session,
328333
server_group,
329334
SVN_CONFIG_OPTION_SERF_LOG_LEVEL,
330335
log_level));
336+
#endif
331337
#endif
332338
}
333339

340+
#ifdef SVN__SERF_EXPERIMENTAL
334341
#if SERF_VERSION_AT_LEAST(1, 4, 0) && !defined(SVN_SERF_NO_LOGGING)
335342
if (log_components != SERF_LOGCOMP_NONE)
336343
{
@@ -348,6 +355,7 @@ load_config(svn_ra_serf__session_t *session,
348355
if (!status)
349356
serf_logging_add_output(session->context, output);
350357
}
358+
#endif
351359
#endif
352360

353361
/* Don't allow the http-max-connections value to be larger than our

subversion/libsvn_ra_serf/update.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,7 @@ get_best_connection(report_context_t *ctx)
616616
}
617617
else
618618
{
619-
#if SERF_VERSION_AT_LEAST(1, 4, 0)
619+
#if defined(SVN__SERF_EXPERIMENTAL) && SERF_VERSION_AT_LEAST(1, 4, 0)
620620
/* Often one connection is slower than others, e.g. because the server
621621
process/thread has to do more work for the particular set of requests.
622622
In the worst case, when REQUEST_COUNT_TO_RESUME requests are queued

subversion/libsvn_ra_serf/util.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,7 @@ load_authorities(svn_ra_serf__connection_t *conn, const char *authorities,
481481
return SVN_NO_ERROR;
482482
}
483483

484+
#ifdef SVN__SERF_EXPERIMENTAL
484485
#if SERF_VERSION_AT_LEAST(1, 4, 0) && defined(SVN__SERF_TEST_HTTP2)
485486
/* Implements serf_ssl_protocol_result_cb_t */
486487
static apr_status_t
@@ -512,6 +513,7 @@ conn_negotiate_protocol(void *data,
512513
return APR_SUCCESS;
513514
}
514515
#endif
516+
#endif
515517

516518
static svn_error_t *
517519
conn_setup(apr_socket_t *sock,
@@ -558,6 +560,7 @@ conn_setup(apr_socket_t *sock,
558560
SVN_ERR(load_authorities(conn, conn->session->ssl_authorities,
559561
conn->session->pool));
560562
}
563+
#ifdef SVN__SERF_EXPERIMENTAL
561564
#if SERF_VERSION_AT_LEAST(1, 4, 0) && defined(SVN__SERF_TEST_HTTP2)
562565
if (APR_SUCCESS ==
563566
serf_ssl_negotiate_protocol(conn->ssl_context, "h2,http/1.1",
@@ -567,6 +570,7 @@ conn_setup(apr_socket_t *sock,
567570
conn->conn,
568571
SERF_CONNECTION_FRAMING_TYPE_NONE);
569572
}
573+
#endif
570574
#endif
571575
}
572576

@@ -2273,7 +2277,7 @@ svn_ra_serf__default_readline(serf_bucket_t *bucket, int acceptable,
22732277
int *found,
22742278
const char **data, apr_size_t *len)
22752279
{
2276-
#if SERF_VERSION_AT_LEAST(1, 4, 0)
2280+
#if defined(SVN__SERF_EXPERIMENTAL) && SERF_VERSION_AT_LEAST(1, 4, 0)
22772281
return serf_default_readline(bucket, acceptable, found, data, len);
22782282
#else
22792283
return bucket_limited_readline(bucket, acceptable, SERF_READ_ALL_AVAIL,

0 commit comments

Comments
 (0)