Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1.2.1: issue with pc.generate() when it is build only shared library. #12225

Closed
kloczek opened this issue Sep 6, 2023 · 27 comments
Closed

1.2.1: issue with pc.generate() when it is build only shared library. #12225

kloczek opened this issue Sep 6, 2023 · 27 comments

Comments

@kloczek
Copy link

kloczek commented Sep 6, 2023

Describe the bug
From https://people.freedesktop.org/~dbn/pkg-config-guide.html:

Requires and Requires.private define other modules needed by the library. It is usually preferred to use the private variant of Requires to avoid exposing unnecessary libraries to the program that is linking with your library. If the program will not be using the symbols of the required library, it should not be linking directly to that library. See the discussion of overlinking for a more thorough explanation.

Since pkg-config always exposes the link flags of the Requires libraries, these modules will become direct dependencies of the program. On the other hand, libraries from Requires.private will only be included when static linking.

Scenario:

  • im meson.buold is defined library libfoo build with
    library(libfoo, 'foo': libfoo_src, dependencies: [ bar1_deps, bar2_deps], <other_args>)
  • for libfoo is generated pkgconfig file using
    pc.generate(libfoo, libraries: libfoo, <other_params>)
  • while project is build
    meson -D default_library=shared

Generated libfoo.pc file will will have Requires.private field with libraries listed in dependencies: [ bar1_deps, bar2_deps].
In this situation it does not make any sense because produced binaries will not contain static libraries.

To Reproduce
You can clone https://gitlab.freedesktop.org/spice/libcacard/ and add below patch (I just found that pc.generate() is wrongly defined) :

--- a/meson.build
+++ b/meson.build
@@ -88,9 +88,7 @@
 )

 pc = import('pkgconfig')
