RNGP - fix: use relative paths for gradle exec invocations (#36080)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36080

For Android release builds on Windows, gradle release build fails if there are spaces in path (https://github.com/facebook/react-native/issues/34878). This is due to gradle improperly handling arguments with spaces (this is also [an open issue](https://github.com/gradle/gradle/issues/6072) on Gradle). Since the Hermes compilation and other Gradle exec invocations involve arguments which will contain spaces (if there are spaces in your path), this also means it is hard to get around this by simply escaping the spaces (eg: by using double quotes), since these arguments are not properly handled by Gradle itself.

As a workaround, this PR uses relative paths for all Gradle commands invoked for Android. As long as there aren't any spaces in the react-native directory structure (i.e this repo), this fix should work.

## Changelog

[Android][Fixed] - Used relative paths for gradle commands

Pull Request resolved: https://github.com/facebook/react-native/pull/36076

Test Plan: `npx react-native run-android` builds and runs the app successfully on Android device, when run inside an RN0711 project with a path containing spaces (and with the changes in this PR applied) on Windows. This includes release builds (i.e with the `--variant=release` flag).

Reviewed By: cipolleschi

Differential Revision: D43080177

Pulled By: cortinico

fbshipit-source-id: 7625f3502af47e9b28c6fc7dfe1459d7c7f1362d

packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/tasks/BundleHermesCTask.kt

@@ -7,6 +7,7 @@
 
 package com.facebook.react.tasks
 
+import com.facebook.react.utils.Os.cliPath
 import com.facebook.react.utils.detectOSAwareHermesCommand
 import com.facebook.react.utils.moveTo
 import com.facebook.react.utils.windowsAwareCommandLine
@@ -135,8 +136,9 @@ abstract class BundleHermesCTask : DefaultTask() {
   internal fun getBundleCommand(bundleFile: File, sourceMapFile: File): List<Any> =
       windowsAwareCommandLine(
           buildList {
+            val rootFile = root.get().asFile
             addAll(nodeExecutableAndArgs.get())
-            add(cliFile.get().asFile.absolutePath)
+            add(cliFile.get().asFile.cliPath(rootFile))
             add(bundleCommand.get())
             add("--platform")
             add("android")
@@ -144,16 +146,16 @@ abstract class BundleHermesCTask : DefaultTask() {
             add(devEnabled.get().toString())
             add("--reset-cache")
             add("--entry-file")
-            add(entryFile.get().asFile.toString())
+            add(entryFile.get().asFile.cliPath(rootFile))
             add("--bundle-output")
-            add(bundleFile.toString())
+            add(bundleFile.cliPath(rootFile))
             add("--assets-dest")
-            add(resourcesDir.get().asFile.toString())
+            add(resourcesDir.get().asFile.cliPath(rootFile))
             add("--sourcemap-output")
-            add(sourceMapFile.toString())
+            add(sourceMapFile.cliPath(rootFile))
             if (bundleConfig.isPresent) {
               add("--config")
-              add(bundleConfig.get().asFile.absolutePath)
+              add(bundleConfig.get().asFile.cliPath(rootFile))
             }
             add("--minify")
             add(minifyEnabled.get().toString())
@@ -165,26 +167,30 @@ abstract class BundleHermesCTask : DefaultTask() {
       hermesCommand: String,
       bytecodeFile: File,
       bundleFile: File
-  ): List<Any> =
-      windowsAwareCommandLine(
-          hermesCommand,
-          "-emit-binary",
-          "-out",
-          bytecodeFile.absolutePath,
-          bundleFile.absolutePath,
-          *hermesFlags.get().toTypedArray())
+  ): List<Any> {
+    val rootFile = root.get().asFile
+    return windowsAwareCommandLine(
+        hermesCommand,
+        "-emit-binary",
+        "-out",
+        bytecodeFile.cliPath(rootFile),
+        bundleFile.cliPath(rootFile),
+        *hermesFlags.get().toTypedArray())
+  }
 
   internal fun getComposeSourceMapsCommand(
       composeScript: File,
       packagerSourceMap: File,
       compilerSourceMap: File,
       outputSourceMap: File
-  ): List<Any> =
-      windowsAwareCommandLine(
-          *nodeExecutableAndArgs.get().toTypedArray(),
-          composeScript.absolutePath,
-          packagerSourceMap.toString(),
-          compilerSourceMap.toString(),
-          "-o",
-          outputSourceMap.toString())
+  ): List<Any> {
+    val rootFile = root.get().asFile
+    return windowsAwareCommandLine(
+        *nodeExecutableAndArgs.get().toTypedArray(),
+        composeScript.cliPath(rootFile),
+        packagerSourceMap.cliPath(rootFile),
+        compilerSourceMap.cliPath(rootFile),
+        "-o",
+        outputSourceMap.cliPath(rootFile))
+  }
 }

packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/tasks/GenerateCodegenArtifactsTask.kt

@@ -8,6 +8,7 @@
 package com.facebook.react.tasks
 
 import com.facebook.react.utils.JsonUtils
+import com.facebook.react.utils.Os.cliPath
 import com.facebook.react.utils.windowsAwareCommandLine
 import org.gradle.api.file.Directory
 import org.gradle.api.file.DirectoryProperty
@@ -63,16 +64,17 @@ abstract class GenerateCodegenArtifactsTask : Exec() {
   }
 
   internal fun setupCommandLine(libraryName: String, codegenJavaPackageName: String) {
+    val workingDir = project.projectDir
     commandLine(
         windowsAwareCommandLine(
             *nodeExecutableAndArgs.get().toTypedArray(),
-            reactNativeDir.file("scripts/generate-specs-cli.js").get().asFile.absolutePath,
+            reactNativeDir.file("scripts/generate-specs-cli.js").get().asFile.cliPath(workingDir),
             "--platform",
             "android",
             "--schemaPath",
-            generatedSchemaFile.get().asFile.absolutePath,
+            generatedSchemaFile.get().asFile.cliPath(workingDir),
             "--outputDir",
-            generatedSrcDir.get().asFile.absolutePath,
+            generatedSrcDir.get().asFile.cliPath(workingDir),
             "--libraryName",
             libraryName,
             "--javaPackageName",

packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/tasks/GenerateCodegenSchemaTask.kt

@@ -7,6 +7,7 @@
 
 package com.facebook.react.tasks
 
+import com.facebook.react.utils.Os.cliPath
 import com.facebook.react.utils.windowsAwareCommandLine
 import org.gradle.api.file.DirectoryProperty
 import org.gradle.api.file.RegularFile
@@ -63,6 +64,7 @@ abstract class GenerateCodegenSchemaTask : Exec() {
   }
 
   internal fun setupCommandLine() {
+    val workingDir = project.projectDir
     commandLine(
         windowsAwareCommandLine(
             *nodeExecutableAndArgs.get().toTypedArray(),
@@ -70,11 +72,11 @@ abstract class GenerateCodegenSchemaTask : Exec() {
                 .file("lib/cli/combine/combine-js-to-schema-cli.js")
                 .get()
                 .asFile
-                .absolutePath,
+                .cliPath(workingDir),
             "--platform",
             "android",
-            generatedSchemaFile.get().asFile.absolutePath,
-            jsRootDir.asFile.get().absolutePath,
+            generatedSchemaFile.get().asFile.cliPath(workingDir),
+            jsRootDir.asFile.get().cliPath(workingDir),
         ))
   }
 }

packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/Os.kt

@@ -7,7 +7,9 @@
 
 package com.facebook.react.utils
 
-object Os {
+import java.io.File
+
+internal object Os {
 
   fun isWindows(): Boolean =
       System.getProperty("os.name")?.lowercase()?.contains("windows") ?: false
@@ -28,4 +30,15 @@ object Os {
           it
         }
       }
+
+  /**
+   * As Gradle doesn't support well path with spaces on Windows, we need to return relative path on
+   * Win. On Linux & Mac we'll default to return absolute path.
+   */
+  fun File.cliPath(base: File): String =
+      if (isWindows()) {
+        this.relativeTo(base).path
+      } else {
+        this.absolutePath
+      }
 }

packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/PathUtils.kt

@@ -11,6 +11,7 @@ package com.facebook.react.utils
 
 import com.facebook.react.ReactExtension
 import com.facebook.react.model.ModelPackageJson
+import com.facebook.react.utils.Os.cliPath
 import java.io.File
 import org.gradle.api.Project
 
@@ -130,7 +131,7 @@ internal fun detectOSAwareHermesCommand(projectRoot: File, hermesCommand: String
   val builtHermesc =
       getBuiltHermescFile(projectRoot, System.getenv("REACT_NATIVE_OVERRIDE_HERMES_DIR"))
   if (builtHermesc.exists()) {
-    return builtHermesc.absolutePath
+    return builtHermesc.cliPath(projectRoot)
   }
 
   // 3. If the react-native contains a pre-built hermesc, use it.
@@ -142,7 +143,7 @@ internal fun detectOSAwareHermesCommand(projectRoot: File, hermesCommand: String
 
   val prebuiltHermes = File(projectRoot, prebuiltHermesPath)
   if (prebuiltHermes.exists()) {
-    return prebuiltHermes.absolutePath
+    return prebuiltHermes.cliPath(projectRoot)
   }
 
   error(

packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/tasks/BundleHermesCTaskTest.kt

@@ -7,7 +7,9 @@
 
 package com.facebook.react.tasks
 
+import com.facebook.react.tests.OS
 import com.facebook.react.tests.OsRule
+import com.facebook.react.tests.WithOs
 import com.facebook.react.tests.createTestTask
 import java.io.File
 import org.junit.Assert.*
@@ -205,6 +207,7 @@ class BundleHermesCTaskTest {
     val task =
         createTestTask<BundleHermesCTask> {
           it.nodeExecutableAndArgs.set(listOf("node", "arg1", "arg2"))
+          it.root.set(tempFolder.root)
           it.cliFile.set(cliFile)
           it.bundleCommand.set("bundle")
           it.devEnabled.set(true)
@@ -244,6 +247,60 @@ class BundleHermesCTaskTest {
     assertEquals(24, bundleCommand.size)
   }
 
+  @Test
+  @WithOs(OS.WIN)
+  fun getBundleCommand_onWindows_returnsWinValidCommandsPaths() {
+    val entryFile = tempFolder.newFile("index.js")
+    val cliFile = tempFolder.newFile("cli.js")
+    val bundleFile = tempFolder.newFile("bundle.js")
+    val sourceMapFile = tempFolder.newFile("bundle.js.map")
+    val resourcesDir = tempFolder.newFolder("res")
+    val bundleConfig = tempFolder.newFile("bundle.config")
+    val task =
+        createTestTask<BundleHermesCTask> {
+          it.nodeExecutableAndArgs.set(listOf("node", "arg1", "arg2"))
+          it.root.set(tempFolder.root)
+          it.cliFile.set(cliFile)
+          it.bundleCommand.set("bundle")
+          it.devEnabled.set(true)
+          it.entryFile.set(entryFile)
+          it.resourcesDir.set(resourcesDir)
+          it.bundleConfig.set(bundleConfig)
+          it.minifyEnabled.set(true)
+          it.extraPackagerArgs.set(listOf("--read-global-cache"))
+        }
+
+    val bundleCommand = task.getBundleCommand(bundleFile, sourceMapFile)
+
+    assertEquals("cmd", bundleCommand[0])
+    assertEquals("/c", bundleCommand[1])
+    assertEquals("node", bundleCommand[2])
+    assertEquals("arg1", bundleCommand[3])
+    assertEquals("arg2", bundleCommand[4])
+    assertEquals(cliFile.relativeTo(tempFolder.root).path, bundleCommand[5])
+    assertEquals("bundle", bundleCommand[6])
+    assertEquals("--platform", bundleCommand[7])
+    assertEquals("android", bundleCommand[8])
+    assertEquals("--dev", bundleCommand[9])
+    assertEquals("true", bundleCommand[10])
+    assertEquals("--reset-cache", bundleCommand[11])
+    assertEquals("--entry-file", bundleCommand[12])
+    assertEquals(entryFile.relativeTo(tempFolder.root).path, bundleCommand[13])
+    assertEquals("--bundle-output", bundleCommand[14])
+    assertEquals(bundleFile.relativeTo(tempFolder.root).path, bundleCommand[15])
+    assertEquals("--assets-dest", bundleCommand[16])
+    assertEquals(resourcesDir.relativeTo(tempFolder.root).path, bundleCommand[17])
+    assertEquals("--sourcemap-output", bundleCommand[18])
+    assertEquals(sourceMapFile.relativeTo(tempFolder.root).path, bundleCommand[19])
+    assertEquals("--config", bundleCommand[20])
+    assertEquals(bundleConfig.relativeTo(tempFolder.root).path, bundleCommand[21])
+    assertEquals("--minify", bundleCommand[22])
+    assertEquals("true", bundleCommand[23])
+    assertEquals("--read-global-cache", bundleCommand[24])
+    assertEquals("--verbose", bundleCommand[25])
+    assertEquals(26, bundleCommand.size)
+  }
+
   @Test
   fun getBundleCommand_withoutConfig_returnsCommandWithoutConfig() {
     val entryFile = tempFolder.newFile("index.js")
@@ -254,6 +311,7 @@ class BundleHermesCTaskTest {
     val task =
         createTestTask<BundleHermesCTask> {
           it.nodeExecutableAndArgs.set(listOf("node", "arg1", "arg2"))
+          it.root.set(tempFolder.root)
           it.cliFile.set(cliFile)
           it.bundleCommand.set("bundle")
           it.devEnabled.set(true)
@@ -274,7 +332,10 @@ class BundleHermesCTaskTest {
     val bytecodeFile = tempFolder.newFile("bundle.js.hbc")
     val bundleFile = tempFolder.newFile("bundle.js")
     val task =
-        createTestTask<BundleHermesCTask> { it.hermesFlags.set(listOf("my-custom-hermes-flag")) }
+        createTestTask<BundleHermesCTask> {
+          it.root.set(tempFolder.root)
+          it.hermesFlags.set(listOf("my-custom-hermes-flag"))
+        }
 
     val hermesCommand = task.getHermescCommand(customHermesc, bytecodeFile, bundleFile)
 
@@ -287,6 +348,31 @@ class BundleHermesCTaskTest {
     assertEquals(6, hermesCommand.size)
   }
 
+  @Test
+  @WithOs(OS.WIN)
+  fun getHermescCommand_onWindows_returnsRelativePaths() {
+    val customHermesc = "hermesc"
+    val bytecodeFile = tempFolder.newFile("bundle.js.hbc")
+    val bundleFile = tempFolder.newFile("bundle.js")
+    val task =
+        createTestTask<BundleHermesCTask> {
+          it.root.set(tempFolder.root)
+          it.hermesFlags.set(listOf("my-custom-hermes-flag"))
+        }
+
+    val hermesCommand = task.getHermescCommand(customHermesc, bytecodeFile, bundleFile)
+
+    assertEquals("cmd", hermesCommand[0])
+    assertEquals("/c", hermesCommand[1])
+    assertEquals(customHermesc, hermesCommand[2])
+    assertEquals("-emit-binary", hermesCommand[3])
+    assertEquals("-out", hermesCommand[4])
+    assertEquals(bytecodeFile.relativeTo(tempFolder.root).path, hermesCommand[5])
+    assertEquals(bundleFile.relativeTo(tempFolder.root).path, hermesCommand[6])
+    assertEquals("my-custom-hermes-flag", hermesCommand[7])
+    assertEquals(8, hermesCommand.size)
+  }
+
   @Test
   fun getComposeSourceMapsCommand_returnsCorrectCommand() {
     val packagerMap = tempFolder.newFile("bundle.js.packager.map")
@@ -296,6 +382,7 @@ class BundleHermesCTaskTest {
     val composeSourceMapsFile = File(reactNativeDir, "scripts/compose-source-maps.js")
     val task =
         createTestTask<BundleHermesCTask> {
+          it.root.set(tempFolder.root)
           it.nodeExecutableAndArgs.set(listOf("node", "arg1", "arg2"))
         }
 
@@ -312,4 +399,34 @@ class BundleHermesCTaskTest {
     assertEquals(outputMap.absolutePath, composeSourcemapCommand[7])
     assertEquals(8, composeSourcemapCommand.size)
   }
+
+  @Test
+  @WithOs(OS.WIN)
+  fun getComposeSourceMapsCommand_onWindows_returnsRelativePaths() {
+    val packagerMap = tempFolder.newFile("bundle.js.packager.map")
+    val compilerMap = tempFolder.newFile("bundle.js.compiler.map")
+    val outputMap = tempFolder.newFile("bundle.js.map")
+    val reactNativeDir = tempFolder.newFolder("node_modules/react-native")
+    val composeSourceMapsFile = File(reactNativeDir, "scripts/compose-source-maps.js")
+    val task =
+        createTestTask<BundleHermesCTask> {
+          it.root.set(tempFolder.root)
+          it.nodeExecutableAndArgs.set(listOf("node", "arg1", "arg2"))
+        }
+
+    val composeSourcemapCommand =
+        task.getComposeSourceMapsCommand(composeSourceMapsFile, packagerMap, compilerMap, outputMap)
+
+    assertEquals("cmd", composeSourcemapCommand[0])
+    assertEquals("/c", composeSourcemapCommand[1])
+    assertEquals("node", composeSourcemapCommand[2])
+    assertEquals("arg1", composeSourcemapCommand[3])
+    assertEquals("arg2", composeSourcemapCommand[4])
+    assertEquals(composeSourceMapsFile.relativeTo(tempFolder.root).path, composeSourcemapCommand[5])
+    assertEquals(packagerMap.relativeTo(tempFolder.root).path, composeSourcemapCommand[6])
+    assertEquals(compilerMap.relativeTo(tempFolder.root).path, composeSourcemapCommand[7])
+    assertEquals("-o", composeSourcemapCommand[8])
+    assertEquals(outputMap.relativeTo(tempFolder.root).path, composeSourcemapCommand[9])
+    assertEquals(10, composeSourcemapCommand.size)
+  }
 }

packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/tasks/GenerateCodegenArtifactsTaskTest.kt

@@ -7,9 +7,8 @@
 
 package com.facebook.react.tasks
 
-import com.facebook.react.tests.OS
-import com.facebook.react.tests.OsRule
-import com.facebook.react.tests.WithOs
+import com.facebook.react.tests.*
+import com.facebook.react.tests.createProject
 import com.facebook.react.tests.createTestTask
 import java.io.File
 import org.junit.Assert.assertEquals
@@ -97,6 +96,44 @@ class GenerateCodegenArtifactsTaskTest {
         task.commandLine.toMutableList())
   }
 
+  @Test
+  @WithOs(OS.WIN)
+  fun setupCommandLine_onWindows_willSetupCorrectly() {
+    val reactNativeDir = tempFolder.newFolder("node_modules/react-native/")
+    val outputDir = tempFolder.newFolder("output")
+
+    val project = createProject()
+    val task =
+        createTestTask<GenerateCodegenArtifactsTask>(project) {
+          it.reactNativeDir.set(reactNativeDir)
+          it.generatedSrcDir.set(outputDir)
+          it.nodeExecutableAndArgs.set(listOf("--verbose"))
+        }
+
+    task.setupCommandLine("example-test", "com.example.test")
+
+    assertEquals(
+        listOf(
+            "cmd",
+            "/c",
+            "--verbose",
+            File(reactNativeDir, "scripts/generate-specs-cli.js")
+                .relativeTo(project.projectDir)
+                .path,
+            "--platform",
+            "android",
+            "--schemaPath",
+            File(outputDir, "schema.json").relativeTo(project.projectDir).path,
+            "--outputDir",
+            outputDir.relativeTo(project.projectDir).path,
+            "--libraryName",
+            "example-test",
+            "--javaPackageName",
+            "com.example.test",
+        ),
+        task.commandLine.toMutableList())
+  }
+
   @Test
   fun resolveTaskParameters_withConfigInPackageJson_usesIt() {
     val packageJsonFile =

packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/tasks/GenerateCodegenSchemaTaskTest.kt

@@ -7,9 +7,8 @@
 
 package com.facebook.react.tasks
 
-import com.facebook.react.tests.OS
-import com.facebook.react.tests.OsRule
-import com.facebook.react.tests.WithOs
+import com.facebook.react.tests.*
+import com.facebook.react.tests.createProject
 import com.facebook.react.tests.createTestTask
 import java.io.File
 import org.junit.Assert.*
@@ -140,13 +139,14 @@ class GenerateCodegenSchemaTaskTest {
           it.codegenDir.set(codegenDir)
           it.jsRootDir.set(jsRootDir)
           it.generatedSrcDir.set(outputDir)
-          it.nodeExecutableAndArgs.set(listOf("--verbose"))
+          it.nodeExecutableAndArgs.set(listOf("node", "--verbose"))
         }
 
     task.setupCommandLine()
 
     assertEquals(
         listOf(
+            "node",
             "--verbose",
             File(codegenDir, "lib/cli/combine/combine-js-to-schema-cli.js").toString(),
             "--platform",
@@ -156,4 +156,39 @@ class GenerateCodegenSchemaTaskTest {
         ),
         task.commandLine.toMutableList())
   }
+
+  @Test
+  @WithOs(OS.WIN)
+  fun setupCommandLine_onWindows_willSetupCorrectly() {
+    val codegenDir = tempFolder.newFolder("codegen")
+    val jsRootDir = tempFolder.newFolder("js")
+    val outputDir = tempFolder.newFolder("output")
+
+    val project = createProject()
+    val task =
+        createTestTask<GenerateCodegenSchemaTask>(project) {
+          it.codegenDir.set(codegenDir)
+          it.jsRootDir.set(jsRootDir)
+          it.generatedSrcDir.set(outputDir)
+          it.nodeExecutableAndArgs.set(listOf("node", "--verbose"))
+        }
+
+    task.setupCommandLine()
+
+    assertEquals(
+        listOf(
+            "cmd",
+            "/c",
+            "node",
+            "--verbose",
+            File(codegenDir, "lib/cli/combine/combine-js-to-schema-cli.js")
+                .relativeTo(project.projectDir)
+                .path,
+            "--platform",
+            "android",
+            File(outputDir, "schema.json").relativeTo(project.projectDir).path,
+            jsRootDir.relativeTo(project.projectDir).path,
+        ),
+        task.commandLine.toMutableList())
+  }
 }

packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/utils/OsTest.kt

@@ -10,14 +10,17 @@ package com.facebook.react.utils
 import com.facebook.react.tests.OS
 import com.facebook.react.tests.OsRule
 import com.facebook.react.tests.WithOs
+import com.facebook.react.utils.Os.cliPath
 import com.facebook.react.utils.Os.unixifyPath
 import org.junit.Assert.*
 import org.junit.Rule
 import org.junit.Test
+import org.junit.rules.TemporaryFolder
 
 class OsTest {
 
   @get:Rule val osRule = OsRule()
+  @get:Rule val tempFolder = TemporaryFolder()
 
   @Test
   @WithOs(OS.LINUX, "amd64")
@@ -56,4 +59,28 @@ class OsTest {
 
     assertEquals("/D/just/a/windows/path/", aWindowsPath.unixifyPath())
   }
+
+  @Test
+  @WithOs(OS.WIN)
+  fun cliPath_onWindows_returnsRelativePath() {
+    val tempFile = tempFolder.newFile("test.txt").apply { createNewFile() }
+
+    assertEquals(tempFile.relativeTo(tempFolder.root).path, tempFile.cliPath(tempFolder.root))
+  }
+
+  @Test
+  @WithOs(OS.LINUX)
+  fun cliPath_onLinux_returnsAbsolutePath() {
+    val tempFile = tempFolder.newFile("test.txt").apply { createNewFile() }
+
+    assertEquals(tempFile.absolutePath, tempFile.cliPath(tempFolder.root))
+  }
+
+  @Test
+  @WithOs(OS.MAC)
+  fun cliPath_onMac_returnsAbsolutePath() {
+    val tempFile = tempFolder.newFile("test.txt").apply { createNewFile() }
+
+    assertEquals(tempFile.absolutePath, tempFile.cliPath(tempFolder.root))
+  }
 }