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: fix build on armv6l #74882
grpc: fix build on armv6l #74882
Conversation
Patch doesn't apply anymore, and I hope it is not needed anymore three years later 🙈 |
It is likely still needed; upstream "declared bankruptcy" on supporting armv6l. |
Can you check if this makes sense against 1.52.1? diff --git a/CMakeLists.txt b/CMakeLists.txt
index feb37d1a7f..847b96de50 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -310,6 +310,7 @@ include(cmake/ccache.cmake)
include(cmake/abseil-cpp.cmake)
include(cmake/address_sorting.cmake)
+include(cmake/atomic.cmake)
include(cmake/benchmark.cmake)
include(cmake/cares.cmake)
include(cmake/protobuf.cmake)
@@ -2494,6 +2495,7 @@ target_link_libraries(grpc
${_gRPC_ADDRESS_SORTING_LIBRARIES}
${_gRPC_RE2_LIBRARIES}
${_gRPC_UPB_LIBRARIES}
+ ${_gRPC_ATOMIC_LIBRARIES}
${_gRPC_ALLTARGETS_LIBRARIES}
absl::cleanup
absl::flat_hash_map
diff --git a/cmake/atomic.cmake b/cmake/atomic.cmake
new file mode 100644
index 0000000000..8de8dcdf99
--- /dev/null
+++ b/cmake/atomic.cmake
@@ -0,0 +1,39 @@
+# Copyright 2019 gRPC authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+include(CheckCXXSourceCompiles)
+include(CMakePushCheckState)
+
+set(ATOMIC_TEST_CXX_SOURCE "
+#include <atomic>
+#include <cstdint>
+std::atomic<std::int64_t> v;
+int main() {
+ return v;
+}")
+
+# Determine if libatomic is needed for C++ atomics.
+cmake_push_check_state(RESET)
+set(CMAKE_REQUIRED_FLAGS -std=c++11)
+check_cxx_source_compiles("${ATOMIC_TEST_CXX_SOURCE}" HAVE_ATOMICS_WITHOUT_LIBATOMIC)
+if(NOT HAVE_ATOMICS_WITHOUT_LIBATOMIC)
+ set(CMAKE_REQUIRED_LIBRARIES atomic)
+ check_cxx_source_compiles("${ATOMIC_TEST_CXX_SOURCE}" HAVE_ATOMICS_WITH_LIBATOMIC)
+ if(HAVE_ATOMICS_WITH_LIBATOMIC)
+ set(_gRPC_ATOMIC_LIBRARIES atomic)
+ else()
+ message(FATAL_ERROR "Could not determine support for atomic operations.")
+ endif()
+endif()
+cmake_pop_check_state()
diff --git a/templates/CMakeLists.txt.template b/templates/CMakeLists.txt.template
index 7be9b0a2ae..bd9093ce0c 100644
--- a/templates/CMakeLists.txt.template
+++ b/templates/CMakeLists.txt.template
@@ -82,6 +82,8 @@
deps.append("${_gRPC_ADDRESS_SORTING_LIBRARIES}")
deps.append("${_gRPC_RE2_LIBRARIES}")
deps.append("${_gRPC_UPB_LIBRARIES}")
+ if target_dict['name'] == 'grpc':
+ deps.append("${_gRPC_ATOMIC_LIBRARIES}")
if target_dict['name'] in ['grpc_authorization_provider']:
deps.append("${_gRPC_RE2_LIBRARIES}")
deps.append("${_gRPC_ALLTARGETS_LIBRARIES}")
@@ -346,6 +348,7 @@
include(cmake/abseil-cpp.cmake)
include(cmake/address_sorting.cmake)
+ include(cmake/atomic.cmake)
include(cmake/benchmark.cmake)
include(cmake/cares.cmake)
include(cmake/protobuf.cmake) |
Yes, that looks good. I was just about to post my version (against master, I haven't checked if it will apply to 1.52.1) as well: lopsided98/grpc@164f552 |
Great, I'll fetch yours. Thanks for the quick turnaround. |
Motivation for this change
As of 1.25.0, grpc needs to be linked to libatomic on armv6l. I have submitted a PR upstream to do this, but I would like to add the patch to nixpkgs to fix the problem without waiting for a new release with the fix.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @LnL7 @marsam