Branko Čibej wrote:
I'm not comfortable with copying whole swathes of APR into our code.

I have made a smaller version (attached), restoring support just back to APR 1.4 (not 1.3), putting all the apr_escape code in one file (128 lines long), omitting EBCDIC support. Does that feel better?

Adding escape_path to the editor invocations was a bug fix, and I'd argue that if someone wants to use an older APR, they can just live with that bug, too. [...]

That's a valid option but I feel it's a last resort. Having decided this was our bug to fix, rather than one that we just inherited through the dependency, then it would seem like a bad thing to provide a way to build Subversion such that the bug that we declared "fixed" is not fixed in that build configuration.

[...] For the rest we'd still want to keep the APR >= 1.4 requirement and APR 
>= 1.5 as the recommended minimum, so for example, leave the get_deps.sh unchanged.

For the rest, this patch does exactly that.

Andrew Marlow wrote:
sounds good to me. I've built recent versions of subversion on RHEL7 and it is a right pain in the neck, chasing those dependencies down to serf, especially since the last time I looked serf depended on python2 instead of python3. Your proposal gets my vote, FWIW.

Thanks.

How's this version for everyone?

- Julian
Restoring support for building with APR 1.4.

This patch changes the minimum required version of APR from 1.5 to 1.4.

The first part of this patch is a partial revert of r1874094 (which changed
the requirement from APR 1.3 to APR 1.5), reverting only the parts needed to
restore APR 1.4 compatibility.

* INSTALL,
  build/generator/gen_win_dependencies.py,
  configure.ac:
  Change minimum APR version from 1.5 to 1.4.

* subversion/include/svn_types.h,
  subversion/libsvn_subr/iter.c:
  Restore the compatibility versions of apr_hash_this_key,
  apr_hash_this_key_len, apr_hash_this_val.

The second part of this patch adds a local copy of the 'apr_escape_shell'
function, copied from APR 1.7.0, if building against APR < 1.5.

* subversion/include/private/svn_dep_compat.h
  (APR_ESCAPE_STRING, apr_escape_shell): Declare if APR < 1.5.

* subversion/libsvn_subr/apr_escape.c
  New, copied from parts of APR 1.7.0.

* subversion/libsvn_subr/cmdline.c
  When APR < 1.5, include 'svn_dep_compat.h' instead of 'apr_escape.h'.
--This line, and those below, will be ignored--

Index: INSTALL
===================================================================
--- INSTALL	(revision 1881769)
+++ INSTALL	(working copy)
@@ -203,13 +203,13 @@ I.    INTRODUCTION
 
       Note: Because previous builds of Subversion may have installed older
       versions of these libraries, you may want to run some of the cleanup
       commands described in section II.B before installing the following.
 
 
-      1.  Apache Portable Runtime 1.5 or newer  (REQUIRED)
+      1.  Apache Portable Runtime 1.4 or newer  (REQUIRED)
 
       Whenever you want to build any part of Subversion, you need the
       Apache Portable Runtime (APR) and the APR Utility (APR-util)
       libraries.
 
       If you do not have a pre-installed APR and APR-util, you will need
