flux-framework / flux-sched

Fluxion Graph-based Scheduler
GNU Lesser General Public License v3.0
84 stars 39 forks source link

up the required C++ standard to 20 #1174

Closed trws closed 1 month ago

trws commented 1 month ago

I realize this is odd timing for this, but it's been sitting for a while, and I keep tripping over myself trying to use interfaces that I know exist but don't in C++14. IIRC @grondo did a test that we can build with gcc12+ everywhere we care about through the devtoolset to build our RPMs, it would help me a lot if we could finish this through as long as it doesn't cause deployment concerns.

problem: Many interfaces are made more cumbersome or slower by forcing the use of only up to c++14.

solution: As previously discussed, up the required C++ standard to 20 since we have devtoolset gcc everywhere that can compile to that standard.

trws commented 1 month ago

Excellent point, @jameshcorbett would you be willing to give it another run through to make sure this doesn't cause problems spinning a new RPM? The last thing I want to do is block us being able to push out an update right now.

jameshcorbett commented 1 month ago

Working through some issues in the buildfarm, it didn't work out of the box:

/builddir/build/BUILD/flux-sched-0.33.1/resource/planner/c++/planner_multi.hpp:96:21:   required from here
/usr/include/boost/detail/allocator_utilities.hpp:125:31: error: no class template named 'rebind' in 'class std::allocator<planner_multi_meta>'
  125 |           rebind<Type>::other other;
      |                               ^~~~~
/usr/include/boost/detail/allocator_utilities.hpp: In instantiation of 'struct boost::detail::allocator::rebinder<std::allocator<planner_multi_meta> >::result<planner_multi_meta>':
/usr/include/boost/detail/allocator_utilities.hpp:133:49:   required from 'struct boost::detail::allocator::compliant_allocator_rebind_to<std::allocator<planner_multi_meta>, planner_multi_meta>'
/usr/include/boost/mpl/eval_if.hpp:60:31:   required from 'struct boost::mpl::eval_if_c<false, 

My only option here is to up the boost version to whatever is standard on LC right? Which on Ruby is 1.66.

trws commented 1 month ago

Try it, I’ll see if I can factor around it. What boost are we currently building with?


Sent from Workspace ONE Boxerhttps://whatisworkspaceone.com/boxer On April 15, 2024 at 3:50:24 PM PDT, James Corbett @.***> wrote:

Working through some issues in the buildfarm, it didn't work out of the box:

/builddir/build/BUILD/flux-sched-0.33.1/resource/planner/c++/planner_multi.hpp:96:21: required from here /usr/include/boost/detail/allocator_utilities.hpp:125:31: error: no class template named 'rebind' in 'class std::allocator' 125 | rebind::other other; | ^~~~~ /usr/include/boost/detail/allocator_utilities.hpp: In instantiation of 'struct boost::detail::allocator::rebinder<std::allocator >::result': /usr/include/boost/detail/allocator_utilities.hpp:133:49: required from 'struct boost::detail::allocator::compliant_allocator_rebind_to<std::allocator, planner_multi_meta>' /usr/include/boost/mpl/eval_if.hpp:60:31: required from 'struct boost::mpl::eval_if_c<false,

My only option here is to up the boost version to whatever is standard on LC right? Which on Ruby is 1.66.

— Reply to this email directly, view it on GitHubhttps://urldefense.us/v3/__https://github.com/flux-framework/flux-sched/pull/1174*issuecomment-2057941503__;Iw!!G2kpM7uM-TzIFchu!xSYvlOVz2XDdxtmmrVhaExzi49nnie02CTYXdDH7Z2JRCcYkQ28XstScRy7CAPEcLTEiXlm8u7nrx9vpOyaNx1p173Q$, or unsubscribehttps://urldefense.us/v3/__https://github.com/notifications/unsubscribe-auth/AAFBFNMDBJDMRKN5H22K2DTY5RKSXAVCNFSM6AAAAABGEWX2UWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJXHE2DCNJQGM__;!!G2kpM7uM-TzIFchu!xSYvlOVz2XDdxtmmrVhaExzi49nnie02CTYXdDH7Z2JRCcYkQ28XstScRy7CAPEcLTEiXlm8u7nrx9vpOyaNGlGOhiU$. You are receiving this because you were mentioned.Message ID: @.***>

jameshcorbett commented 1 month ago

We had buildrequires >= 1.53.0 but I wonder if we were just picking up the default version of 1.66.0 (assuming I understand how the buildfarm works)? Either way, upping it to buildrequires >= 1.66.0 hit the same issue.

grondo commented 1 month ago

but I wonder if we were just picking up the default version of 1.66.0

The buildfarm installs buildrequires using yum/dnf, so will probably pick the latest available version already. Keep in mind that a distro usually has ABI guarantees, so it is unlikely the packaged version of libboost would ever get a big version bump if it would break that guarantee (so usually libs stick at a given version for an entire major release lifetime)

trws commented 1 month ago

Ok, we have an unexpected wrinkle. Boost multiindex apparently expects the rebind property to exist on allocators until 1.67, which was deprecated in C++17 and removed in C++20.

Thankfully this is an interface I know reasonably well, and the whole thing is user-customizable, so I made a little wrapper allocator that provides the missing fields to keep the old version of boost happy. We can remove it if and when we can use a boost that's less behind, but it's more important to keep things working and fix issues right now than muck around with getting new boost versions.

trws commented 1 month ago

Excellent! Ability to format strings with the good version of fmt or std::format will make life much nicer!


Sent from Workspace ONE Boxerhttps://whatisworkspaceone.com/boxer On April 16, 2024 at 10:38:34 AM PDT, James Corbett @.***> wrote:

@jameshcorbett approved this pull request.

Looks reasonable and it works in the buildfarm! I can also bump the gcc-toolset to 13 as we discussed on a call, that seems to work as well.

— Reply to this email directly, view it on GitHubhttps://urldefense.us/v3/__https://github.com/flux-framework/flux-sched/pull/1174*pullrequestreview-2004254584__;Iw!!G2kpM7uM-TzIFchu!3qyd0DvO5UFbPbTGnbiYyihYnnvuew9f9PRfu0bA933d-2TrCHBdMkCQd26xBal5FRmdUlPF-QZRtw_2Vmyo1EOjZAs$, or unsubscribehttps://urldefense.us/v3/__https://github.com/notifications/unsubscribe-auth/AAFBFNJITB7PPG6R2ONLNU3Y5VOZJAVCNFSM6AAAAABGEWX2UWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMBUGI2TINJYGQ__;!!G2kpM7uM-TzIFchu!3qyd0DvO5UFbPbTGnbiYyihYnnvuew9f9PRfu0bA933d-2TrCHBdMkCQd26xBal5FRmdUlPF-QZRtw_2VmyoC6733io$. You are receiving this because you were mentioned.Message ID: @.***>