Skip to content
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

giac: Make cliquer a dependency, libnauty an optional dependency #31403

Closed
mkoeppe opened this issue Feb 16, 2021 · 29 comments
Closed

giac: Make cliquer a dependency, libnauty an optional dependency #31403

mkoeppe opened this issue Feb 16, 2021 · 29 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Feb 16, 2021

giac uses libnauty if it finds it, so it should be an optional dependency.

Also, giac uses cliquer if it finds it, see https://groups.google.com/g/sage-release/c/R-8gvElDIUo/m/tHvIEdaQAAAJ

We add these missing dependencies.

Follow up: if libnauty is not configured to be installed (nor present as a system package), then we should disable its use by giac. The corresponding test in giac's configure script is not robust - it explicitly checks in /usr/local; which may be disabled on macOS using -isysroot.

CC: @kiwifb @dimpase @wdjoyner @sagetrac-parisse @jamesjer

Component: packages: standard

Author: Matthias Koeppe

Branch/Commit: 06c4df1

Reviewer: Dima Pasechnik

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

@mkoeppe mkoeppe added this to the sage-9.3 milestone Feb 16, 2021
@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 24, 2021

comment:1

Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review of ticket status, priority, and last modification date.

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Mar 24, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Jul 19, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Apr 1, 2022
@mkoeppe mkoeppe changed the title giac: Make libnauty an optional dependency giac: Make cliquer a dependency, libnauty an optional dependency Apr 27, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.6 Apr 27, 2022
@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 27, 2022

@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 27, 2022

Commit: 06c4df1

@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 27, 2022

Author: Matthias Koeppe

@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 27, 2022

New commits:

06c4df1build/pkgs/giac/dependencies: Add cliquer, add libnauty as an optional dep

@mkoeppe

This comment has been minimized.

@kiwifb
Copy link
Member

kiwifb commented Apr 27, 2022

comment:10

This is just wrong. libcliquer is not a direct dependency of giac, it only depends on it through nauty.

$ readelf -d /usr/lib64/libgiac.so

Dynamic section at offset 0x1377818 contains 42 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libntl.so.43]
 0x0000000000000001 (NEEDED)             Shared library: [libpari-gmp-tls.so.7]
 0x0000000000000001 (NEEDED)             Shared library: [libgsl.so.27]
 0x0000000000000001 (NEEDED)             Shared library: [liblapack.so.3]
 0x0000000000000001 (NEEDED)             Shared library: [libblas.so.3]
 0x0000000000000001 (NEEDED)             Shared library: [libnauty.so.2]
 0x0000000000000001 (NEEDED)             Shared library: [libcurl.so.4]
 0x0000000000000001 (NEEDED)             Shared library: [libglpk.so.40]
 0x0000000000000001 (NEEDED)             Shared library: [libpng16.so.16]
 0x0000000000000001 (NEEDED)             Shared library: [libecm.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libmpfi.so.0]
 0x0000000000000001 (NEEDED)             Shared library: [libmpfr.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libgmp.so.10]
 0x0000000000000001 (NEEDED)             Shared library: [libstdc++.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [ld-linux-x86-64.so.2]
 0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
 0x000000000000000e (SONAME)             Library soname: [libgiac.so.0]

No libcliquer above. But

$ readelf -d /usr/lib64/libnauty.so

Dynamic section at offset 0x76d08 contains 29 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libcliquer.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [ld-linux-x86-64.so.2]
 0x000000000000000e (SONAME)             Library soname: [libnauty.so.2]

libcliquer is present here. Of course when you look at ldd's output you see all the libraries that are needed in a transitive fashion, so libcliquer shows up there. But libcliquer doesn't appear to be directly linked to libgiac or giac for that matter.

@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 27, 2022

@kiwifb
Copy link
Member

kiwifb commented Apr 27, 2022

comment:12

I am just looking at the configure script for giac

AC_CHECK_LIB(cliquer,main) 
AC_CHECK_LIB(nauty,main)
AC_CHECK_HEADERS(nauty/naututil.h)

It is all automagic. I am fairly sure the libcliquer detection is there so that when nauty is found the libcliquer flags are properly included too. Notice how no headers are checked for cliquer, just adding the library for linking.

@kiwifb
Copy link
Member

kiwifb commented Apr 27, 2022

comment:13

Another point is that the giac actively uses HAVE_LIBNAUTY in some code but HAVE_LIBCLIQUER is only present in configure objects. So nauty is optional and cliquer is just its dependency.

@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 27, 2022

comment:14

Our nauty package also does not declare a dependency on cliquer. The dependency has to go somewhere

@kiwifb
Copy link
Member

kiwifb commented Apr 27, 2022

comment:15

We definitely need to add cliquer to nauty then.

@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 27, 2022

comment:16

Replying to @kiwifb:

Another point is that the giac actively uses HAVE_LIBNAUTY in some code but HAVE_LIBCLIQUER is only present in configure objects. So nauty is optional and cliquer is just its dependency.

This certainly makes sense, given upstream nauty's inadequate build system.

@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 27, 2022

comment:17

Replying to @kiwifb:

We definitely need to add cliquer to nauty then.

Actually it looks like upstream nauty (and thus our package) uses a vendored copy of cliquer.

@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 27, 2022

comment:18

So on systems with nauty linking to an unvendored cliquer (Debian?), I think it will be nondeterministic whether the GIAC build picks up our libcliquer or system libcliquer.

@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 27, 2022

comment:19

Adding the dependency will remove this nondeterminism.

@kiwifb
Copy link
Member

kiwifb commented Apr 27, 2022

comment:20

You are posting too fast for me to make coherent answers :) I guess removing the non-deterministic behavior is the best course of action. debian, gentoo, probably fedora and arch, are unvendoring. I don't know about others or anaconda and brew.

