WebPlatformForEmbedded / libwpe

General-purpose library specifically developed for the WPE-flavored port of WebKit.
BSD 2-Clause "Simplified" License
49 stars 36 forks source link

1.13.2: missig pkgconfig dependency #107

Closed kloczek closed 2 years ago

kloczek commented 2 years ago

Just found that webkitgtk failed on missing xkbcommon. header file.

In file included from /usr/include/wpe-1.0/wpe/wpe.h:38,
                 from /usr/include/wpe-1.0/wpe/wpe-egl.h:37,
                 from /home/tkloczko/rpmbuild/BUILD/webkitgtk-2.36.3/Source/WebCore/platform/graphics/egl/GLContextEGLLibWPE.cpp:39:
/usr/include/wpe-1.0/wpe/input-xkb.h:48:10: fatal error: xkbcommon/xkbcommon.h: No such file or directory
   48 | #include <xkbcommon/xkbcommon.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

And indeed wpe-1.pc file has no Requires: xkbcommon line.

[tkloczko@devel-g2v SPECS]$ rpm -ql libwpe-devel | grep include.*.h$ | xargs grep -h \#include|sort | uniq
#include "export.h"
#include "gamepad.h"
#include "input.h"
#include "input-xkb.h"
#include "keysyms.h"
#include "libwpe-version.h"
#include "loader.h"
#include "pasteboard.h"
#include "renderer-backend-egl.h"
#include "renderer-host.h"
#include <stdbool.h>
#include <stdint.h>
#include "version-deprecated.h"
#include "version.h"
#include "view-backend.h"
#include <wpe/export.h>
#include "wpe.h"
#include <xkbcommon/xkbcommon-compose.h>
#include <xkbcommon/xkbcommon.h>

As wpe iuses xkbcommon headers it should depend on in pkgconfig description

kloczek commented 2 years ago

Kind of only issue is that wpe can be build with and without xkb support and what is in the header files and pkgconfig file should be indicated. Looks like in wpe/input-xkb.h none depends on -DWPE_ENABLE_XKB=1 define.

Also looks like wpe.pc.in file is no longer used because is used meson pkgconfig files generator.

kloczek commented 2 years ago

Temporary I've added patch

--- a/meson.build~      2022-05-16 18:04:51.000000000 +0000
+++ b/meson.build       2022-05-29 02:55:10.721776653 +0000
@@ -106,6 +106,7 @@
        name: 'wpe-' + api_version,
        subdirs: 'wpe-' + api_version,
        libraries: libwpe,
+       requires: 'xkbcommon',
        version: meson.project_version(),
        extra_cflags: pkg_cflags,
 )

to have something working however I'm fully aware that this is not right solution.

eli-schwartz commented 2 years ago

xkbcommon is already in Requires.private, so I wonder why this doesn't already work for you?

kloczek commented 2 years ago

Requires.private is ONLY for statuc linking. From https://people.freedesktop.org/~dbn/pkg-config-guide.html

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. For this reason, it is usually only appropriate to add modules from the same package in Requires.

If you will look on what does cmake pkg_check_modules(), meson dependency() or aclocal PKG_CHECK_MODULES() macro you will find that in all those cases pkg-config is never executed with --print-requires-private and always with --print-requires. Only meson can handle now static linking by usimg pkg-conf with --print-requires-private but it is only used to obtain list of static libraries because it does not matter how you are linking header files of external pkgconfig module will be ALWAYS used.

If heder files of exact pkgconfig module are using other pkgconfig module heders it needs to be listed in Requires.

eli-schwartz commented 2 years ago

The reason I say this is because pkg-config and pkgconf include the cflags of a private requirement even without using --static.

kloczek commented 2 years ago

The reason I say this is because pkg-config and pkgconf include the cflags of a private requirement even without using --static.

This issue is not about --cflags output. In both cases when used is cmake and meson generated wpe-1.0.pc file is without Requires.private And just made experiment. After change wpe-1.0.pc to:

[tkloczko@devel-g2v SPECS]$ pkg-config --cflags wpe-1.0
-I/usr/include/wpe-1.0 -DWPE_ENABLE_XKB=1
[tkloczko@devel-g2v SPECS]$ cat /usr/lib64/pkgconfig/wpe-1.0.pc
prefix=/usr
includedir=${prefix}/include
libdir=${prefix}/lib64

