update custom easyblock for Boost to always build single and multi threaded versions#2456
Conversation
|
Still not warning about ignoring boost_multi_thread... Not sure it's really needed... |
|
This of course need more testing... |
| self.log.info("Building boost_mpi library") | ||
| self.build_boost_variant(bjamoptions + " --with-mpi", paracmd) | ||
| mpi_bjamoptions = " --with-mpi" | ||
| self.build_boost_variant(self.bjamoptions + mpi_bjamoptions, self.paracmd) |
There was a problem hiding this comment.
The goal should be to do a single invocation of b2 with everything. To use mpi you can add using mpi ; to the user-config.jam file. I'll double check if that is the same
There was a problem hiding this comment.
Yeah, didn't get that to work with my first tries, so I decided to PR it as it is to start with...
There was a problem hiding this comment.
I'm not so sure about that. Think Boost.MPI the same way we have Boost.Python, then we would in reality only want it to build the mpi parts, i.e. using --with-mpi, but it would then be the only build/install step it does, just like for the python part.
There was a problem hiding this comment.
Yes, but that is not what it currently is: boost_mpi = True in the EC additionally installs Boost.MPI and we can't change that until EB 5.x
However it makes sense providing a (new) knob to build only Boost.MPI (same as Boost.Python) because we moved Boost to GCCcore, i.e. no MPI there.
We might even have boost_mpi = None being the default and the EasyBlock decides whether to build with or without MPI based on the currently loaded modules/dependencies.
There was a problem hiding this comment.
We could simply check if name = Boost.MPI do --with-mpi and only that (and verify that usempi = True first)
Else if either boost_mpi=True (requires usempi=True) or just usempi = True then add "using mpi" to config and build everything, perhaps verifying that we have MPI in the toolchain in all cases.
usempi=True/False should be the determining factor for MPI since that's what is mostly used in other easyblocks and for simpler easyconfigs.
There was a problem hiding this comment.
Nah, I'd prefer not to change this any more, it does the "same" thing as the old one, i.e. multiple b2 invocations when boost_mpi=True, producing all the libs.
It's better to deal with this MPI stuff in a separate PR.
I.e. my Boost-1.74.0-gompi-2020b.eb which has boost_mpi=True produces all the libs of standard boost + the mpi ones and creates the correct links when using this boost.py.
There was a problem hiding this comment.
Hm, but multiple invocations of b2 was the source of the problem that lead to this. So IMO we should fix that together. I can offer to tackle the remaining issues on top of this. But yeah, maybe in a separate PR, I guess doesn't matter much
There was a problem hiding this comment.
No multiple invocations was not really the problem, it was doing it with layout=system once and layout=tagged once. Now I always do layout=tagged so there is no problem with multiple invocations.
There was a problem hiding this comment.
I mean: if we do a single invocation, then there is no room for such mistakes at all.
There was a problem hiding this comment.
I agree that this can be changed in a follow up PR, let's get this one merged as is to avoid dragging it along...
| lib_mt_suffix += '-x64' | ||
|
|
||
| shlib_ext = get_shared_lib_ext() | ||
| lib_glob = 'lib*%s.%s.%s' % (lib_mt_suffix, shlib_ext, self.version) |
There was a problem hiding this comment.
This replace and glob stuff here gives me headaches. Not sure if all is correct.
Currently we have the following:
- libboost_type_erasure.a
- libboost_type_erasure-mt-x64.a
- libboost_type_erasure-mt-x64.so
- libboost_type_erasure-mt-x64.so.1.74.0
- libboost_type_erasure.so
- libboost_type_erasure.so.1.74.0
So I would say the glob should be lib*.* and the replacement simply: "use current name but remove everything from first '-' until first dot".
This removes the need for the lib_mt_suffix and the arch stuff above and makes it way simpler to understand: "Remove the tag"
There was a problem hiding this comment.
When doing threading=single,multi --layout=tagged you get:
libboost_type_erasure-mt-x64.a
libboost_type_erasure-mt-x64.so@
libboost_type_erasure-mt-x64.so.1.74.0*
libboost_type_erasure-x64.a
libboost_type_erasure-x64.so@
libboost_type_erasure-x64.so.1.74.0*
and nothing else, and that's what the glob/replace deals with.
There was a problem hiding this comment.
and nothing else
The "currently" was referring to current EB installations of Boost.
Ah, forgot the single-threaded. Then glob for lib*-mt*.* instead. Then 2 comments like "Glob for the mt libs" and "remove the tags from the name" would be enough and the code much easier to understand. Maybe even include those names of yours as examples in a comment
There was a problem hiding this comment.
yeah, testing...
There was a problem hiding this comment.
Better now I hope...
Simplyfy lib globbing and replacement for default libs Co-authored-by: Alexander Grund <[email protected]>
… by Alexander Grund <[email protected]>
|
There seem to be problems with older Boost versions that need to be looked at a bit more... |
|
@Flamefire Boost 1.71 (at least) doesn't protect itself correctly in the multiple-variants case: Note that, for 1.71, it's just a warning so the cmake correctly finds all components. |
Yes but the wrong ones: It uses the single threaded ones which would be a breaking change and likely wrong.
We need the tagged layout or it would be a breaking change in EB. However I think we should make the single-threaded build opt-in and clearly warn for known bad versions. Could you attach the 2 files ( Edit: Ok, found it: The multi-thread default was introduced in 1.73: boostorg/boost_install@c7a4270 |
|
By single build above I mean dropping threading=xxx and -layout=... which would produce the same libraries as we used to have, currently checking which version starts supplying cmake files and the -x86 suffix repsectively. |
|
The threading=multi --layout=tagged option was added to boost.py in commit e2e4de1 back in 2017. But 1.74.0 is the first easyconfig which actually turned on boost_multi_thread. |
|
Ah you are right, we always build with system layout and without threading set anyway. Given that the
--> New options |
|
Note that 1.70.0 is the first version with cmake files included. |
…o only add default links when appropriate.
|
@Flamefire @boegel How does this look? |
|
Btw, unless I made a mistake in testing, this produces the exact same result as before for everything up to 1.68.0, from 1.69 it produces an identical result but with tagged layout by default. Still have to finish my changes changes to cmakemake.py to make the last step to complete all of this. |
|
Any update on this one? |
|
LGTM, started a couple tests |
|
Test report by @Flamefire Overview of tested easyconfigs (in order)
Build succeeded for 6 out of 6 (6 easyconfigs in total) |
|
Test report by @SebastianAchilles Overview of tested easyconfigs (in order)
Build succeeded for 4 out of 4 (4 easyconfigs in total) |
|
Test report by @SebastianAchilles Overview of tested easyconfigs (in order)
Build succeeded for 4 out of 4 (4 easyconfigs in total) |
|
|
||
| # boost_multi_thread is deprecated | ||
| if self.cfg['boost_multi_thread'] is not None: | ||
| self.log.deprecated("boost_multi_thread has been deprecated, it has been replaced by tagged_layout. " |
There was a problem hiding this comment.
@akesandgren The easyconfigs for Boost 1.74.0 should be updated accordingly, since they set boost_multi_thread = True.
Couldn't find a PR for that, did I overlook it?
There was a problem hiding this comment.
No I didn't make one yet. Didn't know if we wanted to change that already or not.
Co-authored-by: Alexander Grund <[email protected]>
|
Test report by @boegel Overview of tested easyconfigs (in order)
Build succeeded for 2 out of 2 (2 easyconfigs in total) |
|
Test report by @boegel Overview of tested easyconfigs (in order)
Build succeeded for 13 out of 13 (13 easyconfigs in total) |
…gren/easybuild-easyblocks into 20210602164441_new_pr_jgYrgPRJSF
|
Test report by @boegel Overview of tested easyconfigs (in order)
Build succeeded for 2 out of 2 (2 easyconfigs in total) |
Followup to easybuilders#2456 This now saves some work by building Boost.MPI with the main build and installing directly into the final folder without an intermediate copy step Also a bit of cleanup, fixing comments, ...
Followup to easybuilders#2456 This now saves some work by building Boost.MPI with the main build and installing directly into the final folder without an intermediate copy step Also a bit of cleanup, fixing comments, ...
Followup to easybuilders#2456 This now saves some work by building Boost.MPI with the main build and installing directly into the final folder without an intermediate copy step Also a bit of cleanup, fixing comments, ...
(created using
eb --new-pr)Fixes Boost end of old problem with using CMake and Boost.Python, still need change in cmakemake.py to complete the fix.