restinio
restinio copied to clipboard
Optional dependencies not actually optional.
I've been following RESTinio a bit as I plan to use it for a game and also because I've been making a Macports portfile. Initially, the cmake file was so broken I had to implement a custom install routine in the portfile (which worked but is a PITA to maintain). As such, I was looking into the changes introduced in recent versions and I noticed that in fact most of the issues appear to have been fixed.
However, SObjectizer is still always being expected/included by CMake. Shouldn't this be wrapped in a conditional for tests, samples or somesuch?
Shouldn't this be wrapped in a conditional for tests, samples or somesuch?
Yes, I think we can do something like that. But it can take some time because we are a little busy with other projects. I hope we resolve this issue in several days.
Thanks for reporting it.
Shouldn't this be wrapped in a conditional for tests, samples or somesuch?
Yes, I think we can do something like that. But it can take some time because we are a little busy with other projects. I hope we resolve this issue in several days.
Thanks for reporting it.
For now I'll be adding patches to fix issues in my portfile. Similar issues I see are with OpenSSL which is billed as "optional" (I figure you mean, if you don't need TLS don't include it and it should be fine?). There should perhaps also be a CMake option to handle that and a stub tls in case OpenSSL is not available. As far as I can tell, Restinio doesn't use OpenSSL directly anyway bur rather through asio and asio::ssl
, no?
And pretty much the same thing for PCRE/Boost regex. They're both looked for always and I don't think the CMake file actually allows to choose between the two (or eventually allow supporting both if they're not mutually exclusive)
And pretty much the same thing for PCRE/Boost regex. They're both looked for always and I don't think the CMake file actually allows to choose between the two (or eventually allow supporting both if they're not mutually exclusive)
Usage of PCRE/PCRE2 or Boost is not controlled by CMake. You select which one will be used by manually including restinio/router/pcre_regex_engine.hpp
or restinio/router/boost_regex_engine.hpp
in your C++ file (see this section for more details). CMake script just calls find_package to check presence of these tools.
And pretty much the same thing for PCRE/Boost regex. They're both looked for always and I don't think the CMake file actually allows to choose between the two (or eventually allow supporting both if they're not mutually exclusive)
Usage of PCRE/PCRE2 or Boost is not controlled by CMake. You select which one will be used by manually including
restinio/router/pcre_regex_engine.hpp
orrestinio/router/boost_regex_engine.hpp
in your C++ file (see this section for more details). CMake script just calls find_package to check presence of these tools.
I figured the logic was that. It's a relatively sensible approach but far some failsafe. I'd recommend having CMake switches to choose between either, both or none and installing files as appropriate.
I personally think restinio is a great library with lots of potential, but have seen most of the criticism it has gotten (on reddit for example) is because of the heavy dependencies.
Now, I realize that most of these are truly optional; but by not making them so in CMakeLists, you're probably giving the impression that the dependencies are heavier than they actually are and alienating people who would otherwise use the RESTinio.
Anyway, cheers.
For now I'll be adding patches to fix issues in my portfile.
Can you, please, share your patch with us?
And by the way, regarding MacPorts (no experience with it), is it possible to pass cmake definition to be applied for a given lib?
For now I'll be adding patches to fix issues in my portfile.
Can you, please, share your patch with us?
And by the way, regarding MacPorts (no experience with it), is it possible to pass cmake definition to be applied for a given lib?
You can pass arguments into most steps like configure.args-append
or build.post_args-append
and such; passing options to cmake is just appending -DCMAKE_SOME_VARIABLE=VALUE
to args during configure step.
Can you, please, share your patch with us?
For now I was doing some very basic things, mostly removing some stuff that would not be needed for a basic install. Will definitely share once I've done something more robust.
So... I've decided to make a few substantial changes to the way RESTinio manages at least some of its dependencies after all. I will submit a PR once I'm done, but for now I've got some questions.
Why are there apparently benchmarks (that have another dependency) in the tests folder?
Why are there apparently benchmarks (that have another dependency) in the tests folder?
I suppose it because we saw these benchmarks as part of test suite.
As a comment on how to use this as a "standalone" 3rd party build in a larger docker build we do the following:
wget -O restinio.tar.gz "https://github.com/Stiffstream/restinio/archive/v.0.5.1.tar.gz" &&
mkdir -p restinio &&
tar
--extract
--file restinio.tar.gz
--directory restinio
--strip-components 1
cd restinio
cd dev
sed -i.bak '/add_subdirectory(so_5)/d' CMakeLists.txt
mkdir build && cd build
cmake -DRESTINIO_TEST=OFF -DRESTINIO_SAMPLE=OFF -DRESTINIO_INSTALL_SAMPLES=OFF -DRESTINIO_BENCH=OFF -DRESTINIO_INSTALL_BENCHES=OFF -DRESTINIO_FIND_DEPS=ON ..
./configure
make
sudo make install
cd ..
The only thing that is ugly is the sed to get rid of the add_subdirectory(so_5)
Could add an option to not always add so_5 since it's not actually required, please :-)
Otherwise, thanks for an excellent library!!
I second that the optional dependencies are not optional due to simple fact that the conditional if else with the -RESTINIO_FIND_DEPS
at this line does not contain the further find_package(OpenSSL)
, find_package(PCRE)
, find_package(PCRE2)
, find_package(ZLIB)
.
Moreover, the else condition in case one sets RESTINIO_FIND_DEPS=Off
will look for local folders which is still forcing you to use the dependencies that you don't want.
As @skarlsson suggested the only way to truly chose dependencies is to activate their finding to avoid including relative directories and then remove with sed all find_package outside of the scope of the dependencies conditional.
Personally, I think that it is a huge overhead for integrating a headers only library that doesn't even need to generate a shared library to pass simple dependencies checks that should be inside of the same conditional scope to be able to control their checks through cmake variables.
Certainly my PR would need to be updated anf possibly refined but I think it provides a more than acceptable solution to the issue and would greatly improve usability and inclusion in package managers for different platforms.
I've added an option RESTINIO_ALLOW_SOBJECTIZER
to the main CMakeLists.txt. If that option is OFF
then add_subdirectory(so_5)
won't be used. It allows to run cmake that way:
cmake -DRESTINIO_TEST=OFF \
-DRESTINIO_SAMPLE=OFF \
-DRESTINIO_BENCH=OFF \
-DRESTINIO_ALLOW_SOBJECTIZER=OFF \
-DRESTINIO_FIND_DEPS=ON ..
If RESTINIO_TEST
or RESTINIO_SAMPLE
or RESTINIO_BENCH
is ON
then RESTINIO_ALLOW_SOBJECTIZER
can't be OFF
.
The updated version is in master branch. It would be great if someone tried it and provided some feedback: is that change solve the problem or not?
Works! Thanks!
Hey! I'll try it as soon as I get a chance. I'd still prefer to be able to individually choose the regex implementations but if that's just not an option for you guys I guess I can just turn those into a patch for whichever package manager.
Sent with GitHawk
@Lord-Kamina I would like to join the effort. I do have decent experienc in such tasks. Please take a look at my WIP micro-service based on Restinio: https://github.com/ahmedyarub/micro-service
Simplifying the process of acquiring and building dependencies by including everything in the CMake script https://stiffstream.com/en/docs/restinio/0.5/obtaining.html#prerequisites
you should make a clear statement in the README.md possible just point to the prerequisites you point to here
When I read there that RESTinio was a "header-only C++14 library" I was led to think I could just include it, and be good. But that is not the case.