Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pkgconfig files install path #2917

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

texus
Copy link
Contributor

@texus texus commented Mar 10, 2024

Description

When running CMake without setting CMAKE_INSTALL_PREFIX, and then running CMake again with a different prefix, the install path for the pkgconfig files still points to the old location.

Because the default /usr/local prefix on Linux requires root access, installing SFML without sudo will fail with an error like this:

-- Install configuration: "Release"
-- Installing: /home/texus/Documents/SFML/whatever/lib/libsfml-system-s.a
-- Installing: /home/texus/Documents/SFML/whatever/lib/libsfml-window-s.a
-- Installing: /home/texus/Documents/SFML/whatever/lib/libsfml-network-s.a
-- Installing: /home/texus/Documents/SFML/whatever/lib/libsfml-graphics-s.a
-- Installing: /home/texus/Documents/SFML/whatever/lib/libsfml-audio-s.a
-- Installing: /usr/local/lib/pkgconfig/sfml-all.pc
CMake Error at build/cmake_install.cmake:59 (file):
  file INSTALL cannot copy file
  "/home/texus/Documents/SFML/build/tools/pkg-config/sfml-all.pc" to
  "/usr/local/lib/pkgconfig/sfml-all.pc": Permission denied.

This was fixed in the CSFML project a few weeks ago (see SFML/CSFML#233 and SFML/CSFML#234), this PR applies the same fix to SFML.

This change is NOT backwards compatible. The existing SFML_PKGCONFIG_INSTALL_PREFIX variable will no longer work and is being replaced with a new SFML_PKGCONFIG_INSTALL_DIR variable in cmake.

Tasks

  • Tested on Linux

How to test this PR?

On linux you can run the following commands to get the error:

cmake -S . -B build
cmake --build build
cmake --install build --prefix whatever

On other platforms you can probably also witness the files being installed to the wrong directory, but you will need to set SFML_INSTALL_PKGCONFIG_FILES to ON yourself because it is only enabled by default on Linux and BSD.

Copy link

codecov bot commented Mar 10, 2024

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

Additional details and impacted files

Impacted file tree graph

@@            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     

see 74 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3acc332...e4c2d0a. Read the comment docs.

This allows the pkgconfig install path to still be correct when changing the CMAKE_INSTALL_PREFIX after the first cmake run.
@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 9074962350

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 51.9%

Totals Coverage Status
Change from base Build 9057848497: 0.0%
Covered Lines: 10476
Relevant Lines: 19004

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

None yet

3 participants