@@ -835,13 +835,13 @@ II.   INSTALLATION
         used to generate the project files.
       * Perl 5.8 or higher from https://www.perl.org/get.html
       * Awk (from https://www.cs.princeton.edu/~bwk/btl.mirror/awk95.exe) is
         needed to compile Apache.  Note that this is the actual awk program,
         not an installer - just rename it to awk.exe and it is ready to use.
       * Apache apr, apr-util, and optionally apr-iconv libraries, version
-        1.5 or later (1.2 for apr-iconv). If you are building from a Subversion
+        1.4 or later (1.2 for apr-iconv). If you are building from a Subversion
         checkout and have not downloaded Apache 2, then get these 3 libraries
         from https://www.apache.org/dist/apr/.
       * SQLite 3.8.2 or higher from https://www.sqlite.org/download.html
         (3.8.11.1 or higher recommended)
       * ZLib 1.2 or higher is required and can be obtained from
         http://www.zlib.net/
Index: build/generator/gen_win_dependencies.py
===================================================================
--- build/generator/gen_win_dependencies.py	(revision 1881769)
+++ build/generator/gen_win_dependencies.py	(working copy)
@@ -342,13 +342,13 @@ class GenDependenciesBase(gen_base.Gener
         self._find_python(show_warnings)
       self._find_ruby(show_warnings)
 
   def _find_apr(self):
     "Find the APR library and version"
 
-    minimal_apr_version = (1, 5, 0)
+    minimal_apr_version = (1, 4, 0)
 
     if not self.apr_path:
       sys.stderr.write("ERROR: Use '--with-apr' option to configure APR " + \
                        "location.\n")
       sys.exit(1)
 
Index: configure.ac
===================================================================
--- configure.ac	(revision 1881769)
+++ configure.ac	(working copy)
@@ -88,13 +88,13 @@ AC_SUBST([MKDIR])
 
 # ==== Libraries, for which we may have source to build ======================
 
 dnl verify apr version and set apr flags
 dnl These regular expressions should not contain "\(" and "\)".
 
-APR_VER_REGEXES=["1\.[5-9]\. 2\."]
+APR_VER_REGEXES=["1\.[4-9]\. 2\."]
 
 SVN_LIB_APR($APR_VER_REGEXES)
 
 if test `expr $apr_version : 2` -ne 0; then
   dnl Bump the library so-version to 2 if using APR-2
   dnl (Debian uses so-version 1 for APR-1-with-largefile)
Index: subversion/include/private/svn_dep_compat.h
===================================================================
--- subversion/include/private/svn_dep_compat.h	(revision 1881769)
+++ subversion/include/private/svn_dep_compat.h	(working copy)
@@ -26,12 +26,13 @@
  */
 
 #ifndef SVN_DEP_COMPAT_H
 #define SVN_DEP_COMPAT_H
 
 #include <apr_version.h>
+#include <apr_errno.h>
 
 #ifdef __cplusplus
 extern "C" {
 #endif /* __cplusplus */
 
 /**
@@ -190,11 +191,21 @@ extern "C" {
  */
 #ifndef SQLITE_VERSION_AT_LEAST
 #define SQLITE_VERSION_AT_LEAST(major,minor,patch)                     \
 ((major*1000000 + minor*1000 + patch) <= SVN_SQLITE_MIN_VERSION_NUMBER)
 #endif /* SQLITE_VERSION_AT_LEAST */
 
+/**
+ * Support for 'apr_escape_shell() which was introduced in APR 1.5.
+ */
+#if !APR_VERSION_AT_LEAST(1,5,0)
+/* from apr_escape.h */
+#define APR_ESCAPE_STRING      (-1)
+APR_DECLARE(apr_status_t) apr_escape_shell(char *escaped, const char *str,
+        apr_ssize_t slen, apr_size_t *len);
+#endif
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
 
 #endif /* SVN_DEP_COMPAT_H */
Index: subversion/include/svn_types.h
===================================================================
--- subversion/include/svn_types.h	(revision 1881769)
+++ subversion/include/svn_types.h	(working copy)
@@ -246,12 +246,41 @@ typedef struct svn_version_t svn_version
 #endif
 
 /** @} */
 
 
 
+/** @defgroup apr_hash_utilities APR Hash Table Helpers
+ * These functions enable the caller to dereference an APR hash table index
+ * without type casts or temporary variables.
+ *
+ * These functions are provided by APR itself from version 1.5.
+ * Definitions are provided here for when using older versions of APR.
+ * @{
+ */
+
+#if !APR_VERSION_AT_LEAST(1, 5, 0)
+
+/** Return the key of the hash table entry indexed by @a hi. */
+const void *
+apr_hash_this_key(apr_hash_index_t *hi);
+
+/** Return the key length of the hash table entry indexed by @a hi. */
+apr_ssize_t
+apr_hash_this_key_len(apr_hash_index_t *hi);
+
+/** Return the value of the hash table entry indexed by @a hi. */
+void *
+apr_hash_this_val(apr_hash_index_t *hi);
+
+#endif
+
+/** @} */
+
+
+
 /** On Windows, APR_STATUS_IS_ENOTDIR includes several kinds of
  * invalid-pathname error but not ERROR_INVALID_NAME, so we include it.
  * We also include ERROR_DIRECTORY as that was not included in apr versions
  * before 1.4.0 and this fix is not backported */
 /* ### These fixes should go into APR. */
 #ifndef WIN32
Index: subversion/libsvn_subr/apr_escape.c
===================================================================
--- subversion/libsvn_subr/apr_escape.c	(nonexistent)
+++ subversion/libsvn_subr/apr_escape.c	(working copy)
@@ -0,0 +1,128 @@
+/* Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/* The code in this file is copied from APR (initially from APR 1.7.0)
+ * to provide compatibility for building against APR < 1.5.0.
+ */
+
+#include "private/svn_dep_compat.h"
+
+#if !APR_VERSION_AT_LEAST(1,5,0)
+
+#include <apr_lib.h>
+#include <apr_strings.h>
+
+/* from apr_escape_test_char.h */
+#define T_ESCAPE_SHELL_CMD     (1)
+#define T_ESCAPE_PATH_SEGMENT  (2)
+#define T_OS_ESCAPE_PATH       (4)
+#define T_ESCAPE_ECHO          (8)
+#define T_ESCAPE_URLENCODED    (16)
+#define T_ESCAPE_XML           (32)
+#define T_ESCAPE_LDAP_DN       (64)
+#define T_ESCAPE_LDAP_FILTER   (128)
+
+static const unsigned char test_char_table[256] = {
+    224,222,222,222,222,222,222,222,222,222,223,222,222,222,222,222,222,222,222,222,
+    222,222,222,222,222,222,222,222,222,222,222,222,6,16,127,22,17,22,49,17,
+    145,145,129,80,80,0,0,18,0,0,0,0,0,0,0,0,0,0,16,87,
+    119,16,119,23,16,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
+    0,0,0,0,0,0,0,0,0,0,0,23,223,23,23,0,23,0,0,0,
+    0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
+    0,0,0,23,23,23,17,222,222,222,222,222,222,222,222,222,222,222,222,222,
+    222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,
+    222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,
+    222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,
+    222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,
+    222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,
+    222,222,222,222,222,222,222,222,222,222,222,222,222,222,222,222 
+};
+
+/* from apr_encode_private.h */
+#if APR_CHARSET_EBCDIC
+#error This Subversion compatibility code for APR<1.5 does not support EBCDIC.
+#else                           /* APR_CHARSET_EBCDIC */
+#define ENCODE_TO_ASCII(ch)  (ch)
+#define ENCODE_TO_NATIVE(ch)  (ch)
+#endif                          /* !APR_CHARSET_EBCDIC */
+
+/* we assume the folks using this ensure 0 <= c < 256... which means
+ * you need a cast to (unsigned char) first, you can't just plug a
+ * char in here and get it to work, because if char is signed then it
+ * will first be sign extended.
+ */
+#define TEST_CHAR(c, f)        (test_char_table[(unsigned)(c)] & (f))
+
+APR_DECLARE(apr_status_t) apr_escape_shell(char *escaped, const char *str,
+        apr_ssize_t slen, apr_size_t *len)
+{
+    unsigned char *d;
+    const unsigned char *s;
+    apr_size_t size = 1;
+    int found = 0;
+
+    d = (unsigned char *) escaped;
+    s = (const unsigned char *) str;
+
+    if (s) {
+        if (d) {
+            for (; *s && slen; ++s, slen--) {
+#if defined(OS2) || defined(WIN32)
+                /*
+                 * Newlines to Win32/OS2 CreateProcess() are ill advised.
+                 * Convert them to spaces since they are effectively white
+                 * space to most applications
+                 */
+                if (*s == '\r' || *s == '\n') {
+                    if (d) {
+                        *d++ = ' ';
+                        found = 1;
+                    }
+                    continue;
+                }
+#endif
+                if (TEST_CHAR(*s, T_ESCAPE_SHELL_CMD)) {
+                    *d++ = '\\';
+                    size++;
+                    found = 1;
+                }
+                *d++ = *s;
+                size++;
+            }
+            *d = '\0';
+        }
+        else {
+            for (; *s && slen; ++s, slen--) {
+                if (TEST_CHAR(*s, T_ESCAPE_SHELL_CMD)) {
+                    size++;
+                    found = 1;
+                }
+                size++;
+            }
+        }
+    }
+
+    if (len) {
+        *len = size;
+    }
+    if (!found) {
+        return APR_NOTFOUND;
+    }
+
+    return APR_SUCCESS;
+}
+
+#endif

Property changes on: subversion/libsvn_subr/apr_escape.c
___________________________________________________________________
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Index: subversion/libsvn_subr/cmdline.c
===================================================================
--- subversion/libsvn_subr/cmdline.c	(revision 1881769)
+++ subversion/libsvn_subr/cmdline.c	(working copy)
@@ -36,13 +36,18 @@
 #include <io.h>
 #include <conio.h>
 #endif
 
 #include <apr.h>                /* for STDIN_FILENO */
 #include <apr_errno.h>          /* for apr_strerror */
+#include <apr_version.h>
+#if APR_VERSION_AT_LEAST(1,5,0)
 #include <apr_escape.h>
+#else
+#include "private/svn_dep_compat.h"
+#endif
 #include <apr_general.h>        /* for apr_initialize/apr_terminate */
 #include <apr_strings.h>        /* for apr_snprintf */
 #include <apr_pools.h>
 #include <apr_signal.h>
 
 #include "svn_cmdline.h"
Index: subversion/libsvn_subr/iter.c
===================================================================
--- subversion/libsvn_subr/iter.c	(revision 1881769)
+++ subversion/libsvn_subr/iter.c	(working copy)
@@ -140,6 +140,32 @@ svn_iter_apr_array(svn_boolean_t *comple
  * we can never remove it or change its signature. */
 svn_error_t *
 svn_iter__break(void)
 {
   return &internal_break_error;
 }
+
+#if !APR_VERSION_AT_LEAST(1, 5, 0)
+const void *apr_hash_this_key(apr_hash_index_t *hi)
+{
+  const void *key;
+
+  apr_hash_this((apr_hash_index_t *)hi, &key, NULL, NULL);
+  return key;
+}
+
+apr_ssize_t apr_hash_this_key_len(apr_hash_index_t *hi)
+{
+  apr_ssize_t klen;
+
+  apr_hash_this((apr_hash_index_t *)hi, NULL, &klen, NULL);
+  return klen;
+}
+
+void *apr_hash_this_val(apr_hash_index_t *hi)
+{
+  void *val;
+
+  apr_hash_this((apr_hash_index_t *)hi, NULL, NULL, &val);
+  return val;
+}
+#endif

Reply via email to