Skip to content

Emit correct pkg-config file if paths are absolute#29

Open
marcin-serwin wants to merge 1 commit intosammycage:masterfrom
marcin-serwin:push-vuxnuyswzwln
Open

Emit correct pkg-config file if paths are absolute#29
marcin-serwin wants to merge 1 commit intosammycage:masterfrom
marcin-serwin:push-vuxnuyswzwln

Conversation

@marcin-serwin
Copy link
Contributor

CMAKE_INSTALL_INCLUDEDIR and CMAKE_INSTALL_LIBDIR may be defined to be absolute paths. In this situation they should not be appended to the prefix.

@sammycage
Copy link
Owner

@dg0yt

@marcin-serwin
Copy link
Contributor Author

@dg0yt bump

@dg0yt
Copy link
Contributor

dg0yt commented Jul 24, 2025

FTR CMake now even stronger recommends to not use absolute paths:

"While absolute paths are allowed, they are not recommended as they do not work with the cmake --install command's --prefix option, or with the cpack installer generators."

https://cmake.org/cmake/help/v4.1/module/GNUInstallDirs.html

Does this package really need the extra complexity for supporting a case which is not recommended?

@marcin-serwin
Copy link
Contributor Author

Absolute paths are used extensively by nixpkgs for installing development files into a separate prefix -- ignoring --prefix is precisely what we want in such cases so the warning is not relevant.

The complexity could be lowered a bit by bumping minimum cmake to 3.20 -- it introduces cmake_path command that could be used for path construction.

@dg0yt
Copy link
Contributor

dg0yt commented Jul 24, 2025

get_filename_component should be good enough. If you are so familiar with CMake and your requirement, why not make a PR?

@marcin-serwin
Copy link
Contributor Author

get_filename_component should be good enough.

From its docs:

Changed in version 3.20: This command has been superseded by the cmake_path() command

so not sure if it's worth doing.

If you are so familiar with CMake and your requirement, why not make a PR?

What do you mean? We are literally reviewing my PR...

@dg0yt
Copy link
Contributor

dg0yt commented Jul 24, 2025

In fact I didn't review so far...

CMAKE_INSTALL_INCLUDEDIR and CMAKE_INSTALL_LIBDIR may be defined to be
absolute paths. In this situation they should not be appended to the
prefix.

Signed-off-by: Marcin Serwin <marcin@serwin.dev>
@marcin-serwin
Copy link
Contributor Author

After reading the docs I don't see how get_filename_component could be used to achieve this, it doesn't have the equivalent of APPEND from cmake_path.

@dg0yt
Copy link
Contributor

dg0yt commented Jul 24, 2025

Well, I didn't ask for ping, bump or cmake_path. AFAICT path construction with / is robust enough here.

@marcin-serwin
Copy link
Contributor Author

Thanks for the review. Sorry for bumping you, I was just following what @sammycage did.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants