-
Notifications
You must be signed in to change notification settings - Fork 96
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
RESTinio CMake issues #167
Comments
Incidentally, there's also a typo in the Edit: And nevermind that, because |
Hi! CMake's related topic is hard part to me because I really hate CMake and almost not understand how it works. We have to support it because it's a de-facto standard at the time and we're trying to do it as best as we can, but shit can happens, sorry. As far as I can understand our CMake files supports the following case:
I suppose that the use of restinio via If those scenarios aren't appicable to you then could you please tell us what do you want to have? Maybe you can point to some good written example of similar CMake file(s) in other OpenSource projects? |
CMake is a bit like PHP, in that it's not necessarily a bad language and has a million ways to do anything, but it's full of developers who misuse it horribly and stick to ancient patterns. It's very likely that this has contributed to your hatred of it. I don't have a project in mind with particularly good usage of it but I'll ask around and see if I can find something. I'm willing to eventually making PRs to try and fix at least some of the issues, of course. |
@eao197 I'm taking a crack at fixing some of the issues and several aspects of RESTinio's CMake scripts. How/where exactly are either Catch or Clara included? In the CMake script, I see that using Also, the Would you mind explaining when other libraries are used and how are they obtained in the various scenarios (some of them are pretty obvious. Others not so much) |
Hi!
It's a hard question. When But if
There are two different projects. The first is fmt (fmtlib): MxxRu::arch_externals :fmt do |e|
e.url 'https://github.com/fmtlib/fmt/archive/9.1.0.zip' and the second is MxxRu wrapper for fmtlib named fmtlib_mxxru: MxxRu::arch_externals :fmtlib_mxxru do |e|
e.url 'https://github.com/Stiffstream/fmtlib_mxxru/archive/fmt-5.0.0-1.tar.gz' RESTinio uses fmtlib-9.1.0 and fmtlib_mxxru version
You have to understand that the content of root CMakeLists.txt is a result of long evolution process. Initially our idea (and intent) was to get all dependencies for the development of RESTinio via MxxRu::externals. When we fixed a new version of RESTinio we made a tarball with all dependencies. We thought that a user get this tarball, unpack it and add just one directory to INCLUDE path. So the first versions of CMakeLists.txt were very simple because they were created for such simple scenario. But then we started to get questions like "I'm already have library X in my project how can I tell RESTinio to use it?" So we started to add new conditions to CMakeLists.txt. Moreover, after some time we added a support for Conan and vcpkg. And now our CMakeLists.txt is a mess of workarounds for various cases. It should be refactored and a new strategy for dependency management has to be implemented (there is already a related issue). But because this project is on hold now there is no progress in that area. |
What I'm currently going for is to do something akin to what is proposed in #160, but I thought perhaps I could also leave |
It seems that this issue is no more actual after release of v.0.7.0 where CMake scripts were refactored. |
This is an offshoot from #166. I normally use a modified version of RESTinio to suit my main project. Obviously, for a test case I am trying to set-up vanilla RESTinio to see if the error persists.
However, the CMakeLists contains several errors that make it pretty hard to use. The whole
IF (RESTINIO_MASTER_PROJECT)
means RESTinio cannot be used properly viaadd_subfolder
. I generally always prefer to usefind_package
, and I know that RESTinio provides CMake config files, but given that it's not a library commonly available in many package managers,add_subdirectory
would be a pretty common way to include it in a project.The issue is, when included like that,
RESTINIO_MASTER_PROJECT
is not defined. When that happens, RESTinio does not even try to look for FMT, so thenrestinio/CMakeLists.txt
produces an error because it's trying to link against a non-existent target, and similar errors will likely occur for most other dependencies.If one just removes that check, http_parser is still a problem.
If
RESTINIO_FIND_DEPS
is on butUSE_EXTERNAL_HTTP_PARSER
isn't, RESTinio tries to usefind_package
to find a library that hasn't been installed anywhere because it's included in RESTinio. The correct behavior would be to useadd_subdirectory(nodejs/http_parser)
as is being done whenRESTINIO_FIND_DEPS
is off. It might also be a good idea to haveUSE_EXTERNAL_HTTP_PARSER
be the default.The text was updated successfully, but these errors were encountered: