On 31.07.2021 2:44, Daniel Shahaf wrote:
+ SVN_SERF_LIBS=["$SVN_SERF_LIBS `$PKG_CONFIG $serf_pc_arg --libs-only-L
| $SED -e 's/^-L/-R/g'`"]
I'm sceptical about the regex. For all I know, there may be multiple -L
options and there may be leading spaces before the first one.
More importantly, however, shouldn't we be getting the linker flags from
pkg-config(1)? It would be interesting to build serf with a non-default
prefix and then grep the resulting .pc file for that prefix.
Thanks for reviewing!
Currently, serf has a very specific form of .pc file, hardcoded in its
repo [1]. This only has a single -L. I already build serf with
non-default prefix (hence the struggle and the patch) and it still has
single -L without any -R, that's why libsvn_ra_serf can't find it later
(because in absence of -R it doesn't record -rpath to find serf).
At the same time, -L could happen to be present in the middle of the
path (consider '-L~/SERF-LATEST/'), and avoiding to replace that -L in
the middle is... hard. Just think about parsing all the possible quotes
and escapes!
Therefore, I conclude that it's optimal (by patch complexity vs benefit)
to only replace a single -L in the beginning. At worst, this patch will
leave some space for further improvement. Currently, to my
understanding, patch already fixes the current problem without breaking
things.
[1] https://svn.apache.org/repos/asf/serf/trunk/build/serf.pc.in
