Opened 3 years ago

Closed 3 years ago

#27271 closed enhancement (fixed)

spkg-configure.m4 for ecm

Reported by: dimpase Owned by:
Priority: major Milestone: sage-8.9
Component: build Keywords: spkg-configure
Cc: embray, fbissey Merged in:
Authors: Dima Pasechnik Reviewers: Isuru Fernando
Report Upstream: N/A Work issues:
Branch: 96ade1f (Commits, GitHub, GitLab) Commit: 96ade1f6e46bbe678f383beb7559fedadcbd3a8e
Dependencies: Stopgaps:

Status badges

Description (last modified by fbissey)

it needs gmp, and is not a dependency of anything (besides sagelib)

To get this package on the system:

  • Fedora: gmp-ecm-devel
  • Gentoo: gmp-ecm - (but the version is too old in the main tree. Decent one in the sage-on-gentoo overlay)
  • FreeBSD: gmp-ecm

Change History (23)

comment:1 Changed 3 years ago by embray

  • Milestone changed from sage-8.7 to sage-8.8

Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)

comment:2 Changed 3 years ago by dimpase

  • Branch set to u/dimpase/packages/ecmconfig
  • Commit set to 7d1f15e76c1b01da96fa00a20cdfab05623a1937
  • Status changed from new to needs_review

checking the version by grepping the header, duh...


Last 10 new commits:

b80bf72Reworked this a bit more
0ca3f56fix typo
06798caadded a bit more explanation
b592d77correct logic for SAGE_GMP_PREFIX etc
5057680add the AX_ABSOLUTE_HEADER macro
101537biml in particular is very picky about being given an absolute path to the
862ca6aMerge remote-tracking branch 'trac/develop' into HEAD
03dc987Merged trac #27215 in
98c67d3Merge remote-tracking branch 'trac/public/packages/gmp_m4' into ecm-config
7d1f15espkg-configure for ecm

comment:3 follow-up: Changed 3 years ago by embray

Does this really depend on #27212? I ask just because it's not clear to me exactly how, and this might be useful on its own as well.

comment:4 in reply to: ↑ 3 ; follow-up: Changed 3 years ago by dimpase

Replying to embray:

Does this really depend on #27212? I ask just because it's not clear to me exactly how, and this might be useful on its own as well.

gmp/mpir is a dependency of ecm! (and so not using system's gmp/mpir and using the system's gmp/mpir for ecm would lead to a lot of linking/loading "fun" with sagelib)

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

comment:5 Changed 3 years ago by embray

As for the contents of the ecm/spkg-configure.m4 itself, it looks fine to me at first glance, an I trust you've tested it.

A slightly more "sophisticated" approach might be build a test program which prints the value of the version, but in this case it's simple enough that just grepping for it should be good enough.

comment:6 Changed 3 years ago by dimpase

I am going to submit an upstream patch to get a pkg-config configuration for ecm (more or less the same INRIA upstream accepted such patch for gf2x, so this will make this much easier, once in).

comment:7 in reply to: ↑ 4 Changed 3 years ago by embray

Replying to dimpase:

Replying to embray:

Does this really depend on #27212? I ask just because it's not clear to me exactly how, and this might be useful on its own as well.

gmp/mpir is a dependency of ecm! (and so not using system's gmp/mpir and using the system's gmp/mpir for ecm would lead to a lot of linking/loading "fun" with sagelib)

I see. In that case (and we still need a generic way to do this but I think it's tricky), the ecm/spkg-configure.m4 should do

AC_REQUIRE([SAGE_SPKG_CONFIGURE_GMP])

and then refuse to use the system package if sage_spkg_install_mpir = yes or sage_spkg_install_gmp = yes.

comment:8 Changed 3 years ago by dimpase

OK, so the latter applies to other tickets that have #27212 as a dependence.

comment:9 Changed 3 years ago by git

  • Commit changed from 7d1f15e76c1b01da96fa00a20cdfab05623a1937 to cb3a637b22f7cffcc9a9e7a645499f0861edec54

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

6253f30spkg-configure for ecm
cb3a637added the check for gmp/mpir presence

comment:10 Changed 3 years ago by dimpase

  • Cc fbissey added
  • Dependencies #27212 deleted
  • Description modified (diff)

rebased over 8.8.beta5 and added the missing test. Can be reviewed now.

comment:11 Changed 3 years ago by fbissey

  • Description modified (diff)

comment:12 Changed 3 years ago by dimpase

  • Status changed from needs_review to needs_work

also need to check for ecm executable.

Version 0, edited 3 years ago by dimpase (next)

comment:13 Changed 3 years ago by git

  • Commit changed from cb3a637b22f7cffcc9a9e7a645499f0861edec54 to 54bfb1d0dda028789b697995d86a6cb5f05d33a9

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

eaccefdspkg-configure for ecm
6faf068added the check for gmp/mpir presence
54bfb1dcheck for ecm executable, use temp var for version

comment:14 Changed 3 years ago by dimpase

  • Status changed from needs_work to needs_review

needs review again.

comment:15 Changed 3 years ago by embray

Not sure if it's related to this ticket or not (it wouldn't be caused by it as it hasn't had positive_review yet, but I mean it might be relevant), but I'm seeing some problems on Cygwin with the ECM make check test suite, which I don't think I had problems with before (I did not list ECM in #22866).

I think I might still need to go ahead and make building MPIR a requirement on Cygwin for now anyways. I'll try to investigate this once I can to a state where everything else isn't broken :(

comment:16 Changed 3 years ago by git

  • Commit changed from 54bfb1d0dda028789b697995d86a6cb5f05d33a9 to d9a37478f9621258944169d7e8c7d87914ce1085

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

b52ad46spkg-configure for ecm
b4b099fadded the check for gmp/mpir presence
d9a3747check for ecm executable, use temp var for version

comment:17 Changed 3 years ago by dimpase

rebased oved 8.8.rc0

comment:18 Changed 3 years ago by embray

  • Milestone changed from sage-8.8 to sage-8.9

Moving tickets from the Sage 8.8 milestone that have been actively worked on in the last six months to the next release milestone (optimistically).

comment:19 Changed 3 years ago by isuruf

  • Status changed from needs_review to positive_review

Picks up the conda package correctly.

comment:20 Changed 3 years ago by isuruf

  • Reviewers set to Isuru Fernando

comment:21 Changed 3 years ago by git

  • Commit changed from d9a37478f9621258944169d7e8c7d87914ce1085 to 96ade1f6e46bbe678f383beb7559fedadcbd3a8e
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:

65edbffspkg-configure for ecm
c6feec7added the check for gmp/mpir presence
96ade1fcheck for ecm executable, use temp var for version

comment:22 Changed 3 years ago by dimpase

  • Status changed from needs_review to positive_review

rebased over Sage 8.9.beta3, just in case.

comment:23 Changed 3 years ago by vbraun

  • Branch changed from u/dimpase/packages/ecmconfig to 96ade1f6e46bbe678f383beb7559fedadcbd3a8e
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.