-
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
grpc: Retry CMake configuration if it fails #23153
base: master
Are you sure you want to change the base?
Conversation
7553546
to
0bfc73a
Compare
This comment has been minimized.
This comment has been minimized.
The gRPC CMake configuration step can fail to due to a failure when downloading third party archives. These failures are expected, and the user is instructed to retry. So, if configuration fails, retry it Also set the `gRPC_DOWNLOAD_ARCHIVES` CMake flag to `False`, to avoid downloading these unnecessary archives in the first place, in versions of gRPC that understand the flag
0bfc73a
to
0ac619c
Compare
This comment has been minimized.
This comment has been minimized.
Conan v1 pipeline ✔️All green in build 4 (
Conan v2 pipeline ❌
The v2 pipeline failed. Please, review the errors and note this is required for pull requests to be merged. In case this recipe is still not ported to Conan 2.x, please, ping See details:Failure in build 5 (
Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability. |
@@ -174,6 +174,11 @@ def generate(self): | |||
tc.cache_variables["gRPC_BUILD_GRPC_PYTHON_PLUGIN"] = self.options.python_plugin | |||
tc.cache_variables["gRPC_BUILD_GRPC_RUBY_PLUGIN"] = self.options.ruby_plugin | |||
|
|||
# Never download unnecessary archives | |||
# (supported in gRPC >= 1.62.0) | |||
tc.cache_variables["gRPC_DOWNLOAD_ARCHIVES"] = False |
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.
I'm not particularly familiar with gRPC
- so to confirm, these archives aren't required? I agree that a conan recipe should never attempt to download external files, but if they are required then they should be added to conandata.yml
and downloaded/extracted in the source()
method.
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.
Based on the issue where the flag was introduced, it seems that they're not required for anything directly related to building or testing gRPC
grpc/grpc#34587 (comment)
In addition, for all but one of the archives, no attempt is made to download the archives -- the CMake script checks for the presence of directories that are (in most cases) already present in the Conan source checkout.
@RubenRBS looks like the v2 pipeline is failing on a weird error - it can't find itself as a dependency? |
@js-nano from @RubenRBS on slack, to get the conan 2 pipeline passing you'll need to simplify the def build_requirements(self):
if not self._is_legacy_one_profile:
# For the grpc-cpp-plugin executable at build time
- self.tool_requires(self.tested_reference_str)
self.tool_requires("protobuf/<host_version>") This is due to a limitation in the existing CI infrastructure |
The gRPC CMake configuration step can fail to due to a failure when downloading third party archives. These failures are expected, and the user is instructed to retry. So, if configuration fails, retry it
Also set the
gRPC_DOWNLOAD_ARCHIVES
CMake flag toFalse
, to avoid downloading these unnecessary archives in the first place, in versions of gRPC that understand the flagSpecify library name and version: grpc/all
This fixes #23094