Guarding against SERF_VERSION_AT_LEAST(unreleased version) logic in libsvn_ra_serf

[Evgeny Kotkov via dev]

Hi all,

I noticed that libsvn_ra_serf on trunk currently contains 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:

  subversion/libsvn_ra_serf/eagain_bucket.c(69):
      #if !SERF_VERSION_AT_LEAST(1, 4, 0)
  subversion/libsvn_ra_serf/eagain_bucket.c(102):
      #if SERF_VERSION_AT_LEAST(1, 4, 0)
  subversion/libsvn_ra_serf/sb_bucket.c(120):
      #if !SERF_VERSION_AT_LEAST(1, 4, 0)
  subversion/libsvn_ra_serf/sb_bucket.c(163):
      #if SERF_VERSION_AT_LEAST(1, 4, 0)
  subversion/libsvn_ra_serf/serf.c(168):
      #if SERF_VERSION_AT_LEAST(1, 4, 0) && !defined(SVN_SERF_NO_LOGGING)
  subversion/libsvn_ra_serf/serf.c(255):
      #if SERF_VERSION_AT_LEAST(1, 4, 0) && !defined(SVN_SERF_NO_LOGGING)
  subversion/libsvn_ra_serf/serf.c(322):
      #if SERF_VERSION_AT_LEAST(1, 4, 0) && !defined(SVN_SERF_NO_LOGGING)
  subversion/libsvn_ra_serf/serf.c(334):
      #if SERF_VERSION_AT_LEAST(1, 4, 0) && !defined(SVN_SERF_NO_LOGGING)
  subversion/libsvn_ra_serf/serf.c(594):
      #if defined(SVN_DEBUG) && !SERF_VERSION_AT_LEAST(1,4,0)
  subversion/libsvn_ra_serf/update.c(619):
      #if SERF_VERSION_AT_LEAST(1, 4, 0)
  subversion/libsvn_ra_serf/util.c(483):
      #if SERF_VERSION_AT_LEAST(1, 4, 0) && defined(SVN__SERF_TEST_HTTP2)
  subversion/libsvn_ra_serf/util.c(560):
      #if SERF_VERSION_AT_LEAST(1, 4, 0) && defined(SVN__SERF_TEST_HTTP2)

It seems that this is essentially 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.

I think we should treat this as a compatibility issue.  I can think of the
following ad-hoc fix:

1) Place such code under an additional guard (#ifdef SVN__SERF_EXPERIMENTAL)
   that is never defined in production builds.

2) Backport this change to 1.14.x as well.


Thanks,
Evgeny Kotkov