igraph: 0.8.5 -> 0.9.1

[dotlambda]

Motivation for this change

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Ensured that relevant documentation is up to date
• Fits CONTRIBUTING.md.

$out/lib/pkgconfig/igraph.pc contains incorrect paths:

prefix=/nix/store/ik8rwy9kp31mhm5dni74icr90msy6dzf-igraph-0.9.0
exec_prefix=/nix/store/ik8rwy9kp31mhm5dni74icr90msy6dzf-igraph-0.9.0
libdir=${exec_prefix}//nix/store/ik8rwy9kp31mhm5dni74icr90msy6dzf-igraph-0.9.0/lib
includedir=${prefix}//nix/store/ik8rwy9kp31mhm5dni74icr90msy6dzf-igraph-0.9.0/include

Name: libigraph
Description: A library for creating and manipulating graphs
Version: 0.9.0
URL: https://igraph.org
Libs: -L${libdir} -ligraph
Libs.private: -lm -lstdc++ -lxml2 -lz -lgmp -lblas -lglpk -llapack -larpack
Cflags: -I${includedir}/igraph

Does someone knowledgable about CMake and pkg-config have time to dig into that? Upstream seems to have wanted to fix something similar with igraph/igraph@8bf4adb, but it doesn't work for us.

[SuperSandro2000]

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 117512 run on x86_64-linux 1

6 packages failed to build and are new build failures:

• python38Packages.cozy: log was empty
• python38Packages.kmapper: log was empty
• python38Packages.python-igraph: log was empty
• python39Packages.cozy: log was empty
• python39Packages.kmapper: log was empty
• python39Packages.python-igraph: log was empty

2 packages built:

• hal-hardware-analyzer
• igraph

The following issues got detected with the above build packages.
Please fix at least the ones listed with your changed packages:

hal-hardware-analyzer:

warning: build-tools-in-build-inputs
qt5.wrapQtAppsHook is a build tool so it likely goes to nativeBuildInputs, not buildInputs.

Near pkgs/applications/science/electronics/hal-hardware-analyzer/default.nix:25:3:

   |
25 |   buildInputs = [ qtbase qtsvg boost rapidjson igraph spdlog fmt graphviz wrapQtAppsHook z3 ]
   |   ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/build-tools-in-build-inputs.md

[dotlambda]

I now disabled the two failing tests on aarch64 and unvendored CXSparse. I consider this done.
EDIT: I forgot about the paths in igraph.pc.

[szhorvat]

Some comments on this from an igraph developer:

• bison and flex are not needed to build from the release tarball. They are only needed to build from the git repo directly.
• By default, igraph detects dependencies: it uses external versions when it finds them and uses bundled ("vendored") versions when it does not. Depending on how NixOS works, you may want to not rely on auto-detection and instead explicitly set it to use vendored/external to make the build process fully deterministic. See here: https://igraph.org/c/doc/igraph-Installation.html#igraph-Installation-notes-for-package-maintainers The same goes for which BLAS/LAPACK implementations to use. If there are multiple on a system, it will pick up an arbitrary one, but you can choose a specific one.
• You are currently building a static library. Is this what you want? I would have assumed most Linux distros want the shared one.
• Link-time optimization provides real benefits, and no significant drawbacks in terms of build time, assuming that you build a shared library (building tests will be super slow if you're building the static one). This can also be set to AUTO in which case it'll use it only if it actually works.
• python-igraph 0.9.1 should come out very soon now, and I would recommend waiting for it. In 0.9.1 one can control much better how the C core is built/used (e.g. if you want to use external dependencies or not). If you don't wait, I recommend upgrading to 0.9.1 as soon as it comes out.

[szhorvat]

And of course: thanks for making igraph available to NixOS users!

[szhorvat]

Sorry about so many comments, just one more note that came to mind: the release tarball (not the git repo) includes pre-built HTML documentation in doc/html. Depending on NixOS policies, you may want to explicitly install this as well. I guess it would be better if we made it so that a make install will do this automatically, but for now it's not the case (partly because building docs is expensive and we don't want to do it for every install during development).

[dotlambda]

@szhorvat Thank you so much! I'm building a shared library now and also the docs. Is there a big downside to turning on thread-local storage?

Also, lib/pkgconfig/igraph.pc contains wrong information:

