Opened 12 months ago

Closed 11 months ago

Last modified 11 months ago

#31025 closed enhancement (fixed)

FPLLL 5.4.0 and FPyLLL 0.5.4

Reported by: malb Owned by:
Priority: critical Milestone: sage-9.3
Component: packages: standard Keywords: sd111
Cc: fbissey Merged in:
Authors: Martin Albrecht Reviewers: Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: 29fcb34 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by mkoeppe)

Tarball URLs - see checksums.ini

Change History (44)

comment:1 Changed 12 months ago by malb

  • Branch set to u/malb/fplll_5_4_0_and_fpylll_0_5_3_dev0

comment:2 Changed 12 months ago by malb

  • Authors set to Martin Albrecht
  • Commit set to 8faa924c7f7ac8424af39ccef8d3d0efe22dbe16
  • Component changed from PLEASE CHANGE to packages: standard
  • Description modified (diff)
  • Status changed from new to needs_review
  • Type changed from PLEASE CHANGE to enhancement

New commits:

8faa924FPLLL 5.4.0 and FPyLLL 0.5.3.dev0

comment:3 Changed 12 months ago by fbissey

  • Cc fbissey added

comment:4 Changed 12 months ago by mkoeppe

build/pkgs/fpylll/checksums.ini needs upstream_url - see other python packages for the preferred URL format

comment:5 Changed 12 months ago by git

  • Commit changed from 8faa924c7f7ac8424af39ccef8d3d0efe22dbe16 to 0ab1d0e53a496e1b0e0130058c1c604b43c78478

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

0ab1d0eadd upstream_url

comment:6 Changed 12 months ago by malb

Ta. Fixed.

comment:7 Changed 12 months ago by mkoeppe

  • Keywords sd111 added

comment:8 Changed 12 months ago by mkoeppe

  • Description modified (diff)

comment:9 Changed 12 months ago by mkoeppe

  • Reviewers set to https://github.com/mkoeppe/sage/actions/runs/413724088

comment:10 Changed 12 months ago by mkoeppe

  • Reviewers changed from https://github.com/mkoeppe/sage/actions/runs/413724088 to https://github.com/mkoeppe/sage/actions/runs/414158703

comment:11 Changed 12 months ago by mkoeppe

