Skip to content

boost.py - upgrade to reflect name change of python 3 related lib files in Boost 1.67.0#1472

Merged
boegel merged 3 commits intoeasybuilders:developfrom
jhein32:boost-python-name-update
Aug 14, 2018
Merged

boost.py - upgrade to reflect name change of python 3 related lib files in Boost 1.67.0#1472
boegel merged 3 commits intoeasybuilders:developfrom
jhein32:boost-python-name-update

Conversation

@jhein32
Copy link
Copy Markdown
Contributor

@jhein32 jhein32 commented Aug 14, 2018

With this block, I could build Boost 1.68.0 for

foss/2018b, Python 3.7.0
intel/2018b, Python 2.7.15
intel/2018b, Python 3.7.0

configurations to follow as a PR in the easyconfigs

Comment thread easybuild/easyblocks/b/boost.py Outdated
if LooseVersion(self.version) >= LooseVersion("1.68.0"):
suffix = '%s%s' % (pymajorver, pyminorver)
else:
suffix = pymajorver
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jhein32 The logic is becoming a bit convoluted here, and after double checking I think the right way to update this results in cleaner code. The shift to using double digits for the name of the library with Boost Python bindings was made in Boost 1.67.0, for both Python 2.x & 3.x; see the Python section in https://www.boost.org/users/history/version_1_67_0.html:

The library name now includes the version suffix of the Python version used to compile it.
For example, a variant compiled with Python 2.7 will produce library names boost_python27
and boost_numpy27, etc.

So the logic should become:

if LooseVersion(self.version) >= LooseVersion("1.67.0"):
    suffix = '%s%s' % (pymajorver, pyminorver)
elif LooseVersion(pymajorver) >= LooseVersion('3.0'):
    suffix = pymajorver
else:
    suffix = ''

(this should have been done already in #1424, but there we overlooked that this change was supposed to be made for both Python versions)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assumed that was checked properly beforehand ...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't, and I suspect @rubendibattista missed it too...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was worried about backwards compatibility since I did not know if it was just for new stuff or also for some previous versions and I did not tried with Python 3. So I didn't want to mess up with things I did not test.

I think the proposed solution by @boegel looks good. :)

Sorry about the overlook.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rubendibattista @boegel

Didn't mean to upset and hope I didn't. When I saw the code I assumed there was a reason for it looking as it did. I submitted a revised version (currently testing).

@jhein32
Copy link
Copy Markdown
Contributor Author

jhein32 commented Aug 14, 2018

As a simple check, I did a force rebuild of Boost-1.68.0-intel-2018b-Python-3.7.0.eb with the revised EasyBlock, which succeeded.

@jhein32 jhein32 changed the title boost.py - upgrade to reflect name change of python 3 related lib files in Boost 1.68.0 boost.py - upgrade to reflect name change of python 3 related lib files in Boost 1.67.0 Aug 14, 2018
Copy link
Copy Markdown
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doing some more extensive testing myself to make sure, but looks fine, will merge when I'm done testing

@boegel
Copy link
Copy Markdown
Member

boegel commented Aug 14, 2018

tested with a couple of existing Boost easyconfigs + easybuilders/easybuild-easyconfigs#6700, lgtm

@boegel boegel merged commit ac089b8 into easybuilders:develop Aug 14, 2018
@jhein32
Copy link
Copy Markdown
Contributor Author

jhein32 commented Aug 14, 2018

you are closing #1465 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants