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

Add better autoconf support for libboost #394

Merged
merged 7 commits into from Oct 22, 2018

Conversation

Projects
None yet
4 participants
@dongahn
Copy link
Contributor

dongahn commented Oct 19, 2018

Add powerpc64le and armv7l platforms to ax_boost_base.m4
such that configure will search for the system library path where
boost RPMs install their boost libraries.

Add the actual boost library names in the config error messages
(ax_boost_filesystem.m4, ax_boost_system.m4
and ax_boost_graph.m4).

Add a new m4 script that checks whether the buggy libboost versions
as tested and documented at #297.

Resolve #302, #297 and #396.

@dongahn dongahn requested review from SteVwonder and garlick Oct 19, 2018

@dongahn

This comment has been minimized.

Copy link
Contributor Author

dongahn commented Oct 19, 2018

Travis fails with:

�[0K$ (mkdir -p ../depbuild && cd ../depbuild && bash ${depbuilder} --cachedir=$HOME/local/.cache)
�[31mInvalid requirement: ''cffi>=1.11''
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/pip/req/req_install.py", line 82, in __init__
    req = Requirement(req)
  File "/usr/local/lib/python2.7/dist-packages/pip/_vendor/packaging/requirements.py", line 96, in __init__
    requirement_string[e.loc:e.loc + 8]))
InvalidRequirement: Invalid requirement, parse error at ""'cffi>=1""
�[0m
/usr/local/lib/python2.7/dist-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:318: SNIMissingWarning: An HTTPS request has been made, but the SNI (Subject Name Indication) extension to TLS is not available on this platform. This may cause the server to present an incorrect TLS certificate, which can cause validation failures. You can upgrade to a newer version of Python to solve this. For more information, see 

Does PR #392 need to go in first?

@SteVwonder

This comment has been minimized.

Copy link
Member

SteVwonder commented Oct 19, 2018

Ouch. When I updated the dependencies in the (now old) dep builder, I probably broke something. Sorry! I agree that merging #392 should fix this.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Oct 19, 2018

There's a missing comma in the armv7l patch, but after I add that, configure seems to find all the boost bits!

checking for boostlib >= 1.53.0 (105300)... yes
checking for buggy boostlib versions (105400-105800)... no
checking whether the Boost::System library is available... yes
checking for exit in -lboost_system... yes
checking whether the Boost::Filesystem library is available... yes
checking for exit in -lboost_filesystem... yes
checking whether the Boost::Graph library is available... yes
checking for exit in -lboost_graph... yes
checking whether the Boost::Regex library is available... yes
checking for exit in -lboost_regex... yes
diff --git a/m4/ax_boost_base.m4 b/m4/ax_boost_base.m4
index f750fa4..c9749dc 100644
--- a/m4/ax_boost_base.m4
+++ b/m4/ax_boost_base.m4
@@ -114,7 +114,7 @@ AC_DEFUN([_AX_BOOST_BASE_RUNDETECT],[
     AS_CASE([${host_cpu}],
       [x86_64],[libsubdirs="lib64 libx32 lib lib64"],
       [ppc64|s390x|sparc64|aarch64|ppc64le|powerpc64le],[libsubdirs="lib64 lib lib64"],
-      [armv7l],[libsubdirs="lib/arm-linux-gnueabihf lib"]
+      [armv7l],[libsubdirs="lib/arm-linux-gnueabihf lib"],
       [libsubdirs="lib"]
     )
@dongahn

This comment has been minimized.

Copy link
Contributor Author

dongahn commented Oct 19, 2018

Great! I didn't have a test machine and that crept in. Sorry about that.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Oct 19, 2018

No worries!

@@ -62,7 +62,9 @@ AC_DEFINE_UNQUOTED([LIBLUA_SO], ["$LIBLUA_SONAME"],

PKG_CHECK_MODULES([UUID], [uuid], [], [])
AX_FLUX_CORE

AX_BOOST_BASE([1.53.0], [], [AC_MSG_ERROR([Please install boost >= 1.53)])])

This comment has been minimized.

@SteVwonder

SteVwonder Oct 19, 2018

Member

Can you update this error message to match the one in buggy boost? Please use Boost == 1.53 or > 1.58

@SteVwonder

This comment has been minimized.

Copy link
Member

SteVwonder commented Oct 19, 2018

Other than the one nit, LGTM. @dongahn, maybe you can take a look at #396 and see if the boost portion of that makes sense to include in this PR. If not, I'm happy to merge this PR without it.

The buggy boost check is a great addition, BTW!

@garlick
Copy link
Member

garlick left a comment

Sorry, should have given my comment in a review. This looks good, with that one caveat. Thanks!

@dongahn dongahn force-pushed the dongahn:boost_config branch 2 times, most recently from dba9e04 to abbf1a5 Oct 20, 2018

@dongahn dongahn force-pushed the dongahn:boost_config branch from abbf1a5 to 282b476 Oct 20, 2018

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Oct 20, 2018

Codecov Report

Merging #394 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #394   +/-   ##
=======================================
  Coverage   75.45%   75.45%           
=======================================
  Files          65       65           
  Lines       10512    10512           
=======================================
  Hits         7932     7932           
  Misses       2580     2580

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f8cf83...6176da0. Read the comment docs.

@dongahn dongahn force-pushed the dongahn:boost_config branch from 282b476 to 6176da0 Oct 20, 2018

@dongahn

This comment has been minimized.

Copy link
Contributor Author

dongahn commented Oct 20, 2018

@SteVwonder and @garlick: I addressed both of the issues you found. Also, as part of that, I addressed the issues captured in #396 as well. Thanks.

@SteVwonder: everything turned green and from my perspective this can go in.

@SteVwonder

This comment has been minimized.

Copy link
Member

SteVwonder commented Oct 22, 2018

Thanks @dongahn! Merging.

@SteVwonder SteVwonder merged commit 2a8d570 into flux-framework:master Oct 22, 2018

3 checks passed

codecov/patch Coverage not affected when comparing 8f8cf83...6176da0
Details
codecov/project 75.45% remains the same compared to 8f8cf83
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.