Opened 4 years ago

Closed 4 years ago

#19009 closed enhancement (fixed)

upgrade flint to 2.5.2

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-6.9
Component: packages: standard Keywords:
Cc: jpflori, wbhart Merged in:
Authors: Vincent Delecroix Reviewers: François Bissey
Report Upstream: Reported upstream. Developers acknowledge bug. Work issues:
Branch: 88fe908 (Commits) Commit: 88fe9081e745cc06158b537989221d4b9e9cdafb
Dependencies: Stopgaps:

Description (last modified by fbissey)

flint 2.5.2 is out. Upstream tarball is here

http://flintlib.org/flint-2.5.2.tar.gz

All interesting patches integrated upstream.

Attachments (3)

flint-2.5.log (194.2 KB) - added by vdelecroix 4 years ago.
Vincent build failed on the 10th of August
singular-3.1.7p1.p0.log (56.6 KB) - added by vdelecroix 4 years ago.
Singular failed build on 10th of August
eclib-20150510.log (33.3 KB) - added by vdelecroix 4 years ago.
Failed eclib build on 10th of August

Download all attachments as: .zip

Change History (54)

comment:1 Changed 4 years ago by vdelecroix

The three patches that were written for flint-2.4.5 seem not needed anymore (two modifications are included and I was not sure how to handle the remaining one flint-2.4.5-test.patch). Removing them and launching sage -f flint I ended up with a failure related to NTL. I do not think that I can not help much more right now.

comment:2 follow-up: Changed 4 years ago by jpflori

Can you post the failure log here?

Changed 4 years ago by vdelecroix

Vincent build failed on the 10th of August

comment:3 in reply to: ↑ 2 Changed 4 years ago by vdelecroix

  • Branch set to public/19009
  • Commit set to 75a92830e64d18993909974332f89a980ad6fcac

Replying to jpflori:

Can you post the failure log here?

Done. I also give the branch I started from in case.


New commits:

75a9283Trac 19009: preparation for flint-2.5

comment:4 Changed 4 years ago by vdelecroix

Strange

2631	make[2]: Leaving directory '/opt/sage/local/var/tmp/sage/build/flint-2.5/src'
2632	/bin/sh: 2: ldconfig: not found
2633	Makefile:141: recipe for target 'libflint.so.13.5.0' failed
2634	make[1]: *** [libflint.so.13.5.0] Error 127

What can be this story with ldconfig not being found?

comment:5 Changed 4 years ago by vdelecroix

By replacing ldconfig with /sbin/ldconfig in Makefile.in, I was able to build flint... I am waiting for the rest.

There was a patch about changing a .o to a .lo. Should I keep it?

Changed 4 years ago by vdelecroix

Singular failed build on 10th of August

Changed 4 years ago by vdelecroix

Failed eclib build on 10th of August

comment:6 Changed 4 years ago by git

  • Commit changed from 75a92830e64d18993909974332f89a980ad6fcac to 5af42745bf78d3e9b7c39317a4fc85bd9efea8b1

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

5e3a1faTrac 19009: flint patch
5af4274Trac 19009: Fix eclib and singular

comment:7 Changed 4 years ago by vdelecroix

There seems to be a bug around nmod_mat_cols vs nmod_mat_ncols that prevents to build both singular and eclib. A simple substitution does the trick (see commit 5af4274). I posted a message on flint-devel but as I just registered my message is moderated.

With the current branch I was able to build the whole Sage from scratch.

Last edited 4 years ago by vdelecroix (previous) (diff)

comment:8 Changed 4 years ago by vdelecroix

  • Description modified (diff)
  • Report Upstream changed from N/A to Reported upstream. Developers acknowledge bug.

comment:9 Changed 4 years ago by vdelecroix

  • Description modified (diff)

comment:10 Changed 4 years ago by git

  • Commit changed from 5af42745bf78d3e9b7c39317a4fc85bd9efea8b1 to 09ce2b161d38dd2a1e10b041027f61d32757d790

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

b090371flint-2.5 patch 1: change NTL-interface.o to NTL-interface.lo
0b6d8a2flint-2.5 patch 2: fix ldconfig detection
09ce2b1flint-2.5 patch 3: fix nmod_mat_cols

comment:11 Changed 4 years ago by vdelecroix

  • Authors set to Vincent Delecroix
  • Status changed from new to needs_review

Ready to be tested!

comment:12 Changed 4 years ago by fbissey

Have you run the test suite Vincent? I got rather interesting stuff while working on the Gentoo ebuild

  • the old test patch shouldn't be needed anymore (I originally wrote that patch) because the .o is explicitly requested to be build for the test (quick clever actually, wish I had thought of that).

Something that I will have to take upstream but from which the spkg should be safe. If a previous libflint is already installed the test suite will fail to link because the old one may miss some symbols that are tested and it will be the one found and linked again first. It shouldn't affect sagemath because spkg-install explicitly remove the old flint library before install.

Last edited 4 years ago by fbissey (previous) (diff)

comment:13 Changed 4 years ago by fbissey

The problem is quite linux specific. The FLINT_LIB target creates libflint.so.x.y.z and link it to libflint.so.x but doesn't create libflint.so. That last one is only created on installation. So if you configure flint to only build shared libraries on linux and run make check you don't have a usable libflint.so in your build directory. If you have one in a -Lpath or global location it will be used. If not you'll go for a straight failure.

comment:14 Changed 4 years ago by git

  • Commit changed from 09ce2b161d38dd2a1e10b041027f61d32757d790 to 38a9c00a6fce00ad77f6c5a35bc76a125659fa99

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

8024e13flint-2.5 patch 1: fix ldconfig detection
d9f95f2flint-2.5 patch 2: fix nmod_mat_cols
38a9c00Add description of patches to SPKG.txt

comment:15 Changed 4 years ago by vdelecroix

All flint test passed without the first patch (I ran sage -f -c flint). Thanks!

I also added some description of the two remaining patches in SPKG.txt. According to Bill Hart, they will be in flint-2.5.1.

comment:16 follow-up: Changed 4 years ago by fbissey

  • Reviewers set to François Bissey

I think you did a very good job spotting those issues, although I am not sure ldconfig should be used but that is a matter of taste. I think this is in good shape to go. Send it to the bots!

comment:17 in reply to: ↑ 16 Changed 4 years ago by vdelecroix

Sadly, the bots will not run it because it is about a package... (e.g. they would have to guess where to download the package)

Last edited 4 years ago by vdelecroix (previous) (diff)

comment:18 Changed 4 years ago by fbissey

Well I guess you could at least make the location of the tarball more obvious in the description. It took me a few seconds to work it out.

comment:19 Changed 4 years ago by vdelecroix

  • Description modified (diff)

Sorry for that.

comment:20 follow-up: Changed 4 years ago by fbissey

  • Status changed from needs_review to needs_work

Do not work on OS X. Your ldconfig fix is not appropriate for OS X and other platform lacking ldconfig. I will be in touch with upstream about it.

comment:21 in reply to: ↑ 20 Changed 4 years ago by vdelecroix

Replying to fbissey:

Do not work on OS X. Your ldconfig fix is not appropriate for OS X and other platform lacking ldconfig. I will be in touch with upstream about it.

Bill did something for Cygwin/Mingw (setting LDCONFIG=/dev/null). But not OS X. The last state of configure in the trunk branch shows

# sometimes LDCONFIG is not to be found in the path. Look at some common places.
case "$OS" in
   MINGW*|CYGWIN*)
      LDCONFIG="/dev/null";;
   *)
      if [ -z "$LDCONFIG" ]; then
         if command -v ldconfig > /dev/null; then
            LDCONFIG="ldconfig"
         elif [ -x /sbin/ldconfig ]; then
            LDCONFIG="/sbin/ldconfig"
         else
            echo "Warning: ldconfig was not found. You can specify a path on the command line with LDCONFIG=<name>";
            exit 1
         fi
      fi;;
esac

comment:22 Changed 4 years ago by fbissey

I hadn't noticed that. That's the obvious spot to add something for OS X. Thanks for pointing it out.

comment:23 Changed 4 years ago by vdelecroix

comment:24 Changed 4 years ago by fbissey

I see, I have added stuff to the ongoing conversation I have with Bill in https://github.com/wbhart/flint2/issues/180 I guess if it doesn't pick it up now I should make a further pull request for him to find.

comment:25 Changed 4 years ago by fbissey

Bill has merged my OS X stuff and I compiled/tested trunk inside of sage 6.8 (but not installed in place or rebuild stuff, including sage, against it). It looks like we will have a 2.5.1 release shortly with all those fixes inside. I suggest we move to that.

comment:26 Changed 4 years ago by vdelecroix

  • Description modified (diff)
  • Summary changed from upgrade flint to 2.5 to upgrade flint to 2.5.1

