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: fix build on armv6l #74882

Merged
merged 1 commit into from Dec 3, 2019
Merged

Conversation

lopsided98
Copy link
Contributor

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
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @LnL7 @marsam

@matthewbauer matthewbauer merged commit 49df3e3 into NixOS:master Dec 3, 2019
@lopsided98 lopsided98 deleted the grpc-armv6l branch December 3, 2019 19:48
@mweinelt
Copy link
Member

mweinelt commented Mar 1, 2023

Patch doesn't apply anymore, and I hope it is not needed anymore three years later 🙈

@lopsided98
Copy link
Contributor Author

It is likely still needed; upstream "declared bankruptcy" on supporting armv6l.

@mweinelt
Copy link
Member

mweinelt commented Mar 1, 2023

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)

@lopsided98
Copy link
Contributor Author

lopsided98 commented Mar 1, 2023

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

@mweinelt
Copy link
Member

mweinelt commented Mar 1, 2023

Great, I'll fetch yours. Thanks for the quick turnaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants