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

Deboostify Crow #457

Merged
merged 27 commits into from
Jun 18, 2022
Merged

Deboostify Crow #457

merged 27 commits into from
Jun 18, 2022

Conversation

luca-schlecker
Copy link
Collaborator

@luca-schlecker luca-schlecker commented Jun 6, 2022

This Pr removes Boost as a dependency and will instead rely on the standalone asio library.
CI now installs libasio-dev instead of libboost-all-dev and the documentation resembles that change. (Although the MacOS section could use a double-check, I have no such device on hand)

Proper testing necessary

Unittests are all passing but I think some real-life testing does no harm.

Removed Boost functions and types and their replacements:

  • boost::lexical_cast becomes crow::utility::lexical_cast. Its implementation relies on std::stringstream for converting between types.
  • boost::hash_combine is replaced by a private function. Its implementation idea stems from around the internet and can be found in multiple sources.
  • boost::iequals is replaced by crow::utility::string_equals taking two mandatory std::string arguments and one optional bool controlling the case-sensitivity.
  • boost::system::error_code is replaced by asio::error_code which is an alias for std::error_code.
  • Every boost::asio:: function is replaced with their standalone asio:: counterpart.
  • boost::array is replaced with std::array.
  • boost::posix_time is replaced with a std::chrono equivalent.
  • boost::*_comparable is dropped and the respective operators are implemented manually.
  • boost::lexicographical_compare is replaced with std::lexicographical_compare.
  • boost::optional is replaced with std::unique_ptr.
  • boost::trim is replaced with crow::utility::trim.

Introduced functions (*explicitly* tested?):

  • private void crow::ci_hash::hash_combine. (❌)
  • private bool crow::json::detail::r_string::equals. (❌)
  • Copy-constructor for crow::CookieParser::Cookie. (❌)
  • bool crow::utility::string_equals (✅)
  • T crow::utility::lexical_cast (✅)
  • std::string crow::utility::trim (✅)

Copy link
Member

@The-EDev The-EDev left a comment

Choose a reason for hiding this comment

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

What I found are the comments below and the drone error:

2: ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2: unittest is a Catch v2.13.8 host application.
2: Run with -? for options
2: 
2: -------------------------------------------------------------------------------
2: middleware_cookieparser_format
2: -------------------------------------------------------------------------------
2: /drone/src/tests/unittest.cpp:1667
2: ...............................................................................
2: 
2: /drone/src/tests/unittest.cpp:1713: FAILED:
2:   CHECK( s.find("Expires=Wed, 01 Nov 2000 23:59:59 GMT") != std::string::npos )
2: with expansion:
2:   18446744073709551615 (0xffffffffffffffff)
2:   !=
2:   18446744073709551615 (0xffffffffffffffff)

And It's not a dealbreaker but formatting .drone.yml is somewhat incorrect and qs_parse is somewhat problematic (because while formatting Crow I attempted to keep external libraries away from clang)

include/crow/utility.h Outdated Show resolved Hide resolved
include/crow/http_server.h Outdated Show resolved Hide resolved
include/crow/json.h Show resolved Hide resolved
include/crow/json.h Outdated Show resolved Hide resolved
@luca-schlecker
Copy link
Collaborator Author

I'll have to look into that unittest again. Passing fine on my system.
Unfortunately, I had to make a change to query_string.h and in the process vscode auto-formatted everything according to Crow's guidelines. The yml files were also formatted on save, reversed that change though.

@luca-schlecker luca-schlecker requested a review from The-EDev June 7, 2022 01:08
@luca-schlecker luca-schlecker force-pushed the ls-boostless branch 8 times, most recently from cb65082 to d8721c9 Compare June 7, 2022 16:37
@CrowCpp CrowCpp deleted a comment from crow-clang-format bot Jun 7, 2022
@CrowCpp CrowCpp deleted a comment from crow-clang-format bot Jun 7, 2022
@CrowCpp CrowCpp deleted a comment from crow-clang-format bot Jun 7, 2022
@luca-schlecker luca-schlecker marked this pull request as ready for review June 7, 2022 17:46
@CrowCpp CrowCpp deleted a comment from crow-clang-format bot Jun 7, 2022
@luca-schlecker luca-schlecker marked this pull request as draft June 7, 2022 22:54
@CrowCpp CrowCpp deleted a comment from crow-clang-format bot Jun 7, 2022
Copy link
Member

@The-EDev The-EDev left a comment

Choose a reason for hiding this comment

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

Aside from the comments, I couldn't find anything concerning from looking at the code. I'll do a few local tests (to see if it compiles with GCC 4.8 for example) and we'll see what to do from there :)

include/crow/utility.h Show resolved Hide resolved
include/crow/middlewares/cookie_parser.h Outdated Show resolved Hide resolved
include/crow/json.h Outdated Show resolved Hide resolved
@CrowCpp CrowCpp deleted a comment from crow-clang-format bot Jun 8, 2022
@CrowCpp CrowCpp deleted a comment from crow-clang-format bot Jun 8, 2022
@luca-schlecker luca-schlecker marked this pull request as ready for review June 8, 2022 14:28
@luca-schlecker luca-schlecker requested a review from The-EDev June 8, 2022 14:34
It serves as a replacement for boost::iequals.
Move to pointers instead.
This implementation relies on the safe deletion of `nullptr`.
This makes the code more consistent and also reflects the intention a little better.
Return early if inequality is found.
A private templated function containing the algorithm is used to prevent repetition.
@The-EDev The-EDev merged commit ee0e025 into master Jun 18, 2022
@luca-schlecker luca-schlecker deleted the ls-boostless branch June 18, 2022 14:31
@CrowCpp CrowCpp deleted a comment from crow-clang-format bot Jun 18, 2022
@The-EDev The-EDev mentioned this pull request Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants