sagemath/sage

Optional package deformation fails to build: mpir.h not found

mkoeppe opened this issue · 33 comments

On homebrew-macos-maximal (https://github.com/mkoeppe/sage/runs/959683064) - which uses gmp from the system, not mpir...

 [deformation-d05941b.p0] error installing, exit status 1. End of log file:
  [deformation-d05941b.p0]   /Users/runner/work/sage/sage/.tox/local-homebrew-macos-maximal/local/var/tmp/sage/build/deformation-d05941b.p0/src/generics.h:12:10: fatal error: 'mpir.h' file not found
  [deformation-d05941b.p0]   #include <mpir.h>
  [deformation-d05941b.p0]            ^~~~~~~~
  [deformation-d05941b.p0]   In file included from zero.c:1:
  [deformation-d05941b.p0]   In file included from /Users/runner/work/sage/sage/.tox/local-homebrew-macos-maximal/local/var/tmp/sage/build/deformation-d05941b.p0/src/vec.h:8:
  [deformation-d05941b.p0]   /Users/runner/work/sage/sage/.tox/local-homebrew-macos-maximal/local/var/tmp/sage/build/deformation-d05941b.p0/src/generics.h:12:10: fatal error: 'mpir.h' file not found

Is this package still maintained?

Also on debian-bullseye:

 [deformation-d05941b.p0]       CC   ../build/perm/../perm.lo
  [deformation-d05941b.p0]   /usr/bin/ld: -r and -pie may not be used together
  [deformation-d05941b.p0]   collect2: error: ld returned 1 exit status

a straightforward modification of using GMP in place MPIR seems to work just fine, at least on 64-bit linux. Please review.

Depends on #31525

CC: @jpflori @videlec @kiwifb @slel @sagetrac-spancratz

Component: packages: experimental

Work Issues: fix C code on clang

Author: Matthias Koeppe, Dima Pasechnik

Branch/Commit: 44b1d0a

Reviewer: Matthias Koeppe

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

Description changed:

--- 
+++ 
@@ -12,3 +12,14 @@
 
 Is this package still maintained?
 
+
+
+Also on `debian-bullseye`: 
+
+```
+ [deformation-d05941b.p0]       CC   ../build/perm/../perm.lo
+  [deformation-d05941b.p0]   /usr/bin/ld: -r and -pie may not be used together
+  [deformation-d05941b.p0]   collect2: error: ld returned 1 exit status
+```
+
+
comment:2

I pushed a modif on my github repo for the debian issue.
For the homebrew one, I fear that the package is using some MPIR internals and cannot work with GMP, though that is just a very vague souvenir.

comment:4

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

comment:5

Replying to @jpflori:

I pushed a modif on my github repo for the debian issue.

Could you push a tag please so that there is a cleaner way to refer to this version?

Commit: 9959a92

comment:7

https://github.com/jpflori/deformation/tags
Just did.


New commits:

9959a92WIP
comment:8

we now have a release that uses GMP, and doesn't use MPIR at all.

Author: Matthias Koeppe, Dima Pasechnik

comment:9

this works and passes checks on Ubuntu and Fedora 32


New commits:

313c76bWIP
dd68f33updated to the latest fix, using GMP

Changed commit from 9959a92 to dd68f33

Description changed:

--- 
+++ 
@@ -22,4 +22,6 @@
   [deformation-d05941b.p0]   collect2: error: ld returned 1 exit status
 ```
 
+---
 
+a straightforward modification of using GMP in place MPIR seems to work just fine, at least on 64-bit linux. Please review.
comment:11

I would strongly suggest to use more traditionally named release tags. If there is no version scheme, how about using the date.

comment:12

Also, does not build for me on macos-homebrew:

[deformation-mpir_fix]         ^
[deformation-mpir_fix] nmod_mat_charpoly.c:9:6: error: redefinition of 'nmod_mat_charpoly'
[deformation-mpir_fix] void nmod_mat_charpoly(nmod_poly_t rop, const nmod_mat_t op)
[deformation-mpir_fix]      ^
[deformation-mpir_fix] /usr/local/include/flint/nmod_poly.h:1412:6: note: previous definition is here
[deformation-mpir_fix] void nmod_mat_charpoly(nmod_poly_t p, const nmod_mat_t M)
[deformation-mpir_fix]      ^
[deformation-mpir_fix] 1 error generated.
comment:13

yes, the C code there is far from "normal", there are various ugly bits such as static functions implemented in .h files.
E.g. gcc 8.3 produces a slew of warnings. OK, it's something doable, to fix these.

Work Issues: fix C code on clang

comment:15

Do we assume that we have Flint 2.6 or newer? nmod_mat_charpoly appeared in Flint 2.6,
and we can either use it, or rename the one in deformation
(both ways work, but of course it's better to use one in Flint)

comment:16

Yes, if we make #31525 (Wrap FLINT 2.6 functions, drop support for system FLINT < 2.6) a dependency of this ticket

Dependencies: #31525

comment:18

Unfortunately, some tests fail with Flint 2.6.3, while with Flint 2.5.2 everything passes.
https://github.com/dimpase/deformation/runs/2456263893

...
nmod_mat_charpoly... PASS
monotonic... PASS
make[1]: *** [../Makefile.subdirs:84: ../build/diagfrob/test/t-ell_curves_RUN] Aborted (core dumped)
ell_curves... make[1]: Leaving directory '/home/runner/work/deformation/deformation/diagfrob'
make: *** [Makefile:179: check] Error 2
Error: Process completed with exit code 2.
comment:19

I found a workaround (by chance), it appears that the order the libraries are linked in is important, see dimpase/deformation#2

It likely points to a bug somewhere, though (valgrind shows trouble in some tests, although not in ones which fail), but, well...

Changed commit from dd68f33 to 79a7633

Branch pushed to git repo; I updated commit sha1. New commits:

79a7633update to 20210503, new location for releases

Changed commit from 79a7633 to b21456b

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

ff216bdWIP
197696cupdated to the latest fix, using GMP
4822d1cupdate to 20210503, new location for releases
b21456bupdate checksum
comment:22

OK, this also should work on macOS, and pass checks

Branch pushed to git repo; I updated commit sha1. New commits:

44b1d0aupdate upstream info

Changed commit from b21456b to 44b1d0a

comment:24

the versions are now dates in format YYYYMMDD

Reviewer: Matthias Koeppe

comment:25

Builds OK now on macOS, thanks