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.

Sign up for GitHub

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

Jump to bottom

Support Boost 1.70: #2905

Closed
wants to merge 1 commit into from
Closed

Support Boost 1.70: #2905

wants to merge 1 commit into from

Conversation

seelabs
Copy link
Collaborator

@seelabs seelabs commented Apr 12, 2019

This patch removes calls to several deprecated asio functions.

  • io_service::post becomes post (free function)
  • io_service::work becomes executor_work_guard
  • io_service::wrap becomes bind_executor
  • get_io_context becomes get_executor or get_executor().context()

This patch was tested with boost 1.69 and 1.70. The functions
ripple::get_lowest_layer and beast::create_waitable_timer are required to
handle a breaking difference between these versions. When rippled no longer
needs to support pre 1.70 boost versions, both of these functions may be
removed, and the waitable timer injections may also be removed.

Note: this was tested against boost 1.69 and boost 1.70 rc2. We should not merge this until we test against the actual released boost 1.70.

@HowardHinnant @miguelportilla

@ripplelabs-jenkins
Copy link
Collaborator

ripplelabs-jenkins commented Apr 12, 2019 •
edited
Loading

Jenkins Build Summary

Built from this commit

Built at 20190517 - 21:31:00

Test Results

Build Type Log Result Status
rpm logfile 1177 cases, 0 failed, t: n/a PASS ✅
msvc.Debug logfile 1177 cases, 0 failed, t: 579s PASS ✅
msvc.Debug,
NINJA_BUILD=true		 logfile 1177 cases, 0 failed, t: 601s PASS ✅
msvc.Debug
-Dunity=OFF		 logfile 1176 cases, 0 failed, t: 1095s PASS ✅
msvc.Release logfile 1177 cases, 0 failed, t: 403s PASS ✅
gcc.Release
-Dassert=ON,
MANUAL_TESTS=true		 logfile 915 cases, 0 failed, t: 4m50s PASS ✅
docs,
TARGET=docs		 logfile 1 cases, 0 failed, t: 0m1s PASS ✅
clang.Debug logfile 1177 cases, 0 failed, t: 2m44s PASS ✅
gcc.Debug
-Dcoverage=ON,
TARGET=coverage_report,
SKIP_TESTS=true		 logfile 1177 cases, 0 failed, t: 16m39s PASS ✅
clang.Debug
-Dunity=OFF		 logfile 1176 cases, 0 failed, t: 1m52s PASS ✅
gcc.Debug logfile 1177 cases, 0 failed, t: 2m53s PASS ✅
gcc.Debug
-Dunity=OFF		 logfile 1176 cases, 0 failed, t: 0m43s PASS ✅
clang.Release
-Dassert=ON		 logfile 1177 cases, 0 failed, t: 4m44s PASS ✅
gcc.Release
-Dassert=ON		 logfile 1177 cases, 0 failed, t: 4m51s PASS ✅
gcc.Debug
-Dstatic=OFF		 logfile 1177 cases, 0 failed, t: 2m51s PASS ✅
gcc.Debug
-Dstatic=OFF -DBUILD_SHARED_LIBS=ON		 logfile 1177 cases, 0 failed, t: 2m49s PASS ✅
gcc.Debug,
NINJA_BUILD=true		 logfile 1177 cases, 0 failed, t: 2m45s PASS ✅
clang.Debug
-Dunity=OFF -Dsan=address,
PARALLEL_TESTS=false,
DEBUGGER=false		 logfile 1176 cases, 0 failed, t: 1m4s PASS ✅
clang.Debug
-Dunity=OFF -Dsan=undefined,
PARALLEL_TESTS=false		 logfile 1176 cases, 0 failed, t: 3m51s PASS ✅

HowardHinnant
HowardHinnant approved these changes Apr 12, 2019
View reviewed changes
Copy link
Contributor

@HowardHinnant HowardHinnant left a comment

Choose a reason for hiding this comment

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

Passes all tests on macOS.

Needs these updated build instructions for macOS: HowardHinnant@8553a36

@seelabs
Copy link
Collaborator Author

seelabs commented Apr 12, 2019

Pushed with updated readme with @HowardHinnant changes. Note: reverted the change saying the min version was 1.70. I believe it's still 1.67 with this commit.

@HowardHinnant
Copy link
Contributor

I'll double check that. I tried with boost 1.67 and it failed. But I now think that during yesterday's boost 1.70 linker warning hunt I left my 1.67 in a compromised state. Rebuilding 1.67 and will retest this against it, and will comment back here.

@seelabs
Copy link
Collaborator Author

seelabs commented Apr 12, 2019

Re-added @HowardHinnant docs about 1.70 being the min boost version on mac. There are build issues with older versions.

@HowardHinnant
Copy link
Contributor

More info: I'm unable to build boost 1.67 with the latest Apple developer tools. However boost binaries are available (https://sourceforge.net/projects/boost/files/boost-binaries/). And I've confirmed that this commit works with boost 1.67, assuming 1.67 is already built.

@seelabs
Copy link
Collaborator Author

seelabs commented Apr 12, 2019

Thanks @HowardHinnant Given this is a boost build issue and not a rippled issue, I changed the min version back to 1.67

@seelabs
Copy link
Collaborator Author

seelabs commented Apr 13, 2019

The the gcc.debug jenkins test are failing with ripple.http.Server failed to complete. The child process may have crashed. I'll look into this.

@miguelportilla
Copy link
Contributor

miguelportilla commented Apr 18, 2019 •
edited
Loading

Tested on Windows using Boost 1.67, 1.69 and 1.70 final.

miguelportilla
miguelportilla reviewed Apr 19, 2019
View reviewed changes
src/ripple/server/impl/LowestLayer.h

namespace ripple {

// Before boost 1.70, get_lowest_layer required an explicit templat parameter
Copy link
Contributor

Choose a reason for hiding this comment

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

misspell templat

@nbougalis nbougalis mentioned this pull request Apr 27, 2019
Closed
@seelabs seelabs mentioned this pull request Apr 29, 2019
Merged
@miguelportilla miguelportilla mentioned this pull request May 2, 2019
Closed
@seelabs
Copy link
Collaborator Author

seelabs commented May 2, 2019 •
edited
Loading

Rebased and pushed FindBoost change from @mellery451

@seelabs
Support Boost 1.70:
5ee25a2 
This patch removes calls to several deprecated asio functions.

* `io_service::post` becomes `post` (free function)
* `io_service::work` becomes `executor_work_guard`
* `io_service::wrap` becomes `bind_executor`
* `get_io_context`   becomes `get_executor` or `get_executor().context()`

This patch was tested with boost 1.69 and 1.70. The functions
`ripple::get_lowest_layer` and `beast::create_waitable_timer` are required to
handle a breaking difference between these versions. When rippled no longer
needs to support pre 1.70 boost versions, both of these functions may be
removed, and the waitable timer injections may also be removed.
@seelabs seelabs added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label May 17, 2019
@manojsdoshi manojsdoshi mentioned this pull request May 21, 2019
Merged
@seelabs seelabs deleted the boost-70 branch April 24, 2020 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miguelportilla miguelportilla miguelportilla approved these changes

@HowardHinnant HowardHinnant HowardHinnant approved these changes

Assignees

@miguelportilla miguelportilla

@HowardHinnant HowardHinnant

Labels
Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@seelabs @ripplelabs-jenkins @HowardHinnant @miguelportilla