Opened 5 years ago

Closed 4 years ago

#21083 closed enhancement (fixed)

Upgrade BRiAl and build it with C++11

Reported by: leif Owned by:
Priority: major Milestone: sage-8.1
Component: packages: standard Keywords: unordered_map hash_map segfault C++11
Cc: fbissey Merged in:
Authors: Jeroen Demeyer Reviewers: François Bissey
Report Upstream: Fixed upstream, in a later stable release. Work issues:
Branch: 2d18149 (Commits, GitHub, GitLab) Commit: 2d181499ce9f251d6b5eacda6745d54f6e466189
Dependencies: Stopgaps:

Status badges

Description (last modified by jdemeyer)

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

Fixed upstream: https://github.com/BRiAl/BRiAl/issues/11

Also the patch from https://github.com/BRiAl/BRiAl/pull/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

Change History (22)

comment:1 Changed 5 years ago by leif

  • Description modified (diff)

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

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

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

Replying to Snark:

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.

comment:4 Changed 4 years ago by jdemeyer

  • Description modified (diff)
  • Report Upstream changed from Reported upstream. Developers acknowledge bug. to Fixed upstream, in a later stable release.
  • Summary changed from Upgrade BRiAl and build it without '-std=c++98' when upstream issue #11 is fixed to Upgrade BRiAl and build it without '-std=c++98'

comment:5 Changed 4 years ago by jdemeyer

  • Milestone changed from sage-7.3 to sage-8.1
  • Type changed from task to enhancement

comment:6 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:7 Changed 4 years ago by jdemeyer

  • Branch set to u/jdemeyer/upgrade_brial_and_build_it_without___std_c__98_

comment:8 Changed 4 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Commit set to 82e4fcad1760c3d2a46007c3164d3eb4909ac46b
  • Status changed from new to needs_review

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


New commits:

82e4fcaUpgrade BRiAl to version 1.0.1

comment:9 Changed 4 years ago by jdemeyer

  • Summary changed from Upgrade BRiAl and build it without '-std=c++98' to Upgrade BRiAl and build it with C++11

comment:10 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:11 Changed 4 years ago by fbissey

  • Reviewers set to François Bissey
  • Status changed from needs_review to positive_review

Looks good to me.

comment:12 Changed 4 years ago by vbraun

  • Status changed from positive_review to needs_work

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

comment:13 Changed 4 years ago by fbissey

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.

comment:14 Changed 4 years ago by fbissey

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.

comment:15 follow-up: Changed 4 years ago by jdemeyer

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

comment:16 in reply to: ↑ 15 Changed 4 years ago by fbissey

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.

comment:17 Changed 4 years ago by jdemeyer

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.

comment:18 Changed 4 years ago by fbissey

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.

comment:19 Changed 4 years ago by git

  • Commit changed from 82e4fcad1760c3d2a46007c3164d3eb4909ac46b to 2d181499ce9f251d6b5eacda6745d54f6e466189

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

2d18149Disable test suite, which requires Boost

comment:20 Changed 4 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:21 Changed 4 years ago by fbissey

  • Status changed from needs_review to positive_review

Looks good.

comment:22 Changed 4 years ago by vbraun

  • Branch changed from u/jdemeyer/upgrade_brial_and_build_it_without___std_c__98_ to 2d181499ce9f251d6b5eacda6745d54f6e466189
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.