-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
add libsystemd dependency to grpc #22436
add libsystemd dependency to grpc #22436
Conversation
implements #21253 |
This comment has been minimized.
This comment has been minimized.
@uilianries @SSE4 please approve |
recipes/grpc/all/conanfile.py
Outdated
@@ -97,6 +97,7 @@ def requirements(self): | |||
self.requires("re2/20230301") | |||
self.requires("zlib/[>=1.2.11 <2]") | |||
self.requires("protobuf/3.21.12", transitive_headers=True, transitive_libs=True, run=can_run(self)) | |||
self.requires("libsystemd/255") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's probably not needed on all platforms
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
libsystemd is available only on Linux, so with this change grpc can't be built on Windows/Macos
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
valid point. Thanks for saving CI time :)
e31e8e0
to
1ebbc37
Compare
This comment has been minimized.
This comment has been minimized.
67b74f9
to
750ec58
Compare
This comment has been minimized.
This comment has been minimized.
Running into this myself today, thanks for the PR. However, the "libsystemd/255" conan package appears to require conan v1.60. Not sure if it is appropriate to add that as an explicit check in this project so it is more clear to consumers? The root cause is that libsystemd/255 fails to import "move_folder_contents", a 1.60.0 feature.
|
Is there documentation anywhere for pulling down this repository and doing something like "export-pkg" locally so I can test out this PR? I have cloned the repo but I am getting weird errors when attempting to install/build/export-pkg the I am not 100% sure, but it seems like these are recipes but they don't have the original sources? So that's why I can't build/export them locally? |
@bhutch29 Doing |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! Thanks a lot for your contribution, we appreciate it.
Co-authored-by: Juan <[email protected]>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@juansblanco, thank you! |
Conan v1 pipeline ✔️All green in build 11 (
Conan v2 pipeline ✔️
All green in build 12 ( |
@uilianries @SSE4 please review |
Specify library name and version: grpc/all