Opened 2 years ago
Closed 13 months 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: |
Description (last modified by )
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 22 months ago by
- Description modified (diff)
comment:2 follow-up: ↓ 5 Changed 22 months ago by
comment:3 Changed 22 months ago by
- Milestone changed from sage-9.2 to sage-9.3
comment:4 Changed 18 months ago by
- 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 17 months ago by
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 17 months ago by
- Branch set to u/mkoeppe/optional_package_deformation_fails_to_build__mpir_h_not_found
comment:7 Changed 17 months ago by
- Commit set to 9959a926052630c04b2375df4114fc4f284b6b0c
comment:8 Changed 16 months ago by
we now have a release that uses GMP, and doesn't use MPIR at all.
comment:9 Changed 16 months ago by
- 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
comment:10 Changed 16 months ago by
- Description modified (diff)
comment:11 Changed 16 months ago by
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 16 months ago by
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 16 months ago by
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 16 months ago by
- Status changed from needs_review to needs_work
- Work issues set to fix C code on clang
comment:15 Changed 16 months ago by
- 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 16 months ago by
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 16 months ago by
- Dependencies set to #31525
- Status changed from needs_info to needs_work
comment:18 Changed 16 months ago by
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 16 months ago by
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...
comment:20 Changed 15 months ago by
- Commit changed from dd68f33963a2158799d7bb723d1025fe939ff0d5 to 79a7633388c3f81846e4b4a9c52ce1a0fede00c8
Branch pushed to git repo; I updated commit sha1. New commits:
79a7633 | update to 20210503, new location for releases
|
comment:21 Changed 15 months ago by
- Commit changed from 79a7633388c3f81846e4b4a9c52ce1a0fede00c8 to b21456b039b6b6a90e66a6f0c828fcb9aa8e2430
comment:22 Changed 15 months ago by
- Status changed from needs_work to needs_review
OK, this also should work on macOS, and pass checks
comment:23 Changed 15 months ago by
- Commit changed from b21456b039b6b6a90e66a6f0c828fcb9aa8e2430 to 44b1d0a533e9a2b5df3e6785b07464504daaa770
Branch pushed to git repo; I updated commit sha1. New commits:
44b1d0a | update upstream info
|
comment:24 Changed 15 months ago by
the versions are now dates in format YYYYMMDD
comment:25 Changed 15 months ago by
- Reviewers set to Matthias Koeppe
- Status changed from needs_review to positive_review
Builds OK now on macOS, thanks
comment:26 Changed 15 months ago by
- Cc slelievre spancratz added
comment:27 Changed 13 months ago by
- Priority changed from major to critical
comment:28 Changed 13 months ago by
- Branch changed from u/dimpase/packages/deformat/no_mpir to 44b1d0a533e9a2b5df3e6785b07464504daaa770
- Resolution set to fixed
- Status changed from positive_review to closed
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.