Opened 6 years ago

Closed 6 years ago

#21690 closed defect (fixed)

fp(y)lll 5.x build fails on Cygwin

Reported by: embray Owned by:
Priority: minor Milestone: sage-7.4
Component: porting: Cygwin Keywords: windows cygwin fplll
Cc: malb Merged in:
Authors: Erik Bray Reviewers: Jean-Pierre Flori
Report Upstream: Fixed upstream, but not in a stable release. Work issues:
Branch: 1517ea7 (Commits, GitHub, GitLab) Commit: 1517ea7830e7a2542c3069239e5bbcbf248dc342
Dependencies: #21675, #20692 Stopgaps:

Status badges

Description (last modified by embray)

Relevant portion of the build log:

[fplll-5.0.3rc0] depbase=`echo fplll.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
[fplll-5.0.3rc0] /bin/sh ../libtool  --tag=CXX   --mode=compile g++ -std=c++11 -DHAVE_CONFIG_H -I. -I..  -DFPLLL_DEFAULT_STRATEGY_PATH=\"/home/embray/src/sagemath/sage/local/share/fplll/strategies\" -DFPLLL_DEFAULT_STRATEGY=\"/home/embray/src/sagemath/sage/local/share/fplll/strategies/default.json\" -I./.. -I/home/embray/src/sagemath/sage/local/include/  -I/home/embray/src/sagemath/sage/local/include/ -L/home/embray/src/sagemath/sage/local/lib -O0 -MT fplll.lo -MD -MP -MF $depbase.Tpo -c -o fplll.lo fplll.cpp &&\
[fplll-5.0.3rc0] mv -f $depbase.Tpo $depbase.Plo
[fplll-5.0.3rc0] libtool: compile:  g++ -std=c++11 -DHAVE_CONFIG_H -I. -I.. -DFPLLL_DEFAULT_STRATEGY_PATH=\"/home/embray/src/sagemath/sage/local/share/fplll/strategies\" -DFPLLL_DEFAULT_STRATEGY=\"/home/embray/src/sagemath/sage/local/share/fplll/strategies/default.json\" -I./.. -I/home/embray/src/sagemath/sage/local/include/ -I/home/embray/src/sagemath/sage/local/include/ -L/home/embray/src/sagemath/sage/local/lib -O0 -MT fplll.lo -MD -MP -MF .deps/fplll.Tpo -c fplll.cpp  -DDLL_EXPORT -DPIC -o .libs/fplll.o
[fplll-5.0.3rc0] In file included from ../fplll/nr/../defs.h:55:0,
[fplll-5.0.3rc0]                  from ../fplll/nr/nr.h:8,
[fplll-5.0.3rc0]                  from ../fplll/nr/numvect.h:19,
[fplll-5.0.3rc0]                  from nr/matrix.h:21,
[fplll-5.0.3rc0]                  from util.h:23,
[fplll-5.0.3rc0]                  from fplll.h:25,
[fplll-5.0.3rc0]                  from fplll.cpp:18:
[fplll-5.0.3rc0] ../fplll/nr/../nr/dpe.h: In function 'void dpe_ugly_log(dpe_struct*, const dpe_struct*)':
[fplll-5.0.3rc0] ../fplll/nr/../nr/dpe.h:775:42: error: 'M_LN2' was not declared in this scope
[fplll-5.0.3rc0]    dpe_set_d (x, ((double) DPE_EXP(y)) *  M_LN2 + log (DPE_MANT(y)));
[fplll-5.0.3rc0]                                           ^
[fplll-5.0.3rc0] In file included from ../fplll/nr/nr.h:10:0,
[fplll-5.0.3rc0]                  from ../fplll/nr/numvect.h:19,
[fplll-5.0.3rc0]                  from nr/matrix.h:21,
[fplll-5.0.3rc0]                  from util.h:23,
[fplll-5.0.3rc0]                  from fplll.h:25,
[fplll-5.0.3rc0]                  from fplll.cpp:18:
[fplll-5.0.3rc0] ../fplll/nr/nr_rand.inl: In static member function 'static void fplll::RandGen::init_with_time2()':
[fplll-5.0.3rc0] ../fplll/nr/nr_rand.inl:29:28: error: 'gettimeofday' was not declared in this scope
[fplll-5.0.3rc0]      gettimeofday(&time,NULL);
[fplll-5.0.3rc0]                             ^
[fplll-5.0.3rc0] In file included from bkz.h:24:0,
[fplll-5.0.3rc0]                  from fplll.h:28,
[fplll-5.0.3rc0]                  from fplll.cpp:18:
[fplll-5.0.3rc0] lll.h: In destructor 'fplll::LLLReduction<ZT, FT>::~LLLReduction()':
[fplll-5.0.3rc0] lll.h:39:21: error: 'LDConvHelper' has not been declared
[fplll-5.0.3rc0]    ~LLLReduction() { LDConvHelper::free(); }
[fplll-5.0.3rc0]                      ^
[fplll-5.0.3rc0] make[6]: *** [Makefile:851: fplll.lo] Error 1

This only started since #21221 was merged.

There are two issues here, one of which is definitely an upstream bug, and the other of which is more of an omission but could be addressed upstream as well.

  1. Compiling with -std=c++11 fails on Cygwin. I haven't pinpointed the exact cause, but this is not the first time (e.g. #21185) this has been an issue on Cygwin. Really this is a bug with newlib, which has bugs in its headers w.r.t. C standard versions. Compiling with -std=gnu++11 fixes the first two issues above. This could be addressed either in Sage's spkg-install or in a patch to fplll's configure.ac.
  1. The second issue is simply a bug. fplll does not define the macro FPLLL_WITH_LONG_DOUBLE on Cygwin, so the definition of the destructor ~LLLReduction() should be be bracketed with #ifdef FPLLL_WITH_LONG_DOUBLE

Upstream bug report for fplll: https://github.com/fplll/fplll/issues/212 Upstream bug report for fpylll: https://github.com/fplll/fpylll/issues/40

(P.S. I can't wait to get that cygwin build bot up and running :D)

Change History (27)

comment:1 Changed 6 years ago by embray

Description: modified (diff)

comment:2 Changed 6 years ago by embray

Description: modified (diff)

comment:3 Changed 6 years ago by jdemeyer

Dependencies: #21675

comment:4 Changed 6 years ago by embray

Description: modified (diff)
Report Upstream: N/ANot yet reported upstream; Will do shortly.

comment:5 Changed 6 years ago by embray

Description: modified (diff)

comment:6 Changed 6 years ago by embray

Authors: Erik Bray
Branch: u/embray/cygwin/ticket-21690
Commit: 91814c2bc80cf62e8d86379fbd708c8011cd1241
Status: newneeds_review

Here's an initial fix (against #21675) which includes the patch to lll.h and a patch to spkg-install.

I'm also about to try a patch to the configure.ac for supporting Cygwin.


New commits:

88a7a70Make sure dowgrading from fplll to libfplll works.
3951f91Rename fplll -> libfplll
618a581sagelib no longer depends on fplll
91814c2Fix issues with building fplll 5.x on Cygwin as reported in

comment:7 Changed 6 years ago by embray

Description: modified (diff)
Report Upstream: Not yet reported upstream; Will do shortly.Reported upstream. No feedback yet.

comment:8 Changed 6 years ago by embray

Status: needs_reviewneeds_work
Summary: fplll 5.x build fails on Cygwinfp(y)lll 5.x build fails on Cygwin

Looks like fpylll also has a problem on Cygwin. I'm going to address that as part of this ticket while I'm at it. It appears to be another issue with long double.

comment:9 Changed 6 years ago by embray

Description: modified (diff)

comment:10 Changed 6 years ago by tscrim

Cc: tscrim added

comment:11 Changed 6 years ago by embray

Cc: tscrim removed
Report Upstream: Reported upstream. No feedback yet.Reported upstream. Developers acknowledge bug.

comment:12 Changed 6 years ago by embray

My patch for the issue in fpylll is here: https://github.com/fplll/fpylll/pull/41 I've confirmed that it solves the problem for me and passes the existing tests. I could update this ticket to include the patch, but there's no longer any urgency on this for me (as I can proceed with the build) so I'll wait and see.

comment:13 Changed 6 years ago by git

Commit: 91814c2bc80cf62e8d86379fbd708c8011cd1241855752d71251de0de76be1c7a4cb08044a183721

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

27cc968Add license notice.
6c9d6d1Warn but don't fail if there are no patches to apply
229199fApply sage-apply-patches standard on all packages during spkg-install
5b00b25Fixup
5343f3bUse sage-apply-patches in more packages.
2f728e2Make sure dowgrading from fplll to libfplll works.
1a372f4Rename fplll -> libfplll
32aad0asagelib no longer depends on fplll
719a5acFix issues with building fplll 5.x on Cygwin as reported in
855752dAdd patch for fpylll that addresses the build failure on Cygwin

comment:14 Changed 6 years ago by embray

Dependencies: #21675#21675, #20692
Status: needs_workneeds_review

Added additional patch needed for fpylll to build/work on Cygwin. Rebased on #20692 which I'm making a dependency of this ticket, just so I don't have to rebase that branch again. This ticket can wait on #20692.

comment:15 Changed 6 years ago by embray

Report Upstream: Reported upstream. Developers acknowledge bug.Fixed upstream, but not in a stable release.

comment:16 Changed 6 years ago by cheuberg

Status: needs_reviewneeds_work

does not merge with 7.5.rc2

comment:17 Changed 6 years ago by embray

Hm, I wouldn't complain but this one never even got a review.

comment:18 Changed 6 years ago by jpflori

Please update the branch and I'll review it.

comment:19 Changed 6 years ago by git

Commit: 855752d71251de0de76be1c7a4cb08044a18372124abf0de4ae3d30a6c89d1107b08a51ff0431645

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

1517ea7Fix issues with building fplll 5.x on Cygwin as reported in
24abf0dAdd patch for fpylll that addresses the build failure on Cygwin

comment:20 Changed 6 years ago by embray

Status: needs_workneeds_review

Rebased.

comment:21 Changed 6 years ago by jpflori

Reviewers: Jean-Pierre Flori
Status: needs_reviewpositive_review

Looks ok to me. I trust Erik tested it properly.

comment:22 Changed 6 years ago by vbraun

Status: positive_reviewneeds_work
Applying ../patches/ticket-21690.patch
patching file setup.py
Reversed (or previously applied) patch detected!  Assume -R? [n] 
Apply anyway? [n] 
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file setup.py.rej
patching file src/fpylll/config.pyx
Reversed (or previously applied) patch detected!  Assume -R? [n] 
Apply anyway? [n] 
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file src/fpylll/config.pyx.rej
patching file src/fpylll/fplll/bkz.pyx
Reversed (or previously applied) patch detected!  Assume -R? [n] 
Apply anyway? [n] 
Skipping patch.
16 out of 16 hunks ignored -- saving rejects to file src/fpylll/fplll/bkz.pyx.rej
patching file src/fpylll/fplll/decl.pxd
Reversed (or previously applied) patch detected!  Assume -R? [n] 
Apply anyway? [n] 
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file src/fpylll/fplll/decl.pxd.rej
patching file src/fpylll/fplll/enumeration.pyx
Reversed (or previously applied) patch detected!  Assume -R? [n] 
Apply anyway? [n] 
Skipping patch.
8 out of 8 hunks ignored -- saving rejects to file src/fpylll/fplll/enumeration.pyx.rej
patching file src/fpylll/fplll/gso.pyx
Reversed (or previously applied) patch detected!  Assume -R? [n] 
Apply anyway? [n] 
Skipping patch.
30 out of 30 hunks ignored -- saving rejects to file src/fpylll/fplll/gso.pyx.rej
patching file src/fpylll/fplll/lll.pyx
Reversed (or previously applied) patch detected!  Assume -R? [n] 
Apply anyway? [n] 
Skipping patch.
9 out of 9 hunks ignored -- saving rejects to file src/fpylll/fplll/lll.pyx.rej
patching file src/fpylll/fplll/pruner.pyx
Reversed (or previously applied) patch detected!  Assume -R? [n] 
Apply anyway? [n] 
Skipping patch.
3 out of 3 hunks ignored -- saving rejects to file src/fpylll/fplll/pruner.pyx.rej
patching file src/fpylll/numpy.pyx
Reversed (or previously applied) patch detected!  Assume -R? [n] 
Apply anyway? [n] 
Skipping patch.
2 out of 2 hunks ignored -- saving rejects to file src/fpylll/numpy.pyx.rej
patching file src/fpylll/util.pyx
Reversed (or previously applied) patch detected!  Assume -R? [n] 
Apply anyway? [n] 
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file src/fpylll/util.pyx.rej
Error applying '../patches/ticket-21690.patch'

comment:23 Changed 6 years ago by embray

Um, this obviously used to work. Was the fpylll package updated?

comment:24 Changed 6 years ago by git

Commit: 24abf0de4ae3d30a6c89d1107b08a51ff04316451517ea7830e7a2542c3069239e5bbcbf248dc342

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

comment:25 Changed 6 years ago by embray

Status: needs_workneeds_review

Updated the branch to drop to patch for fpylll since it is no longer needed since the fpylll version was bumped in #21728.

comment:26 Changed 6 years ago by jpflori

Status: needs_reviewpositive_review

comment:27 Changed 6 years ago by vbraun

Branch: u/embray/cygwin/ticket-216901517ea7830e7a2542c3069239e5bbcbf248dc342
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.