mm2 / Little-CMS

A free, open source, CMM engine. It provides fast transforms between ICC profiles.
https://www.littlecms.com
MIT License
549 stars 174 forks source link

meson: Create separate pkgconfig files for plugins #392

Closed Biswa96 closed 1 year ago

Biswa96 commented 1 year ago

Fixes https://github.com/mm2/Little-CMS/issues/391

mm2 commented 1 year ago

Hi, please see the discussion here : https://github.com/mm2/Little-CMS/issues/391#issuecomment-1611351494

This is for me to understand why you need this. In the case you want to commercially distribute your program using the system libraries, which is something you should avoid, just not calling the plug-in init function in your program prevents any GPL'd code to be linked. With separated pc files, you are forcing open-source clients to include a different configuration for each plug-in, which breaks compatibility with current distribution and offers nothing in exchange. Or maybe I'm missing something important?

Biswa96 commented 1 year ago

I was thinking of general use cases. For example, one can check if fastfloat or threaded plugin is available through pkgconfig files. Some projects like cairo^1, harfbuzz^2 etc. have different pkgconfig files for different libraries.

Biswa96 commented 1 year ago

Also, the lcms2.pc has -llcms2 -llcms2_fast_float in Libs field. If the program is not using the plugins, -llcms2_fast_float is not required.

mm2 commented 1 year ago

I have taken some time to check this issue, because I believed the design of plug-ins was done correctly. I corroborate it is. You need not separate configurations.

I have installed the core lcms2 plus both plugins. The lcms2.pc is like this:

prefix=/usr/local
exec_prefix=${prefix}
libdir=${exec_prefix}/lib
includedir=${prefix}/include

Name: lcms2
Description: LCMS Color Management Library
Version: 2.15
Libs: -L${libdir} -llcms2  -llcms2_fast_float -llcms2_threaded
Libs.private: -lm -lpthread
Cflags: -I${includedir}

Then I wrote a small program using only core functions:

test1.c:

#include <lcms2.h>

int main()
{
        printf("lcms version %d\n", cmsGetEncodedCMMversion());
        return 0;
}

Then I compile the program using pkconfig


gcc -o test1 test1.c  `pkg-config --cflags --libs lcms2`

Let's see what is in the binary

marti@I7:~/lcms2-2.15$ nm test1 | grep cms
                 U cmsGetEncodedCMMversion

And as shared object:

marti@I7:~/lcms2-2.15$ ldd test1
        linux-vdso.so.1 (0x00007fff475ef000)
        liblcms2.so.2 => /usr/local/lib/liblcms2.so.2 (0x00007fe333a78000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fe333886000)
        libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fe333737000)
        libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007fe333714000)
        /lib64/ld-linux-x86-64.so.2 (0x00007fe333af1000)

No GPL'd code here.

Now I add a plug-in to the program:

test2.c:

#include <lcms2.h>
#include <lcms2_threaded.h>

int main()
{
        cmsPlugin(cmsThreadedExtensions(CMS_THREADED_GUESS_MAX_THREADS, 0));

        printf("lcms version %d\n", cmsGetEncodedCMMversion());
        return 0;
}

And compile in the exact same way

gcc -o test2 test2.c  `pkg-config --cflags --libs lcms2`

Let's see what is in the binary

marti@I7:~/lcms2-2.15$ nm test2 | grep cms
                 U cmsGetEncodedCMMversion
                 U cmsPlugin
                 U cmsThreadedExtensions
marti@I7:~/lcms2-2.15$

And what is being linked,

marti@I7:~/lcms2-2.15$ ldd test2
        linux-vdso.so.1 (0x00007fffe47aa000)
        liblcms2.so.2 => /usr/local/lib/liblcms2.so.2 (0x00007fc1fac19000)
        liblcms2_threaded.so.1 => not found
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fc1faa27000)
        libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fc1fa8d8000)
        libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007fc1fa8b5000)
        /lib64/ld-linux-x86-64.so.2 (0x00007fc1fac92000)

The plug-in is included dynamically in this case.

So, please explain me why you need separate pkgconfig files if with just one you have full control.

Here are the source files

Worst part is, any application using this feature, just activating the plug-in in the C code, would be broken badly if I split the pc files. The compilation would fail with a linker message about a library not found when it was previously working. And at the end there is no need at all.

Biswa96 commented 1 year ago

Thanks for the explanation. I agree that there are indeed some negative side effects to consider.

mm2 commented 1 year ago

Thanks for checking the issue!

eli-schwartz commented 1 year ago

I was thinking of general use cases. For example, one can check if fastfloat or threaded plugin is available through pkgconfig files. Some projects like cairo, harfbuzz etc. have different pkgconfig files for different libraries.

I would suggest instead adding a variable features=fastfloat,threaded to the .pc file (in meson, pass variables: ['features=...'] to the pkgconfig.generate() call).

People seeking to confirm which plugins are available would do the same lcms2 lookup they used to do, but this time with --variable features to get a list of features available.

This is also done by e.g. libcurl.pc via supported_protocols and supported_features, so it's not a novel approach.

And it has no effect on people's current usage of the pkg-config files.

mm2 commented 1 year ago

Sure, we can add a variable on pc files, and probably that would be handy for someone, but I still like the "keep as simple as possible" approach, that's "little" CMS, remember? I guess just checking for plug-in header existence would be enough, then let the linker to do its job.

kmilos commented 1 year ago

And what is being linked

As @eli-schwartz pointed out to me, the --as-needed linker option that you rely on here (and that was introduced as default not that long ago) might not be the default (or even available?) on all platforms and toolchains.

Perhaps you can then add it to the .pc file linker options if you're reliant on it (and the linker supports it) and want to "keep as simple as possible"?

mm2 commented 1 year ago

I was thinking this option relates to weak references. So that means without this flag, if I link a small program by using -lm I got all math library included, no matter I only used a single floor()? Anyway I still think for commercial program redistribution nobody should use system libraries but local copies

eli-schwartz commented 1 year ago

If you statically link then the linker optimizes out anything that it can. Since you passed the unused static libraries on the command line, it does more work -- opening the archive and searching for symbols in it -- but then realizes it needs nothing, tosses it all out (or just keeps floor.o and tosses everything else out) and moves on.

If you dynamically link then the linker just adds a tag for the dynamic loader to load that library at runtime, which is actually needed if you use floor() -- yes, the whole library, because dynamic linking and you can't dynamically link to just one symbol in the .so file, you can only link to the .so file itself.

The interesting question is what happens if you don't use any functions at all. By default the linker adds that dependency anyway, and the --as-needed flag tells it to omit the NEEDED tag in this case.

Again, this only matters when doing dynamic linking.