From 25e8699b9c1f218fffbb8ebe59877c8865cdc878 Mon Sep 17 00:00:00 2001 From: Nicola Corti Date: Mon, 4 Nov 2024 11:04:47 -0800 Subject: [PATCH] Properly handle paths with spaces in autolinking Summary: Fixes https://github.com/facebook/react-native/issues/47364 Fixes https://github.com/facebook/react-native/issues/47377 Fixes https://github.com/facebook/react-native/issues/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 --- ...rateAutolinkingNewArchitecturesFileTask.kt | 11 +++++--- ...AutolinkingNewArchitecturesFileTaskTest.kt | 27 ++++++++++++++++--- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/packages/gradle-plugin/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/tasks/GenerateAutolinkingNewArchitecturesFileTask.kt b/packages/gradle-plugin/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/tasks/GenerateAutolinkingNewArchitecturesFileTask.kt index 0de22fc0eb5af0..38a31b86ac57b1 100644 --- a/packages/gradle-plugin/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/tasks/GenerateAutolinkingNewArchitecturesFileTask.kt +++ b/packages/gradle-plugin/react-native-gradle-plugin/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/packages/gradle-plugin/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/tasks/GenerateAutolinkingNewArchitecturesFileTaskTest.kt b/packages/gradle-plugin/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/tasks/GenerateAutolinkingNewArchitecturesFileTaskTest.kt index 7abe10595b2e91..b8d14bf7bfa172 100644 --- a/packages/gradle-plugin/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/tasks/GenerateAutolinkingNewArchitecturesFileTaskTest.kt +++ b/packages/gradle-plugin/react-native-gradle-plugin/src/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",