Malformed generated liboqs.pc file

[Sigmanificient]

Describe the bug
Hi, while upgrading nixpkgs's liboqs (0.8.0 -> 0.10.0), i ran into an issue concerning the generated liboqs.pc file. Nix kindly reported the following malformation:

Broken paths found in a .pc file! /nix/store/2r1kngq4xq5q2zag4wpw1sq0y0p2v4zk-liboqs-0.10.0-dev/lib/pkgconfig/liboqs.pc
The following lines have issues (specifically '//' in paths).
2:libdir=${prefix}//nix/store/m6lrn85hylxmnhz1zhsy6sibq0jnxlnb-liboqs-0.10.0/lib
It is very likely that paths are being joined improperly.
ex: "${prefix}/@CMAKE_INSTALL_LIBDIR@" should be "@CMAKE_INSTALL_FULL_LIBDIR@"

Reverting this commit fixes the problem, generating the proper pc file:

prefix=/nix/store/vg0fz9670mimw8b24f4bx8sj499bskgh-liboqs-0.10.0
libdir=${prefix}/lib
includedir=/nix/store/f8jcidncc42iprkp21n8sdsh8hcbkhh7-liboqs-0.10.0-dev/include

Name: liboqs
Description: Library for quantum-safe cryptographic algorithms
Version: 0.10.0
Requires.private: openssl
Cflags: -I${includedir}
Libs: -L${libdir} -loqs

Environment (please complete the following information):

• OS: NixOS 23.11
• OpenSSL version: 3.0.13
• Compiler version used: gcc 13.2.0
• Build variables used:
  • "-DOQS_DIST_BUILD=ON"
  • "-DOQS_BUILD_ONLY_LIB=ON"
• liboqs version: 0.10.0

[baentsch]

Thanks for this bug report. Checking the cmake documentation seems to indicate we should indeed use "CMAKE_INSTALL_FULL_LIBDIR/INCLUDEDIR" instead. Any objections @vt-alt ?

[vt-alt]

Well, I don't know anything about Nix. That path looks bizarre:

2:libdir=${prefix}//nix/store/m6lrn85hylxmnhz1zhsy6sibq0jnxlnb-liboqs-0.10.0/lib

In ALT we have correct paths in final liboqs.pc;

prefix=/usr
libdir=${prefix}/lib64
includedir=${prefix}/include

(Using ${prefix} is common practice.) I tested with proposed changes

-libdir=${prefix}/@CMAKE_INSTALL_LIBDIR@
-includedir=${prefix}/@CMAKE_INSTALL_INCLUDEDIR@
+libdir=@CMAKE_INSTALL_FULL_LIBDIR@
+includedir=@CMAKE_INSTALL_FULL_INCLUDEDIR@

And it produced correct paths either:

prefix=/usr
libdir=/usr/lib64
includedir=/usr/include

So I don't object this improvement. But who knows what it will produce on Nix.
Documentation says

an absolute path is constructed typically by prepending the value of the CMAKE_INSTALL_PREFIX variable

So there are chances it will produce just the same value.

There are also chances this is just Nix packaging bug which we should not try to "fix".

[vt-alt]

Also, the docs say:

LIBDIR
object code libraries (lib or lib64)

On Debian, this may be lib/ when CMAKE_INSTALL_PREFIX is /usr.

So this should be lib, lib64, or lib/<multiarch-tuple> on Debian and not something like /nix/store/m6lrn85hylxmnhz1zhsy6sibq0jnxlnb-liboqs-0.10.0/lib.

By this I conclude Nix [packager] maybe misdefining it.

[vt-alt]

I checked on Debian 12 in Docker:

root@e3e602170cd4:/# cat /usr/lib/x86_64-linux-gnu/pkgconfig/liboqs.pc
prefix=/usr
libdir=${prefix}/lib
includedir=${prefix}/include
...

In Fedora Rawhide:

[root@b309a2d23cc7 /]# cat /usr/lib64/pkgconfig/liboqs.pc
prefix=/usr
libdir=${prefix}/lib64
includedir=${prefix}/include
...

All expected correct values.

[vt-alt]

So my thought is, while I don't oppose the change, that the values are already perfectly correct, and the change will not change much, and, perhaps, don't fix Nix packaging.

[baentsch]

Thanks for the analysis, @vt-alt . Based on this I tend to agree we leave things as-is.

By this I conclude Nix [packager] maybe misdefining it.

