Opened 9 years ago
Closed 7 years ago
#15015 closed defect (fixed)
Update to MPIR 2.7.0-alpha12
Reported by: | jpflori | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.5 |
Component: | packages: standard | Keywords: | mpir spkg cygwin |
Cc: | kcrisman, dimpase, tscrim, was | Merged in: | |
Authors: | Jean-Pierre Flori, Jeroen Demeyer | Reviewers: | Jean-Pierre Flori |
Report Upstream: | Fixed upstream, but not in a stable release. | Work issues: | |
Branch: | d97c066 (Commits, GitHub, GitLab) | Commit: | d97c06604293f765fd0c8de6189d05cedcebea3c |
Dependencies: | Stopgaps: |
Description (last modified by )
MPIR 2.6.0 does not build on Cygwin 64. The next version should fix this problem (and others). Patches from Sage will mostly be integrated.
upstream at: http://mpir.org/mpir-2.7.0-alpha12.tar.bz2
Attachments (1)
Change History (63)
comment:1 Changed 9 years ago by
comment:2 Changed 9 years ago by
- Description modified (diff)
comment:3 Changed 9 years ago by
- Dependencies set to MPIR 2.6.1 upstream
- Description modified (diff)
- Milestone changed from sage-5.12 to sage-pending
- Summary changed from Let MPIR >=2.6.0 build on Cygwin to Update to MPIR 2.6.1 when it's released
comment:4 Changed 9 years ago by
For the record, the current spkg built for me on cygwin64.
comment:5 Changed 9 years ago by
On top of that the current MPIR 2.6.0 fails to properly build on Cygwin when CXX support is not enabled. Therefore it is not possible/easy to build GCC on Cygwin. That is because of a dirty hack in the autotools stuff already present in GMP before MPIR was forked. This is also fixed in the current git version.
comment:6 Changed 8 years ago by
- Summary changed from Update to MPIR 2.6.1 when it's released to Update to MPIR 2.7.0 when it's released
Otherwise we'd probably have to close it as won't fix. :-)
Current upstream tarball: http://mpir.org/mpir-2.7.0-alpha4.tar.bz2
Still needs to be tested on Darwin PowerPC I think. (moufang? Does it still live?)
And probably also Macs with more recent versions of Xcode.
comment:7 follow-ups: ↓ 8 ↓ 9 Changed 8 years ago by
So are you saying that JP should make a tarball for this as an alpha? I don't have time to test this on PPC until that exists. Currently trying to built 6.2.beta7, I'll wait until I get a clean build for that platform before I try new things. (Hopefully rpy is the only remaining problem there.)
comment:8 in reply to: ↑ 7 Changed 8 years ago by
Replying to kcrisman:
So are you saying that JP should make a tarball for this as an alpha?
I think that would be convenient for testing, and there are still some issues on Cygwin* JP knows about.
By the way, note that the latest changes in the alpha4 tarball aren't yet committed on github (it's still at alpha3).
comment:9 in reply to: ↑ 7 Changed 8 years ago by
Replying to kcrisman:
So are you saying that JP should make a tarball for this as an alpha? I don't have time to test this on PPC until that exists. Currently trying to built 6.2.beta7, I'll wait until I get a clean build for that platform before I try new things. (Hopefully rpy is the only remaining problem there.)
I happens I did and almost have an almost completely working out of the box cygwin64 branch. I just have to cherry-pick commits and push them to different branches on trac.
comment:10 Changed 8 years ago by
- Branch set to u/jpflori/ticket/15015
- Commit set to 5d7a2e99278f657b5d17f527d3d8f1c38e125a02
- Dependencies MPIR 2.6.1 upstream deleted
- Description modified (diff)
There are still issues with PPL... See https://groups.google.com/forum/?fromgroups=#!topic/mpir-devel/JFDaMAc0YpY
New commits:
a564118 | Update MPIR to 2.7.0.alpha4
|
5d7a2e9 | Remove unnecessary fix for MPIR.
|
comment:11 follow-up: ↓ 13 Changed 8 years ago by
A few comments/questions:
- Do we lose the Clang / XCode 5 patch? (I did have other
clang
patches elsewhere, also just preventingconfigure
to fail for no real reason.)
- Sun compiler patch (probably less important)?
- What is
patch ... || continue
supposed to achieve? (Theif [ $? -ne 0 ]
afterwards gets useless.)
- In
if uname -a | grep x86_64; then ...
any output (stdout
andstderr
) should get redirected to/dev/null
.
Should #16149 be added as a dependency of this ticket, or do you want to deal with that in MPIR's spkg-install
? (I'd prefer the former, but am also undecided about the solution to choose on #16149, hence "needs info".)
comment:12 follow-up: ↓ 14 Changed 8 years ago by
I recall we also had issues with AVX-enabled CPUs on Darwin and (not that) old Apple assemblers not supporting AVX instructions. IIRC Jeroen removed a work-around (with a bunch of other things dealing with proper CFLAGS
) from spkg-install
a while ago, so this is presumably no longer an issue (just because we meanwhile require some more recent version of Xcode I guess), but I might be wrong.
comment:13 in reply to: ↑ 11 ; follow-ups: ↓ 15 ↓ 19 Changed 8 years ago by
Replying to leif:
A few comments/questions:
All (Most?) should be upstream now if I did not screw up a long time ago.
- Do we lose the Clang / XCode 5 patch? (I did have other
clang
patches elsewhere, also just preventingconfigure
to fail for no real reason.)
Must admit I don't remember about that one.
- Sun compiler patch (probably less important)?
This one is in (IIRC).
- What is
patch ... || continue
supposed to achieve? (Theif [ $? -ne 0 ]
afterwards gets useless.)
When there is no patch at all I guess. Not sure naymore.
- In
if uname -a | grep x86_64; then ...
any output (stdout
andstderr
) should get redirected to/dev/null
.
Maybe!
Should #16149 be added as a dependency of this ticket, or do you want to deal with that in MPIR's
spkg-install
? (I'd prefer the former, but am also undecided about the solution to choose on #16149, hence "needs info".)
The CXXFLAGS issue should be fixed in MPIR (maybe not alpha4, but in git master).
comment:14 in reply to: ↑ 12 Changed 8 years ago by
Replying to leif:
I recall we also had issues with AVX-enabled CPUs on Darwin and (not that) old Apple assemblers not supporting AVX instructions. IIRC Jeroen removed a work-around (with a bunch of other things dealing with proper
CFLAGS
) fromspkg-install
a while ago, so this is presumably no longer an issue (just because we meanwhile require some more recent version of Xcode I guess), but I might be wrong.
I don't now. We surely added thing for apple ocmpiler/assembler/linker but couldn't promise it was AVX related.
comment:15 in reply to: ↑ 13 Changed 8 years ago by
Replying to jpflori:
Replying to leif:
Should #16149 be added as a dependency of this ticket, or do you want to deal with that in MPIR's
spkg-install
? (I'd prefer the former, but am also undecided about the solution to choose on #16149, hence "needs info".)The CXXFLAGS issue should be fixed in MPIR (maybe not alpha4, but in git master).
No, a slightly different one (but with the same effect). AC_PROG_CXX
in MPIR is now correctly called after CXXFLAGS
have been saved, and MPIR now behaves correctly (I'd say). But Sage sets (and exports) CXXFLAGS
to the empty string, so MPIR doesn't touch them (again assuming they were intentionally set), such that we end up with missing flags (unless the user had really set them, or CFLAGS
, to something meaningful).
comment:16 Changed 8 years ago by
I see. Thanks for the explanation.
comment:17 Changed 8 years ago by
P.S.: In spkg-install
, we could do (almost) the same with CXXFLAGS
as we do with CFLAGS
. In the first "dry" configure
run, we already unset them, so we could grep MPIR's from the Makefile and at least prepend those settings to our effective CXXFLAGS
for the second, real configure
run.
(So far, we didn't touch CXXFLAGS
at all, which I'd consider a small deficiency or inconsistency anyway.)
comment:18 Changed 8 years ago by
[Slight correction to my previous post.]
comment:19 in reply to: ↑ 13 Changed 8 years ago by
Replying to jpflori:
- What is
patch ... || continue
supposed to achieve? (Theif [ $? -ne 0 ]
afterwards gets useless.)When there is no patch at all I guess.
Ah, thought of something like that. But it should be
for foo in bar*; do [ -r "$foo" ] || continue # ignore non-readable / non-existent files process "$foo" if [ $? -ne 0 ]; then # processing $foo failed, err out ... fi done
comment:20 Changed 8 years ago by
Sure, I messed up :(
comment:21 Changed 8 years ago by
- Commit changed from 5d7a2e99278f657b5d17f527d3d8f1c38e125a02 to da483e1e109f71c93d22fa1ab115b12ede4fcaca
comment:22 Changed 8 years ago by
All patches from Sage seem to be integrated upstream, good! The changes here look fine, the main part of reviewing will be checking that it actually works. I will upgrade this to the latest MPIR alpha and report back.
comment:23 Changed 8 years ago by
- Description modified (diff)
comment:24 Changed 8 years ago by
I'm not sure the problem with ppl is fixed in the latest alpha... Some hints to fix it must lurk on flint-devel.
comment:25 Changed 8 years ago by
See https://groups.google.com/d/msg/mpir-devel/JFDaMAc0YpY/h774pq9wN2EJ And IIRC it does not only happens on Cygwin(64) but also on more usual systems.
comment:26 Changed 8 years ago by
- Branch changed from u/jpflori/ticket/15015 to u/jdemeyer/ticket/15015
- Created changed from 08/06/13 21:44:07 to 08/06/13 21:44:07
- Modified changed from 08/28/14 10:21:45 to 08/28/14 10:21:45
comment:27 Changed 8 years ago by
- Commit changed from da483e1e109f71c93d22fa1ab115b12ede4fcaca to 77251224787d5140af17a32e7cb3059effe22670
In spkg-install
, I also changed
echo "Checking what CFLAGS MPIR would use if they were empty..." (unset CFLAGS CPPFLAGS CXXFLAGS && ./configure $MPIR_CONFIGURE) &>configure-empty.log
to
echo "Configuring MPIR with empty CFLAGS to determine the defaults:" (unset CFLAGS CPPFLAGS CXXFLAGS && ./configure $MPIR_CONFIGURE)
I think it's better to display the first configure
run, in case it needs to be debugged or just to show the user that Sage isn't stuck.
New commits:
1607b8f | Merge remote-tracking branch 'origin/develop' into ticket/15015
|
7725122 | Cosmetic changes
|
comment:28 Changed 8 years ago by
On Linux x86_64, g++ (Gentoo 4.6.4 p1.2, pie-0.5.2) 4.6.4, while compiling the Sage library:
gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/usr/local/src/sage-config/local/include -I/usr/local/src/sage-config/local/include/csage -I/usr/local/src/sage-config/src -I/usr/local/src/sage-config/src/sage/ext -I/usr/local/src/sage-config/local/include/python2.7 -c build/cythonized/sage/libs/ppl.cpp -o build/temp.linux-x86_64-2.7/build/cythonized/sage/libs/ppl.o -fno-strict-aliasing -w In file included from build/cythonized/sage/libs/ppl.cpp:352:0: ../local/include/gmpxx.h:1548:3: error: ‘__gmp_expr<__mpz_struct [1], __mpz_struct [1]>::__gmp_expr(intmax_t)’ cannot be overloaded ../local/include/gmpxx.h:1539:3: error: with ‘__gmp_expr<__mpz_struct [1], __mpz_struct [1]>::__gmp_expr(long int)’ ../local/include/gmpxx.h:1552:3: error: ‘__gmp_expr<__mpz_struct [1], __mpz_struct [1]>::__gmp_expr(uintmax_t)’ cannot be overloaded ../local/include/gmpxx.h:1540:3: error: with ‘__gmp_expr<__mpz_struct [1], __mpz_struct [1]>::__gmp_expr(long unsigned int)’ ../local/include/gmpxx.h:1615:16: error: ‘__gmp_expr<__mpz_struct [1], __mpz_struct [1]>& __gmp_expr<__mpz_struct [1], __mpz_struct [1]>::operator=(intmax_t)’ cannot be overloaded ../local/include/gmpxx.h:1604:16: error: with ‘__gmp_expr<__mpz_struct [1], __mpz_struct [1]>& __gmp_expr<__mpz_struct [1], __mpz_struct [1]>::operator=(long int)’ ../local/include/gmpxx.h:1619:16: error: ‘__gmp_expr<__mpz_struct [1], __mpz_struct [1]>& __gmp_expr<__mpz_struct [1], __mpz_struct [1]>::operator=(uintmax_t)’ cannot be overloaded ../local/include/gmpxx.h:1606:16: error: with ‘__gmp_expr<__mpz_struct [1], __mpz_struct [1]>& __gmp_expr<__mpz_struct [1], __mpz_struct [1]>::operator=(long unsigned int)’ error: command 'gcc' failed with exit status 1
comment:29 Changed 8 years ago by
Yup that is the problem I was mentioning.
comment:30 Changed 8 years ago by
Could you try with the attached patch? If it works, I'll forward it to Bill.
Changed 8 years ago by
comment:31 Changed 8 years ago by
Groumpf, thepatch is reversed. Take the .2 one.
comment:32 Changed 8 years ago by
Strange I don't have problems, even without my latest patch, on a machine where I seem to remember I used to have problems.
comment:33 Changed 8 years ago by
Works for me...
comment:34 Changed 8 years ago by
In fact I also need the patch, but not to build PPL anymore, but some ppl related file within the Sage library.
comment:35 Changed 8 years ago by
- Description modified (diff)
comment:36 Changed 8 years ago by
- Commit changed from 77251224787d5140af17a32e7cb3059effe22670 to aca71cb72ef80895a6228efde9965367152dba4f
comment:37 Changed 8 years ago by
- Commit changed from aca71cb72ef80895a6228efde9965367152dba4f to 67babebcf82ac594043c634b8e63d4036c7818c6
Branch pushed to git repo; I updated commit sha1. New commits:
67babeb | Fix doctests due to changed xgcd results
|
comment:38 Changed 8 years ago by
This now builds and passes all doctests on 64-bit for me.
Test please!
comment:39 Changed 8 years ago by
- Cc was added
William: this changes one doctest in src/sage/tests/book_stein_modform.py
comment:40 Changed 8 years ago by
- Commit changed from 67babebcf82ac594043c634b8e63d4036c7818c6 to 8c7fbbd95d470a15e3e57b4e2573685bd726a7bd
Branch pushed to git repo; I updated commit sha1. New commits:
8c7fbbd | Re-enable "not tested" test from #4357
|
comment:41 Changed 8 years ago by
Unrelated to this ticket, I have re-enabled a doctest in book_stein_modform.py
which was fixed in #4357.
comment:42 Changed 8 years ago by
For info, I agree that we should output the first configure run on screen. Some users already complained that MPIR compilation was stuck because of the long time it can take on some systems and nothing was getting printed on screen.
comment:43 Changed 8 years ago by
Works fine for me (except for numerical noise I also get with the previous MPIR, and that is surely ppc64 specific).
Would you consider adding a spkg-src script?
comment:44 Changed 8 years ago by
- Reviewers set to Jean-Pierre Flori
- Summary changed from Update to MPIR 2.7.0 when it's released to Update to MPIR 2.7.0.alpha12
This works perfectly fine for me (though I did not test Cygwin, but that's a low level priority and can be fixed later in case it is needed).
The current tarball is the upstream one and includes windows stuff we used to delete. Apart from that, I would positively review the ticket, even without a proper spkg-src script.
comment:45 Changed 8 years ago by
- Summary changed from Update to MPIR 2.7.0.alpha12 to Update to MPIR 2.7.0 when it's released
Upstream has indicated that they consider the changed output of mpz_gcdext()
a bug.
Therefore, I think it is premature to upgrade now.
comment:46 Changed 8 years ago by
Oh yeah, I read about that one...
comment:47 Changed 8 years ago by
- Description modified (diff)
Because of the precedent at #17169, I would simply use the upstream tarball and not add a spkg-src
script.
comment:48 Changed 8 years ago by
I don't really agree... If we keep on like that Sage will grow huge. Hard disks are cheap but internet connections aren't always, e.g. I get a poor bandwith between my office and boxen. Let's discuss this on sage-devel.
comment:50 Changed 7 years ago by
The gcdext change in MPIR was to be expected (at least the gcdext(1,1)
example Jeroen found) and I personally don't think it's a bug.
I'm not sure what Bill will think though.
At the very least it makes MPIR consistent with GMP.
comment:51 Changed 7 years ago by
(And ok for not shipping any spkg-src
script.)
comment:52 Changed 7 years ago by
- Status changed from new to needs_info
comment:53 Changed 7 years ago by
The only thing really annoying me right now is the name of the tarball. When we update to another alpha or the final version it will be a pain, so we should still repackage the upstream tarball to change the dir name wihtin it (rather than renaming the tarball).
comment:54 Changed 7 years ago by
- Status changed from needs_info to needs_work
- Work issues set to fix tarball name issue
comment:55 Changed 7 years ago by
- Description modified (diff)
- Milestone changed from sage-pending to sage-6.5
- Status changed from needs_work to needs_review
- Work issues fix tarball name issue deleted
comment:56 Changed 7 years ago by
- Commit changed from 8c7fbbd95d470a15e3e57b4e2573685bd726a7bd to aefe0ebcbbeeae156ea4923eee35dd112dfa6d70
Branch pushed to git repo; I updated commit sha1. New commits:
aefe0eb | Upgrade to MPIR 2.7.0-alpha12
|
comment:57 Changed 7 years ago by
New commits:
aefe0eb | Upgrade to MPIR 2.7.0-alpha12
|
comment:58 Changed 7 years ago by
- Summary changed from Update to MPIR 2.7.0 when it's released to Update to MPIR 2.7.0-alpha12
comment:59 Changed 7 years ago by
- Commit changed from aefe0ebcbbeeae156ea4923eee35dd112dfa6d70 to d97c06604293f765fd0c8de6189d05cedcebea3c
comment:60 Changed 7 years ago by
- Status changed from needs_review to positive_review
Looks fine.
I had a vague souvenir we had trouble with dashes in version numbers but it does not seem to be the case anymore. Maybe we could also switch back to verbatim version numbers for Singular 4-0-1.
comment:61 Changed 7 years ago by
Got two files with failing doctests but cannot reproduce these failures:
src/sage/modular/modform/ambient.py
src/sage/graphs/genus.pyx
comment:62 Changed 7 years ago by
- Branch changed from u/jdemeyer/ticket/15015 to d97c06604293f765fd0c8de6189d05cedcebea3c
- Resolution set to fixed
- Status changed from positive_review to closed
This should surely be renamed to "Update to MPIR 2.6.1" when the official version get released (no idea when...).