Gradle: extend the algoritm to find hermesc paths.

Summary:
This diff extends the Gradle algo used to search for `hermesc`.
Currently we look into `node_modules/hermes-engine/%OS-BIN%/hermesc`

With this change the algo will look into:
- A user provided path to hermesc
- Built from source version of hermesc (for users of New Architecture)
- Bundled version of hermesc inside react-native
- hermesc from the hermes-engine NPM package

I've added tests for the new algo. I also realized our tests were broken
(since they stopped running on CI), I fixed them as well.

Changelog:
[Android] [Changed] - Gradle: extend the algoritm to find hermesc paths

Reviewed By: ShikaSD

Differential Revision: D35649911

fbshipit-source-id: d4bcbe06a6bfa8d98b91c1612fc28b300de91661

Author: cortinico

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

@@ -141,9 +141,11 @@ abstract class ReactExtension @Inject constructor(project: Project) {
 
   /** Hermes Config */
 
-  /** The command to use to invoke hermes. Default is `hermesc` for the correct OS. */
-  val hermesCommand: Property<String> =
-      objects.property(String::class.java).convention("node_modules/hermes-engine/%OS-BIN%/hermesc")
+  /**
+   * The command to use to invoke hermesc (the hermes compiler). Default is "", the plugin will
+   * autodetect it.
+   */
+  val hermesCommand: Property<String> = objects.property(String::class.java).convention("")
 
   /** Toggle Hermes for the whole build. Default: false */
   val enableHermes: Property<Boolean> = objects.property(Boolean::class.java).convention(false)

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

@@ -12,6 +12,14 @@ object Os {
   fun isWindows(): Boolean =
       System.getProperty("os.name")?.lowercase()?.contains("windows") ?: false
 
+  fun isMac(): Boolean = System.getProperty("os.name")?.lowercase()?.contains("mac") ?: false
+
+  fun isLinuxAmd64(): Boolean {
+    val osNameMatch = System.getProperty("os.name")?.lowercase()?.contains("linux") ?: false
+    val archMatch = System.getProperty("os.arch")?.lowercase()?.contains("amd64") ?: false
+    return osNameMatch && archMatch
+  }
+
   fun String.unixifyPath() =
       this.replace('\\', '/').replace(":", "").let {
         if (!it.startsWith("/")) {

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

@@ -11,7 +11,6 @@ package com.facebook.react.utils
 
 import com.facebook.react.ReactExtension
 import java.io.File
-import org.apache.tools.ant.taskdefs.condition.Os
 
 /**
  * Computes the entry file for React Native. The Algo follows this order:
@@ -44,13 +43,16 @@ internal fun detectedCliPath(
 
 /**
  * Computes the `hermesc` command location. The Algo follows this order:
- * 1. The path provided by the `hermesCommand` config in the `reactApp` Gradle extension
- * 2. The file located in `node_modules/hermes-engine/%OS-BIN%/hermesc` where `%OS-BIN%` is
- * substituted with the correct OS arch.
- * 3. Fails otherwise
+ * 1. The path provided by the `hermesCommand` config in the `react` Gradle extension
+ * 2. The file located in `node_modules/react-native/sdks/hermes/build/bin/hermesc`. This will be
+ * used if the user is building Hermes from source.
+ * 3. The file located in `node_modules/react-native/sdks/hermesc/%OS-BIN%/hermesc` where `%OS-BIN%`
+ * is substituted with the correct OS arch. This will be used if the user is using a precompiled
+ * hermes-engine package.
+ * 4. Fails otherwise
  */
 internal fun detectedHermesCommand(config: ReactExtension): String =
-    detectOSAwareHermesCommand(config.hermesCommand.get())
+    detectOSAwareHermesCommand(config.root.get().asFile, config.hermesCommand.get())
 
 private fun detectEntryFile(entryFile: File?, reactRoot: File): File =
     when {
@@ -112,20 +114,48 @@ private fun detectCliPath(
 
 // Make sure not to inspect the Hermes config unless we need it,
 // to avoid breaking any JSC-only setups.
-private fun detectOSAwareHermesCommand(hermesCommand: String): String {
-  // If the project specifies a Hermes command, don't second guess it.
-  if (!hermesCommand.contains("%OS-BIN%")) {
-    return hermesCommand
+internal fun detectOSAwareHermesCommand(projectRoot: File, hermesCommand: String): String {
+  // 1. If the project specifies a Hermes command, don't second guess it.
+  if (hermesCommand.isNotBlank()) {
+    val osSpecificHermesCommand =
+        if ("%OS-BIN%" in hermesCommand) {
+          hermesCommand.replace("%OS-BIN%", getHermesOSBin())
+        } else {
+          hermesCommand
+        }
+    return osSpecificHermesCommand
+        // Execution on Windows fails with / as separator
+        .replace('/', File.separatorChar)
+  }
+
+  // 2. If the project is building hermes-engine from source, use hermesc from there
+  val builtHermesc = File(projectRoot, HERMESC_BUILT_FROM_SOURCE_PATH)
+  if (builtHermesc.exists()) {
+    return builtHermesc.absolutePath
+  }
+
+  // 3. If the react-native contains a pre-built hermesc, use it.
+  val prebuiltHermesPath =
+      HERMESC_IN_REACT_NATIVE_PATH
+          .replace("%OS-BIN%", getHermesOSBin())
+          // Execution on Windows fails with / as separator
+          .replace('/', File.separatorChar)
+
+  val prebuiltHermes = File(projectRoot, prebuiltHermesPath)
+  if (prebuiltHermes.exists()) {
+    return prebuiltHermes.absolutePath
   }
 
-  // Execution on Windows fails with / as separator
-  return hermesCommand.replace("%OS-BIN%", getHermesOSBin()).replace('/', File.separatorChar)
+  error(
+      "Couldn't determine Hermesc location. " +
+          "Please set `react.hermesCommand` to the path of the hermesc binary file. " +
+          "node_modules/react-native/sdks/hermesc/%OS-BIN%/hermesc")
 }
 
-private fun getHermesOSBin(): String {
-  if (Os.isFamily(Os.FAMILY_WINDOWS)) return "win64-bin"
-  if (Os.isFamily(Os.FAMILY_MAC)) return "osx-bin"
-  if (Os.isOs(null, "linux", "amd64", null)) return "linux64-bin"
+internal fun getHermesOSBin(): String {
+  if (Os.isWindows()) return "win64-bin"
+  if (Os.isMac()) return "osx-bin"
+  if (Os.isLinuxAmd64()) return "linux64-bin"
   error(
       "OS not recognized. Please set project.react.hermesCommand " +
           "to the path of a working Hermes compiler.")
@@ -136,3 +166,8 @@ internal fun projectPathToLibraryName(projectPath: String): String =
         .split(':', '-', '_', '.')
         .joinToString("") { token -> token.replaceFirstChar { it.uppercase() } }
         .plus("Spec")
+
+private const val HERMESC_IN_REACT_NATIVE_PATH =
+    "node_modules/react-native/sdks/hermesc/%OS-BIN%/hermesc"
+private const val HERMESC_BUILT_FROM_SOURCE_PATH =
+    "node_modules/react-native/sdks/hermes/build/bin/hermesc"

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

@@ -97,7 +97,6 @@ class GenerateCodegenArtifactsTaskTest {
 
     assertEquals(
         listOf(
-            "yarn",
             "--verbose",
             File(reactNativeDir, "scripts/generate-specs-cli.js").toString(),
             "--platform",

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

@@ -105,7 +105,6 @@ class GenerateCodegenSchemaTaskTest {
 
     assertEquals(
         listOf(
-            "yarn",
             "--verbose",
             File(codegenDir, "lib/cli/combine/combine-js-to-schema-cli.js").toString(),
             File(outputDir, "schema.json").toString(),

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

@@ -21,20 +21,26 @@ class OsTest {
 
   @Test
   @WithOs(OS.UNIX)
-  fun isWindows_onUnix_returnsFalse() {
+  fun onUnix_checksOsCorrectly() {
     assertFalse(Os.isWindows())
+    assertFalse(Os.isMac())
+    assertFalse(Os.isLinuxAmd64())
   }
 
   @Test
   @WithOs(OS.MAC)
-  fun isWindows_onMac_returnsTrue() {
+  fun onMac_checksOsCorrectly() {
     assertFalse(Os.isWindows())
+    assertTrue(Os.isMac())
+    assertFalse(Os.isLinuxAmd64())
   }
 
   @Test
   @WithOs(OS.WIN)
   fun isWindows_onWindows_returnsTrue() {
     assertTrue(Os.isWindows())
+    assertFalse(Os.isMac())
+    assertFalse(Os.isLinuxAmd64())
   }
 
   @Test

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

@@ -8,6 +8,9 @@
 package com.facebook.react.utils
 
 import com.facebook.react.TestReactExtension
+import com.facebook.react.tests.OS
+import com.facebook.react.tests.OsRule
+import com.facebook.react.tests.WithOs
 import java.io.File
 import org.gradle.testfixtures.ProjectBuilder
 import org.junit.Assert.*
@@ -18,6 +21,7 @@ import org.junit.rules.TemporaryFolder
 class PathUtilsTest {
 
   @get:Rule val tempFolder = TemporaryFolder()
+  @get:Rule val osRule = OsRule()
 
   @Test
   fun detectedEntryFile_withProvidedVariable() {
@@ -60,7 +64,7 @@ class PathUtilsTest {
           parentFile.mkdirs()
           writeText("<!-- nothing to see here -->")
         }
-    extension.cliPath.set(project.projectDir + "/abs/fake-cli.sh")
+    extension.cliPath.set(project.projectDir.toString() + "/abs/fake-cli.sh")
 
     val actual = detectedCliPath(project.projectDir, extension)
 
@@ -77,7 +81,7 @@ class PathUtilsTest {
           writeText("<!-- nothing to see here -->")
         }
     extension.cliPath.set("fake-cli.sh")
-    extension.reactRoot.set(project.projectDir + "/react-root")
+    extension.root.set(File(project.projectDir.toString(), "react-root"))
 
     val actual = detectedCliPath(project.projectDir, extension)
 
@@ -104,6 +108,7 @@ class PathUtilsTest {
   fun detectedCliPath_withCliPathFromExtensionInParentFolder() {
     val rootProject = ProjectBuilder.builder().build()
     val project = ProjectBuilder.builder().withParent(rootProject).build()
+    project.projectDir.mkdirs()
     val extension = TestReactExtension(project)
     val expected = File(rootProject.projectDir, "cli-in-root.sh").apply { writeText("#!/bin/bash") }
     extension.cliPath.set("../cli-in-root.sh")
@@ -148,15 +153,11 @@ class PathUtilsTest {
     assertEquals(expected.toString(), actual)
   }
 
-  @Test
-  fun detectedHermesCommand_withOSSpecificBin() {
+  @Test(expected = IllegalStateException::class)
+  fun detectedHermesCommand_failsIfNotFound() {
     val extension = TestReactExtension(ProjectBuilder.builder().build())
 
     val actual = detectedHermesCommand(extension)
-
-    assertTrue(actual.startsWith("node_modules/hermes-engine/"))
-    assertTrue(actual.endsWith("hermesc"))
-    assertFalse(actual.contains("%OS-BIN%"))
   }
 
   @Test
@@ -178,4 +179,56 @@ class PathUtilsTest {
   fun projectPathToLibraryName_withDotsAndUnderscores() {
     assertEquals("SampleAndroidAppSpec", projectPathToLibraryName("sample_android.app"))
   }
+
+  @Test
+  fun detectOSAwareHermesCommand_withProvidedCommand() {
+    assertEquals(
+        "./my-home/hermes", detectOSAwareHermesCommand(tempFolder.root, "./my-home/hermes"))
+  }
+
+  @Test
+  fun detectOSAwareHermesCommand_withHermescBuiltLocally() {
+    tempFolder.newFolder("node_modules/react-native/sdks/hermes/build/bin/")
+    val expected = tempFolder.newFile("node_modules/react-native/sdks/hermes/build/bin/hermesc")
+
+    assertEquals(expected.toString(), detectOSAwareHermesCommand(tempFolder.root, ""))
+  }
+
+  @Test
+  @WithOs(OS.MAC)
+  fun detectOSAwareHermesCommand_withBundledHermescInsideRN() {
+    tempFolder.newFolder("node_modules/react-native/sdks/hermesc/osx-bin/")
+    val expected = tempFolder.newFile("node_modules/react-native/sdks/hermesc/osx-bin/hermesc")
+
+    assertEquals(expected.toString(), detectOSAwareHermesCommand(tempFolder.root, ""))
+  }
+
+  @Test(expected = IllegalStateException::class)
+  @WithOs(OS.MAC)
+  fun detectOSAwareHermesCommand_failsIfNotFound() {
+    detectOSAwareHermesCommand(tempFolder.root, "")
+  }
+
+  @Test
+  @WithOs(OS.MAC)
+  fun detectOSAwareHermesCommand_withProvidedCommand_takesPrecedence() {
+    tempFolder.newFolder("node_modules/react-native/sdks/hermes/build/bin/")
+    tempFolder.newFile("node_modules/react-native/sdks/hermes/build/bin/hermesc")
+    tempFolder.newFolder("node_modules/react-native/sdks/hermesc/osx-bin/")
+    tempFolder.newFile("node_modules/react-native/sdks/hermesc/osx-bin/hermesc")
+
+    assertEquals(
+        "./my-home/hermes", detectOSAwareHermesCommand(tempFolder.root, "./my-home/hermes"))
+  }
+
+  @Test
+  @WithOs(OS.MAC)
+  fun detectOSAwareHermesCommand_withoutProvidedCommand_builtHermescTakesPrecedence() {
+    tempFolder.newFolder("node_modules/react-native/sdks/hermes/build/bin/")
+    val expected = tempFolder.newFile("node_modules/react-native/sdks/hermes/build/bin/hermesc")
+    tempFolder.newFolder("node_modules/react-native/sdks/hermesc/osx-bin/")
+    tempFolder.newFile("node_modules/react-native/sdks/hermesc/osx-bin/hermesc")
+
+    assertEquals(expected.toString(), detectOSAwareHermesCommand(tempFolder.root, ""))
+  }
 }

packages/rn-tester/android/app/build.gradle

@@ -83,7 +83,7 @@ react {
     root = rootDir
     inputExcludes = ["android/**", "./**", ".gradle/**"]
     composeSourceMapsPath = "$rootDir/scripts/compose-source-maps.js"
-    hermesCommand = "$rootDir/node_modules/hermes-engine/%OS-BIN%/hermesc"
+    hermesCommand = "$rootDir/sdks/hermes/build/bin/hermesc"
     enableHermesForVariant { def v -> v.name.contains("hermes") }
 
     // Codegen Configs

react.gradle

@@ -30,7 +30,7 @@ def reactRoot = file(config.root ?: "../../")
 def inputExcludes = config.inputExcludes ?: ["android/**", "ios/**"]
 def bundleConfig = config.bundleConfig ? "${reactRoot}/${config.bundleConfig}" : null ;
 def enableVmCleanup = config.enableVmCleanup == null ? true : config.enableVmCleanup
-def hermesCommand = config.hermesCommand ?: "../../node_modules/hermes-engine/%OS-BIN%/hermesc"
+def hermesCommand = config.hermesCommand
 
 /**
  * Detects CLI location in a similar fashion to the React Native CLI
@@ -90,15 +90,36 @@ def getHermesOSBin() {
 // Make sure not to inspect the Hermes config unless we need it,
 // to avoid breaking any JSC-only setups.
 def getHermesCommand = {
-    // If the project specifies a Hermes command, don't second guess it.
-    if (!hermesCommand.contains("%OS-BIN%")) {
-        return hermesCommand
+    // 1. If the project specifies a Hermes command, don't second guess it.
+    if (config.hermesCommand?.trim()) {
+        if (hermesCommand.contains("%OS-BIN%")) {
+            return hermesCommand
+                .replaceAll("%OS-BIN%", getHermesOSBin())
+                .replace('/' as char, File.separatorChar)
+        } else {
+            return hermesCommand
+                .replace('/' as char, File.separatorChar)
+        }
+    }
+
+    // 2. If the project is building hermes-engine from source, use hermesc from there
+    def builtHermesc = new File(reactRoot, "node_modules/react-native/sdks/hermes/build/bin/hermesc")
+    if (builtHermesc.exists()) {
+        return builtHermesc.getAbsolutePath()
+    }
+
+    // 3. If the react-native contains a pre-built hermesc, use it.
+    def prebuiltHermesPath = "node_modules/react-native/sdks/hermesc/%OS-BIN%/hermesc"
+        .replaceAll("%OS-BIN%", getHermesOSBin())
+        .replace('/' as char, File.separatorChar);
+    def prebuiltHermes = new File(reactRoot, prebuiltHermesPath)
+    if (prebuiltHermes.exists()) {
+        return prebuiltHermes.getAbsolutePath()
     }
 
-    // Execution on Windows fails with / as separator
-    return hermesCommand
-            .replaceAll("%OS-BIN%", getHermesOSBin())
-            .replace('/' as char, File.separatorChar);
+    throw new Exception("Couldn't determine Hermesc location. " +
+        "Please set `project.ext.react.hermesCommand` to the path of the hermesc binary file. " +
+        "node_modules/react-native/sdks/hermesc/%OS-BIN%/hermesc");
 }
 
 // Set enableHermesForVariant to a function to configure per variant,