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

[gherkin-cpp, messages-cpp] unable to use Cmake FetchContent to include gherkin-cpp as-source. #192

Closed
daantimmer opened this issue Nov 13, 2023 · 8 comments

Comments

@daantimmer
Copy link

daantimmer commented Nov 13, 2023

👓 What did you see?

It is currently not possible to use CMake's FetchContent to include gherkin-cpp and messages-cpp within a project. Both projects are currently setup with a Linux workflow in mind. 'Compile library separately, install library separately, use CMake 'Find' to include the pre-build binary.
This is however not the only way of using CMake projects. Often times it is useful to include the library directly as a dependency, and build the library while building the rest of the project. This especially useful for single-stage builds, instead of multi-stage builds.

✅ What did you expect to see?

Being able to FetchContent() gherkin-cpp

📦 Which tool/library version are you using?

cmake version 3.27.7

🔬 How could we reproduce it?

Will provide an example repository when this is picked up

Steps to reproduce the behavior:

  1. Install '...' version '...'
  2. Create a file called '....'
  3. Run command '....'
  4. See error '....'

📚 Any additional context?

I am building a cucumber runner using cpp. Where gherkin-cpp is the backend for parsing the gherkin files and providing me with a nice json object. The cucumber runner will be made open source (MIT licensed)


This text was originally generated from a template, then edited by hand. You can modify the template here.

@chybz
Copy link
Contributor

chybz commented Nov 13, 2023

@daantimmer Thanks for reporting your issue.

FYI it's not a "Linux workflow".

You are responsible to install the dependencies using whatever is appropriate to your specific use case. There are so many options out there and this project does not enforce a specific use case.

It's not clear if you ask for gherkin/cpp to FetchContent messages/cpp or if you encounter issues using FetchContent on gherkin/cpp.

Would you like to provide further details ?

@daantimmer
Copy link
Author

daantimmer commented Nov 13, 2023

@chybz
I have created an example project. Using our own open source devcontainer, which contains gcc, clang and clang-cl to cross compile for windows. The project can be found here: https://github.com/daantimmer/gherkin-cpp-container

One of the usecases that exists is the following: add the required dependencies using FetchContent. See:

This is the only way to get things FetchContent-ed correctly together with FindPackage.

As you can see in this CI/build run: https://github.com/daantimmer/gherkin-cpp-container/actions/runs/6855495998 none of the targets can build. That is because all the libraries and executables within gherkin-cpp and messages-cpp have been setup using CMAKE_SOURCE_DIR.

CMAKE_SOURCE_DIR uses the root CMakeLists.txt. Which does not work in the case of FetchContent, because the CMAKE_SOURCE_DIR is completely wrong for the directory tree created by FetchContent.

I've created a fork of cucumber/gherkin and cucumber/messages in which I am trying to 'fix' this. To get it to a compiling state:

For the messages repository I had to leave in the target_include_directories for cucumber-messages, because util.hpp is (accidentally?) residing in the same directory as all the sources. I would assume it has to move to the include directory.

After these fixes are applied the projects are atleast able to be configurable and getting to a stage where they can start compiling. As can be seen here:
https://github.com/daantimmer/gherkin-cpp-container/actions/runs/6856150561
Although, now many other actual build errors pop-up, which is part of #191

@daantimmer
Copy link
Author

@chybz any input from your side?

1 similar comment
@daantimmer
Copy link
Author

@chybz any input from your side?

@chybz
Copy link
Contributor

chybz commented Dec 28, 2023

@daantimmer sorry for the late update, was busy elsewhere. Made some updates and progress, though nothing is finished yet. Fixes should be available within a couple of days now, if I'm not "distracted".

@daantimmer
Copy link
Author

@chybz note that I've already made quite some changes in my fork for both gherkin-cpp and messages-cpp. Perhaps you can take inspiration from there for all minimum required changes.

@daantimmer
Copy link
Author

daantimmer commented Dec 28, 2023

I've added the gherkin pull request: #202
I've added the messages pull reqest: cucumber/messages#191

This was the minimum required to get your library working using FetchContent and as a dependency.

For your information, I am already using/integrating the gherkin-cpp library from my fork for a currently private hosted, but soon to be made MIT open source once I get clearance from the company, a cucumber runner written in cpp based on gherkin-cpp. I just need to finish up some formalities.

@chybz
Copy link
Contributor

chybz commented Jan 18, 2024

Closing as issue fixed by work on #202

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 a pull request may close this issue.

2 participants