Properly handle paths with spaces in autolinking

Summary: Fixes facebook#47364 Fixes facebook#47377 Fixes facebook#37124 We're having problems is a path contains a space ' ' because when autolinking, the `add_subdirectory()` function of CMake consider the path with space as 2 parameters. This fixes it by properly quoting the path. Changelog: [Android] [Fixed] - Properly handle paths with spaces in autolinking Differential Revision: D65434413
cortinico · Nov 4, 2024 · 25e8699 · 25e8699
1 parent 44f2a08
commit 25e8699
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 8 deletions.
diff --git a/...n/src/main/kotlin/com/facebook/react/tasks/GenerateAutolinkingNewArchitecturesFileTask.kt b/...n/src/main/kotlin/com/facebook/react/tasks/GenerateAutolinkingNewArchitecturesFileTask.kt
@@ -59,15 +59,15 @@ abstract class GenerateAutolinkingNewArchitecturesFileTask : DefaultTask() {
           val cxxModuleCMakeListsPath = dep.cxxModuleCMakeListsPath
           if (libraryName != null && cmakeListsPath != null) {
             // If user provided a custom cmakeListsPath, let's honor it.
-            val nativeFolderPath = cmakeListsPath.replace("CMakeLists.txt", "")
+            val nativeFolderPath = sanitizeCmakeListsPath(cmakeListsPath)
             addDirectoryString +=
-                "add_subdirectory($nativeFolderPath ${libraryName}_autolinked_build)"
+                "add_subdirectory(\"$nativeFolderPath\" ${libraryName}_autolinked_build)"
           }
           if (cxxModuleCMakeListsPath != null) {
             // If user provided a custom cxxModuleCMakeListsPath, let's honor it.
-            val nativeFolderPath = cxxModuleCMakeListsPath.replace("CMakeLists.txt", "")
+            val nativeFolderPath = sanitizeCmakeListsPath(cxxModuleCMakeListsPath)
             addDirectoryString +=
-                "\nadd_subdirectory($nativeFolderPath ${libraryName}_cxxmodule_autolinked_build)"
+                "\nadd_subdirectory(\"$nativeFolderPath\" ${libraryName}_cxxmodule_autolinked_build)"
           }
           addDirectoryString
         }
@@ -159,6 +159,9 @@ abstract class GenerateAutolinkingNewArchitecturesFileTask : DefaultTask() {
     const val COMPONENT_DESCRIPTOR_FILENAME = "ComponentDescriptors.h"
     const val COMPONENT_INCLUDE_PATH = "react/renderer/components"
 
+    internal fun sanitizeCmakeListsPath(cmakeListsPath: String): String =
+        cmakeListsPath.replace("CMakeLists.txt", "").replace(" ", "\\ ")
+
     // language=cmake
     val CMAKE_TEMPLATE =
         """

diff --git a/...c/test/kotlin/com/facebook/react/tasks/GenerateAutolinkingNewArchitecturesFileTaskTest.kt b/...c/test/kotlin/com/facebook/react/tasks/GenerateAutolinkingNewArchitecturesFileTaskTest.kt
@@ -11,6 +11,7 @@ import com.facebook.react.model.ModelAutolinkingConfigJson
 import com.facebook.react.model.ModelAutolinkingDependenciesJson
 import com.facebook.react.model.ModelAutolinkingDependenciesPlatformAndroidJson
 import com.facebook.react.model.ModelAutolinkingDependenciesPlatformJson
+import com.facebook.react.tasks.GenerateAutolinkingNewArchitecturesFileTask.Companion.sanitizeCmakeListsPath
 import com.facebook.react.tests.createTestTask
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.Rule
@@ -145,9 +146,9 @@ class GenerateAutolinkingNewArchitecturesFileTaskTest {
       # or link against a old prefab target (this is needed for React Native 0.76 on).
       set(REACTNATIVE_MERGED_SO true)
       
-      add_subdirectory(./a/directory/ aPackage_autolinked_build)
-      add_subdirectory(./another/directory/ anotherPackage_autolinked_build)
-      add_subdirectory(./another/directory/cxx/ anotherPackage_cxxmodule_autolinked_build)
+      add_subdirectory("./a/directory/" aPackage_autolinked_build)
+      add_subdirectory("./another/directory/with\ spaces/" anotherPackage_autolinked_build)
+      add_subdirectory("./another/directory/cxx/" anotherPackage_cxxmodule_autolinked_build)
       
       set(AUTOLINKED_LIBRARIES
         react_codegen_aPackage
@@ -258,6 +259,24 @@ class GenerateAutolinkingNewArchitecturesFileTaskTest {
                 .trimIndent())
   }
 
+  @Test
+  fun sanitizeCmakeListsPath_withPathEndingWithFileName_removesFilename() {
+    val input = "./a/directory/CMakeLists.txt"
+    assertThat(sanitizeCmakeListsPath(input)).isEqualTo("./a/directory/")
+  }
+
+  @Test
+  fun sanitizeCmakeListsPath_withSpaces_removesSpaces() {
+    val input = "./a/dir ectory/with spaces/"
+    assertThat(sanitizeCmakeListsPath(input)).isEqualTo("./a/dir\\ ectory/with\\ spaces/")
+  }
+
+  @Test
+  fun sanitizeCmakeListsPath_withPathEndingWithFileNameAndSpaces_sanitizesIt() {
+    val input = "./a/dir ectory/CMakeLists.txt"
+    assertThat(sanitizeCmakeListsPath(input)).isEqualTo("./a/dir\\ ectory/")
+  }
+
   private val testDependencies =
       listOf(
           ModelAutolinkingDependenciesPlatformAndroidJson(
@@ -276,7 +295,7 @@ class GenerateAutolinkingNewArchitecturesFileTaskTest {
               buildTypes = emptyList(),
               libraryName = "anotherPackage",
               componentDescriptors = listOf("AnotherPackageComponentDescriptor"),
-              cmakeListsPath = "./another/directory/CMakeLists.txt",
+              cmakeListsPath = "./another/directory/with spaces/CMakeLists.txt",
               cxxModuleCMakeListsPath = "./another/directory/cxx/CMakeLists.txt",
               cxxModuleHeaderName = "AnotherCxxModule",
               cxxModuleCMakeListsModuleName = "another_cxxModule",