matplotlib/mplcairo

pkgconfig version checks are broken

QuLogic opened this issue · 5 comments

has_pkgconfig_raqm is set by the result of get_pkgconfig, but the latter returns the output of pkg-config --atleast-version. This does not print anything at all; the availability of raqm should be determined by the exit code of the pkg-config process.

The other calls with --atleast-version are similarly broken.

Also, the --cflags/--libs for raqm don't seem to be added to anything?

Thanks for the report. I think

diff --git i/setup.py w/setup.py
index 1f4f4c0..b06fd18 100644
--- i/setup.py
+++ w/setup.py
@@ -111,14 +111,18 @@ class build_ext(build_ext):
             is_arch = "Arch Linux" in Path("/etc/os-release").read_text()
         except OSError:
             is_arch = False
-        has_pkgconfig_raqm = False
-        if not is_arch:
+        if is_arch:
+            has_pkgconfig_raqm = False
+        else:
             try:
-                has_pkgconfig_raqm = get_pkgconfig(
-                    f"--atleast-version={MIN_RAQM_VERSION}", "raqm")
+                get_pkgconfig(f"--atleast-version={MIN_RAQM_VERSION}", "raqm")
             except (FileNotFoundError, CalledProcessError):
-                pass
-        if not has_pkgconfig_raqm:
+                has_pkgconfig_raqm = False
+            else:
+                has_pkgconfig_raqm = True
+        if has_pkgconfig_raqm:
+            ext.extra_compile_args += get_pkgconfig("--cflags", "raqm")
+        else:
             (tmp_include_dir / "raqm-version.h").write_text("")  # Touch it.
             with urllib.request.urlopen(
                     f"https://raw.githubusercontent.com/HOST-Oman/libraqm/"

should fix the problem, can you confirm?

Regarding the other call to --atleast-version (checking the cairo version): I don't use the return value at all, I just want to throw an exception if the version is too old (which get_pkgconfig will do, as it uses check_output).

Regarding the raqm --cflags and --libs: the patch above adds the required --cflags, but I don't want to add --libs, as I would prefer keeping the same strategy as for cairo, which is to load the symbols at runtime (for cairo this is done from the python side with dlopen(..., RTLD_GLOBAL), for raqm with "normal" dlopen/dlsym calls).

Yes, this patch worked. I understand the runtime linking for wheels, but I wonder if it should fully link for packages. I'm not certain if it would be better though.

I pushed the patch.
I guess you can always force libcairo/libraqm to be linked with something like LDFLAGS=$(pkg-config --libs cairo raqm) python setup.py bdist_wheel? I'd have to keep the dlopen/dlsym code anyways...

Fixed by 11bb8a3