Indeed, the NIX path appears strange: It seems it causes a full path to be appended to "${prefix}": Can you check (why/whether) this so, @Sigmanificient ? Could you check whether the change suggested by the cmake documentation would indeed (or not) improve the situation for you?

[Sigmanificient]

@vt-alt Using the following patch:

--- a/src/liboqs.pc.in	2024-05-09 14:54:04.989767336 +0200
+++ b/src/liboqs.pc.in	2024-05-09 14:54:50.361324577 +0200
@@ -1,6 +1,6 @@
 prefix=@CMAKE_INSTALL_PREFIX@
-libdir=${prefix}/@CMAKE_INSTALL_LIBDIR@
-includedir=${prefix}/@CMAKE_INSTALL_INCLUDEDIR@
+libdir=@CMAKE_INSTALL_FULL_LIBDIR@
+includedir=@CMAKE_INSTALL_FULL_INCLUDEDIR@
 
 Name: @PROJECT_NAME@
 Description: Library for quantum-safe cryptographic algorithms

The package build successfully and the generated pc file is also seems to be correct:

$ nix-build -A liboqs.dev
/nix/store/944qsiqygszzvj2s98swyl9bl2k7mvlb-liboqs-0.10.0-dev
$ cat result-dev/lib/pkgconfig/liboqs.pc
prefix=/nix/store/86dhaq2f3kh3fy5ck8szj7xl68lhagii-liboqs-0.10.0
libdir=/nix/store/86dhaq2f3kh3fy5ck8szj7xl68lhagii-liboqs-0.10.0/lib
includedir=/nix/store/944qsiqygszzvj2s98swyl9bl2k7mvlb-liboqs-0.10.0-dev/include

Name: liboqs
Description: Library for quantum-safe cryptographic algorithms
Version: 0.10.0
Requires.private: openssl
Cflags: -I${includedir}
Libs: -L${libdir} -loqs

[vt-alt]

@Sigmanificient Do you redefine CMAKE_INSTALL_LIBDIR somehow while building liboqs? Looks like it already contains prefix which seems incorrect (see the docs). It should contain only lib for your case.

Also, can you show your full cmake command? it's hard to guess what went wrong without this or verbose build log.

[Sigmanificient]

I use the attributes used in the derivation:

  patches = [
    ./fix-openssl-detection.patch
    # liboqs.pc.in path were modified in this commit
    # causing malformed path with double slashes.
    ./fix-pc-file-variables.patch
  ];

  nativeBuildInputs = [ cmake ninja ];
  buildInputs = [ openssl ];

  cmakeFlags = [
    "-DBUILD_SHARED_LIBS=${if enableStatic then "OFF" else "ON"}"
    "-DOQS_DIST_BUILD=ON"
    "-DOQS_BUILD_ONLY_LIB=ON"
  ];

  dontFixCmake = true; # fix CMake file will give an error
  outputs = [ "out" "dev" ];

