Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade LiDIA to v2.3.0+latte-patches-2019-05-01 #27746

Closed
mkoeppe opened this issue Apr 29, 2019 · 30 comments
Closed

Upgrade LiDIA to v2.3.0+latte-patches-2019-05-01 #27746

mkoeppe opened this issue Apr 29, 2019 · 30 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Apr 29, 2019

This upgrade fixes compile errors on Mac with clang 6.0.1 as reported here:
https://groups.google.com/d/msg/sage-devel/UtdbqZy-1VE/Jlxfj3FxDAAJ
as well as clang 7.

Upstream issues:
mkoeppe/LiDIA#1

Package:

wget -P upstream https://github.com/mkoeppe/LiDIA/releases/download/v2.3.0%2Blatte-patches-2019-05-02/lidia-2.3.0+latte-patches-2019-05-02.tar.gz

Upstream: Fixed upstream, in a later stable release.

CC: @dimpase @jplab @sophiasage

Component: packages: experimental

Author: Matthias Koeppe

Branch/Commit: ba3e3e3

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/27746

@mkoeppe mkoeppe added this to the sage-8.8 milestone Apr 29, 2019
@dimpase
Copy link
Member

dimpase commented Apr 29, 2019

comment:1

Also, mkoeppe/LiDIA#2 - can this be merged upstream?

@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 29, 2019

comment:3

What happened in this bug report is that CXX has been set to "g++ -std=gnu++11" (where g++ is the system's clang compiler).
This causes an error.

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Member Author

mkoeppe commented May 1, 2019

Changed upstream from Reported upstream. Developers acknowledge bug. to Fixed upstream, in a later stable release.

@mkoeppe mkoeppe changed the title Fix LiDIA installation on macOS Upgrade LiDIA to v2.3.0+latte-patches-2019-05-01 May 1, 2019
@mkoeppe
Copy link
Member Author

mkoeppe commented May 1, 2019

@mkoeppe
Copy link
Member Author

mkoeppe commented May 1, 2019

New commits:

d0d3954Upgrade LiDIA to 2.3.0+latte-patches-2019-05-01

@mkoeppe
Copy link
Member Author

mkoeppe commented May 1, 2019

Commit: d0d3954

@mkoeppe
Copy link
Member Author

mkoeppe commented May 1, 2019

Author: Matthias Koeppe

@dimpase
Copy link
Member

dimpase commented May 1, 2019

comment:7

OK, this also needs adjustement w.r.t. to #27212. (Somehow lidia went under the radar there).

What is the correct way to configure it if GMP is installed in the standard location,
searched by default by compiler/linker? Does it still need these --with-extra...?

Otherwise, spkg-install should contain something like

sdh_configure --with-arithmetic=gmp \
              --with-extra-includes="$SAGE_GMP_INCLUDE" \
              --with-extra-libs="$SAGE_GMP_PREFIX/lib" \
              --enable-shared=yes --enable-static=no

ok, so an experiment tells me that --with-extra- may be skipped if the GMP location is searched by compiler/linker.

@dimpase
Copy link
Member

dimpase commented May 1, 2019

comment:8

otherwise, I checked that things build with gcc 8.3 on linux and with clang 6.0.1 on freebsd.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 1, 2019

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

625a1e3build/pkgs/lidia/spkg-install: Use

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 1, 2019

Changed commit from d0d3954 to 625a1e3

@sophiasage
Copy link
Mannequin

sophiasage mannequin commented May 2, 2019

@sophiasage
Copy link
Mannequin

sophiasage mannequin commented May 2, 2019

Attachment: lidia-2.3.0+latte-patches-2019-05-01.log

@dimpase
Copy link
Member

dimpase commented May 2, 2019

comment:10

in the setting where location of GMP headers and libs has to be specified in C(XX)FLAGS and in LDFLAGS, lidia's configure returns a false negative, as it ignores them for no good reason. So one needs

--- /dev/null
+++ b/build/pkgs/lidia/patches/accept_gmp.patch
@@ -0,0 +1,13 @@
+diff --git a/configure b/configure
+index dd9b0ad..77a438a 100755
+--- a/configure
++++ b/configure
+@@ -18092,7 +18092,7 @@ EOF
+   test $ac_status = 0; }; } > /dev/null 2>&1; then
+                       lidia_cv_gmp="yes"
+               else
+-                      lidia_cv_gmp="no"
++                      lidia_cv_gmp="yes"
+               fi
+ 
+ fi

to tell it to shut up and trust that Sage has it all sorted already.

@mkoeppe
Copy link
Member Author

mkoeppe commented May 2, 2019

comment:11

The correct fix, of course, would be in lidia's acinclude.m4.

@dimpase
Copy link
Member

dimpase commented May 2, 2019

comment:12

Of course, but I saw way too many semi-broken homegrown m4 macros to recognise GMP lately to be bothered about it.

The only harm of this patch is that, potentially, if something goes horribly wrong, you get a linker error...

@dimpase
Copy link
Member

dimpase commented May 2, 2019

comment:13
Changed 3 hours ago by selia
Attachment lidia-2.3.0+latte-patches-2014-10-04.p0.log​ added
Changed 3 hours ago by selia
Attachment lidia-2.3.0+latte-patches-2019-05-01.log​ added

@sophiasage, have you pulled the branch of this ticket?
There should not be any patches to apply in build/pkgs/lidia/patches/.

@mkoeppe
Copy link
Member Author

mkoeppe commented May 2, 2019

comment:14

Dima, could you test branch config_flags from https://github.com/mkoeppe/LiDIA/tree/config_flags

@dimpase
Copy link
Member

dimpase commented May 2, 2019

comment:15

running ./bootstrap gives a lot of warnings like

library/number_fields/Makefile.am:42: warning: source file '$(LIDIA_NF_SRCDIR)/quadratic_order/quadratic_order1.cc' is in a subdirectory,
library/number_fields/Makefile.am:42: but option 'subdir-objects' is disabled

Is it normal?

@dimpase
Copy link
Member

dimpase commented May 2, 2019

comment:16

OK, https://github.com/mkoeppe/LiDIA/tree/config_flags works for me in instead of the patch in comment:10.

By the way, not only with clang 6, but with clang 7 too.

@mkoeppe
Copy link
Member Author

mkoeppe commented May 2, 2019

comment:17

Thanks for testing!

@dimpase
Copy link
Member

dimpase commented May 2, 2019

comment:18

once you made a new tarball, and provided a link to it, please feel free to set this to positive review.

@dimpase
Copy link
Member

dimpase commented May 2, 2019

Reviewer: Dima Pasechnik

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Member Author

mkoeppe commented May 2, 2019

comment:21

Replying to @dimpase:

running ./bootstrap gives a lot of warnings like

library/number_fields/Makefile.am:42: warning: source file '$(LIDIA_NF_SRCDIR)/quadratic_order/quadratic_order1.cc' is in a subdirectory,
library/number_fields/Makefile.am:42: but option 'subdir-objects' is disabled

Is it normal?

I see these warnings too, but I decided not to address them at this time.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 2, 2019

Changed commit from 625a1e3 to ba3e3e3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 2, 2019

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

ba3e3e3Update LiDIA to 2.3.0+latte-patches-2019-05-02

@vbraun
Copy link
Member

vbraun commented May 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants