We need pc files

[nacho]

Apart from the one added right now for libxml we need the following:

• protobuf-c
• lz4
• turbojpeg
• libsasl2 (cyrus-sasl)
• opus
• portaudio
• wing (I am checking why it is not installed)

[guruDanny67]

Turbojpeg should be already present, for the others I agree, now that I know how to use them, with meson, I make .pc files for all the libraries I use :)

[nacho]

wing should be fixed in, still need to make a new release There is another one that has issues: pango. The pc files contains variables for the include dir and the libdir and pkg-config is not able to handle those dynamically... I wonder if this is a bug on the pkg-config that we are using.

[nacho]

gdk-pixbuf-2.0.pc is another one giving problems

[nacho]

This makes me wonder if the problematic point here is the new pkgconf

[nacho]

And atk is another

[guruDanny67]

The atk, pango & gdk-pixbuf .pc files are used by the check_libs project (that we should rename in 'check_pc_files', btw) and they work correctly.

I use, at work, the gtkmm3 .pc files (that uses all the gtk3 stack) and they work.

I used meson to find the dependency and also the solution file is ok, with all the include & libs loaded.

What is exactly your problem?

Thanks

Il giorno gio 30 ago 2018 alle ore 11:06 Ignacio Casal Quinteiro < notifications@github.com> ha scritto:

And atk is another

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/wingtk/gvsbuild/issues/276#issuecomment-417246554, or mute the thread https://github.com/notifications/unsubscribe-auth/ALgbgcbzC2i2qHKR2GfZg0ig-Ckvq_OTks5uV6sngaJpZM4WRQuX .

[nacho]

It is simple to test this:

copy the gtk directory somewhere else, i.e to "pippo" and run the following:

.\pippo\x64\release\bin\pkg-config.exe --libs --cflags pango -IC:/gtk-build/gtk/x64/release/include/pango-1.0 -IC:/gtk-build/gtk/x64/release/include/glib-2.0 -IC:/gtk-build/gtk/x64/release/lib/glib-2.0/include -IC:/gtk-build/pippo/x64/release/bin/../include/fribidi -lpango-1.0 -lgobject-2.0 -lglib-2.0 -lintl

see that it points to gtk instead of pippo

now modify the pango.pc file to have just the prefix hardcoded and the other vars referring to prefix, you will see the following:

.\pippo\x64\release\bin\pkg-config.exe --libs --cflags pango -IC:/gtk-build/pippo/x64/release/bin/../include/pango-1.0 -IC:/gtk-build/gtk/x64/release/include/glib-2.0 -IC:/gtk-build/gtk/x64/release/lib/glib-2.0/include -IC:/gtk-build/pippo/x64/release/bin/../include/fribidi -LC:/gtk-build/pippo/x64/release/bin/../lib -lpango-1.0 -lgobject-2.0 -lglib-2.0 -lintl

This is how pango.pc looks after the changes

prefix=C:\gtk-build\gtk\x64\release exec_prefix=${prefix} libdir=${prefix}/lib includedir=${prefix}/include Name: Pango Description: Internationalized text handling Version: 1.42.3 Requires: glib-2.0 gobject-2.0 Requires.private: fribidi Libs: -L${libdir} -lpango-1.0 Libs.private: -lm Cflags: -I${includedir}/pango-1.0

[guruDanny67]

I see, I've also made some test and it's not clear, with pkgconf, when the dir is substituted from the one found on the prefix (in some test works, in some other not).

We should make (or fix) all the .pc to have relative paths (include=$(prefix}/include) and investigate better.

If we want to remove the pkg-config dependency to glib I found, on sourceforge, pkg-config-lite that is the pkg-config standard (not updated, is the 0.28.0 release) with two files with the glib parts/functions used by the package and so buildable directly).

[nacho]

I think fighting pc files might not be a good idea, we either fix pkgconf or we switch back to pkg-config. @TingPing thoughts?

[guruDanny67]

I take a look to use the old pkg-config, optional to the new pkgconf, but now glib, with meson, depends on pkg-config so it's not builtable in a easy way. We can surely upgrade pkgconf to the latest release or use for pkg-config the internal glib, maybe updated.

Pkg-config is 3 source files so have a meson.build for it is really easy, what's complicated is the glib dependency.

I'll take a look in the weekend.

Il lun 3 set 2018, 11:07 Ignacio Casal Quinteiro notifications@github.com ha scritto:

I think fighting pc files might not be a good idea, we either fix pkgconf or we switch back to pkg-config. @TingPing https://github.com/TingPing thoughts?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/wingtk/gvsbuild/issues/276#issuecomment-418051293, or mute the thread https://github.com/notifications/unsubscribe-auth/ALgbgcbrSst_G-mWIB5gZ6XCXdalVUy7ks5uXPFqgaJpZM4WRQuX .

[nacho]

I already tried the last version of pkgconf and it does not work either. Also I had to manually patch it since it does not build anymore with meson.

[guruDanny67]

I see: from a 1.5.3 package that says it's version 1.4.0 I don't expect too much ;(

I think that it's better to put our efforts to make the old pkg-config works as before. As I told I take a look this weekend, the wheater seems bad in Italy ...

[nacho]

probably the right solution would be to reimplement pkg-config inside meson

[guruDanny67]

I open a PR to use the old pkg-config (with --with-pkg-config) to use, optionally, the old system and see if it's better then pkgconf.

I put the glib 2.58.0 modules used by pkg-config, trying to limit the executable size but I'm not able to make the -Gy option work (put all functions in separate blocks) so we have a 500k executable.

If you think is ok you can also merge this pull request: being optional if nothing is added to the scripts the behaviour is the same of the old one, unless we change the default and make the pkgconf optional.

with best regards

[TingPing]

Sorry I didn't notice this thread:

I see, I've also made some test and it's not clear, with pkgconf, when the dir is substituted from the one found on the prefix (in some test works, in some other not).

https://github.com/pkgconf/pkgconf/commit/60c05f5621b771a288f80b1239fd0a287c58f3d4

/* Some pc files will use absolute paths for all of their directories
 * which is broken when redefining the prefix. We try to outsmart the
 * file and rewrite any directory that starts with the same prefix.
 */

This matches pkg-config's implementation, at least what I remember when I wrote it.

[nacho]

@TingPing awesome! note though that pkgconf is now in a different repo

[nacho]

@TingPing also, can you backport this patch in gvsbuild?

[TingPing]

That patch was before gvsbuild even used pkgconf.

[nacho]

Then the patch is broken :( see the tests I made a couple of weeks ago

[TingPing]

Uh, well yea:

copy the gtk directory somewhere else, i.e to "pippo" and run the following:

Just don't do that. The build dir isn't portable, was never portable, and will never be portable.

[nacho]

@TingPing well the only reason this is not supported right now is because of pkgconf... I've been using the builddir in a different directory for years already, but this has become an issue now that I tried to port the project to meson and using pkg-config...

[TingPing]

I've been using the builddir in a different directory for years already,

It worked because of chance. You should understand undefined behavior working does not mean it is supported.

but this has become an issue now that I tried to port the project to meson and using pkg-config...

Yes because Meson projects very rarely use ${prefix}. We should probably change that in GNOME at least.

As I said my behavior in pkgconf matches pkg-config so that part never changed and was always possible to break.

[nacho]

As I said, I was not using pc files before since I was using the visual studio projects and linking directly the .lib files. Now I want a way to handle this, and I think it is just a matter of fixing pkgconf. As far as I see it the problem is that in some cases the prefix is not substituted... I'll try to dig further on this once I finish my work on glib-networking but till then any help is appreciated, specially since you already touched that part of the code

[TingPing]

Now I want a way to handle this, and I think it is just a matter of fixing pkgconf. As far as I see it the problem is that in some cases the prefix is not substituted...

So as far as I'm concerned the current code is correct in all definitions of it:

• It matches pkg-config implementation, that is 99% of the spec basically, to argue against it must have strong reasons
• It avoids "guessing". It can't possibly know that C:\foo should be $prefix unless pkgconf is in C:\foo. If pkgconf is in C:\bar then it can only assume the absolute path was done on purpose because the library is installed elsewhere (You can't even know how much of the path is the prefix if its from another prefix).

[TingPing]

So my only suggestion is for work to be done in Meson's pkgconfig module so that it understands prefix and automatically rewrites to use it. EDIT: And even then automatic is wrong sometimes, maybe an option to disable.

[guruDanny67]

I make some more test, moving the gtk/win32/release directory around and, for the most part, pkgconf seems to work correctly, as stated. I don't like the fact that the .. is not collapsed (you got -Lc:/my_moved_dir/bin/../lib instead of -Lc:/my_moved_dir/lib) but I can live with this :)

I may have found the problem, at least with atk, pango & gdk-pibuf.

It seems that sometimes the prefix is substituted with the pkgconf running path and sometimes not but is probably an error in the .pc file: for atk the prefix is written with the window's backslash path separator while the libdir & includedir are written with the unix slash path separator:

prefix=c:\gtk-build_bld\vs2017\gtk\Win32\release exec_prefix=c:\gtk-build_bld\vs2017\gtk\Win32\release libdir=c:/gtk-build/_bld/vs2017/gtk/Win32/release/lib includedir=c:/gtk-build/_bld/vs2017/gtk/Win32/release/include

Looking for the cflags, with this .pc file, you always get what's written in the .pc file (c:/gtk-build/..../include) because the pkgconf doesn't understand that's the initial parts of the paths are the same.

Writing the prefix with the unix slash fixes the think and moving the file around moves also the generated response for libs & cflags.

We can open a issue on pkgconf to have the path always normalized in one way or another (backslash or slash) but it's surely good to fix our .pc files to have the same initial path or, better, always use relative paths (e.g. ${prefix}/whatever)

With best regards.

[TingPing]

It seems that sometimes the prefix is substituted with the pkgconf running path and sometimes not but is probably an error in the .pc file: for atk the prefix is written with the window's backslash path separator while the libdir & includedir are written with the unix slash path separator

Good catch!, Yea pkgconf just does a straight strncmp() on the paths.

[nacho]

Not able to currently test it but I guess this would work...

diff --git a/libpkgconf/pkg.c b/libpkgconf/pkg.c
index 0c3a692..9ed568f 100644
--- a/libpkgconf/pkg.c
+++ b/libpkgconf/pkg.c
@@ -188,22 +188,40 @@ determine_prefix(const pkgconf_pkg_t *pkg, char *buf, size_t buflen)
        return buf;
 }

