Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 44.13%. Comparing base (6eaf300) to head (e0b6879).
Report is 44 commits behind head on master.

❗ Current head e0b6879 differs from pull request most recent head e4c2d0a. Consider uploading reports for the commit e4c2d0a to get more accurate results

@@            Coverage Diff             @@
##           master    #2917      +/-   ##
==========================================
+ Coverage   43.65%   44.13%   +0.47%     
==========================================
  Files         253      253              
  Lines       20940    21269     +329     
  Branches     5139     5210      +71     
==========================================
+ Hits         9142     9387     +245     
- Misses      10785    11057     +272     
+ Partials     1013      825     -188     

Pull Request Test Coverage Report for Build 12436619949

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.006%) to 57.009%

Totals
Change from base Build 12423971359:	-0.006%
Covered Lines:	12053
Relevant Lines:	19987

💛 - Coveralls

@ChrisThrasher has since implemented this in #2835

Does that also work for you @texus?

The issue still occurs.
PR #2835 appears to be about different problem.

I've rebased my PR on the master branch (to solve the conflict with the change from that PR).

I'm starting to think that we don't actually need SFML_RELATIVE_INSTALL_LIBDIR anymore when SFML_PKGCONFIG_INSTALL_PREFIX is replaced with SFML_PKGCONFIG_INSTALL_DIR. The old issue was about appending CMAKE_INSTALL_FULL_LIBDIR to CMAKE_INSTALL_PREFIX failing when SFML_PKGCONFIG_INSTALL_DIR was an absolute path. However this PR removes the CMAKE_INSTALL_PREFIX variable from this code.

I'm not familiar enough with pkg-config files to actually check whether the generated files are correct though.

I think I understand enough about the pkg-config issues in the meantime, but I'm waiting until there is a consensus on SFML/CSFML#374 to keep the SFML implementation the same as in CSFML.

There are 3 distinct pkg-config issues that exist:

1. When running cmake multiple times with different CMAKE_INSTALL_PREFIX values, the pkg-config files are installed to the location of the initial value instead of the latest one.

   This is the issue that is solved by this PR and was already solved in CSFML before.

   Note that this issue is not specific to using --install with --prefix, the following example also triggers the issue:

   cmake -S . -B build
   cmake -S . -B build -D CMAKE_INSTALL_PREFIX=whatever
   cmake --build build
   cmake --build build --target install

2. The lib directory inside the generated .pc files is incorrect when CMAKE_INSTALL_LIBDIR is an absolute path.

   This issue was resolved in SFML last year (Fix pkg-config support on Nix #2835) by introducing the SFML_RELATIVE_INSTALL_LIBDIR variable.
   The same issue is being resolved in CSFML now (Fix Nix pkg-config support CSFML#374), but there are actually 2 possible ways of solving it.

3. Calling cmake --install with a --prefix option that is different from the earlier CMAKE_INSTALL_PREFIX value leads to a wrong path in the prefix value inside the .pc files.

   This is a new issue that has not been addressed in either SFML or CSFML.
   This issue also affects other projects and it does not appear like CMake provides any good solution to resolve this:

   • https://gitlab.kitware.com/cmake/cmake/-/issues/24887
   • https://discourse.cmake.org/t/how-to-generate-pc-pkg-config-file-supporting-prefix-of-the-cmake-install/4109

   I personally think that it would be fine to just ignore this (at least for now) and expect people to use CMAKE_INSTALL_PREFIX.

I'm waiting until there is a consensus on SFML/CSFML#374 to keep the SFML implementation the same as in CSFML.

SFML sets the precedent that CSFML follows so this PR does not need to concern itself too much with being consistent with CSFML.

I'm starting to think that we don't actually need SFML_RELATIVE_INSTALL_LIBDIR anymore when SFML_PKGCONFIG_INSTALL_PREFIX is replaced with SFML_PKGCONFIG_INSTALL_DIR.

Do you still think this may be true? If we can fix this bug while also getting rid of some extra intermediate variables then I'd prefer that.

SFML sets the precedent that CSFML follows so this PR does not need to concern itself too much with being consistent with CSFML.

The same changes are needed in both libraries though, so the exact same discussion in CSFML can be had right here for SFML. Which is why I still think that the CSFML discussion is relevant.

I'm starting to think that we don't actually need SFML_RELATIVE_INSTALL_LIBDIR anymore when SFML_PKGCONFIG_INSTALL_PREFIX is replaced with SFML_PKGCONFIG_INSTALL_DIR.

Do you still think this may be true? If we can fix this bug while also getting rid of some extra intermediate variables then I'd prefer that.

It still has a use in the existing code, but it can be removed depending on which decision is taken in the @CMAKE_INSTALL_FULL_LIBDIR@ vs ${exec_prefix}/@CSFML_RELATIVE_INSTALL_LIBDIR@ situation.

• The advantage of @CMAKE_INSTALL_FULL_LIBDIR@ is requiring less code, the SFML_RELATIVE_INSTALL_LIBDIR variable can be removed.
• The advantage of ${exec_prefix}/@CSFML_RELATIVE_INSTALL_LIBDIR@ is that it is more consistent with other pkg-config files. From the pkg-config files installed on my linux, almost 90% use some kind of prefix for their libdir value where only 10% use a hardcoded path (which is what @CMAKE_INSTALL_FULL_LIBDIR@ would do). It also makes the .pc file somewhat more relocatable, but with prefix being a hardcoded value the files have never been relocatable anyway (so I don't think it is that important).