comment:27 Changed 4 years ago by vdelecroix

  • Branch changed from public/19009 to u/vdelecroix/19009
  • Commit changed from 38a9c00a6fce00ad77f6c5a35bc76a125659fa99 to 14480381065f4490dfcd477d6abb680852a8361d
  • Status changed from needs_work to needs_review

Back to needs review with flint-2.5.1!


New commits:

1448038Trac 19009: upgrade flint to 2.5.1

comment:28 follow-up: Changed 4 years ago by wbhart

I'm afraid there is going to be a 2.5.2. This soname versioning is causing problems everywhere, even with the fixes.

There's also a bug in gmpcompat.h (fixed in trunk).

I hope it won't take long.

Unfortunately libtool is broken on msys2, so we can't even use that.

And some systems don't make /dev/null executable. What a mess this soname business is!

comment:29 in reply to: ↑ 28 Changed 4 years ago by vdelecroix

  • Description modified (diff)
  • Summary changed from upgrade flint to 2.5.1 to upgrade flint to 2.5.2

Replying to wbhart:

I'm afraid there is going to be a 2.5.2. This soname versioning is causing problems everywhere, even with the fixes.

We will wait for that... there is no hurry on the Sage side.

comment:30 Changed 4 years ago by vdelecroix

  • Status changed from needs_review to needs_work

comment:31 Changed 4 years ago by wbhart

Can someone verify that "true" is available on OSX as an alternative to /dev/null and that it ignores arguments, i.e.

true blah 1 2 3 "hello"

does nothing. I want to use this instead of /dev/null in lieu of LDCONFIG, since more and more systems are making /dev/null not executable.

comment:32 Changed 4 years ago by fbissey

Mirage:~ fbissey$ uname -a
Darwin Mirage.local 14.4.0 Darwin Kernel Version 14.4.0: Thu May 28 11:35:04 PDT 2015; root:xnu-2782.30.5~1/RELEASE_X86_64 x86_64
Mirage:~ fbissey$ which true
/usr/bin/true
Mirage:~ fbissey$ true blah 1 2 3 "hello"
Mirage:~ fbissey$ 

More seriously I am not sure where your problem is. On OS X when I found out that LDCONFIG needed a value I put a. Nothing will try to execute or run the content of LDCONFIG in configure if it is passed as an argument and putting /dev/null where you did is just as good. Once LDCONFIG is defined the only place it can be executed is in the Makefile and then it only happens on system with FLINT_SOLIB which should only be true on linux. So not executed on any other platform. Actually if you want to clean that up, only care about LDCONFIG if FLINT_SOLIB is defined in configure.

comment:33 Changed 4 years ago by wbhart

Sorry, I'm not sure I understand what you mean. LDCONFIG is intended to be executed by the Makefile. I don't see where I said it was to be executed by configure. Of course I meant to use true in lieu of ldconfig, not LDCONFIG.

/dev/null doesn't work on some systems (the build fails) because permission to execute /dev/null is denied. Unfortunately, such systems don't set a meaningful uname and so we can't even detect them. true seems to be more portable than /dev/null, at least until someone decides it is a security threat and stops people executing it, as happened with /dev/null.

I will try to clean up when LDCONFIG is set to something other than true. I think having it bomb out with an error is not the right strategy. Even on systems that set FLINT_SOLIB, if they don't have ldconfig it is most likely because they don't need it, in which case configure shouldn't just exit.

comment:34 Changed 4 years ago by fbissey

Have you thought about using - in front of $LDCONFIG https://www.gnu.org/software/make/manual/html_node/Errors.html#Errors ? That would help it not to bomb out I think.

Last edited 4 years ago by fbissey (previous) (diff)

comment:35 follow-up: Changed 4 years ago by wbhart

I don't know why, but putting a - in from of $LDCONFIG didn't actually work. It just said,

/bin/sh: 2: -ldconfig: not found

So much for removing the minus before passing it to the shell!

Edit: ok it works if I put it earlier. It obviously considers the whole block a "line".

Last edited 4 years ago by wbhart (previous) (diff)

comment:36 Changed 4 years ago by git

  • Commit changed from 14480381065f4490dfcd477d6abb680852a8361d to e30b2ab649045654edb4ff079b69ae54c957ce12

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

e30b2abTrac 19009: upgrade flint to 2.5.2

comment:37 Changed 4 years ago by vdelecroix

  • Description modified (diff)
  • Status changed from needs_work to needs_review

comment:38 Changed 4 years ago by wbhart

  • Description modified (diff)