@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 27, 2022

comment:21

Sorry - I should take a break

@kiwifb
Copy link
Member

kiwifb commented Apr 27, 2022

comment:22

It doesn't help that it is 9:50am here and I am in the middle of meetings and trying to help people with my "day job" at the same time.

@dimpase
Copy link
Member

dimpase commented Apr 28, 2022

comment:24

This misses the issue of system-wide nauty, which should only be used if it's linked to libcliquer, no?
E.g. on Debian stable I see

$ ldd /usr/lib/x86_64-linux-gnu/libnauty.so
        linux-vdso.so.1 (0x00007ffe223f8000)
        libcliquer.so.1 => /lib/libcliquer.so.1 (0x00007e4c9cd43000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007e4c9cb7e000)
        /lib64/ld-linux-x86-64.so.2 (0x00007e4c9cfe4000)

but apparently this is not the case on Ubuntu 21.

@kiwifb
Copy link
Member

kiwifb commented Apr 28, 2022

comment:25

If ubuntu 21 uses a vendored version of cliquer, it doesn't have to be linked to an external library.

That's one of the issue we have here. Some distribution will use external cliquer while some other will not. That's why the branch is just going with "let's just make sure it is always present".

@dimpase
Copy link
Member

dimpase commented Apr 28, 2022

comment:26

Replying to @kiwifb:

If ubuntu 21 uses a vendored version of cliquer, it doesn't have to be linked to an external library.

Huh? The problem is that nauty must be linked to libcliquer (vendored or nor), not the other way around.

@kiwifb
Copy link
Member

kiwifb commented Apr 28, 2022

comment:27

OK my phrasing may not have been the best. If ubuntu 21's nauty uses a vendored libcliquer - like the sage optional package does - it may not need libcliquer.

If ubuntu 21's nauty does use an external libcliquer and it isn't installed along nauty then it is a bug in that version of ubuntu.

@dimpase
Copy link
Member

dimpase commented Apr 28, 2022

comment:28

Replying to @kiwifb:

OK my phrasing may not have been the best. If ubuntu 21's nauty uses a vendored libcliquer - like the sage optional package does - it may not need libcliquer.

If it used a vendored libcliquer we won't be seeing this bug reported.
According to https://ubuntu.pkgs.org/21.10/ubuntu-universe-amd64/libnauty2_2.7r1+ds-2_amd64.deb.html libcliquer is "Required".

If ubuntu 21's nauty does use an external libcliquer and it isn't installed along nauty then it is a bug in that version of ubuntu.

cc-ing Fedora's people.

@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 May 3, 2022
@mkoeppe
Copy link
Member Author

mkoeppe commented Jul 7, 2022

@dimpase
Copy link
Member

dimpase commented Jul 8, 2022

comment:31

lgtm

@dimpase
Copy link
Member

dimpase commented Jul 8, 2022

Reviewer: Dima Pasechnik

@mkoeppe
Copy link
Member Author

mkoeppe commented Jul 8, 2022

comment:32

Thanks!

@vbraun
Copy link
Member

vbraun commented Jul 9, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants