On 16.09.2020 23:14, Julian Foad wrote:
> Dear devs,
>
> I propose a patch restoring support for APR 1.4 so that svn 1.14 can
> be built and packaged on CentOS 7.
>
> The WANdisco folks continue to package svn for a variety of target
> systems. Packaging for CentOS 7 which has APR 1.4.8, they ran into
> the problem that just before svn 1.14.0 we bumped the dependency
> requirement to APR 1.5.0 and removed support for older APR versions.
>
> Here's the change we made:
>> r1874094 | jamessan | 2020-02-17 03:49:42
>> Require at least version 1.5 of APR
[...]
>>
>
> The context is we'd recently started using apr_pescape_shell() in
> r1874057, shortly afterwards changing to using apr_escape_shell()
> instead, and those escaping functions are new in 1.5.0.
>
> Ian at WANdisco told me:
>> The dependencies we have issues with are apr and apr-util as 1.14
>> requires at least APR 1.5 and Red Hat ship 1.4.8 in EL7. I tried
>> ignoring the version and using the old apr but it doesn't compile due
>> to some missing symbols.
>>
>> Because of this if we want to ship any 1.14 build for EL7 we have to
>> build our own (later) APR and APR-Util packages which in turn means
>> libserf and apache httpd would need to link against these too.
>
> They are looking for a more self-contained solution.
>
> I propose adding support more or less as in the attached patch. It:
>
> - reverts r1874094, thus restoring the support we had for APR 1.4
> (and even 1.3 by the look of it), but we never had backward
> compatibility support for apr_escape_shell();
IIRC we skipped APR 1.4, going from requiring 1.3 to 1.5.
> - adds a local copy of the 'apr_escape_shell()' function, copied
> from APR 1.7.0.
>
> One adjustments is needed before this is ready to commit: I must make
> the 'apr_escape_shell' part conditional on APR version. Note also
> that the support for 'apr_escape_shell()' adds 4 files, as that's how
> the code was laid out in APR. We could combine them but I preferred
> to copy it as exactly as possible.
>
> If this is an acceptable approach, I would propose it for backport to
> a 1.14.x point release.
>
> WANdisco would prefer to package our upstream svn source release
> exactly rather than patch it in their packaging, and that is why I am
> proposing adding this support.
>
> Of course I understand that APR pre-1.5 is pretty old and the argument
> to not complicate the current code with too much backward
> compatibility support code. So the question is whether this is too
> much or whether supporting building on distros like CentOS 7 out of
> the box is the better option.
I'm not comfortable with copying whole swathes of APR into our code.
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.
So instead of copying a bunch of APR source into our code, I'd suggest
to add conditional code that invokes the editor without properly
escaping the paths (or whatever we did before that change). 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.
I do worry that would make the resulting patch even more complicated in
the end.
-- Brane
Received on 2020-09-17 08:16:31 CEST