Opened 2 years ago

Closed 2 years ago

#29430 closed enhancement (fixed)

spkg-configure.m4 for flintqs

Reported by: mjo Owned by:
Priority: major Milestone: sage-9.1
Component: build: configure Keywords:
Cc: dimpase, embray, mkoeppe, fbissey Merged in:
Authors: Michael Orlitzky Reviewers: Matthias Koeppe, Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: 5990654 (Commits, GitHub, GitLab) Commit: 5990654aa3f16aed4e28425e4f0f64ddf7ea9902
Dependencies: Stopgaps:

Status badges

Description

This package only installs the QuadraticSieve executable, which needs to be in the PATH for qsieve() to work in Sage.

Change History (30)

comment:1 Changed 2 years ago by mjo

  • Branch set to u/mjo/ticket/29430
  • Commit set to 580c0ee678f3c04e191613256921c05e4dfa83a9
  • Status changed from new to needs_review

New commits:

​580c0eeTrac #29430: new spkg-configure.m4 for flintqs.

comment:2 Changed 2 years ago by mkoeppe

  • Status changed from needs_review to needs_work

Needs distros/

comment:3 Changed 2 years ago by git

  • Commit changed from 580c0ee678f3c04e191613256921c05e4dfa83a9 to 2b1e41b0bc0041028e7576d29c4b0d014fbe1685

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

​2b1e41bTrac #29430: add distros/* information for flintqs.

comment:4 Changed 2 years ago by mjo

  • Status changed from needs_work to needs_review

I added debian, fedora, and gentoo (I added the Gentoo package a few seconds before opening this ticket) based on repology.org results. Ubuntu and nix have packages too, but I don't see ubuntu.txt or nix.txt anywhere in the tree.

comment:5 Changed 2 years ago by mkoeppe

ubuntu uses debian.txt

comment:6 Changed 2 years ago by mkoeppe

See #29130 for nix

comment:7 Changed 2 years ago by mkoeppe

Have you checked arch?

comment:8 Changed 2 years ago by dimpase

on conda it's flintqs too.

comment:10 Changed 2 years ago by dimpase

Fedora does not have flintqs

comment:11 Changed 2 years ago by git

  • Commit changed from 2b1e41b0bc0041028e7576d29c4b0d014fbe1685 to b34335174b52d8377aa8bbe22c145b70bfb09f54

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

​b343351Trac #29430: add distros/* information for flintqs.

comment:12 Changed 2 years ago by mjo

Force pushed a rename fedora -> conda. The arch.txt was already there (before I added the m4 file).

comment:13 Changed 2 years ago by dimpase

  • Reviewers set to Matthias Koeppe, Dima Pasechnik
  • Status changed from needs_review to positive_review

lgtm - but add author's name

comment:14 Changed 2 years ago by mjo

  • Authors set to Michael Orlitzky

comment:15 Changed 2 years ago by mkoeppe

  • Status changed from positive_review to needs_review

Let me run some tests first.

comment:16 Changed 2 years ago by dimpase

well, I did test on Debian. There is basically one version of this packages around.

comment:17 Changed 2 years ago by mkoeppe

  • Status changed from needs_review to positive_review

Tests ran at ​https://github.com/mkoeppe/sage/actions/runs/66814284

comment:18 Changed 2 years ago by mjo

  • Status changed from positive_review to needs_work

Sorry, I'll have to add "mpir" to the DEPCHECK here. I didn't realize sage installed it in place of gmp with everything renamed mpir->gmp.

comment:19 Changed 2 years ago by dimpase

you need to check for BOTH gmp and mpir, not just one of them.

comment:20 Changed 2 years ago by dimpase

e.g. 

SAGE_SPKG_CONFIGURE([givaro], [
    m4_pushdef([SAGE_GIVARO_MINVER],["40101"])
    SAGE_SPKG_DEPCHECK([gmp mpir], [
...

comment:21 Changed 2 years ago by mjo

Thanks, I see that now. I was rather confused about how all these packages explicitly looking for libgmp in ./configure could use MPIR in sage. I'm halfway through a build test for a ppl spkg-configure but as soon as it finishes I'll fix this up.

comment:22 Changed 2 years ago by dimpase

  • Branch changed from u/mjo/ticket/29430 to public/packages/flintqs-config
  • Commit changed from b34335174b52d8377aa8bbe22c145b70bfb09f54 to 1fdea6a54fe90ba109ed1b40d1526256a9c3a342
  • Status changed from needs_work to needs_review

added this fix, rebased over latest beta

New commits:

​6311b9cTrac #29430: new spkg-configure.m4 for flintqs.
​db57b3fTrac #29430: add distros/* information for flintqs.
​1fdea6aadding mpir into deps

comment:23 Changed 2 years ago by dimpase

  • Status changed from needs_review to positive_review

lgtm

comment:24 Changed 2 years ago by mkoeppe

If this only installs an executable, I don't think we need any depcheck at all because there cannot be a library conflict.

comment:25 Changed 2 years ago by mkoeppe

  • Status changed from positive_review to needs_info

comment:26 follow-up: Changed 2 years ago by mjo

The QuadraticSieve executable is the only thing used, and if it exists on the system, it should work fine.. does anyone philosophically object if --without-system-gmp (or mpir) is ignored for some packages?

comment:27 in reply to: ↑ 26 Changed 2 years ago by mkoeppe

Replying to mjo:

does anyone philosophically object if --without-system-gmp (or mpir) is ignored for some packages?

I don't think this is a concern.

comment:28 Changed 2 years ago by mjo

  • Branch changed from public/packages/flintqs-config to u/mjo/ticket/29430
  • Commit changed from 1fdea6a54fe90ba109ed1b40d1526256a9c3a342 to 5990654aa3f16aed4e28425e4f0f64ddf7ea9902
  • Status changed from needs_info to needs_review

Ok, depcheck is gone.

New commits:

​34a9dbfTrac #29430: new spkg-configure.m4 for flintqs.
​5990654Trac #29430: add distros/* information for flintqs.

comment:29 Changed 2 years ago by mkoeppe

  • Status changed from needs_review to positive_review

comment:30 Changed 2 years ago by vbraun

  • Branch changed from u/mjo/ticket/29430 to 5990654aa3f16aed4e28425e4f0f64ddf7ea9902
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.