-pc.generate(
-  libraries: libcacard,
-  requires_private: ['glib-2.0'],
+pc.generate(libcacard,
   subdirs: 'cacard',
   version: meson.project_version(),
   name: 'cacard',

Without that patch generated libcacard.pc is without `Libs: entry')

Expected behavior
If library pkgconfig file is generated and only shared library is build Requires.private should not be added because product of the build will not possible to use for static linking.

system parameters

  • Is this a just a plain native build (for the same computer)
  • what operating system Linux
  • what Python version are you using e.g. 3.8.18
  • what meson --version: 1.2.1
  • what ninja --version N/A
@kloczek
Copy link
Author

kloczek commented Sep 6, 2023

Other scenarios to check:

  • when is used meson -D default_library=both generated .pc file should be with both Requires and Requires.private

@kloczek
Copy link
Author

kloczek commented Sep 6, 2023

Currently generated libcacard.pc:

prefix=/usr
includedir=${prefix}/include
libdir=${prefix}/lib64

Name: cacard
Description: CAC (Common Access Card) library
Version: 2.8.1
Requires.private: glib-2.0 >=  2.32, nss >=  3.12.8, libpcsclite
Libs: -L${libdir} -lcacard
Cflags: -I${includedir}/cacard

None of the header files ${includedir}/cacard includes glib, nss or libpcsclite header files so API defined in those headers do not depends on these libraries headers.

@eli-schwartz
Copy link
Member

None of the header files ${includedir}/cacard includes glib, nss or libpcsclite header files so API defined in those headers do not depends on these libraries headers.

In the general case we do need to assume they might, which is exactly why we intentionally behave the way we do right now.

@xclaesse
Copy link
Member

xclaesse commented Sep 6, 2023

If library pkgconfig file is generated and only shared library is build Requires.private should not be added because product of the build will not possible to use for static linking.

It is a common mistake to think Requires.private is only for static linking. It is also used in the case lib A headers includes headers from lib B but an apps using lib A does not need to link with lib B. It still needs cflags from lib B to find its headers.

There is a long description of the problem there: https://bugs.freedesktop.org/show_bug.cgi?id=105572#c1.

pkg-config has no way to tell a dependency is internal and is needed only for static linking.

@xclaesse
Copy link
Member

xclaesse commented Sep 6, 2023

Note that if you use shared_library() it will not add Requires.private, but when you use library() it will add Requires.private regardless of default_library option. That has been changed years ago and I'm still totally against that inconsistency, but it was a compromise for distro that were not happy to add extra useless dev dependencies.

@kloczek
Copy link
Author

kloczek commented Sep 6, 2023

Note that if you use shared_library() it will not add Requires.private, but when you use library() it will add Requires.private regardless of default_library option. That has been changed years ago and I'm still totally against that inconsistency, but it was a compromise for distro that were not happy to add extra useless dev dependencies.

This is not mistake. Please one more time look on quoted documentation.
In case of libcacard and use it on building spice-gtk meson fails with

subprojects/spice-common/meson.build:144:16: ERROR: Dependency lookup for libcacard with method 'pkgconfig' failed: Could not generate cargs for libcacard:
Package nss was not found in the pkg-config search path.
Perhaps you should add the directory containing `nss.pc'
to the PKG_CONFIG_PATH environment variable
Package 'nss', required by 'libcacard', not found
Package 'libpcsclite', required by 'libcacard', not found


A full log can be found at /home/tkloczko/rpmbuild/BUILD/spice-gtk-0.42/x86_64-redhat-linux-gnu/meson-logs/meson-log.txt

Which does not make ANY sense.
As I wrote in this produced libcacard builds only shared (default) library binaries and on use it in spice-gtk wants to have libraries used only on bluing libcacard.

It is yet another issue in meson unittests/linuxliketests.py where is:

        cmd = [PKG_CONFIG, 'requires-test']
        out = self._run(cmd + ['--print-requires'], override_envvars=env).strip().split('\n')
        if not is_openbsd():
            self.assertEqual(sorted(out), sorted(['libexposed', 'libfoo >= 1.0', 'libhello']))
        else:
            self.assertEqual(sorted(out), sorted(['libexposed', 'libfoo>=1.0', 'libhello']))

        cmd = [PKG_CONFIG, 'requires-private-test']
        out = self._run(cmd + ['--print-requires-private'], override_envvars=env).strip().split('\n')
        if not is_openbsd():
            self.assertEqual(sorted(out), sorted(['libexposed', 'libfoo >= 1.0', 'libhello']))
        else:
            self.assertEqual(sorted(out), sorted(['libexposed', 'libfoo>=1.0', 'libhello']))

So exactly this part is causing that Requires and Requires.private are combined UNCINDITIOANLLY (no matter is project links statically od dynamically linked binaries).

My understanding is that use library() is possible to decide on use meson meson -D default_library=[static|shared|both] how to link project.

IMO main reason why it is so inconsistently treated/combined is yer another issue in rpm pkgconfig generator where in checking packaged .pc files is used pkgconfig executed with -print-requires --print-requires-private
https://github.com/rpm-software-management/rpm/blob/08ae4ee23e7b746c529abef8ddc86cf650177e69/scripts/pkgconfigdeps.sh#L42-L50

In other words there are some other branches of hart inconsistency caused by rpm.

@kloczek
Copy link
Author

kloczek commented Sep 6, 2023

In the general case we do need to assume they might, which is exactly why we intentionally behave the way we do right now.

And that is nothing more than only unnecessary/incorrect assumption.
In such case all what is needed is to add additional Requires: [ bar1_deps, bar2_deps]. in `pc.generate() call.

@xclaesse
Copy link
Member

xclaesse commented Sep 6, 2023

Package 'nss', required by 'libcacard', not found
Package 'libpcsclite', required by 'libcacard', not found

As I wrote in this produced libcacard builds only shared (default) library binaries and on use it in spice-gtk wants to have libraries used only on bluing libcacard.

Meson has no way to know if nss and libpcsclite headers are exposed in libcacard headers. They very well could be needed even when building only shared libraries. It is a fundamental limitation of pkg-config, there is no way to tell of a dependency is only needed for static linking, or also needed for its headers.

@xclaesse
Copy link
Member

xclaesse commented Sep 6, 2023

That means that libcacard does need a build-dep on nss and libpcsclite, even if it's not used at all.

@kloczek
Copy link
Author

kloczek commented Sep 6, 2023

Additionally I can tell you that up to meson 1.0.0 I've been using patch which removed propagate Requires.private base in distribution on which I'm working intentionally there are not static libraries variants.
On move to 1.0.0 I had impression that this was fixed so I've decided to drop that patch.
Now after almost year of using meson >= 1.0.0 I found first project (spice-gtk) in which this issue stroke me again.

By this in last 3-4 years I'v been able to catch only handful (less than 10) number of projects (on scale +5k of all packages in distribution) which was necessity to correct pkgconfig calls on finding dependencies.
All necessary adjustments already have ben integrated in affected projects in last 3-4 years so risk of breaking something by literally following pkg config documentation is really low.

@xclaesse
Copy link
Member

xclaesse commented Sep 6, 2023

So exactly this part is causing that Requires and Requires.private are combined UNCINDITIOANLLY (no matter is project links statically od dynamically linked binaries).

That's true and that's intentional. cflags (not libs) are always combined from Requires and Requires.private. That's how pkg-config works. For better or worse...

@kloczek
Copy link
Author

kloczek commented Sep 6, 2023

That means that libcacard does need a build-dep on nss and libpcsclite, even if it's not used at all.

EXACTLY!! 😄
Those additional libraries are ONLY libpcsclite build stage dependency ot other projects build dependency if someone will produce static libcacard archives and will decide build project like spice-gtk with libcacard static library.
Those build dependencies should not pop up when someone will be building spice-gtk with SHARDED libcacard .

@xclaesse
Copy link
Member

xclaesse commented Sep 6, 2023

A big part of the problem is pkg-config is unmaintained. I did propose to add Requires.internal back in 2018.

I think the solution is;

  1. Add Requires.internal in pkgconf (maybe already done?).
  2. Add pkg.generate(..., internal: true) option to add deps into Requires.internal instead of Requires.private. Doing that would genearte pc file usable only by pkgconf.
  3. pkg.generate(..., requires_private: foo, internal: true) would promote foo from Requires.internal to Requires.private, just like we have requires: foo currently promoting from Requires.private to Requires.
  4. Fight with Debian to make them abandon pkg-config.

@kloczek
Copy link
Author

kloczek commented Sep 6, 2023

A big part of the problem is pkg-config is unmaintained. I did propose to add Requires.internal back in 2018.

That is not true.
2.0.3 has been released 4 days ago https://github.com/pkgconf/pkgconf/tags

@kloczek
Copy link
Author

kloczek commented Sep 6, 2023

A big part of the problem is pkg-config is unmaintained. I did propose to add Requires.internal back in 2018.

So .. because Requires.private is inconsistently used ignoring pkgconfig documentation in meson and rpm lets introduce Requires.internal 😋

@xclaesse
Copy link
Member

xclaesse commented Sep 6, 2023

The doc is not wrong, it is incomplete. It should say in addition that cflags are always taken from Requires.private regardless of --static flag. Read https://bugs.freedesktop.org/show_bug.cgi?id=105572#c1, it's not a bug, it's intentional and makes sense. It's just that pkg-config can only express 2 out of 3 dependency types. It's a missing feature, not a bug.

@kloczek
Copy link
Author

kloczek commented Sep 6, 2023

Since pkg-config always exposes the link flags of the Requires libraries, these modules will become direct dependencies of the program. On the other hand, libraries from Requires.private will only be included when STATIC LINKING.

Hmm IMO that is 100% clear and complete.

@xclaesse
Copy link
Member

xclaesse commented Sep 6, 2023

On the other hand, libraries from Requires.private will only be included when STATIC LINKING.

It tells about libraries, not cflags! That's the whole story here!

@kloczek
Copy link
Author

kloczek commented Sep 6, 2023

Look .. only scenario which needs to handled is when project A is linked with libX and that library could be build as stared and/or static one.
There is no ANY other needs to propagate anywhere else libX dependencies.

@kloczek
Copy link
Author

kloczek commented Sep 6, 2023

On the other hand, libraries from Requires.private will only be included when STATIC LINKING.

It tells about libraries, not cflags! That's the whole story here!

As I wrote already if libX used header files of libY it should be listed in Requires because no matter how project A will be linked against libX (statically or dynamically) libY header files will be used as well.

@xclaesse
Copy link
Member

xclaesse commented Sep 6, 2023

In your specific case maybe, but that's not always the case, and Meson has no way to know.

Visible dependencies: Types from libbar are exposed in some header files of libfoo, but only a subset of users of libfoo will need to call functions from libbar. If they do, they are expected to carry out their own pkg-config check for libbar. Example: libfoo = GDK, libbar = libX11.

@xclaesse
Copy link
Member

xclaesse commented Sep 6, 2023

As I wrote already if libX used header files of libY it should be listed in Requires because no matter how project A will be linked against libX (statically or dynamically) libY header files will be used.

That's wrong, you did not understand. There are cases you need headers from libX but not link to libX.

@xclaesse
Copy link
Member

xclaesse commented Sep 6, 2023

In the link I posted, Requires.private correspond to "visible dependencies" and pkg-config has just no way to express "Invisible dependencies" which is what you want.

@kloczek
Copy link
Author

kloczek commented Sep 6, 2023

In your specific case maybe, but that's not always the case, and Meson has no way to know.

In the link I posted, Requires.private correspond to "visible dependencies" and pkg-config has just no way to express "Invisible dependencies" which is what you want.

Control question: what is ANY other need to propagate libX build dependencies from libY especially on building project A than what I've described?

If answer will be none it will automatically mean that it is not specific byt GENERIC case.

@eli-schwartz
Copy link
Member

And that is nothing more than only unnecessary/incorrect assumption.
In such case all what is needed is to add additional Requires: [ bar1_deps, bar2_deps]. in pc.generate() call.

That would incorrectly over-link the shared libraries of those dependencies in cases where the headers are needed transitively but not the libraries directly.

This was already explained to you, but you're not listening.

@eli-schwartz
Copy link
Member

More generally, the bug report is incorrect, and submitted by someone who often... misunderstands requirements.

There's a real issue here, but it's not the one you reported, and it's one we have an existing ticket for -- plus one we know about quite well.

This bug report has outlived its usefulness.

@eli-schwartz eli-schwartz closed this as not planned Won't fix, can't repro, duplicate, stale Sep 6, 2023
@mesonbuild mesonbuild locked and limited conversation to collaborators Sep 6, 2023
@xclaesse
Copy link
Member

xclaesse commented Sep 6, 2023

Dummy example:

liba.h

typedef int a_int;

libb.h

#include <liba.h>
int b_something(a_int val);

app.c

#include <libb.h>
int main() {
  b_something(42);
}

=> app does not have to link to liba but it must have its header. liba is a build-dep not a runtime dep.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants