cinecert / asdcplib

AS-DCP and AS-02 File Access Library
Other
73 stars 55 forks source link

Customize namespace names #50

Closed ArnaudBienner closed 3 years ago

ArnaudBienner commented 4 years ago

Use CMake definitions to override the default library namespaces. This can be useful when including asdcplib's objects in another static library in order to avoid symbol clashes in applications that link to both asdcplib and the library including asdcplib.

palemieux commented 4 years ago

This can be useful when including asdcplib's objects in another static library in order to avoid symbol clashes in applications that link to both asdcplib and the library including asdcplib.

@ArnaudBienner Can you provide a concrete example? It is the first time I run into this issue and am very curious.

ArnaudBienner commented 4 years ago

@palemieux This is for the case where you will build a library or SDK using asdcplib, maybe with custom changes, maybe without exposing asdcplib API (but providing your own, high level API instead) and want to let people use it into their project, where they already use asdcplib. Without this modification, linking will fail because of name clashes.

palemieux commented 4 years ago

Without this modification, linking will fail because of name clashes.

@ArnaudBienner why would this happen if the the SDK has its own namespace? Even better, can you provide a sample project?

ArnaudBienner commented 4 years ago

Note those changes are for CMake only. This won't work with autoconf as is. @jhursty is there a desire to continue supporting autoconf?

jhursty commented 4 years ago

@ArnaudBienner, please be more specific about what won't work. Is it just that the alternative namespace name feature is unavailable via autoconf or does the whole build break? Autoconf support is required. I'll have some time next week to assist with it.

kblinova commented 4 years ago

@jhursty during CMake build steps we copy *.h.template files into .h files which define the namespaces. This step would need to be added to autoconf workflow to be able to build the rest of the libraries.

ArnaudBienner commented 4 years ago

@ArnaudBienner why would this happen if the the SDK has its own namespace? Even better, can you provide a sample project?

The issue here is not about the SDK namespace, but about the namespaces of the libraries it uses (asdcplib in our case). I'm not aware of any way to change the namespace of a library except by modifying the code: same code will provide the same symbols. I know different different compilers have different behaviors about hiding/exporting symbols by default (and it also depends on whether your building a static or shared library if I'm right). If I recall correctly GCC and Clang will export everything expect locals symbols by default, except you use -fvisibility=hiddencompiler option and code annotation, and MSVC has the opposite behavior (don't export anything by default).

With the proposed change here, you (as a SDK writer) don't have to worry about this anymore. Also, if that is needed, this allows you to export asdcplib symbols while avoiding name clashes, supposing you need to make some asdcplib functions/structures available externally in your SDK (something obviously not possible if you hide those symbols).

pfirth3 commented 4 years ago

We use asdcplib in a number of projects/libraries. We agree with making these changes and think its necessary.

palemieux commented 4 years ago

@pfirth3 I still do not understand the need. Say

If an application includes Library A and Library B separately, then symbols from Library A remain available under libA and those from Library B under libB.

What am I missing? I am really curious.

kblinova commented 4 years ago

Library B includes Library A for convenience, so that an app that does not use Library A does not need bother with having it in order to link with Library B.

But that means if an app happens to also use Library A, there would be linking errors for multiply defined symbols. That is the reason for namespace customization. Library B can modify the namespaces in Library A so any application that links with Library B and Library A (any version of Library A, btw, not necessarily the same as used by Library B) would not have linking errors.

ArnaudBienner commented 4 years ago

@palemieux If your libB uses a modified version of libA, and your application already uses an unmodified version of libA, how do would you do? Of course, ideally you should have only one version of libA but this might not possible for a lot of reasons: because you build a SDK with modifications not merged upstream (because it is still under review, or because those modifications are too specific to be approved and merged upstream). And the other way around, because the application using your SDK already uses a modified version of libA and don't want to use yours. And you can't really force them to not use their libA.

Hope this clarifies things.

palemieux commented 4 years ago

If your libB uses a modified version of libA, and your application already uses an unmodified version of libA,

Thanks for the clarification. I now understand the scenario.

how do would you do?

I would not try not to be in that situation :)

your application already uses an unmodified version of libA, how do would you do?

Why wouldn't your application switch to the modified version of libA?

kblinova commented 4 years ago

As @ArnaudBienner said, the LibA version packaged with LibB could be an older version, or it could be compiled with different flags from what the app needs.

Other widely used libraries (such as boost) provide a way to customize the namespace.

Besides the work to integrate it, this change seems to make asdcplib more flexible. Do you see any disadvantages, @palemieux ?

In most cases, this is a transparent change. Only when creating new classes does one need to remember to use a define to declare the namespace. If one forgets, there will be a compile error and it's easy to fix. References to ASDCP, AS_02 and Kumu namespaces are aliased so they can still be used as before in class name resolutions (i.e. ASDCP::MXF::Writer).

palemieux commented 4 years ago

Besides the work to integrate it, this change seems to make asdcplib more flexible. Do you see any disadvantages, @palemieux ?

... and the complexity and the work to maintain the #defines, I do not downsides.

ArnaudBienner commented 3 years ago

Closing as this now has conflicts: we will try to propose a new PR soon.