+/* Changes all the backslashes to slashes */
+static const char *
+normalize_path(char *buf)
+{
+       char *p = buf;
+
+       while (*p) {
+               if (*p == '\\')
+                       *p = '/';
+               p++;
+       }
+}
+
 static bool
 pkgconf_pkg_parser_value_set(pkgconf_pkg_t *pkg, const size_t lineno, const char *keyword, char *value)
 {
+       char normalized_value[PKGCONF_ITEM_SIZE];
+
        (void) lineno;

+       pkgconf_strlcpy(normalized_value, value, sizeof normalized_value);
+       normalize_path(normalized_value);
+
        /* Some pc files will use absolute paths for all of their directories
         * which is broken when redefining the prefix. We try to outsmart the
         * file and rewrite any directory that starts with the same prefix.
         */
        if (pkg->owner->flags & PKGCONF_PKG_PKGF_REDEFINE_PREFIX && pkg->orig_prefix
-           && !strncmp(value, pkg->orig_prefix->value, strlen(pkg->orig_prefix->value)))
+           && !strncmp(normalized_value, pkg->orig_prefix->value, strlen(pkg->orig_prefix->value)))
        {
                char newvalue[PKGCONF_ITEM_SIZE];

                pkgconf_strlcpy(newvalue, pkg->prefix->value, sizeof newvalue);
-               pkgconf_strlcat(newvalue, value + strlen(pkg->orig_prefix->value), sizeof newvalue);
+               pkgconf_strlcat(newvalue, normalized_value + strlen(pkg->orig_prefix->value), sizeof newvalue);
                pkgconf_tuple_add(pkg->owner, &pkg->vars, keyword, newvalue, false);
        }
        else if (strcmp(keyword, pkg->owner->prefix_varname) || !(pkg->owner->flags & PKGCONF_PKG_PKGF_REDEFINE_PREFIX))
