[vlc-devel] [PATCH] contrib: rnnoise: prefix common symbols

[Rémi Denis-Courmont]

Le tiistaina 20. lokakuuta 2020, 13.53.21 EEST Alexandre Janniaux a écrit :
> Hi,
> 
> On Tue, Oct 20, 2020 at 11:33:09AM +0300, Rémi Denis-Courmont wrote:
> > Hi,
> > 
> > No, there is nothing unavoidable about it.
> 
> Sure, instead you can provide something different than a static framework
> like a bunch of static libraries or a dynamic frameworks. Both are
> tradeoff on library complexity, startup cost and potential code elision.

No. The point is simply that libraries, be they dynamic or static, do not 
trigger duplicate symbol errors when linked with correctly. The fact that this 
causes link-time errors can only mean that there is an error at one or more 
stages of the build system.

Of course, this assumes that the duplicate symbol definitions of identical or 
equivalent. This is (presumably) true for RN noise, but it is not true in 
general. Thus you have to link dynamically or in 2+ stages to make things work 
overall.

> > Partial linking is the only way to avoid overlapping symbols with
> > different definitions, and is the only supported way to link VLC modules
> > statically. Anything else is known broken for well over a decade.
> You cannot partially link a static library into an object
> without extracting the objects and including them in the
> linking, which defeat the argument on static library.

I'm not sure what that's supposed to mean by that. But you cannot make a 
static VLC build without partial linking. We tried static linking in a single 
pass. It did not work, and we conceded 12 years ago. So if it were so that you 
could not do partial linking, then you could not build a static VLC. Though 
based on old and incomplete experiments, I believe that partial linking is 
possible

> So what you're saying is that we should partially link the
> contrib itself to mask the symbols we don't want exposed.
> It's possible yes, but orders of magnitude more difficult
> than just be aware of duplicated symbols.

No, I never said that.

> > But that's not even the problem here. Multiple equivalent definitions of
> > the same function in different libraries is fine even without partial
> > linking. If there is a duplicate symbol error the build flags are wrong,
> > very very wrong.
> Like I said twice, this is not about build flags/link flags.

Like I said way more than twice, this *is* a problem with build/link flags.


This patch is a brittle kludge of the very kind that I don't want to see in 
contribs, and it does not fix the real root cause of the problem.

-- 
レミ・デニ-クールモン
http://www.remlab.net/