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

grpc: Retry CMake configuration if it fails #23153

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions recipes/grpc/all/conanfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import yaml

from conan import ConanFile
from conan.errors import ConanInvalidConfiguration
from conan.errors import ConanInvalidConfiguration, ConanException
from conan.tools.apple import is_apple_os
from conan.tools.build import cross_building, valid_min_cppstd, check_min_cppstd
from conan.tools.cmake import cmake_layout, CMake, CMakeToolchain, CMakeDeps
Expand Down Expand Up @@ -198,6 +198,11 @@ def generate(self):
tc.cache_variables["gRPC_BUILD_GRPC_RUBY_PLUGIN"] = self.options.ruby_plugin
tc.cache_variables["gRPC_BUILD_GRPCPP_OTEL_PLUGIN"] = self.options.get_safe("otel_plugin", False)

# Never download unnecessary archives
# (supported in gRPC >= 1.62.0)
tc.cache_variables["gRPC_DOWNLOAD_ARCHIVES"] = False
Copy link
Contributor

@samuel-emrys samuel-emrys Jun 15, 2024

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.

Copy link
Author

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.



# Consumed targets (abseil) via interface target_compiler_feature can propagate newer standards
if not valid_min_cppstd(self, self._cxxstd_required):
tc.cache_variables["CMAKE_CXX_STANDARD"] = self._cxxstd_required
Expand Down Expand Up @@ -250,7 +255,13 @@ def _patch_sources(self):
def build(self):
self._patch_sources()
cmake = CMake(self)
cmake.configure()

# The CMake configure step can fail spuriously, but succeed on a retry
try:
cmake.configure()
except ConanException:
cmake.configure()

cmake.build()

@property
Expand Down