-
Notifications
You must be signed in to change notification settings - Fork 24
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
Doesn't build against igraph 0.10.3 (default version on fedora 38 and debian 12) #44
Comments
igraph has undergone a significant refactoring for version 0.10. Now all index-like quantities are represented as integers, while previously they were often floating point numbers, a consequence of igraph's R heritage (R uses floats for everything). Breaking changes are described here: https://github.com/igraph/igraph/blob/master/CHANGELOG.md#0100---2022-09-05 In my experience, refactoring old code to work with 0.10 is not usually difficult. The main change that's needed is to replace some If the maintainers of this library have any questions regarding this, feel free to ask me. |
Great, thanks @sporksmith for the bug report and @szhorvat for the details. Should hopefully be a straightforward fix since igraph has some way to find the version: |
You could use CMake's Trying to support both 0.10.x and earlier versions using |
We currently support back to Ubuntu 18.04. If we can't Maybe instead we can build igraph from source as a cmake external project and statically link it into tgen. igraph is GPL though, so we couldn't ever share tgen binaries I think? Another option could be to bring rust into tgen and write a C wrapper around a petgraph implementation of tgen's graph code. |
I briefly checked to see if there's something like an "igraph-0.9" package in fedora 38, but I don't see one. Just "igraph", at version 0.10.3. Maybe the user can enable some alternate package repo that might have it? I'm not a regular fedora user, so I might be missing an obvious solution here...
|
We used to do this with CentOS 8 back when we used igraph in Shadow, where we'd download from an older EPEL repository: |
This is possible with
Well, technically you always can, it might just take an unpleasantly large number of
Note that all major Linux distros and package managers have 0.10 already: https://repology.org/project/igraph/versions Arch/Debian/Fedora have it, and most other distros are based on those. Debian also has older versions, see The reason why igraph 0.10 has so many breaking changes is the following: igraph has accumulated a lot of cruft and some non-ideal design decisions during the past 16 years. These were holding it back, and limiting improvements. We are working towards a 1.0 release with a clean and stable API that we can build on top of in the next 10 years. We tried to squeeze the most significant breaking changes into 0.10 to minimize the inconvenience for downstream users, necessitating only a single large refactoring. |
We definitely empathize with having to balance the tradeoffs between changing the API and supporting technical debt of old designs that restrict new features :) It's good to see igraph continuing to be maintained, and changing indexes to be integers is definitely a nice change. Our problem here is mostly a package management issue: How can we continue to support our existing supported platforms without adding an extra step requiring users to build igraph from source (and requiring them to deal with the usual building/linking issues that arise)? |
igraph 0.10.0 has some API changes that we don't handle. We may be able to support it through some #ifdef macros, as we already do for 0.9.x, though the devs suggest this might be painful. For now, having cmake bail out and explain that the igraph version is incompatible is better than having a bunch of compiler errors. See shadow#44
igraph 0.10.0 has some API changes that we don't handle. We may be able to support it through some #ifdef macros, as we already do for 0.9.x, though the devs suggest this might be painful. For now, having cmake bail out and explain that the igraph version is incompatible is better than having a bunch of compiler errors. See shadow#44
igraph 0.10.0 has some API changes that we don't handle. We may be able to support it through some #ifdef macros, as we already do for 0.9.x, though the devs suggest this might be painful. For now, having cmake bail out and explain that the igraph version is incompatible is better than having a bunch of compiler errors. See #44
Good point - changed our cmake to look for <0.10.0 to at least make the problem and potential resolution clearer for now. #45
It looks like most... don't? e.g. Debian 10 and 11 don't, all Fedoras < 38, all current Ubuntus... Maybe users could try installing an Also I'd think installing a "foreign" package from an older distro is more likely to work than from a newer one? I think dynamically linked libraries are usually backwards compatible with a binary compiled against an older version, but not always forwards compatible.
Yeah, we currently try to support compiling on all supported platforms, so that may be tricky. Also I didn't see any packages in Fedora 38 with alt versions of igraph, but maybe they'll end up adding one. So far I think it's probably worth a try to see to use |
A small marginally related thing I noticed when looking at the code:
I grepped the sources, and it seems that you are only using simple graph manipulation functionality. You are not using any network analysis functions at the moment. This means that none of the functions that are used have changed their behaviour (other than perhaps bugfixes, such as more stringent argument checks). They only changed API. So going the |
I just worked through this issue for myself on Fedora 38, so I'll share what I did for everybody else's benefit (as well as my own memory).
I arrived at this solution after first trying to build the igraph 0.9 shared libraries from https://github.com/igraph/igraph/releases/download/0.9.9/igraph-0.9.9.tar.gz, installing to my $HOME/.local. I just couldn't get cmake for TGen to find igraph in that location. If anyone can tell me how to do that with cmake, I'd be grateful, in case I ever run into a similar problem again. I guess I could have installed everything to the default locations with sudo, but I was afraid to try that... but maybe it would be equivalent to what I ended up doing with the RPM packages(?). I'd also like to report that I'm still using the igraph 0.7 package from the EPEL on a RHEL 8 (formerly CentOS 8) system, as @stevenengler mentioned above, to run TGen with Shadow. Edited to add: |
Debian 12/bookworm is now Debian's stable, and also has igraph 0.10 |
This can be worked around on bookworm by adding the bullseye sources and installing igraph from that. e.g.: $ cat <<EOF > /etc/apt/sources.list.d/bullseye.sources
Types: deb
URIs: http://deb.debian.org/debian
Suites: bullseye bullseye-updates
Components: main
Signed-By: /usr/share/keyrings/debian-archive-keyring.gpg
Types: deb
URIs: http://deb.debian.org/debian-security
Suites: bullseye-security
Components: main
Signed-By: /usr/share/keyrings/debian-archive-keyring.gpg
EOF
$ apt-get install -y libigraph-dev/bullseye Unfortunately unlike the binary package for libigraph, the |
I poked at this a little bit. I pulled in igraph as a subdirectory using By default it builds as a static library, and then tgen statically links. This is technically nice, but might be tricky since igraph is GPL v2. I think it might technically be ok if we don't distribute pre-built binaries (which we currently don't)? I briefly experimented with forcing it to build a dynamic library instead. That also works but we might need to tinker with the build a bit to be get it to create |
IANAL, but my understanding is that you are free to distribute binaries with GPL'd components. It's just that this binary distribution, as a whole, will be covered by the GPL with all that it implies (e.g. need to share the sources). In what situation do you expect this to be a problem? All this has no impact on the license that covers tgen sources. |
Fix build for igraph v0.10.x This fixes the tgen build on systems with igraph library versions >= 0.10.0. We no longer need to use workarounds to install older version from backports or previous release repositories. The general strategy here was to implement the tgen code according to the newer igraph api changes from v0.10.x, and then set up the messy compat definitions in tgen-igraph-compat.h that we can eventually just delete if we ever decide we no longer want to support igraph versions older than v0.10.0. We also stop setting the C attribute table multiple times during program execution, and just sets it once at program startup as explained in #44 (comment). Fixes #44
Now that shadow/tgen#44 is fixed, we no longer need to do this.
I get:
The text was updated successfully, but these errors were encountered: