[ucrt64|mingw64] pkgconf 1.9.3-1 issue with library ordering

[1480c1]

Description

Installing opusfile and running pkgconf --static --libs opusurl provides incorrect ordering of libraries

cddeg@ccom-laptop UCRT64 /c/Users/cddeg
$ pkgconf.exe --version
1.8.0

cddeg@ccom-laptop UCRT64 /c/Users/cddeg
$ pkgconf --static --libs opusurl
-LC:/msys64/ucrt64/lib -lopusurl -lopusfile -LC:/msys64/ucrt64/lib -logg -lopus -lm -lssp -LC:/msys64/ucrt64/lib -lssl -LC:/msys64/ucrt64/lib -lws2_32 -lgdi32 -lcrypt32 -lcrypto -lws2_32 -lgdi32 -lcrypt32

cddeg@ccom-laptop UCRT64 /c/Users/cddeg
$ pacman -Su
:: Starting core system upgrade...
 there is nothing to do
:: Starting full system upgrade...
resolving dependencies...
looking for conflicting packages...

Packages (1) mingw-w64-ucrt-x86_64-pkgconf-1.9.3-1

Total Installed Size:  0.49 MiB
Net Upgrade Size:      0.01 MiB

:: Proceed with installation? [Y/n] y
(1/1) checking keys in keyring                                           [#######################################] 100%
(1/1) checking package integrity                                         [#######################################] 100%
(1/1) loading package files                                              [#######################################] 100%
(1/1) checking for file conflicts                                        [#######################################] 100%
(1/1) checking available disk space                                      [#######################################] 100%
:: Processing package changes...
(1/1) upgrading mingw-w64-ucrt-x86_64-pkgconf                            [#######################################] 100%

cddeg@ccom-laptop UCRT64 /c/Users/cddeg
$ pkgconf.exe --version
1.9.3

cddeg@ccom-laptop UCRT64 /c/Users/cddeg
$ pkgconf --static --libs opusurl
-LC:/msys64/ucrt64/lib -lssl -LC:/msys64/ucrt64/lib -lcrypto -lws2_32 -lgdi32 -lcrypt32 -lopusurl -lopusfile -LC:/msys64/ucrt64/lib -logg -lopus -lm -lssp

for reference, on a linux install

$ pkgconf --static --libs opusurl
-lopusurl -lopusfile -lm -logg -lopus -lm -lssl -ldl -pthread -lcrypto -ldl -pthread

haven't tested other environments

Verification

• [X] I have verified that my MSYS2 is up-to-date before submitting the report (see https://www.msys2.org/docs/updating/)

Windows Version

Microsoft Windows [Version 10.0.22621.608]

MINGW environments affected

• [X] MINGW64
• [X] MINGW32
• [X] UCRT64
• [ ] CLANG64
• [ ] CLANG32
• [ ] CLANGARM64

Expected behavior

probably something similar to linux output with opusfile first then its dependencies

Actual behavior

opusfile somehow comes near the middle

Repro steps

1. install pkgconf and opusfile for the subsystem
2. run pkgconf --static --libs opusurl and check the output
3. create file with:

#include <stdint.h>
#include <opus/opusfile.h>

int main() { return (intptr_t)opus_server_info_init; }

4. run gcc -static $(pkgconf --static --cflags opusurl) temp.c $(pkgconf --static --libs opusurl)

tested with both 1.8.0 and 1.9.3, and 1.8.0 works but not 1.9.3

Also, linux version is 1.8.0, so it seems they haven't updated it there (https://archlinux.org/packages/core/x86_64/pkgconf/)

Are you willing to submit a PR?

Yes

[1480c1]

Tested with pkgconf's git master as well and still same behavior, so it is still present there.

Bisected to the first (non-build broken) commit to https://github.com/pkgconf/pkgconf/commit/197fcadd4c44504808a44fb257275684b5207b10

Something within pkgconf_queue_collect_dependents() is causing the issue, as commenting out the function causes the gcc line to compile and link correctly and print out the excepted order of libraries

Specifically, commenting out

	PKGCONF_FOREACH_LIST_ENTRY(pkg->required.head, node)
	{
		pkgconf_dependency_t *flattened_dep;

		flattened_dep = pkgconf_dependency_copy(client, node->data);

		pkgconf_node_insert(&flattened_dep->iter, flattened_dep, &world->required);
	}

seems to be enough

[1480c1]

Interestingly, this does not reproduce on Linux:

[ccom1@ccom-pc pkgconf (master 0226cdd) 15:42:03]$ ./bui/pkgconf --static --libs opusurl --version
1.9.3
[ccom1@ccom-pc pkgconf (master 0226cdd) 15:42:07]$ ./bui/pkgconf --static --libs opusurl
-lopusurl -lopusfile -lm -logg -lopus -lm -lssl -lcrypto -ldl -pthread 

I am not sure how though

commit 0226cdda6d6c764bf18df24869c043c85e11cdd1 (HEAD -> master, origin/master, origin/HEAD)
Merge: bddf164 fa803c7
Author: Ariadne Conill <[email protected]>
Date:   Sat Aug 20 13:27:45 2022 +0000

    Merge pull request 'meson: use a feature option for tests instead of boolean' (#244) from dcbaker/pkgconf:tests-feature into master
    
    Reviewed-on: https://gitea.treehouse.systems/ariadne/pkgconf/pulls/244

tried to see if there were major differences between the output of --debug, and it seems the windows one has additional lines (besides the ones mentioning /ucrt64 -> C:\msys64\ucrt64)

../libpkgconf/pkg.c:1375 [pkgconf_pkg_verify_dependency]: trying to verify dependency: libcrypto
../libpkgconf/pkg.c:761 [pkgconf_pkg_find]: looking for: libcrypto
../libpkgconf/cache.c:102 [pkgconf_cache_lookup]: found: libcrypto @00000198E91E8820
../libpkgconf/pkg.c:564 [pkgconf_pkg_ref]: libcrypto refcount@00000198E91E8820: 4
../libpkgconf/pkg.c:793 [pkgconf_pkg_find]: libcrypto is cached
../libpkgconf/pkg.c:564 [pkgconf_pkg_ref]: libcrypto refcount@00000198E91E8820: 5
../libpkgconf/queue.c:199 [flatten_dependency_set]: added libcrypto to dep table
../libpkgconf/pkg.c:587 [pkgconf_pkg_unref]: libcrypto refcount@00000198E91E8820: 4
../libpkgconf/pkg.c:1375 [pkgconf_pkg_verify_dependency]: trying to verify dependency: libssl
../libpkgconf/pkg.c:761 [pkgconf_pkg_find]: looking for: libssl
../libpkgconf/cache.c:102 [pkgconf_cache_lookup]: found: libssl @00000198E91E86B0
../libpkgconf/pkg.c:564 [pkgconf_pkg_ref]: libssl refcount@00000198E91E86B0: 3
../libpkgconf/pkg.c:793 [pkgconf_pkg_find]: libssl is cached
../libpkgconf/pkg.c:564 [pkgconf_pkg_ref]: libssl refcount@00000198E91E86B0: 4
../libpkgconf/queue.c:183 [flatten_dependency_set]: dedup libssl = libcrypto?
../libpkgconf/queue.c:199 [flatten_dependency_set]: added libssl to dep table
../libpkgconf/pkg.c:587 [pkgconf_pkg_unref]: libssl refcount@00000198E91E86B0: 3
../libpkgconf/pkg.c:1375 [pkgconf_pkg_verify_dependency]: trying to verify dependency: opusfile
../libpkgconf/pkg.c:761 [pkgconf_pkg_find]: looking for: opusfile
../libpkgconf/cache.c:102 [pkgconf_cache_lookup]: found: opusfile @00000198E91E97E0
../libpkgconf/pkg.c:564 [pkgconf_pkg_ref]: opusfile refcount@00000198E91E97E0: 3
../libpkgconf/pkg.c:793 [pkgconf_pkg_find]: opusfile is cached
../libpkgconf/pkg.c:564 [pkgconf_pkg_ref]: opusfile refcount@00000198E91E97E0: 4
../libpkgconf/queue.c:183 [flatten_dependency_set]: dedup opusfile = libcrypto?
../libpkgconf/queue.c:183 [flatten_dependency_set]: dedup opusfile = libssl?
../libpkgconf/queue.c:199 [flatten_dependency_set]: added opusfile to dep table
../libpkgconf/pkg.c:587 [pkgconf_pkg_unref]: opusfile refcount@00000198E91E97E0: 3

Attempting to debug leads to

So, uhh, qsort issue? Doesn't make much sense to me as to why

[1480c1]

nvm, making qsort match the sorting doesn't change anything wrt the output, wrong tree bark

[1480c1]

Wait, it breaks the linux output to match the Windows output, huh

[1480c1]

diff --git a/libpkgconf/queue.c b/libpkgconf/queue.c
index 61637e3..53f6241 100644
--- a/libpkgconf/queue.c
+++ b/libpkgconf/queue.c
@@ -144,7 +144,9 @@ dep_sort_cmp(const void *a, const void *b)
        const pkgconf_dependency_t *depA = *(void **) a;
        const pkgconf_dependency_t *depB = *(void **) b;
 
-       return depB->match->hits - depA->match->hits;
+       if (depB->match->hits < depA->match->hits || depB < depA)
+               return -1;
+       return 1;
 }
 
 static inline void

seems to fix it for opusurl on windows and does not break linux, but I don't know if that will cause a regression in any other packages

~~It does seem to change the output, so probably not suitable for upstream~~ wait, forgot my machine has 1.8, let me try again

[ccom1@ccom-pc pkgconf (master 0226cdd) 17:04:33]$ ./bui/pkgconf --static --libs opusurl
-lopusurl -lopusfile -lm -logg -lopus -lm -lssl -lcrypto -ldl -pthread 
[ccom1@ccom-pc pkgconf (master 0226cdd) 17:04:41]$ pkgconf --static --libs opusurl
-lopusurl -lopusfile -lm -logg -lopus -lm -lssl -ldl -pthread -lcrypto -ldl -pthread

Seems to keep the same output for linux (bui is upstream master, bui1 is with the diff applied)

[ccom1@ccom-pc pkgconf (master 0226cdd) 17:06:04]$ ./bui/pkgconf --static --libs opusurl
-lopusurl -lopusfile -lm -logg -lopus -lm -lssl -lcrypto -ldl -pthread 
[ccom1@ccom-pc pkgconf (master 0226cdd) 17:06:07]$ ./bui1/pkgconf --static --libs opusurl
-lopusurl -lopusfile -lm -logg -lopus -lm -lssl -lcrypto -ldl -pthread

[1480c1]

Of course, the above relies on stuff that I'm not sure if defined for qsort and pointers, but 🤷

[1480c1]

opened an upstream issue https://github.com/pkgconf/pkgconf/issues/268

[lazka]

imo if you want stable output you'd want something like:

size_t res = depB->match->hits - depA->match->hits;
if (res == 0)
    res = strcmp(depB->package, depA->package);
if (res == 0)
    res = strcmp(depB->version, depA->version);
if (res == 0)
    res = strcmp(depB->match->id, depA->match->id);
return res;

but if the interpretation of the result depends on the properties of the sort algo then imo there is something else broken before that (or there are dep cycles and linux is lucky?)

[revelator]

im wondering if this is also the culprit responsible for not being able to read absolute windows paths from the pkgconfig files ?, when trying to build ffmpeg it chokes on this line -ic:/msys2/mingw32/lib/libstdc++.a though i notice it is using the wrong -i flag should be -I not lower case -i i think. though from most other sources that include the static version of libstdc++ it is usually just /path/lib/staticlibraryname.a without either. guess we need a sed script to yank this out. the failing pkgconfig is from the vulkan loader.