Skip to content

Fix pkgconfig file#716

Merged
lshaw8317 merged 1 commit intoBlosc:mainfrom
avalentino:bugfix/pkgconfig
Feb 16, 2026
Merged

Fix pkgconfig file#716
lshaw8317 merged 1 commit intoBlosc:mainfrom
avalentino:bugfix/pkgconfig

Conversation

@avalentino
Copy link
Member

The curerrent implementation does not work on systems, like e.g. Debian, where the pkgconfig file is installed in
"/usr/lib/x86_64-linux-gnu/pkgconfig/blosc2.pc".

Since the current implementation assumes "prefix=${pcfiledir}/../.." (corretponding to "/user/lib" in this case), the path to include files ("includedir=${prefix}/include") results to be not correct.

The fix has been only tested on Debian.

@avalentino avalentino changed the title Fix pkgconfig file [WIP] Fix pkgconfig file Feb 15, 2026
@avalentino avalentino marked this pull request as draft February 15, 2026 20:06
The curerrent implementation does not work on systems, like e.g.
Debian, where the pkgconfig file is installed in
"/usr/lib/x86_64-linux-gnu/pkgconfig/blosc2.pc".

Since the current implementation assumes "prefix=${pcfiledir}/../.."
(corretponding to "/user/lib" in this case), the path to include files
("includedir=${prefix}/include") results to be not correct.

The fix has been only tested on Debian.
@avalentino avalentino changed the title [WIP] Fix pkgconfig file Fix pkgconfig file Feb 15, 2026
@avalentino avalentino marked this pull request as ready for review February 15, 2026 20:56
@lshaw8317
Copy link
Collaborator

Hi, thanks for noticing this, and yes it is absolutely necessary to fix this. What do you think of this proposal?

prefix=@CMAKE_INSTALL_PREFIX@
exec_prefix=${prefix}
libdir=@CMAKE_INSTALL_FULL_LIBDIR@
includedir=@CMAKE_INSTALL_FULL_INCLUDEDIR@

@avalentino
Copy link
Member Author

Not sure
I was not able to find any official documentation for CMAKE_INSTALL_FULL_INCLUDEDIR and CMAKE_INSTALL_FULL_LIBDIR.
Just a mention for the first one.

@avalentino
Copy link
Member Author

What I put in my implementation exploits the same variable that are used in the install commands

@lshaw8317
Copy link
Collaborator

From https://cmake.org/cmake/help/latest/module/GNUInstallDirs.html:

CMAKE_INSTALL_FULL_<dir>
The absolute path generated from the corresponding CMAKE_INSTALL_<dir> value. If the value is not already an absolute path, an absolute path is constructed typically by prepending the value of the CMAKE_INSTALL_PREFIX variable, except in special cases as documented below.

These variables shouldn't be used in install() commands as they do not work with the cmake --install command's --prefix option, or with the cpack installer generators.

where <dir> is one of:

...

LIBDIR
object code libraries (lib or lib64)

On Debian, this may be lib/<multiarch-tuple> when CMAKE_INSTALL_PREFIX is /usr.

INCLUDEDIR
C header files (include)

@lshaw8317
Copy link
Collaborator

What I put in my implementation exploits the same variable that are used in the install commands

Ok, let's go with your solution then I think. It was what we had before I made the breaking change.

@lshaw8317 lshaw8317 merged commit 576b1ec into Blosc:main Feb 16, 2026
18 checks passed
@avalentino avalentino deleted the bugfix/pkgconfig branch February 16, 2026 09:52
@avalentino
Copy link
Member Author

From https://cmake.org/cmake/help/latest/module/GNUInstallDirs.html:

CMAKE_INSTALL_FULL_


The absolute path generated from the corresponding CMAKE_INSTALL_ value. If the value is not already an absolute path, an absolute path is constructed typically by prepending the value of the CMAKE_INSTALL_PREFIX variable, except in special cases as documented below.
These variables shouldn't be used in install() commands as they do not work with the cmake --install command's --prefix option, or with the cpack installer generators.
where is one of:
...
LIBDIR
object code libraries (lib or lib64)
On Debian, this may be lib/ when CMAKE_INSTALL_PREFIX is /usr.
INCLUDEDIR
C header files (include)

ah, sorry, I totally missed it
In that case I think that your solution in better

@lshaw8317
Copy link
Collaborator

I think the merged solution is fine, and reasonably robust. If in the future it causes problems we can revisit the discussion here I guess. Thanks very much!

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.

2 participants