rules_foreign_cc icon indicating copy to clipboard operation
rules_foreign_cc copied to clipboard

CMake build script always sets `CMAKE_RANLIB` to an empty string.

Open ormandi opened this issue 10 months ago • 3 comments

The cmake build script always overrides the cache value CMAKE_RANLIB with an empty string.

This is, I assume, due to the following snippet:

    # However, if no CMAKE_RANLIB was passed, pass the empty value for it explicitly,
    # as it is legacy and autodetection of ranlib made by CMake automatically
    # breaks some cross compilation builds,
    # see https://github.com/envoyproxy/envoy/pull/6991
    if not params.cache.get("CMAKE_RANLIB"):
        params.cache.update({"CMAKE_RANLIB": ""})

As far as I can tell, the condition of this if-statement always evaluates to True since as params.cache is built the item corresponding to the key CMAKE_RANLIB is popped (i.e. removed) if it was present (here) and naturally evaluates to True if the mentioned key was not present at all.

I assume to fix this one need to do basically do two things:

  • Check the condition of the if-statement based on the input variable user_cache which stores the cache entries as they are taken
  • And put the original value back into params.cache from user_cache if it is present. I.e. something like this:
diff --git a/foreign_cc/private/cmake_script.bzl b/foreign_cc/private/cmake_script.bzl
index bfe21bd..b4410ee 100644
--- a/foreign_cc/private/cmake_script.bzl
+++ b/foreign_cc/private/cmake_script.bzl
@@ -113,8 +113,15 @@ def create_cmake_script(
     # as it is legacy and autodetection of ranlib made by CMake automatically
     # breaks some cross compilation builds,
     # see https://github.com/envoyproxy/envoy/pull/6991
-    if not params.cache.get("CMAKE_RANLIB"):
+    # We do this by checking the key in `user_cache` since from `params.cache`
+    # this value is moved even if it was present before by the following call:
+    # https://github.com/bazelbuild/rules_foreign_cc/blob/99d018f0592bd7de492cbc2fa5e44828aea1ef4c/foreign_cc/private/cmake_script.bzl#L290
+    # Additionally, we also put the original value back if it got removed by
+    # previously.
+    if not user_cache.get("CMAKE_RANLIB"):
         params.cache.update({"CMAKE_RANLIB": ""})
+    else:
+        params.cache.update({"CMAKE_RANLIB": user_cache.get("CMAKE_RANLIB")})

     # Avoid CMake passing the wrong linker flags when cross compiling
     # by setting CMAKE_SYSTEM_NAME and CMAKE_SYSTEM_PROCESSOR,

ormandi avatar Apr 04 '24 16:04 ormandi

Any updates on this?

dotnwat avatar Aug 19 '24 19:08 dotnwat

I think this is currently preventing Arrow (17.0.0) from being built with the cmake rule:

CMake Error at cmake_modules/ThirdpartyToolchain.cmake:949 (find_program):
  Could not find EP_CMAKE_RANLIB using the following names: :
Call Stack (most recent call first):
  CMakeLists.txt:544 (include)

I see -DCMAKE_RANLIB= in the cmake command line.

szym avatar Sep 08 '24 20:09 szym

@szym yes, we are also seeing this Arrow

dotnwat avatar Sep 09 '24 15:09 dotnwat