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 BRiAl and build it with C++11 #21083

Closed
nexttime mannequin opened this issue Jul 24, 2016 · 24 comments
Closed

Upgrade BRiAl and build it with C++11 #21083

nexttime mannequin opened this issue Jul 24, 2016 · 24 comments

Comments

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jul 24, 2016

We currently work around BRiAl 0.8.5 not building in C++11 mode by passing -std=c++98, which is odd.

Fixed upstream: BRiAl/BRiAl#11

Also the patch from BRiAl/BRiAl#15 is added, which is needed for #23943.

Tarball: https://github.com/BRiAl/BRiAl/releases/download/1.0.1/brial-1.0.1.tar.bz2

Upstream: Fixed upstream, in a later stable release.

CC: @kiwifb

Component: packages: standard

Keywords: unordered_map hash_map segfault C++11

Author: Jeroen Demeyer

Branch/Commit: 2d18149

Reviewer: François Bissey

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

@nexttime nexttime mannequin added this to the sage-7.3 milestone Jul 24, 2016
@nexttime

This comment has been minimized.

@SnarkBoojum
Copy link
Mannequin

SnarkBoojum mannequin commented Aug 19, 2017

comment:2

It looks like upstream closed issue 11, and in fact version 1.0.1 is out.

@kiwifb
Copy link
Member

kiwifb commented Aug 19, 2017

comment:3

Replying to @SnarkBoojum:

It looks like upstream closed issue 11, and in fact version 1.0.1 is out.

Yes upstream is me for all that's worth [in a maintenance capacity only) and I meant to do this. Anyway those issues were technically addressed in 0.8.7, brial should compile with C++11. The only issue is that you need at least C++98. If we decide to default it to C++11 we must make sure that the sage extension is also compiled with C++11.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

jdemeyer commented Oct 2, 2017

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

@jdemeyer jdemeyer changed the title Upgrade BRiAl and build it without '-std=c++98' when upstream issue #11 is fixed Upgrade BRiAl and build it without '-std=c++98' Oct 2, 2017
@jdemeyer jdemeyer modified the milestones: sage-7.3, sage-8.1 Oct 2, 2017
@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

jdemeyer commented Oct 2, 2017

@jdemeyer
Copy link

jdemeyer commented Oct 2, 2017

Author: Jeroen Demeyer

@jdemeyer
Copy link

jdemeyer commented Oct 2, 2017

Commit: 82e4fca

@jdemeyer
Copy link

jdemeyer commented Oct 2, 2017

comment:8

Thanks to the changes to upstream, this is pretty straight-forward now.


New commits:

82e4fcaUpgrade BRiAl to version 1.0.1

@jdemeyer jdemeyer changed the title Upgrade BRiAl and build it without '-std=c++98' Upgrade BRiAl and build it with C++11 Oct 2, 2017
@jdemeyer

This comment has been minimized.

@kiwifb
Copy link
Member

kiwifb commented Oct 9, 2017

Reviewer: François Bissey

@kiwifb
Copy link
Member

kiwifb commented Oct 9, 2017

comment:11

Looks good to me.

@vbraun
Copy link
Member

vbraun commented Oct 15, 2017

comment:12

Fails without boost:

[brial-1.0.1] checking whether the Boost::Unit_Test_Framework library is available... yes
[brial-1.0.1] configure: error: Could not find a version of the library!
[brial-1.0.1] Error configuring BRiAl

@kiwifb
Copy link
Member

kiwifb commented Oct 15, 2017

comment:13

Right, my fault upstream when I resurrected the testsuite. I'll need to just mark the testsuite disabled if not found rather than it being mandatory.

@kiwifb
Copy link
Member

kiwifb commented Oct 16, 2017

comment:14

Actually we should be able to fix this by just adding --with-boost-unit-test-framework=no, it default to yes and assumes that it means mandatory.

@jdemeyer
Copy link

comment:15

So you mean add --with-boost-unit-test-framework=no as ./configure flag and then run the testsuite as usual?

@kiwifb
Copy link
Member

kiwifb commented Oct 17, 2017

comment:16

Replying to @jdemeyer:

So you mean add --with-boost-unit-test-framework=no as ./configure flag

Yes

and then run the testsuite as usual?

No - or yes if you consider there is currently no spkg-check for brial. You need the boost-unit-test-framework library to link the testsuite (libboost_unit_test_framework.so). And boost_cropped is just headers, no libraries so we cannot run the testsuite without the full boost package.

@jdemeyer
Copy link

comment:17

I think I understand. The BRiAl build in Sage fails because it doesn't find the boost unit testing framework. However, since Sage doesn't run the testsuite, it doesn't actually need boost at all.

@kiwifb
Copy link
Member

kiwifb commented Oct 17, 2017

comment:18

It still needs boost headers (boost_cropped), they are used in brial in several places. But the test suite relies on something that is only built with the full boost.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 18, 2017

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

2d18149Disable test suite, which requires Boost

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 18, 2017

Changed commit from 82e4fca to 2d18149

@kiwifb
Copy link
Member

kiwifb commented Oct 18, 2017

comment:21

Looks good.

@vbraun
Copy link
Member

vbraun commented Oct 20, 2017

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

3 participants