Opened 12 months ago

Closed 8 days ago

#30325 closed defect (fixed)

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

Reported by: mkoeppe Owned by:
Priority: critical Milestone: sage-9.4
Component: packages: experimental Keywords:
Cc: jpflori, vdelecroix, fbissey, slelievre, spancratz Merged in:
Authors: Matthias Koeppe, Dima Pasechnik Reviewers: Matthias Koeppe
Report Upstream: N/A Work issues: fix C code on clang
Branch: 44b1d0a (Commits, GitHub, GitLab) Commit: 44b1d0a533e9a2b5df3e6785b07464504daaa770
Dependencies: #31525 Stopgaps:

Status badges

Description (last modified by dimpase)

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.

Change History (28)

comment:1 Changed 10 months ago by mkoeppe

  • Description modified (diff)

comment:2 follow-up: Changed 10 months ago by jpflori

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:3 Changed 10 months ago by mkoeppe

  • Milestone changed from sage-9.2 to sage-9.3

comment:4 Changed 6 months ago by mkoeppe

  • Milestone changed from sage-9.3 to sage-9.4

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

comment:5 in reply to: ↑ 2 Changed 4 months ago by mkoeppe

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?

comment:6 Changed 4 months ago by mkoeppe

  • Branch set to u/mkoeppe/optional_package_deformation_fails_to_build__mpir_h_not_found

comment:7 Changed 4 months ago by jpflori

  • Commit set to 9959a926052630c04b2375df4114fc4f284b6b0c

comment:8 Changed 3 months ago by dimpase

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

comment:9 Changed 3 months ago by dimpase

  • Authors set to Matthias Koeppe, Dima Pasechnik
  • Branch changed from u/mkoeppe/optional_package_deformation_fails_to_build__mpir_h_not_found to u/dimpase/packages/deformat/no_mpir
  • Commit changed from 9959a926052630c04b2375df4114fc4f284b6b0c to dd68f33963a2158799d7bb723d1025fe939ff0d5
  • Component changed from packages: optional to packages: experimental
  • Status changed from new to needs_review

this works and passes checks on Ubuntu and Fedora 32


New commits:

313c76bWIP
dd68f33updated to the latest fix, using GMP

comment:10 Changed 3 months ago by dimpase

  • Description modified (diff)

comment:11 Changed 3 months ago by mkoeppe

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

comment:12 Changed 3 months ago by mkoeppe

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 Changed 3 months ago by dimpase

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.

comment:14 Changed 3 months ago by dimpase

  • Status changed from needs_review to needs_work
  • Work issues set to fix C code on clang

comment:15 Changed 3 months ago by dimpase

  • Status changed from needs_work to needs_info

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 Changed 3 months ago by mkoeppe

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

comment:17 Changed 3 months ago by dimpase

  • Dependencies set to #31525
  • Status changed from needs_info to needs_work

comment:18 Changed 3 months ago by dimpase

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 Changed 3 months ago by dimpase

I found a workaround (by chance), it appears that the order the libraries are linked in is important, see https://github.com/dimpase/deformation/issues/2

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

Last edited 3 months ago by dimpase (previous) (diff)

comment:20 Changed 3 months ago by git

  • Commit changed from dd68f33963a2158799d7bb723d1025fe939ff0d5 to 79a7633388c3f81846e4b4a9c52ce1a0fede00c8

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

79a7633update to 20210503, new location for releases

comment:21 Changed 3 months ago by git

  • Commit changed from 79a7633388c3f81846e4b4a9c52ce1a0fede00c8 to b21456b039b6b6a90e66a6f0c828fcb9aa8e2430

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 Changed 3 months ago by dimpase

  • Status changed from needs_work to needs_review

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

comment:23 Changed 3 months ago by git

  • Commit changed from b21456b039b6b6a90e66a6f0c828fcb9aa8e2430 to 44b1d0a533e9a2b5df3e6785b07464504daaa770

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

44b1d0aupdate upstream info

comment:24 Changed 3 months ago by dimpase

the versions are now dates in format YYYYMMDD

comment:25 Changed 3 months ago by mkoeppe

  • Reviewers set to Matthias Koeppe
  • Status changed from needs_review to positive_review

Builds OK now on macOS, thanks

comment:26 Changed 3 months ago by slelievre

  • Cc slelievre spancratz added

comment:27 Changed 2 weeks ago by mkoeppe

  • Priority changed from major to critical

comment:28 Changed 8 days ago by vbraun

  • Branch changed from u/dimpase/packages/deformat/no_mpir to 44b1d0a533e9a2b5df3e6785b07464504daaa770
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.