On 14.08.2021 15:13, Daniel Shahaf wrote:
>> Currently, serf has a very specific form of .pc file, hardcoded in
its repo
>> [1].
> That's an implementation detail and may change in a future release.
> Don't rely on it.
>> This only has a single -L.
> You shouldn't rely on this either, if possible, though I don't
> immediately have a*specific* example of when it would break. Perhaps
> when some linker has a -LX flag?
> Don't parse all quotes and escapes. Delegate to the real parser
> (sh(1)?) to do that for you.
> The patch works in one specific case but will not work in other cases.
> For that matter, it won't work if serf.pc starts to emit one leading
> space in front of its -L argument.
> The patch needs to be compatible not only with past and current serf but
> with future ones as well.
I checked serf and the contents of the string hasn't changed in the last
10 years, ever since it was introduced in commit on 2011-08-06 11:52:19.
On top of that, 1.3.8 was released on 2015-08-31 and 1.3.9 was released
on 2020-07-06, so it's not even clear if there will be new releases at all.
Therefore, I find it very unlikely to change. So it's not worthy to
write complex code to deal with unknown future. It sounds more efficient
to deal with it when it happens.
Also, I'm not very experienced with Linux, but my general impression is
that using shell to parse arguments from a string is non trivial and
prone to various escape/quotes issues.
This is one extra reason to not try to over-complicate things in hopes
to predict unlikely future.
So yes, it sounds to me that the current solution is optimal: it fixes
the existing problem, doesn't add new problems, and is simple.
> Could you justify the s/-L/-R/ approach, please? Do the other
> build/*/*.m4 dependency handlers do this?
The problem about serf is that it doesn't have a libtool's `.la` file.
At the same time, libtool only adds runpath for dependencies that has
`.la` files. Dependencies such as --with-apr, --with-apr-util,
--with-sasl do have .la files.
In my SVN build, this leaves two dependencies without `.la` files:
1) --with-serf, which I patch here
2) --with-zlib, that has the same problem. But a typical Linux has
system zlib (on my Ubuntu, it's even needed to run `sudo` !) and
in absence of RUNPATH, SVN secretly loads system zlib instead of
the one specified in `--with-zlib`. I discovered this just today.
I guess I'll leave it for someone else to fix.
