emsec / hal

HAL – The Hardware Analyzer
MIT License
619 stars 74 forks source link

update dependency installer for Arch #493

Closed leongross closed 1 year ago

leongross commented 1 year ago

Update update_dependecy.sh for Arch Linux to use igraph version 0.9.9-1

szhorvat commented 1 year ago

You may want to warn users that downgrading igraph will break other packages, notably python-igraph.

This is another reason why if HAL decides to stick with old igraph versions for a while, it may be a good idea to just vendor the igraph sources. igraph uses CMake since version 0.9, so integration through a git submodule and add_subdirectory will be easy.

joern274 commented 1 year ago

Following the add_subdirectory suggestion I would like to offer an alternative solution how to provide igraph-0.9.10 for all supported platforms. Please have a look at the HAL branch fix/igraph_subdirectory

@leongross , can you please verify that this branch builds and runs on your Arch system ?

@szhorvat , do you have time to review the changes? There is at least one issue I am not so happy about : when downloading the igraph-0.9.10.tar.gz tarball from github it will only build after patching the version number at two different locations. Did I miss something or is that intended by maintainer?

szhorvat commented 1 year ago

it will only build after patching the version number at two different locations. Did I miss something or is that intended by maintainer?

https://github.com/igraph/igraph/archive/refs/tags/0.9.10.tar.gz is just a snapshot of the repository generated by GitHub automatically. When building igraph from the contents of the repository, it is necessary for an actual git repo to be backing it, not just its contents. Otherwise the version number detection will fail.

If you are aiming to use specifically version 0.9.10, then the solution is to download the release tarball:

https://github.com/igraph/igraph/releases/download/0.9.10/igraph-0.9.10.tar.gz

Note the difference: you need igraph-0.9.10.tar.gz, not 0.9.10.tar.gz.

One additional advantage to this is that flex/bison won't be needed. The parser sources are already included in the release tarball.


An alternative solution is to use a git submodule. Then you can even use the most recent release/0.9 branch, which has some extra fixes on top of 0.9.10. These fixes were necessary for the R and Mathematica interfaces of igraph, but they are not released within the 0.9 series (of course the 0.10 series contains them). But this will still only work within a git repo, so I guess it's not good for you.

szhorvat commented 1 year ago

Another comment is that instead of installing igraph in a temporary location, it should be possible to use CMake's add_subdirectory.

There is an example that shows this here: https://github.com/igraph/igraph-example/tree/master/cmake-project-2 (ignore the git submodule part in the example—the point is to have igraph in a separate directory and use add_subdirectory()).

I think this would be a simpler solution for you. Hopefully, there should be no issues.

I should say though that I am a physicist, not a programmer, so my knowledge of arcane technical details only goes so far 🙂

joern274 commented 1 year ago

Thanks again @szhorvat for helpful comments. To pick the wrong tarball URL was silly of me, I have corrected that.

There is still the need to patch src/CMakeLists.txt unless we are fine with building libigraph.so.0.0.0 without correct version number. Of course not a big deal.

Being a particle physicist myself I would rather avoid fundamental changes in HAL building philosophy which was developed before I entered the project. Some of them are:

You are right, installation of igraph could be omitted but it requires very little time and resources. It is done automatically by externalProject_add and makes troubleshooting easier, so I'd like to keep it in place.

Last not least, keep in mind that this igraph fix is only temporary. Once we drop Ubuntu20.04 support we are more than happy to migrate to igraph0.10 or higher using your patches.

szhorvat commented 1 year ago

There is still the need to patch src/CMakeLists.txt unless we are fine with building libigraph.so.0.0.0 without correct version number. Of course not a big deal.

The SONAME is set properly only starting with igraph 0.10. If you need to set it for 0.9, please use the name libigraph.so.2.0.0 to stay consistent with Debian/Ubuntu. For 0.10, we use libigraph.so.3.0.0.

leongross commented 1 year ago

@joern274 I built the branch on my machine and except for some uncritical warnings everything worked fine. I also loaded up a netlist and this worked fine as well.