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

rapidjson: fix cmake_find_package{,_multi} names #1180

Merged
merged 1 commit into from
Mar 25, 2020

Conversation

madebr
Copy link
Contributor

@madebr madebr commented Mar 24, 2020

Specify library name and version: rapidjson/1.1.0

  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

@madebr madebr mentioned this pull request Mar 24, 2020
4 tasks
@conan-center-bot
Copy link
Collaborator

All green in build 1 (a82e62ce11c9766184619ec1d695bff45ca7cd29)! 😊

@@ -24,3 +24,7 @@ def package(self):

def package_id(self):
self.info.header_only()

def package_info(self):
self.cpp_info.names["cmake_find_package"] = "RapidJSON"
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this break current dependence that are using rapidjson?

The doc says

The alternate name

But it's not very clear if it replaces the default package name

Copy link
Contributor Author

@madebr madebr Mar 25, 2020

Choose a reason for hiding this comment

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

Running grep -Irn rapidjson | grep -i requires on current master returns no matches.

Copy link
Contributor Author

@madebr madebr Mar 25, 2020

Choose a reason for hiding this comment

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

This indeed might break external recipes, but the cmake script of rapidjson also uses RapidJSON as cmake name.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's the external dependencies that I had in mind. I agree is should make the original of the project.
I take it from your reply is does indeed replace the default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SSE4 SSE4 requested a review from uilianries March 25, 2020 08:27
@danimtb danimtb merged commit 04463b7 into conan-io:master Mar 25, 2020
@toshi38
Copy link

toshi38 commented Mar 25, 2020

As a heads up, this does indeed break external dependencies. For anyone finding this with a search you'll likely need to change the casing to RapidJSON

CMake Error at <project>/service/CMakeLists.txt:7 (find_package):
  By not providing "Findrapidjson.cmake" in CMAKE_MODULE_PATH this project
  has asked CMake to find a package configuration file provided by
  "rapidjson", but CMake did not find one.

  Could not find a package configuration file provided by "rapidjson" with
  any of the following names:

    rapidjsonConfig.cmake
    rapidjson-config.cmake

  Add the installation prefix of "rapidjson" to CMAKE_PREFIX_PATH or set
  "rapidjson_DIR" to a directory containing one of the above files.  If
  "rapidjson" provides a separate development package or SDK, be sure it has
  been installed.


-- Configuring incomplete, errors occurred!

@uilianries
Copy link
Member

@toshi38 The author uses the name RapidJSON: https://github.com/Tencent/rapidjson/blob/master/RapidJSONConfig.cmake.in

If any other project requires rapidjsonConfig.make, it needs to be updated.

@madebr madebr deleted the rapidjson_name branch March 28, 2020 00:32
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.

None yet

7 participants