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

Fix compatibility with boost 1.67.0 #2193

Merged
merged 1 commit into from Jul 14, 2018

Conversation

Projects
4 participants
@AMDmi3
Copy link
Contributor

commented Jun 28, 2018

From cmake's FindBoost.cmake:

# Note that Boost Python components require a Python version suffix
# (Boost 1.67 and later), e.g. ``python36`` or ``python27`` for the
# versions built against Python 3.6 and 2.7, respectively.  This also
# applies to additional components using Python including
# ``mpi_python`` and ``numpy``.  Earlier Boost releases may use
# distribution-specific suffixes such as ``2``, ``3`` or ``2.7``.
# These may also be used as suffixes, but note that they are not
# portable.

Note that this can break compatibility with earlier versions of boost, and probably requires some conditions to be implemented properly. However, without some form of this patch the game won't build with recent boost which is already quite wide spread among the distributions:

Boost versions in different repositories

@Vezzra

This comment has been minimized.

Copy link
Member

commented Jun 29, 2018

Note that this can break compatibility with earlier versions of boost, and probably requires some conditions to be implemented properly.

Well, maybe I'm missing something, but regarding the conditions you mention here, wouldn't it be sufficient to just check if the boost version found is >= 1.67, and depending on that use either python and Boost_PYTHON_LIBRARY or python27 and Boost_PYTHON27_LIBRARY?

@Vezzra Vezzra added this to the v0.4.8 (optional) milestone Jun 29, 2018

@AMDmi3 AMDmi3 force-pushed the AMDmi3:boost_1_67_compat branch from 8535ddf to 6185687 Jun 29, 2018

@AMDmi3

This comment has been minimized.

Copy link
Contributor Author

commented Jun 29, 2018

Well, maybe I'm missing something, but regarding the conditions you mention here, wouldn't it be sufficient to just check if the boost version found is >= 1.67, and depending on that use either python and Boost_PYTHON_LIBRARY or python27 and Boost_PYTHON27_LIBRARY?

Yes, that should work. Actually, one can just set Boost_PYTHON_LIBRARY to ${Boost_PYTHON27_LIBRARY} for >= 1.67, however find_package is a bit more tricky as we don't know whether we want to pass python or python27 to it before we call it, so it has to be split into two. See the updated patch - it works for me and should be backwards compatible.

@geoffthemedio geoffthemedio requested a review from adrianbroher Jun 29, 2018

@Vezzra
Copy link
Member

left a comment

See the updated patch - it works for me and should be backwards compatible.

Looks fine to me, but still, @geoffthemedio is right, want to know what @adrianbroher has to say.

@adrianbroher

This comment has been minimized.

Copy link
Contributor

commented Jul 14, 2018

No complaints, should go in as-is. Also this should be cherry-picked to 0.4.8.

@adrianbroher adrianbroher merged commit fc8a7ae into freeorion:master Jul 14, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@AMDmi3

This comment has been minimized.

Copy link
Contributor Author

commented Jul 15, 2018

Thanks!

@Vezzra Vezzra added this to Proposed in 0.4.8 Release Jul 17, 2018

@Vezzra Vezzra moved this from Proposed to Accepted in 0.4.8 Release Jul 17, 2018

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.