@@ -215,7 +233,7 @@ pkgconf_pkg_parser_value_set(pkgconf_pkg_t *pkg, const size_t lineno, const char

                if (relvalue != NULL)
                {
-                       pkg->orig_prefix = pkgconf_tuple_add(pkg->owner, &pkg->vars, "orig_prefix", value, true);
+                       pkg->orig_prefix = pkgconf_tuple_add(pkg->owner, &pkg->vars, "orig_prefix", normalized_value, true);
                        pkg->prefix = pkgconf_tuple_add(pkg->owner, &pkg->vars, keyword, relvalue, false);
                }

[nacho]

Also on Windows we might want to use strncasecmp instead of strncmp btw

[TingPing]

Also on Windows we might want to use strncasecmp instead of strncmp btw

Indeed.

+normalize_path(char *buf)

This should be just a noop on unix.

[pbor]

Actually on unix (and also on windows after replacing backslashes) it should normalize consecutive slashes into a single slash

[nacho]

Still untested but this should do it also fixing the comment from @pbor:

diff --git a/libpkgconf/pkg.c b/libpkgconf/pkg.c
index 0c3a692..a346dfd 100644
--- a/libpkgconf/pkg.c
+++ b/libpkgconf/pkg.c
@@ -188,22 +188,72 @@ determine_prefix(const pkgconf_pkg_t *pkg, char *buf, size_t buflen)
        return buf;
 }

+static void
+remove_additional_separators(char *buf)
+{
+       char *p = buf;
+
+       while (*p) {
+               char *q;
+
+               q = p;
+               while (*q && *q == '/')
+                       q++;
+
+               if (p != q)
+                       memmove (p, q, strlen (q) + 1);
+
+               p++;
+       }
+}
+
+static void
+canonicalize_path(char *buf)
+{
+#ifdef _WIN32
+       char *p = buf;
+
+       while (*p) {
+               if (*p == '\\')
+                       *p = '/';
+               p++;
+       }
+#endif
+
+       remove_additional_separators(buf);
+}
+
+static bool
+is_path_prefix_equal(const char *path1, const char *path2, size_t path2_len)
+{
+#ifdef _WIN32
+       return !_strnicmp(path1, path2, path2_len);
+#else
+       return !strncmp(path1, path2, path2_len);
+#endif
+}
+
 static bool
 pkgconf_pkg_parser_value_set(pkgconf_pkg_t *pkg, const size_t lineno, const char *keyword, char *value)
 {
+       char canonicalized_value[PKGCONF_ITEM_SIZE];
+
        (void) lineno;

+       pkgconf_strlcpy(canonicalized_value, value, sizeof canonicalized_value);
+       canonicalize_path(canonicalized_value);
+
        /* Some pc files will use absolute paths for all of their directories
         * which is broken when redefining the prefix. We try to outsmart the
         * file and rewrite any directory that starts with the same prefix.
         */
        if (pkg->owner->flags & PKGCONF_PKG_PKGF_REDEFINE_PREFIX && pkg->orig_prefix
-           && !strncmp(value, pkg->orig_prefix->value, strlen(pkg->orig_prefix->value)))
+           && is_path_prefix_equal(canonicalized_value, pkg->orig_prefix->value, strlen(pkg->orig_prefix->value)))
        {
                char newvalue[PKGCONF_ITEM_SIZE];

                pkgconf_strlcpy(newvalue, pkg->prefix->value, sizeof newvalue);
-               pkgconf_strlcat(newvalue, value + strlen(pkg->orig_prefix->value), sizeof newvalue);
+               pkgconf_strlcat(newvalue, canonicalized_value + strlen(pkg->orig_prefix->value), sizeof newvalue);
                pkgconf_tuple_add(pkg->owner, &pkg->vars, keyword, newvalue, false);
        }
        else if (strcmp(keyword, pkg->owner->prefix_varname) || !(pkg->owner->flags & PKGCONF_PKG_PKGF_REDEFINE_PREFIX))
@@ -215,7 +265,7 @@ pkgconf_pkg_parser_value_set(pkgconf_pkg_t *pkg, const size_t lineno, const char

                if (relvalue != NULL)
                {
-                       pkg->orig_prefix = pkgconf_tuple_add(pkg->owner, &pkg->vars, "orig_prefix", value, true);
+                       pkg->orig_prefix = pkgconf_tuple_add(pkg->owner, &pkg->vars, "orig_prefix", canonicalized_value, true);
                        pkg->prefix = pkgconf_tuple_add(pkg->owner, &pkg->vars, keyword, relvalue, false);
                }
                else

[nacho]

I'll fire up now the windows vm and give this a try, let's see how it goes...

[nacho]

https://git.dereferenced.org/pkgconf/pkgconf/pulls/9

[danyeaw]

@nacho Is this still an issue we need to resolve?

[nacho]

I think we can close it