cpp: Find dependencies when not using CPM to download them

[musicinmybrain]

🤔 What's changed?

Find dependencies (currently: nlohmann_json) using find_package when not using CPM to download them.

⚡️ What's your motivation?

Before this PR:

$ cmake -S. -Bbuild -DCUCUMBER_MESSAGES_FETCH_DEPS:BOOL=OFF
-- The CXX compiler identification is GNU 16.1.1
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Configuring done (0.2s)
CMake Error at CMakeLists.txt:96 (target_link_libraries):
  Target "cucumber_messages_lib" links to:

    nlohmann_json::nlohmann_json

  but the target was not found.  Possible reasons include:

    * There is a typo in the target name.
    * A find_package call is missing for an IMPORTED target.
    * An ALIAS target is missing.



-- Generating done (0.0s)
CMake Generate step failed.  Build files cannot be regenerated correctly.

After this PR (with a system-wide installation of json-devel on Fedora 44):

$ cmake -S. -Bbuild -DCUCUMBER_MESSAGES_FETCH_DEPS:BOOL=OFF
-- The CXX compiler identification is GNU 16.1.1
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found nlohmann_json: /usr/share/cmake/nlohmann_json/nlohmann_jsonConfig.cmake (found suitable version "3.12.0", minimum required is "3.12.0")
-- Configuring done (0.2s)
-- Generating done (0.0s)
-- Build files have been written to: /home/ben/src/forks/cucumber-messages/cpp/build

🏷️ What kind of change is this?

• 🐛 Bug fix (non-breaking change which fixes a defect)

♻️ Anything particular you want feedback on?

N/A

📋 Checklist:

• I agree to respect and uphold the Cucumber Community Code of Conduct
• I've changed the behaviour of the code
  • I have added/updated tests to cover my changes.
• My change requires a change to the documentation.
  • I have updated the documentation accordingly.
• Users should know about my change
  • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

---

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

[daantimmer]

@musicinmybrain would you consider removing the fetch deps check instead? CPM uses find package underneath.

Out of interest, how are you using cucumber/messages in your project?

[daantimmer]

Also, @musicinmybrain sorry for breaking your dependency chain. I thought I had all use cases covered.

[musicinmybrain]

Thanks for the quick response!

Out of interest, how are you using cucumber/messages in your project?

I’m maintaining the cucumber-messages package in Fedora. This includes the Python and Ruby implementations, which actually have things in the distribution that depend on them, and the C++ shared library just because it hasn’t been much extra work to ship it as well.

@musicinmybrain would you consider removing the fetch deps check instead? CPM uses find package underneath.

Hmm, I could do that. The disadvantage is that this is a lot less helpful for fully offline build environments like what we have in Fedora. In order to use CPM to find the system nlohmann_json, I would need to add e.g. https://github.com/cpm-cmake/CPM.cmake/releases/download/v0.40.2/CPM.cmake as an additional package source, upload it to our lookaside cache and include it in the source package, and then ensure it’s copied into the correct CPM_DOWNLOAD_LOCATION at the start of the package build process. This is perfectly feasible, but it would be easier for me to just carry a downstream-only two-line patch like my original suggestion.

In other words, I don’t just want to not fetch dependencies, I want to not fetch anything.

[musicinmybrain]

Also, @musicinmybrain sorry for breaking your dependency chain. I thought I had all use cases covered.

It’s no problem! This is the kind of thing that, as a distribution packager, I’m used to encountering and bringing up with upstreams.

[daantimmer]

You have convincing arguments. I'll apply the same logic to my pending cucumber/gherkin PR. (I initially was going to get rid of the fetch_deps option and always use CPM, which I applied in my pending gherkin PR. But I'll revert that from the PR.

While we are at it, I have another pending/draft PR on cucumber/messages: #473. This PR consists of two big breaking changes. (Essentially I am piggy backing one on the other). Would you be so kind to give some of your thoughts in/on that PR?

As for this PR, the change is fine @mpkorstanje Only thing that might be needed is a small update to the changelog.md @musicinmybrain

[daantimmer]

@musicinmybrain also, "beer mat/coaster thought", would it be worth it to add a sort of test, to test this usecase? Although I think this mostly needs to be resolved in the workflow file, adding a separate job to first download nlohmann + build&install. And then do a cmake configure/build for cucumber/messages/cpp.

I don't like it when workflows break without tests falling over.