prefix=/nix/store/dybiry1yagfscc5zrv19yjidhgfwxm2a-igraph-0.9.1
exec_prefix=/nix/store/dybiry1yagfscc5zrv19yjidhgfwxm2a-igraph-0.9.1
libdir=${exec_prefix}//nix/store/dybiry1yagfscc5zrv19yjidhgfwxm2a-igraph-0.9.1/lib
includedir=${prefix}//nix/store/dybiry1yagfscc5zrv19yjidhgfwxm2a-igraph-0.9.1/include

Name: libigraph
Description: A library for creating and manipulating graphs
Version: 0.9.1
URL: https://igraph.org
Libs: -L${libdir} -ligraph
Libs.private: -lm -lstdc++ -lxml2 -lz -lgmp -lblas -lcxsparse -lglpk -llapack -larpack
Cflags: -I${includedir}/igraph

[dotlambda]

I included a patch to fix the paths in igraph.pc.

python-igraph's tests fail with

Fatal error at src/core/error.c:252 : Corrupt finally stack: trying to pop 1 element(s) when only 0 left.

[szhorvat]

Is there a big downside to turning on thread-local storage?

There should not be, but this functionality is not well tested. Without it, igraph will not be thread-safe. If used from multiple threads, things are pretty much guaranteed to break. With it, some functions that depend on external libraries will still not be thread safe (in particular anything that uses an external ARPACK won't, and anything that uses GLPK depends on how GLPK was built).

[szhorvat]

python-igraph's tests fail with

Fatal error at src/core/error.c:252 : Corrupt finally stack: trying to pop 1 element(s) when only 0 left.

If you use python-igraph 0.9.0, but link to a different C/igraph than a matched one (the vendored one), then this will happen. It's because C/igraph 0.9.1 is less tolerant of internal errors than 0.9.0, and will now trigger a fatal error on this specific type of error. This bug is already fixed in python-igraph, and the fix will be in version 0.9.1, hopefully out next week.

[ntamas]

@dotlambda @szhorvat igraph.pc is generated at build time from igraph.pc.in, which fills the paths from the ${CMAKE_INSTALL_PREFIX} variable. If the installation is performed into a temporary directory, then this temporary directory will appear in igraph.pc because as far as CMake is concerned, this is where igraph is being installed.

On the other hand, I totally understand that this is unsuitable for package maintainers because the typical way of building a package on pretty much any Linux-based system is to install the library into a staging area and then bundle the staging area into a package file. If anyone knows about any other CMake-based packages that handle this better than igraph, I'd be interested to learn about best practices.

[dotlambda]

@ntamas We don't install it into a temporary directory. The only problem is that CMAKE_INSTALL_PREFIX is already a part of e.g. CMAKE_INSTALL_LIBDIR. Hence the /nix/store/dybiry1yagfscc5zrv19yjidhgfwxm2a-igraph-0.9.1 part appears twice in the path.

[ntamas]

On my system ${CMAKE_INSTALL_LIBDIR} is a relative path (lib or lib64) so it has to be prefixed. The same applies to ${CMAKE_INSTALL_INCLUDEDIR} (it resolves to include on my machine). Are you overriding ${CMAKE_INSTALL_LIBDIR} somewhere in the build process or is this the default behaviour of CMake on NixOS?

[ntamas]

In particular, these variables are supposed to be populated by the GNUInstallDirs CMake module, and it seems to provide two variants for each variable, one with a relative path and one with an absolute path (see here). The one with the absolute path should have _FULL_ in its name.

[dotlambda]

That's the way NixOS sets up these variables. See https://github.com/jtojnar/cmake-snips#assuming-cmake_install_dir-is-relative-path.

[dotlambda]

I suggest you simply use the _FULL_ version in igraph.pc.

[ntamas]

Thanks for the reference -- we'll patch this on our side for 0.9.2 so we can deal with both absolute and relative paths in CMAKE_INSTALL_LIBDIR et al.

[SuperSandro2000]

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 117512 run on x86_64-linux 1

8 packages built:

• hal-hardware-analyzer
• igraph
• python38Packages.cozy
• python38Packages.kmapper
• python38Packages.python-igraph
• python39Packages.cozy
• python39Packages.kmapper
• python39Packages.python-igraph