I have now put up flint-2.5.2. I've tested it on Ubuntu and Msys2 and Cygwin to ensure we didn't break anything there. I'd appreciate testing on OSX to verify that all is fine there.

comment:39 Changed 4 years ago by vdelecroix

  • Description modified (diff)

comment:40 Changed 4 years ago by wbhart

By the way, in case it is relevant, the BSD's use different soname policies than Linux, Cygwin, Msys2 or OSX. So flint is almost certainly broken on BSD. But I recently lost access to the last BSD machine I could use. So there's not much I can do for BSD. No doubt Solaris/SunOS is different again.

comment:41 Changed 4 years ago by git

  • Commit changed from e30b2ab649045654edb4ff079b69ae54c957ce12 to 70b510791d8898766a79d0ae7171e38e40537467

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

70b5107Trac 19009: upgrade flint to 2.5.2

comment:42 Changed 4 years ago by vdelecroix

I just installed FreeBSD in a virtual machine. Though I was not able to run the Makefile. I got plenty of errors

make: "/usr/home/vincent/flint2/Makefile" line 35: Need an operator
make: "/usr/home/vincent/flint2/Makefile" line 37: Need an operator
make: "/usr/home/vincent/flint2/Makefile" line 39: Need an operator
make: "/usr/home/vincent/flint2/Makefile" line 63: Need an operator
make: "/usr/home/vincent/flint2/Makefile" line 122: Need an operator
make: "/usr/home/vincent/flint2/Makefile" line 126: Need an operator
make: "/usr/home/vincent/flint2/Makefile" line 128: Need an operator
make: "/usr/home/vincent/flint2/Makefile" line 192: Need an operator
make: "/usr/home/vincent/flint2/Makefile" line 198: Need an operator
make: "/usr/home/vincent/flint2/Makefile" line 200: Need an operator
make: "/usr/home/vincent/flint2/Makefile" line 201: duplicate script for target "define" ignored
...

Looks like the BSD make is not exactly behaving like the linux one.

comment:43 Changed 4 years ago by fbissey

Yes. You should use GNU make rather than BSD make. A common problem not just on BSD. Usually it is found as gmake on those platforms.

comment:44 Changed 4 years ago by wbhart

If you can tell me what uname and uname -m return on that particular VM I can at least hack in my best guess (in the next release -- I'm not fixing to issue another patch to 2.5).

And yeah, you'll need a bunch of gnu tools to get anywhere on BSD. gmake and probably others.

comment:45 follow-up: Changed 4 years ago by jhpalmieri

On OS X, it passes its test suite and all of Sage's tests pass. (The checksums didn't match, though. I had to fix those first.)

comment:46 in reply to: ↑ 45 Changed 4 years ago by vdelecroix

Replying to jhpalmieri:

On OS X, it passes its test suite and all of Sage's tests pass. (The checksums didn't match, though. I had to fix those first.)

Where you at commit 70b5107? The sha1sum of the flint-2.5.2.tar.gz that I downloaded started with 04534a2b.

Last edited 4 years ago by vdelecroix (previous) (diff)

comment:47 Changed 4 years ago by wbhart

I sneaked an extra commit into the flint-2.5.2 tarball just a couple of minutes after I announced 2.5.2. It's just an extra line in the NEWS file.

I would rewrite history, but my time machine is currently stuck in subspace and I have a plasma leak on deck 7.

comment:48 Changed 4 years ago by git

  • Commit changed from 70b510791d8898766a79d0ae7171e38e40537467 to 88fe9081e745cc06158b537989221d4b9e9cdafb

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

88fe908Trac 19009: upgrade flint to 2.5.2

comment:49 Changed 4 years ago by vdelecroix

With the good checksum.

comment:50 in reply to: ↑ 35 Changed 4 years ago by fbissey

  • Description modified (diff)
  • Status changed from needs_review to positive_review

Replying to wbhart:

I don't know why, but putting a - in from of $LDCONFIG didn't actually work. It just said,

/bin/sh: 2: -ldconfig: not found

So much for removing the minus before passing it to the shell!

Edit: ok it works if I put it earlier. It obviously considers the whole block a "line".

Looking back at it, yes of course not just the ldconfig call. I am not even sure I want to call it a block when you use a continuation line.

I am putting this back to positive review.

comment:51 Changed 4 years ago by vbraun

  • Branch changed from u/vdelecroix/19009 to 88fe9081e745cc06158b537989221d4b9e9cdafb
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.