Add dummy script package libgraphviz (pygraphviz dependency)

edd8e884-f507-429a-b577-5d554626c0fe commented 3 years ago

We got pygraphviz/graphviz_wrap.c:2711:10: fatal error: graphviz/cgraph.h: No such file or directory, see the log in attachment.

CC: @mkoeppe @dcoudert

Component: packages: optional

Author: Matthias Koeppe

Branch/Commit: d40fcb9

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/32825

edd8e884-f507-429a-b577-5d554626c0fe commented 3 years ago

comment:1

Attachment: pygraphviz.log

dcoudert commented 3 years ago

comment:2

I tried ./sage -pip install pygraphviz on a linux fedora desktop and it works fine.

mkoeppe commented 3 years ago

comment:3

More detail is needed here - on what kind of system; how was graphviz installed etc. Top-level config.log please

edd8e884-f507-429a-b577-5d554626c0fe commented 3 years ago

comment:4

The system is a standard Debian bullseye, architecture x86_64, graphviz was installed from the packages. I first saw the error on some VM, and could reproduce it on my laptop.

edd8e884-f507-429a-b577-5d554626c0fe commented 3 years ago

Attachment: config.log

edd8e884-f507-429a-b577-5d554626c0fe commented 3 years ago

comment:5

I attached the config.log.

mkoeppe commented 3 years ago

comment:6

There is a gap in our dependencies. The graphviz package only checks for executables in the system package test (spkg-configure.m4). But pygraphviz needs header files and presumably the shared library -- which on debian are in a separate system package https://packages.debian.org/bookworm/libgraphviz-dev

$ cat build/pkgs/graphviz/spkg-configure.m4 
SAGE_SPKG_CONFIGURE([graphviz], [
    dnl We check all executables that are tested by sage.features.graphviz
    AC_CHECK_PROGS([DOT], [dot])
    AS_IF([test x$DOT = x], [sage_spkg_install_graphviz=yes])
    AC_CHECK_PROGS([NEATO], [neato])
    AS_IF([test x$NEATO = x], [sage_spkg_install_graphviz=yes])
    AC_CHECK_PROGS([TWOPI], [twopi])
    AS_IF([test x$TWOPI = x], [sage_spkg_install_graphviz=yes])
])
$ cat build/pkgs/graphviz/distros/debian.txt 
graphviz
$ cat build/pkgs/pygraphviz/dependencies 
$(PYTHON) graphviz | $(PYTHON_TOOLCHAIN)

The solution is to add a dummy script package libgraphviz in the same way that it is done for nauty, libnauty.

edd8e884-f507-429a-b577-5d554626c0fe commented 3 years ago

comment:8

Replying to @mkoeppe:

The solution is to add a dummy script package libgraphviz in the same way that it is done for nauty, libnauty.

This looks verbose, why not simply put both graphviz and libgraphviz-dev in the distro file of the graphviz spkg, as we do for openssl ?

mkoeppe commented 3 years ago

comment:9

because other uses of graphviz do not need the shared library

mkoeppe commented 3 years ago

Branch: u/mkoeppe/add_dummy_script_package_libgraphviz__pygraphvizdependency

mkoeppe commented 3 years ago

Author: Matthias Koeppe

mkoeppe commented 3 years ago

New commits:

d40fcb9 build/pkgs/libgraphviz: New, use instead of graphviz as dependency of pygraphviz

mkoeppe commented 3 years ago

Commit: d40fcb9

mkoeppe commented 3 years ago

comment:12

Tested successfully with tox -e docker-debian-bullseye-maximal -- pygraphviz

dimpase commented 2 years ago

comment:13

lgtm

dimpase commented 2 years ago

Reviewer: Dima Pasechnik

mkoeppe commented 2 years ago

comment:14

Thanks!

vbraun commented 2 years ago

Changed branch from u/mkoeppe/add_dummy_script_package_libgraphviz__pygraphvizdependency to d40fcb9

sagemath / sage

Add dummy script package libgraphviz (pygraphviz dependency) #32825