Nix logs report the following cmake flags (I've put each on their own line):

-GNinja
-DCMAKE_FIND_USE_SYSTEM_PACKAGE_REGISTRY=OFF
-DCMAKE_FIND_USE_PACKAGE_REGISTRY=OFF
-DCMAKE_EXPORT_NO_PACKAGE_REGISTRY=ON
-DCMAKE_BUILD_TYPE=Release
-DBUILD_TESTING=OFF
-DCMAKE_INSTALL_LOCALEDIR=/nix/store/86dhaq2f3kh3fy5ck8szj7xl68lhagii-liboqs-0.10.0/share/locale
-DCMAKE_INSTALL_LIBEXECDIR=/nix/store/86dhaq2f3kh3fy5ck8szj7xl68lhagii-liboqs-0.10.0/libexec
-DCMAKE_INSTALL_LIBDIR=/nix/store/86dhaq2f3kh3fy5ck8szj7xl68lhagii-liboqs-0.10.0/lib
-DCMAKE_INSTALL_DOCDIR=/nix/store/86dhaq2f3kh3fy5ck8szj7xl68lhagii-liboqs-0.10.0/share/doc/liboqs
-DCMAKE_INSTALL_INFODIR=/nix/store/86dhaq2f3kh3fy5ck8szj7xl68lhagii-liboqs-0.10.0/share/info
-DCMAKE_INSTALL_MANDIR=/nix/store/86dhaq2f3kh3fy5ck8szj7xl68lhagii-liboqs-0.10.0/share/man
-DCMAKE_INSTALL_OLDINCLUDEDIR=/nix/store/944qsiqygszzvj2s98swyl9bl2k7mvlb-liboqs-0.10.0-dev/include
-DCMAKE_INSTALL_INCLUDEDIR=/nix/store/944qsiqygszzvj2s98swyl9bl2k7mvlb-liboqs-0.10.0-dev/include
-DCMAKE_INSTALL_SBINDIR=/nix/store/86dhaq2f3kh3fy5ck8szj7xl68lhagii-liboqs-0.10.0/sbin
-DCMAKE_INSTALL_BINDIR=/nix/store/86dhaq2f3kh3fy5ck8szj7xl68lhagii-liboqs-0.10.0/bin
-DCMAKE_INSTALL_NAME_DIR=/nix/store/86dhaq2f3kh3fy5ck8szj7xl68lhagii-liboqs-0.10.0/lib
-DCMAKE_POLICY_DEFAULT_CMP0025=NEW
-DCMAKE_OSX_SYSROOT=
-DCMAKE_FIND_FRAMEWORK=LAST
-DCMAKE_STRIP=/nix/store/8mjb3ziimfi3rki71q4s0916xkm4cm5p-gcc-wrapper-13.2.0/bin/strip
-DCMAKE_RANLIB=/nix/store/8mjb3ziimfi3rki71q4s0916xkm4cm5p-gcc-wrapper-13.2.0/bin/ranlib
-DCMAKE_AR=/nix/store/8mjb3ziimfi3rki71q4s0916xkm4cm5p-gcc-wrapper-13.2.0/bin/ar
-DCMAKE_C_COMPILER=gcc
-DCMAKE_CXX_COMPILER=g++
-DCMAKE_INSTALL_PREFIX=/nix/store/86dhaq2f3kh3fy5ck8szj7xl68lhagii-liboqs-0.10.0
-DBUILD_SHARED_LIBS=ON
-DOQS_DIST_BUILD=ON
-DOQS_BUILD_ONLY_LIB=ON

Here is the full logs as a gist: https://gist.github.com/Sigmanificient/28ddd430f5a439f3a703b13d12818dc9

[vt-alt]

Thanks.

-DCMAKE_INSTALL_PREFIX=/nix/store/86dhaq2f3kh3fy5ck8szj7xl68lhagii-liboqs-0.10.0
-DCMAKE_INSTALL_LIBDIR=/nix/store/86dhaq2f3kh3fy5ck8szj7xl68lhagii-liboqs-0.10.0/lib

So CMAKE_INSTALL_LIBDIR with absolute path is added automatically by Nix build system. But this is "not recommended" is the docs. Perhaps, bug report should be filed to Nix authors to fix this? Or at least their opinion should be consulted. But I understand that some packages would be broken by the either choice, so they may be reluctant to change anything.

And it looks like cmake is smart enough to strip LIBDIR prefix from CMAKE_INSTALL_FULL_LIBDIR.

Well, in that case, it maybe more portable to patch liboqs.pc.in to use CMAKE_INSTALL_FULL_*s.

ps. I am curious how other Nix packages workaround this issue.

[vt-alt]

JFYI

ps. I am curious how other Nix packages workaround this issue.

Looking at https://github.com/NixOS/nixpkgs I see a lot of setting like this (overriding default values):

  cmakeFlags = [
..
    "-DCMAKE_INSTALL_LIBDIR=lib"
    "-DCMAKE_INSTALL_INCLUDEDIR=include"

nixpkgs (master)$ git grep -e "-DCMAKE_INSTALL_LIBDIR=lib" | wc -l
114

Also sometimes they patch the sources to use CMAKE_INSTALL_FULL_LIBDIR or use postPatch to fix .pc files.

  postPatch = ''
    substituteInPlace cmake/libappimage.pc.in \
      --replace 'libdir=''${prefix}/@CMAKE_INSTALL_LIBDIR@' 'libdir=@CMAKE_INSTALL_FULL_LIBDIR@' \
      --replace 'includedir=''${prefix}/@CMAKE_INSTALL_INCLUDEDIR@' 'includedir=@CMAKE_INSTALL_FULL_INCLUDEDIR@'
  '';

nixpkgs (master)$ git grep -e "CMAKE_INSTALL_FULL_LIBDIR" | wc -l
124

So it seems that experienced Nix packagers already aware of the problem and workaround it on their own.

[baentsch]

Thanks very much for this thorough analysis of Nix packaging, @vt-alt . I take from this that liboqs doesn't have to change anything and will close the issue if there's no howls of protest.