Name: wpe-1.0
Description: The wpe library
Version: 1.13.2
Requires.private: xkbcommon
Libs: -L${libdir} -lwpe-1.0
Cflags: -I${includedir}/wpe-1.0 -DWPE_ENABLE_XKB=1

pkg-config --cflags shows

[tkloczko@devel-g2v SPECS]$ pkg-config --cflags wpe-1.0
-I/usr/include/wpe-1.0 -DWPE_ENABLE_XKB=1

Reason for that is because in bot cases includedir is the same. As long as pkg-config is always executed with --print-requires content of the Cflags: lines is irrelevant because none of those lines is able to cause that pkg-config --print-requires wpe-1.0 can show that it deepens on xkbcommon.

Issue is still the same .. no Requires: dependencies, and I want only to point that in none of the cases use cmake or meson to build wpe Requires.private is not present in installed .pc wile.

kloczek commented 2 years ago

With that temporary altered content of the wpe-1.0.pc with Requires.private

[tkloczko@devel-g2v SPECS]$ pkg-config --print-requires-private wpe-1.0
xkbcommon
[tkloczko@devel-g2v SPECS]$ pkg-config --print-requires wpe-1.0
[tkloczko@devel-g2v SPECS]$
eli-schwartz commented 2 years ago

This issue is not about --cflags output.

The issue is about resolving:

/usr/include/wpe-1.0/wpe/input-xkb.h:48:10: fatal error: xkbcommon/xkbcommon.h: No such file or directory
   48 | #include <xkbcommon/xkbcommon.h>

Which means the issue is about making sure that pkg-config --cflags wpe-1.0 produces the correct flags to make header dependencies on xkbcommon/xkbcommon.h resolve. i.e. this is very much about --cflags output.

kloczek commented 2 years ago

--cflags only alters -I<foo> list .. not list of installed pkgconfig dependencies.

kloczek commented 2 years ago

Did you try to reporoduce that iisue? try any prjext which is using wpe and is using pkgconfig. At least two projects are using now wpe.

[tkloczko@devel-g2v SPECS]$ grep 'pkgconfig(wpe-1.0)' *
webkitgtk.spec:BuildRequires:   pkgconfig(wpe-1.0)
wpebackend-fdo.spec:BuildRequires:      pkgconfig(wpe-1.0)
aperezdc commented 2 years ago

In the case when support for xkbcommon is enabled when building libwpe:

From a theoretical point of view, an application that uses libxkbcommon directly (and maybe calls functions from libwpe that deal with libxkbcommon features) should specify libxkbcommon as a dependency on its own, instead of relying on libwpe to indirectly pull it.

I think we can get out of this pickle by keeping Requires.private: xkbcommon (and in the case of the CMake build system change it to produce that line), and fixing up include/wpe/input-xkb.h to not include libxkbcommon headers (possible, due to only opaque types being used). Let me try writing a patch for this.

kloczek commented 2 years ago

From a theoretical point of view, an application that uses libxkbcommon directly (and maybe calls functions from libwpe that deal with libxkbcommon features) should specify libxkbcommon as a dependency on its own, instead of relying on libwpe to indirectly pull it.

I think we can get out of this pickle by keeping Requires.private: xkbcommon (and in the case of the CMake build system change it to produce that line), and fixing up include/wpe/input-xkb.h to not include libxkbcommon headers (possible, due to only opaque types being used). Let me try writing a patch for this.

eli-schwartz commented 2 years ago

@kloczek,

Unfortunately you are wrong. Every version of pkg-config treats Requires.private as required to be installed, providing additional Cflags (public) and Libs.private (private).

Using pkg-config --static controls whether or not "requires" will provide their Libs. The Cflags are used either way.

There have been proposals to add a third field to indicate "internal" requires which aren't parsed at all unless --static is used.

kloczek commented 2 years ago

Every version of pkg-config treats Requires.private as required to be installed, providing additional Cflags (public) and Libs.private (private).

I've already showed you examples of commands whcih soes not show such connection. As well your clainm is only unproven claim which is agains what is literally in pkgconfig documentation.

Libs.private generally is for the cases when libatries have no pkgconfig files and it is as well for static linking only.