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

CMake build script always sets CMAKE_RANLIB to an empty string. #1189

Open
ormandi opened this issue Apr 4, 2024 · 3 comments
Open

CMake build script always sets CMAKE_RANLIB to an empty string. #1189

ormandi opened this issue Apr 4, 2024 · 3 comments

Comments

@ormandi
Copy link

ormandi commented Apr 4, 2024

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,
@dotnwat
Copy link

dotnwat commented Aug 19, 2024

Any updates on this?

@szym
Copy link

szym commented Sep 8, 2024

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.

@dotnwat
Copy link

dotnwat commented Sep 9, 2024

@szym yes, we are also seeing this Arrow

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

No branches or pull requests

3 participants