Buildsystem: Use pkg-config for miniupnpc and mbedtls

[dustdfg]

Miniupnpc added pkg-config files in 2.2.3 but we require 2.2.8 miniupnp/miniupnp@5a39800

MbedTLS added pkg-config files in 3.6.0 while we require 3.6.1 https://github.com/Mbed-TLS/mbedtls/blob/development/ChangeLog

[dustdfg]

DISCLAIMER: I haven't checked if it builds or not with system libraries at all. I've just noticed those strings and checked if the upstreams started to provide pkg-config files and if our required versions are higher than those versions which added pkg-config support

[akien-mga]

To be clear, we don't require miniupnpc 2.2.8 or mbedtls 3.6.1. These are just the versions we have builtin, but Godot should still compile with earlier versions.

But at least for miniupnpc the pkgconfig file was added in a somewhat old release so we can expect Linux distros will have caught up with it.

For mbedtls, it might be an issue however, we still support mbedtls 2.28 and a lot of Linux distros still have that as their default version. So we might want to keep the old method there for the time being, or let Linux distros patch the detect.py file for their own needs, it's not too difficult.

[akien-mga]

Maybe we can do something like this instead to preserve support for mbedtls 2.28 LTS:

diff --git a/platform/linuxbsd/detect.py b/platform/linuxbsd/detect.py
index 2fd573da75..e8c2f88435 100644
--- a/platform/linuxbsd/detect.py
+++ b/platform/linuxbsd/detect.py
@@ -279,16 +279,18 @@ def configure(env: "SConsEnvironment"):
         env.ParseConfig("pkg-config libwebp --cflags --libs")
 
     if not env["builtin_mbedtls"]:
-        # mbedTLS does not provide a pkgconfig config yet. See https://github.com/ARMmbed/mbedtls/issues/228
-        env.Append(LIBS=["mbedtls", "mbedcrypto", "mbedx509"])
+        # mbedTLS only provides a pkgconfig file since 3.6.0, but we still support 2.28.x,
+        # so fallback to manually specifying LIBS if it fails.
+        try:
+            env.ParseConfig("pkg-config mbedtls mbedcrypto mbedx509 --cflags --libs")
+        except:
+            env.Append(LIBS=["mbedtls", "mbedcrypto", "mbedx509"])
 
     if not env["builtin_wslay"]:
         env.ParseConfig("pkg-config libwslay --cflags --libs")
 
     if not env["builtin_miniupnpc"]:
-        # No pkgconfig file so far, hardcode default paths.
-        env.Prepend(CPPPATH=["/usr/include/miniupnpc"])
-        env.Append(LIBS=["miniupnpc"])
+        env.ParseConfig("pkg-config miniupnpc --cflags --libs")
 
     # On Linux wchar_t should be 32-bits
     # 16-bit library shouldn't be required due to compiler optimizations

(Or if SCons provides a version of ParseConfig that doesn't raise an exception but just returns an error code, we could use that.)

[dustdfg]

Looks reasonable but it didn't throw any exception even on empty command 😅
Moreover in the start of the file we check if pkg-config exist by using pkgconf_error = os.system("pkg-config --version > /dev/null") so...

[dustdfg]

And later check if lib is available with if os.system("pkg-config --exists xkbcommon") == 0: # 0 means found

[dustdfg]

Ohh... It looks like there is also another "bug" but I am not sure if it should be named so... For some libs we check if lib exist with pkg-config --exists but not for all. It is strange

[akien-mga]

It does throw an exception for me if there's no pkg-config file for mbedtls. But indeed os.system("pkg-config --exists mbedtls") == 0 is a better option, so I suggest this diff, which works fine for me locally (uses the first branch when mbedtls-devel is installed, and the else branch otherwise).

diff --git a/platform/linuxbsd/detect.py b/platform/linuxbsd/detect.py
index 2fd573da75..c8202b147d 100644
--- a/platform/linuxbsd/detect.py
+++ b/platform/linuxbsd/detect.py
@@ -279,16 +279,18 @@ def configure(env: "SConsEnvironment"):
         env.ParseConfig("pkg-config libwebp --cflags --libs")
 
     if not env["builtin_mbedtls"]:
-        # mbedTLS does not provide a pkgconfig config yet. See https://github.com/ARMmbed/mbedtls/issues/228
-        env.Append(LIBS=["mbedtls", "mbedcrypto", "mbedx509"])
+        # mbedTLS only provides a pkgconfig file since 3.6.0, but we still support 2.28.x,
+        # so fallback to manually specifying LIBS if it fails.
+        if os.system("pkg-config --exists mbedtls") == 0:  # 0 means found
+            env.ParseConfig("pkg-config mbedtls mbedcrypto mbedx509 --cflags --libs")
+        else:
+            env.Append(LIBS=["mbedtls", "mbedcrypto", "mbedx509"])
 
     if not env["builtin_wslay"]:
         env.ParseConfig("pkg-config libwslay --cflags --libs")
 
     if not env["builtin_miniupnpc"]:
-        # No pkgconfig file so far, hardcode default paths.
-        env.Prepend(CPPPATH=["/usr/include/miniupnpc"])
-        env.Append(LIBS=["miniupnpc"])
+        env.ParseConfig("pkg-config miniupnpc --cflags --libs")
 
     # On Linux wchar_t should be 32-bits
     # 16-bit library shouldn't be required due to compiler optimizations

[Repiteo]

Thanks!

[darksylinc]

This PR breaks builtin_miniupnpc=yes.

See #99196