On local-conda-forge-ubuntu-standard (https://github.com/mkoeppe/sage/runs/1533944840)

  build/src/fpylll/fplll/enumeration.cpp: In function 'PyObject* __pyx_pf_6fpylll_5fplll_11enumeration_11Enumeration_6get_nodes(__pyx_obj_6fpylll_5fplll_11enumeration_Enumeration*, PyObject*)':
  build/src/fpylll/fplll/enumeration.cpp:7430:99: error: no matching function for call to 'fplll::Enumeration<fplll::Z_NR<__mpz_struct [1]>, fplll::FP_NR<double> >::get_nodes(int&)'
       __pyx_t_3 = __Pyx_PyInt_From_unsigned_long(__pyx_v_self->_core.mpz_d->get_nodes(__pyx_v__level)); if (unlikely(!__pyx_t_3)) __PYX_ERR(0, 576, __pyx_L1_error)
                                                                                                     

comment:12 Changed 12 months ago by fbissey

Works fine in Gentoo. I must I never know where to find the appropriate logs in your runs. Sometimes it is easy and sometimes I just get lost. I found the good stuff in the raw logs.

2020-12-11T03:40:06.7154190Z   building 'fpylll.fplll.enumeration' extension
2020-12-11T03:40:06.7164990Z   /home/runner/work/sage/sage/.tox/local-conda-forge-ubuntu-standard/conda/bin/x86_64-conda-linux-gnu-cc -Wno-unused-result -Wsign-compare -DNDEBUG -fwrapv -O2 -Wall -Wstrict-prototypes -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -pipe -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -pipe -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /home/runner/work/sage/sage/.tox/local-conda-forge-ubuntu-standard/conda/include -DNDEBUG -D_FORTIFY_SOURCE=2 -O2 -isystem /home/runner/work/sage/sage/.tox/local-conda-forge-ubuntu-standard/conda/include -fPIC -Isrc/fpylll/fplll -I/home/runner/work/sage/sage/.tox/local-conda-forge-ubuntu-standard/local/lib/python3.8/site-packages/cysignals -I/home/runner/work/sage/sage/.tox/local-conda-forge-ubuntu-standard/local/include -I/home/runner/work/sage/sage/.tox/local-conda-forge-ubuntu-standard/local/lib/python3.8/site-packages/numpy/core/include -I/home/runner/work/sage/sage/.tox/local-conda-forge-ubuntu-standard/local/include -I/home/runner/work/sage/sage/.tox/local-conda-forge-ubuntu-standard/conda/include/python3.8 -c build/src/fpylll/fplll/enumeration.cpp -o build/temp.linux-x86_64-3.8/build/src/fpylll/fplll/enumeration.o -std=c++11 -fvisibility-inlines-hidden -std=c++17 -fmessage-length=0 -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /home/runner/work/sage/sage/.tox/local-conda-forge-ubuntu-standard/conda/include
2020-12-11T03:40:06.7174509Z   cc1plus: warning: command line option '-Wstrict-prototypes' is valid for C/ObjC but not for C++
2020-12-11T03:40:06.7176260Z   build/src/fpylll/fplll/enumeration.cpp: In function 'PyObject* __pyx_pf_6fpylll_5fplll_11enumeration_11Enumeration_6get_nodes(__pyx_obj_6fpylll_5fplll_11enumeration_Enumeration*, PyObject*)':
2020-12-11T03:40:06.7178575Z   build/src/fpylll/fplll/enumeration.cpp:7430:99: error: no matching function for call to 'fplll::Enumeration<fplll::Z_NR<__mpz_struct [1]>, fplll::FP_NR<double> >::get_nodes(int&)'
2020-12-11T03:40:06.7180107Z        __pyx_t_3 = __Pyx_PyInt_From_unsigned_long(__pyx_v_self->_core.mpz_d->get_nodes(__pyx_v__level)); if (unlikely(!__pyx_t_3)) __PYX_ERR(0, 576, __pyx_L1_error)

I have a feeling -std=c++17 is responsible for this, I compile with c++11 in Gentoo (probably a default somewhere).

comment:13 follow-up: Changed 12 months ago by malb

Is there perhaps a race condition that FPyLLL is built before the new FPLLL?

comment:14 in reply to: ↑ 13 Changed 12 months ago by fbissey

Replying to malb:

Is there perhaps a race condition that FPyLLL is built before the new FPLLL?

Looking at the log again, I'd say it looks like it!

comment:15 Changed 12 months ago by mkoeppe

This build is accepting a system package

Checking whether SageMath should install SPKG fplll...
checking whether any of mpfr is installed as or will be installed as SPKG... no
checking for fplll >= 5.3... yes
configure: will use system package and not install SPKG fplll

If the new fpylll has other version requirements, build/pkgs/fplll/spkg-configure.m4 needs to be changed

comment:16 Changed 12 months ago by mkoeppe

Same failure also on fedora-32-standard https://github.com/mkoeppe/sage/runs/1533944016 and other builds

comment:17 Changed 12 months ago by mkoeppe

If practical at all, I would suggest to upstream to allow the Python bindings to accept a range of fplll versions...

comment:18 Changed 12 months ago by git

  • Commit changed from 0ab1d0e53a496e1b0e0130058c1c604b43c78478 to 9eb14e445b2bea0ab0178f426787b8d5a1085527

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

9eb14e4update required FPLLL version

comment:19 Changed 12 months ago by malb

Updated accordingly, I don't think we can commit to supporting a range of FPLLL versions in FPyLLL

comment:20 follow-up: Changed 12 months ago by mkoeppe

Perhaps then an upper bound for the version needs to be set as well?

comment:21 follow-up: Changed 12 months ago by mkoeppe

  • Status changed from needs_review to needs_info

Likewise, should we set a range of acceptable fpylll versions (in the install-requires of sagelib)?

comment:22 in reply to: ↑ 20 ; follow-up: Changed 12 months ago by malb

Replying to mkoeppe:

Perhaps then an upper bound for the version needs to be set as well?

I think an upper bound is unclear, we could enforce a specific version, e.g. 5.4.0?

comment:23 in reply to: ↑ 21 ; follow-up: Changed 12 months ago by malb

Replying to mkoeppe:

Likewise, should we set a range of acceptable fpylll versions (in the install-requires of sagelib)?

For now, Sage only makes very high-level use of FPyLLL, i.e. it should work with many versions.

comment:24 in reply to: ↑ 22 ; follow-up: Changed 12 months ago by mkoeppe

Replying to malb:

Replying to mkoeppe:

Perhaps then an upper bound for the version needs to be set as well?

I think an upper bound is unclear, we could enforce a specific version, e.g. 5.4.0?

So there is no expectation that FPyLLL 0.5.3.dev0 can work with any FPLLL 5.4.x?

comment:25 in reply to: ↑ 23 ; follow-up: Changed 12 months ago by mkoeppe

Replying to malb:

Replying to mkoeppe:

Likewise, should we set a range of acceptable fpylll versions (in the install-requires of sagelib)?

For now, Sage only makes very high-level use of FPyLLL, i.e. it should work with many versions.

To clarify my question, the Sage distribution ships a specific version of FPLLL (5.4.0 with this ticket); but if fpylll comes from PyPI, what future versions should we expect to support this version of FPLLL?

If FPLLL/FPyLLL do not have a policy regarding version compatibility, it would probably pay off to set one...

comment:26 in reply to: ↑ 24 Changed 12 months ago by malb

Replying to mkoeppe:

So there is no expectation that FPyLLL 0.5.3.dev0 can work with any FPLLL 5.4.x?

No expectation at all.

comment:27 in reply to: ↑ 25 Changed 12 months ago by malb

Replying to mkoeppe:

To clarify my question, the Sage distribution ships a specific version of FPLLL (5.4.0 with this ticket); but if fpylll comes from PyPI, what future versions should we expect to support this version of FPLLL?

If FPLLL/FPyLLL do not have a policy regarding version compatibility, it would probably pay off to set one...

Realistically the only viable policy is: whatever versions were released at the same time should work together. So I think it makes sense to enforce a tuple: FPLLL=X,FPyLLL=Y

comment:28 Changed 12 months ago by git

  • Commit changed from 9eb14e445b2bea0ab0178f426787b8d5a1085527 to 26032b18f9b352d5ca357582939fe64851f503a9

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

37414ebupdate to FPyLLL 0.5.4
26032b1enforce exact FPLLL

comment:29 Changed 12 months ago by malb

  • Summary changed from FPLLL 5.4.0 and FPyLLL 0.5.3.dev0 to FPLLL 5.4.0 and FPyLLL 0.5.4

comment:30 follow-up: Changed 12 months ago by mkoeppe

Based on your explanations, I think fpylll's install-requires.txt should also fix the version.

comment:31 Changed 12 months ago by mkoeppe

  • Reviewers changed from https://github.com/mkoeppe/sage/actions/runs/414158703 to https://github.com/mkoeppe/sage/actions/runs/426748847
  • Status changed from needs_info to needs_work

comment:33 Changed 12 months ago by mkoeppe

No, that's not related to this ticket

comment:34 in reply to: ↑ 30 Changed 12 months ago by mkoeppe

Replying to mkoeppe:

Based on your explanations, I think fpylll's install-requires.txt should also fix the version.

As in:

diff --git a/build/pkgs/fpylll/install-requires.txt b/build/pkgs/fpylll/install-requires.txt
index 6e7d17221c..df75cd9fa7 100644
--- a/build/pkgs/fpylll/install-requires.txt
+++ b/build/pkgs/fpylll/install-requires.txt
@@ -1 +1 @@
-fpylll >=0.5.4
+fpylll ==0.5.4

comment:35 Changed 12 months ago by mkoeppe

I would also suggest to add some comments:

diff --git a/build/pkgs/fplll/spkg-configure.m4 b/build/pkgs/fplll/spkg-configure.m4
index 6da0045fe0..b98a71938e 100644
--- a/build/pkgs/fplll/spkg-configure.m4
+++ b/build/pkgs/fplll/spkg-configure.m4
@@ -4,6 +4,9 @@ SAGE_SPKG_CONFIGURE([fplll], [
     dnl if there's a usable system copy of fplll. Unless there's
     dnl a system that ships fplll without fplll.pc file, falling
     dnl back to a manual header/library search is pointless.
+    dnl
+    dnl Trac #31025: FPLLL/FPyLLL make no guarantee regarding compatibility
+    dnl other than "whatever versions were released at the same time should work together"
     PKG_CHECK_MODULES([FPLLL],
                       [fplll == 5.4],
                       [sage_spkg_install_fplll=no],
diff --git a/build/pkgs/fpylll/install-requires.txt b/build/pkgs/fpylll/install-requires.txt
index 6e7d17221c..4b425c186e 100644
--- a/build/pkgs/fpylll/install-requires.txt
+++ b/build/pkgs/fpylll/install-requires.txt
@@ -1 +1,3 @@
-fpylll >=0.5.4
+# Trac #31025: FPLLL/FPyLLL make no guarantee regarding compatibility
+# other than "whatever versions were released at the same time should work together"
+fpylll ==0.5.4

comment:36 Changed 11 months ago by git

  • Commit changed from 26032b18f9b352d5ca357582939fe64851f503a9 to 29fcb343129730537546e2c7c0d65906bb602de5

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

29fcb34implement suggested changes by Matthias

comment:37 Changed 11 months ago by malb

  • Status changed from needs_work to needs_review

comment:38 Changed 11 months ago by mkoeppe

  • Reviewers changed from https://github.com/mkoeppe/sage/actions/runs/426748847 to Matthias Koeppe
  • Status changed from needs_review to positive_review

comment:39 Changed 11 months ago by mkoeppe

  • Priority changed from major to critical

Marking it as critical because some rolling distributions are already picking up 5.4.0

comment:40 Changed 11 months ago by vbraun

  • Branch changed from u/malb/fplll_5_4_0_and_fpylll_0_5_3_dev0 to 29fcb343129730537546e2c7c0d65906bb602de5
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:41 follow-up: Changed 11 months ago by dcoudert

  • Commit 29fcb343129730537546e2c7c0d65906bb602de5 deleted

For some reason, even after make distclean, configure is unable to find homebrew install of fplll 5.4.0 (on macOS). Then, the compilation fails on fpylll (may be a mix of fplll installations).

After modifying spkg-configure.m4 to change fplll == 5.4.0 to fplll >= 5.4.0 and adding file distros/homebrew.txt with fplll, configure finds the local install and everything goes well.

Should we open a ticket for that ?

comment:42 in reply to: ↑ 41 Changed 11 months ago by mkoeppe

Replying to dcoudert:

Should we open a ticket for that ?

Yes please. What version of fplll does pkg-config report?

comment:43 Changed 11 months ago by dcoudert

not sure how to get that, but inside config.log, I see:

## ------------------------------------------------------ ##
## Checking whether SageMath should install SPKG fplll... ##
## ------------------------------------------------------ ##
configure:19538: checking whether any of mpfr is installed as or will be installed as SPKG
configure:19547: result: no
configure:19552: checking for fplll >= 5.4
configure:19559: $PKG_CONFIG --exists --print-errors "fplll >= 5.4"
configure:19562: $? = 0
configure:19576: $PKG_CONFIG --exists --print-errors "fplll >= 5.4"
configure:19579: $? = 0
configure:19617: result: yes
configure:19629: will use system package and not install SPKG fplll

and I have fplll 5.4.0 installed by homebrew.

Follow-up on #31127

comment:44 Changed 11 months ago by mkoeppe

Another follow up in #31146: cygwin-standard: fpylll build fails

Note: See TracTickets for help on using tickets.