cmake/utils.cmake doesn't properly quote arguments to "ar"

[linuxturtle]

When utils.cmake builds a directive file for "ar" as part of building libshaderc/libshaderc_combined.a, it fails to properly quote the arguments to "ar", thus causing "ar" to fail if the build directory happens to contain any characters which might be construed to mean something special on the command line. Here's an example from me trying to build a debian package:

[313/319] cd "/build/shaderc-2018.0~dev+20/debian/build/libshaderc" && /usr/bin/ar -M < 
shaderc_combined.ar
FAILED: libshaderc/libshaderc_combined.a 
cd "/build/shaderc-2018.0~dev+20/debian/build/libshaderc" && /usr/bin/ar -M < shaderc_combined.ar
/usr/bin/ar: /build/shaderc-2018.0~dev: No such file or directory
+Syntax error in archive script, line 1

Here is a patch to fix the problem:

diff --git a/src/cmake/utils.cmake b/src/cmake/utils.cmake
index ed3c733..519d28f 100644
--- a/src/cmake/utils.cmake
+++ b/src/cmake/utils.cmake
@@ -186,9 +186,9 @@ function(shaderc_combine_static_lib new_target target)
       DEPENDS ${all_libs}
       COMMAND libtool -static -o ${libname} ${lib_target_list})
   else()
-    string(REPLACE ";" "> \naddlib $<TARGET_FILE:" temp_string "${all_libs}")
+    string(REPLACE ";" "> '\naddlib '$<TARGET_FILE:" temp_string "${all_libs}")
     set(start_of_file
-      "create ${libname}\naddlib $<TARGET_FILE:${temp_string}>")
+      "create '${libname}'\naddlib '$<TARGET_FILE:${temp_string}>'")
     set(build_script_file "${start_of_file}\nsave\nend\n")
 
     file(GENERATE OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/${new_target}.ar"

[linuxturtle]

Hmm, I should point out that this patch doesn't touch the "MSVC" or "APPLE" versions of output. I assume they'd need similar changes, but am not familiar with either environment.

[dneto0]

Thanks for the good bug report and suggested solution. I think this flow is used on OSX but not on Windows. OSX uses Bash as well, so this solution will work.

[dneto0]

Actually, I see the MSVC case there too.

I tried your solution and it doesn't seem to work when the directory name has spaces in it.

[linuxturtle]

Ugh, you are correct. I thought I had fixed the problem, but didn't realize in my quick test I was actually building in a directory w/out "+" or " " in its path :(.

After some searching, it looks like this is a problem with the "ar" scripting language. In https://sourceware.org/binutils/docs/binutils/ar-scripts.html#ar-scripts, both the "+" and characters are called out as having special meaning, with no way to escape them. It may be the only way to solve this problem is to call "ar" with command line arguments instead of using "-M". I'll keep playing with it to see if I can come up with a working solution, but the one I submitted at first definitely does not work.

[dneto0]

Ok. Thanks. Yeah, I was thinking a custom_command and associated custom_target would be the way to go.

[linuxturtle]

So, after much mucking about, it appears that there are only two ways to combine multiple .a static library archives together into a single library. By far, the easiest way is what's being done, writing a MRI script, and running it with "ar -M". The other way is to take each library archive, extract all of the .o files out of it into some temporary directory, then add them all to a new archive with ar from the command line. This method is fraught with peril, as more than one .a may contain .o files of the same name, so symbols could be overwritten and lost.

So, I think the only way to fix this is to do it in two steps via some sort of script:

1. Copy all of the individual library archives into a temporary directory, whose relative path we know to be free of special characters
2. Write a MRI script to combine the temporary libraries into a single archive library, and execute it with "ar -M"
3. Delete the temporary directory

I'll work on such a script.

[linuxturtle]

OK, this feels like an overly complicated kludge, but I can't figure out any other way to do it. I basically create a shell script which uses "realpath" to output the proper MRI script using relative paths (which hopefully won't contain any special characters). Then I pipe the output of that script into "ar -M". I've been able to successfully build using this patch.

One thing I wonder about, but I'm not versed enough in shaderc or cmake to answer it: why is libtool used to build/manipulate static libraries on the mac, but not on linux? Using libtool would seemingly make the problem a lot simpler.

--- a/src/cmake/utils.cmake
+++ b/src/cmake/utils.cmake
@@ -186,18 +186,18 @@
       DEPENDS ${all_libs}
       COMMAND libtool -static -o ${libname} ${lib_target_list})
   else()
-    string(REPLACE ";" "> \naddlib $<TARGET_FILE:" temp_string "${all_libs}")
+    string(REPLACE ";" ">\"\)\" \necho \"addlib \$\(realpath --relative-to=\"${CMAKE_CURRENT_BINARY_DIR}\" \"$<TARGET_FILE:" temp_string "${all_libs}")
     set(start_of_file
-      "create ${libname}\naddlib $<TARGET_FILE:${temp_string}>")
-    set(build_script_file "${start_of_file}\nsave\nend\n")
+      "echo \"create \$\(realpath --relative-to=\"${CMAKE_CURRENT_BINARY_DIR}\" \"${libname}\"\)\"\necho \"addlib \$\(realpath --relative-to=\"${CMAKE_CURRENT_BINARY_DIR}\" \"$<TARGET_FILE:${temp_string}>\"\)\"")
+    set(build_script_file "${start_of_file}\necho \"save\"\necho \"end\"\n")
 
-    file(GENERATE OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/${new_target}.ar"
+    file(GENERATE OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/${new_target}.sh"
         CONTENT ${build_script_file}
         CONDITION 1)
 
-    add_custom_command(OUTPUT  ${libname}
+    add_custom_command(OUTPUT ${libname}
       DEPENDS ${all_libs}
-      COMMAND ${CMAKE_AR} -M < ${new_target}.ar)
+      COMMAND /bin/sh ${new_target}.sh | ${CMAKE_AR} -M)
   endif()
 
   add_custom_target(${new_target}_genfile ALL

[kroppt]

@linuxturtle

So, I think the only way to fix this is to do it in two steps via some sort of script:

1. Copy all of the individual library archives into a temporary directory, whose relative path we know to be free of special characters

2. Write a MRI script to combine the temporary libraries into a single archive library, and execute it with "ar -M"

3. Delete the temporary directory

I'll work on such a script.

This idea could work, however it would not make sense to continue using the MRI script. The MRI script doesn't appear to support any escaping.

https://sourceware.org/binutils/docs/binutils/ar-scripts.html

The ar command language is not designed to be equivalent to the command-line options; in fact, it provides somewhat less control over archives. The only purpose of the command language is to ease the transition to GNU ar for developers who already have scripts written for the MRI "librarian" program.

If you are using a list of object files, using an MRI script adds no value. The code probably uses the script currently because it can extract object files from other static libraries in one step. That is not a one-step operation using the ordinary flags.

[qwertychouskie]

Just ran into this issue trying to build and package the latest SuperTuxKart RC (1.4-rc1). What is the recommended fix here to avoid build failures?

[kroppt]

@qwertychouskie

Just ran into this issue trying to build and package the latest SuperTuxKart RC (1.4-rc1). What is the recommended fix here to avoid build failures?

Build it in a path that does not contain any whitespace to avoid the problem.

[kroppt]

This was completed by the PR and can be closed. The fix is available starting in v2023.8.