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.

Sign up for GitHub

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

Jump to bottom

[grpc] Use build_modules to find executable grpc_cpp_plugin #5404

Merged
merged 14 commits into from
May 5, 2021
Merged

[grpc] Use build_modules to find executable grpc_cpp_plugin #5404

merged 14 commits into from
May 5, 2021

Conversation

jgsogo
Copy link
Contributor

@jgsogo jgsogo commented May 3, 2021 •
edited
Loading

Specify library name and version: grpc/1.37.0

Taken from protobuf package, this approach using build_modules will find the executable from a package in build context and also for the native one.

Approach is a bit different, exporting the build_module and using an auxiliary target... submitting for consideration.

@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@jgsogo jgsogo marked this pull request as ready for review May 3, 2021 15:32
@github-actions github-actions bot mentioned this pull request May 3, 2021
Open
@prince-chrismc
Copy link
Contributor

qchateau@83da72f

There a new point release of you want to cherry pick it

@jgsogo @prince-chrismc
Update recipes/grpc/all/conanfile.py
5fdbb6c 
Co-authored-by: Chris Mc <prince.chrismc@gmail.com>
@jgsogo
Copy link
Contributor Author

jgsogo commented May 4, 2021

What is your opinion about this approach with the exec component? It is a fake component (maybe I should call it _exec?) but I cannot add these build-modules at the package level when I have already declared some components. Anyway, I prefer this approach exporting the .cmake file than creating the file from the recipe itself (like protobuf).

jgsogo and others added 2 commits May 4, 2021 08:47
@jgsogo
Merge remote-tracking branch 'conan/master' into grpc/grpc_cpp_plugin
f52ef2f
@jgsogo @qchateau
grpc: add version 1.37.1 (#6)
74029ec 
Generated and committed by [Conan Center Bot](https://github.com/qchateau/conan-center-bot)
Find more updatable recipes in the [GitHub Pages](https://qchateau.github.io/conan-center-bot/)

Co-authored-by: Quentin Chateau via Conan Center Bot <quentin.chateau@gmail.com>
@conan-center-bot
Copy link
Collaborator

All green in build 5 (74029ec5b74201027e76255e09e1ecd74f07ec70):

  • grpc/1.37.1@:
    All packages built successfully! (All logs)

  • grpc/1.37.0@:
    All packages built successfully! (All logs)

@prince-chrismc
Copy link
Contributor

I like it separated since it's "cleaner" but there's a practicality of it all being in the same spot which I can live with.

The tie breaking for me is the template rendering.

protobuf dynamically counts the directories (maybe that's a little overkill)

conan-center-index/recipes/protobuf/all/conanfile.py

Line 123 in 7f66a09

protoc_rel_path = "{}bin/{}".format("".join(["../"] * module_folder_depth), protoc_filename)

However I'd much prefer a seperate file in the case of Qt6 is another example where content repeats many times in the same file and it depends on the options.

conan-center-index/recipes/qt/6.x.x/conanfile.py

Line 583 in 7f66a09

INTERFACE_INCLUDE_DIRECTORIES "${{CMAKE_CURRENT_LIST_DIR}}/../../../include/Qt{0}/{1};${{CMAKE_CURRENT_LIST_DIR}}/../../../include/Qt{0}/{1}/Qt{0}"

@jgsogo jgsogo mentioned this pull request May 4, 2021
Merged
@conan-center-bot conan-center-bot requested a review from danimtb May 5, 2021 11:59
@uilianries
Copy link
Member

/cc @Croydon as reviewer.

@conan-center-bot conan-center-bot merged commit 74588f1 into conan-io:master May 5, 2021
@jgsogo jgsogo deleted the grpc/grpc_cpp_plugin branch May 5, 2021 15:48
AlvaroFS pushed a commit to AlvaroFS/conan-center-index that referenced this pull request May 7, 2021
@jgsogo @prince-chrismc
@qchateau @AlvaroFS
(conan-io#5404) [grpc] Use build_modules to find executable grpc_cpp_…
1b89abc 
…plugin

* [google-cloud-cpp] Add library

* start to work on deps

* exclude tests, use config for CMake packages

* Add build module to find grpc_cpp_plugin

* [crc32c] Add recipe

* add crc32c dep

* update grpc

* remove, not for this PR

* [grpc] Use build_modules to find the grcp_cpp_plugin

* Update recipes/grpc/all/conanfile.py

Co-authored-by: Chris Mc <prince.chrismc@gmail.com>

* grpc: add version 1.37.1 (conan-io#6)

Generated and committed by [Conan Center Bot](https://github.com/qchateau/conan-center-bot)
Find more updatable recipes in the [GitHub Pages](https://qchateau.github.io/conan-center-bot/)

Co-authored-by: Quentin Chateau via Conan Center Bot <quentin.chateau@gmail.com>

Co-authored-by: Chris Mc <prince.chrismc@gmail.com>
Co-authored-by: Quentin Chateau via Conan Center Bot <quentin.chateau@gmail.com>
SpaceIm
SpaceIm reviewed May 20, 2021
View reviewed changes
recipes/grpc/all/conanfile.py
Comment on lines +237 to +240
if self.options.cpp_plugin:
module_target_rel_path = os.path.join("lib", "cmake", "grpc_cpp_plugin.cmake")
self.cpp_info.components["execs"].build_modules["cmake_find_package"] = [module_target_rel_path]
self.cpp_info.components["execs"].build_modules["cmake_find_package_multi"] = [module_target_rel_path]
Copy link
Contributor

Choose a reason for hiding this comment

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

How did it pass the hooks? This folder is not in buiddirs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SpaceIm SpaceIm SpaceIm left review comments

@prince-chrismc prince-chrismc prince-chrismc approved these changes

@uilianries uilianries uilianries approved these changes

@SSE4 SSE4 SSE4 approved these changes

@danimtb danimtb Awaiting requested review from danimtb

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jgsogo @conan-center-bot @prince-chrismc @uilianries @SSE4 @SpaceIm