SFML/SFML

Wrong usage of GNUInstallDirs when generating pkg-config files

ThwyIgo opened this issue · 8 comments

ThwyIgo commented

Prerequisite Checklist

Describe your issue here

SFML uses GNUInstallDirs variables (such as CMAKE_INSTALL_LIBDIR) to generate pkg-config files (located in tools/pkg-config).
The problem is that variables like CMAKE_INSTALL_LIBDIR aren't guaranteed to be a path relative to CMAKE_INSTALL_PREFIX, they can even be an absolute path!
This is described in the official documentation, where it says "However, an absolute path is also allowed" under CMAKE_INSTALL_<dir>.

All files in tools/pkg-config/*.pc.in define libdir as ${exec_prefix}/@CMAKE_INSTALL_LIBDIR@, which might concatenate two absolute paths together. This is the case on NixOS for example.

This problem should be easy to fix using cmake_path() as described below:
https://github.com/jtojnar/cmake-snips#concatenating-paths-when-building-pkg-config-files

Also, the install location of *.pc files is affected. Check: this line in SFML 2.6.x and also this line in SFML master.
Both define SFML_PKGCONFIG_DIR variable using CMAKE_INSTALL_LIBDIR which later gets concatenated with CMAKE_INSTALL_PREFIX.

Your Environment

  • OS / distro / window manager: Linux / NixOS / XMonad
  • SFML version: 2.6.1
  • Compiler / toolchain: g++ / gcc
  • Special compiler / CMake flags: -DSFML_INSTALL_PKGCONFIG_FILES=TRUE -DSFML_USE_SYSTEM_DEPS=TRUE

Steps to reproduce

If it helps, I'm packaging SFML 2.6.1 on NixOS here.
I made a temporary workaround to fix *.pc.in files using sed, but the install location is still wrong.

You can build the derivation and check where pkg-config files were installed. In my system, they're at
/nix/store/<hash>-sfml-2.6.1/nix/store/<hash>-sfml-2.6.1/lib/pkgconfig
Clearly two absolute paths were concatenated.

Expected behavior

pkg-config files installed in /nix/store/<hash>-sfml-2.6.1/lib/pkgconfig
libdir defined as ${exec_prefix}/lib

Actual behavior

pkg-config files installed in /nix/store/<hash>-sfml-2.6.1/nix/store/<hash>-sfml-2.6.1/lib/pkgconfig
libdir defined as ${exec_prefix}/<CMAKE_INSTALL_PREFIX>/lib

ChrisThrasher commented

c20ab7d

I recently merged something after 2.6.1 was released that touches this code. Can you test with the 2.6.x branch to confirm this bug is still present?

ThwyIgo commented

Just tested with commits c1c65b5 and c20ab7d.
The error is present in both. Nix is nice and it even gives the following error message:

The following lines have issues (specifically '//' in paths).
3:libdir=${exec_prefix}//nix/store/y4c1dwdd3930v58iga8vn43sm413b1g5-sfml-2.6.1/lib
It is very likely that paths are being joined improperly.
ThwyIgo commented

I didn't test, but something like this might help:

cmake_path(RELATIVE_PATH CMAKE_INSTALL_FULL_LIBDIR
    BASE_DIRECTORY ${CMAKE_INSTALL_PREFIX}
    OUTPUT_VARIABLE RELATIVE_LIB_DIR)

Then in *.pc.in files:
libdir=${exec_prefix}/@RELATIVE_LIB_DIR@

ChrisThrasher commented

master...install_libdir

I took a shot at fixing this. Check this out and let me know what you think.

EDIT: I just realized this is using a CMake 3.20 feature that will have to get rewritten for 2.6.x to be compatible with an older CMake version. v3 can use this solution but I'll eventually have to rewrite this for v2.

ThwyIgo commented

It looks like install_libdir is based on 2.6.x branch, so I did a git merge origin/install_libdir on master, then tried to compile.
It failed, but it was because tools/pkg-config/sfml-window.pc.in was still using the old variable. I changed it to use SFML_RELATIVE_INSTALL_LIBDIR and it worked!!

The final sfml-all.pc (and other .pc files) got installed to the right path and it looks like this:

prefix=/nix/store/4ag1is97xq8w2lm9mzfgqrpgaiycp659-sfml-3.0.0
exec_prefix=${prefix}
libdir=${exec_prefix}/lib
includedir=${prefix}/include

Perfect!

And I'm sorry I didn't mention cmake_path() works only with CMake >= 3.20, I actually didn't make a pull request because I don't really know how to fix the problem without it. I think I would end up reimplementing cmake_path() for 3.7.x lol.

EDIT: I only compiled SFML. I haven't tested it with an SFML 3.0.0 project that uses the library.

ChrisThrasher commented

Thanks for testing this out!

It looks like install_libdir is based on 2.6.x branch

I'm going to fix this in SFML 2 first. Eventually we'll merge this fix into v3 as well.

I fixed that issue with sfml-window.pc.in and then I converted the code to be compliant with CMake 3.7. Do you mind testing out the install_libdir branch again to make sure it works? If so we can make a PR for this and get it merged soon.

ThwyIgo commented

Just tested 872facc and compiled a simple SFML project. It's working just fine.
I used CMake 3.27.7, but yeah, file(RELATIVE_PATH ...) is indeeed compatible with CMake 3.7, I didn't know it was a thing haha.

Thank you!