From 54e9994a6b5f48d761f1dc6cefbe9c14f4ddda50 Mon Sep 17 00:00:00 2001
From: Tobias Mayer <tobim@fastmail.fm>
Date: Mon, 14 Aug 2023 08:57:36 +0200
Subject: [PATCH] grpc: refactor cxxStandard selection

GRPC is less aggressive in overriding `CMAKE_CXX_STANDARD` nowadays.
This allows us to take a less invasive approach to ensure that the
provided abseil package is configured with the same implementation
for the provided c++17 compatibility shims.
Instead of always setting `CMAKE_CXX_STANDARD`, we only do it to
override hard-coded downgrade on darwin. With that, we can use the
default configuration for abseil-cpp, reducing the number of instances
of that library in the build closure to one.
---
 .../libraries/arrow-cpp/default.nix           |  7 ++---
 .../libraries/google-cloud-cpp/default.nix    |  1 -
 pkgs/development/libraries/grpc/default.nix   | 27 ++++++++++---------
 pkgs/top-level/all-packages.nix               |  7 +----
 4 files changed, 17 insertions(+), 25 deletions(-)

diff --git a/pkgs/development/libraries/arrow-cpp/default.nix b/pkgs/development/libraries/arrow-cpp/default.nix
index f61d9aa2215..4d89ce13967 100644
--- a/pkgs/development/libraries/arrow-cpp/default.nix
+++ b/pkgs/development/libraries/arrow-cpp/default.nix
@@ -39,11 +39,8 @@
 , enableShared ? !stdenv.hostPlatform.isStatic
 , enableFlight ? true
 , enableJemalloc ? !stdenv.isDarwin
-  # boost/process is broken in 1.69 on darwin, but fixed in 1.70 and
-  # non-existent in older versions
-  # see https://github.com/boostorg/process/issues/55
-, enableS3 ? (!stdenv.isDarwin) || (lib.versionOlder boost.version "1.69" || lib.versionAtLeast boost.version "1.70")
-, enableGcs ? (!stdenv.isDarwin) && (lib.versionAtLeast grpc.cxxStandard "17") # google-cloud-cpp is not supported on darwin, needs to support C++17
+, enableS3 ? true
+, enableGcs ? !stdenv.isDarwin
 }:
 
 assert lib.asserts.assertMsg
diff --git a/pkgs/development/libraries/google-cloud-cpp/default.nix b/pkgs/development/libraries/google-cloud-cpp/default.nix
index f285aee0b7c..9aa1284bbee 100644
--- a/pkgs/development/libraries/google-cloud-cpp/default.nix
+++ b/pkgs/development/libraries/google-cloud-cpp/default.nix
@@ -120,7 +120,6 @@ stdenv.mkDerivation rec {
     # this adds a good chunk of time to the build
     "-DBUILD_TESTING:BOOL=ON"
     "-DGOOGLE_CLOUD_CPP_ENABLE_EXAMPLES:BOOL=OFF"
-    "-DCMAKE_CXX_STANDARD=${grpc.cxxStandard}"
   ] ++ lib.optionals (apis != [ "*" ]) [
     "-DGOOGLE_CLOUD_CPP_ENABLE=${lib.concatStringsSep ";" apis}"
   ];
diff --git a/pkgs/development/libraries/grpc/default.nix b/pkgs/development/libraries/grpc/default.nix
index 7435ac723f7..70034f2d2e4 100644
--- a/pkgs/development/libraries/grpc/default.nix
+++ b/pkgs/development/libraries/grpc/default.nix
@@ -56,10 +56,22 @@ stdenv.mkDerivation rec {
     "-DgRPC_PROTOBUF_PROVIDER=package"
     "-DgRPC_ABSL_PROVIDER=package"
     "-DBUILD_SHARED_LIBS=ON"
-    "-DCMAKE_CXX_STANDARD=${passthru.cxxStandard}"
   ] ++ lib.optionals (stdenv.hostPlatform != stdenv.buildPlatform) [
     "-D_gRPC_PROTOBUF_PROTOC_EXECUTABLE=${buildPackages.protobuf}/bin/protoc"
-  ];
+  ]
+  # The build scaffold defaults to c++14 on darwin, even when the compiler uses
+  # a more recent c++ version by default [1]. However, downgrades are
+  # problematic, because the compatibility types in abseil will have different
+  # interface definitions than the ones used for building abseil itself.
+  # [1] https://github.com/grpc/grpc/blob/v1.57.0/CMakeLists.txt#L239-L243
+  ++ (let
+    defaultCxxIsOlderThan17 =
+      (stdenv.cc.isClang && lib.versionAtLeast stdenv.cc.cc.version "16.0")
+       || (stdenv.cc.isGNU && lib.versionAtLeast stdenv.cc.cc.version "11.0");
+    in lib.optionals (stdenv.hostPlatform.isDarwin && defaultCxxIsOlderThan17)
+  [
+    "-DCMAKE_CXX_STANDARD=17"
+  ]);
 
   # CMake creates a build directory by default, this conflicts with the
   # basel BUILD file on case-insensitive filesystems.
@@ -81,17 +93,6 @@ stdenv.mkDerivation rec {
 
   enableParallelBuilds = true;
 
-  passthru.cxxStandard =
-    let
-      # Needs to be compiled with -std=c++11 for clang < 11. Interestingly this is
-      # only an issue with the useLLVM stdenv, not the darwin stdenv…
-      # https://github.com/grpc/grpc/issues/26473#issuecomment-860885484
-      useLLVMAndOldCC = (stdenv.hostPlatform.useLLVM or false) && lib.versionOlder stdenv.cc.cc.version "11.0";
-      # With GCC 9 (current aarch64-linux) it fails with c++17 but OK with c++14.
-      useOldGCC = !(stdenv.hostPlatform.useLLVM or false) && lib.versionOlder stdenv.cc.cc.version "10";
-    in
-    (if useLLVMAndOldCC then "11" else if useOldGCC then "14" else "17");
-
   passthru.tests = {
     inherit (python3.pkgs) grpcio-status grpcio-tools;
     inherit arrow-cpp;
diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix
index 7f636a5ae62..2c6a00a2867 100644
--- a/pkgs/top-level/all-packages.nix
+++ b/pkgs/top-level/all-packages.nix
@@ -21597,12 +21597,7 @@ with pkgs;
 
   grilo-plugins = callPackage ../development/libraries/grilo-plugins { };
 
-  grpc = callPackage ../development/libraries/grpc {
-    # grpc builds with c++17 so abseil must also be built that way
-    abseil-cpp = abseil-cpp_202206.override {
-      cxxStandard = grpc.cxxStandard;
-    };
-  };
+  grpc = callPackage ../development/libraries/grpc { };
 
   gsettings-qt = libsForQt5.callPackage ../development/libraries/gsettings-qt { };