From 8ae9c2cff6feaf06daa6f9edac4b2302eb58f43a Mon Sep 17 00:00:00 2001 From: Nicola Corti Date: Tue, 26 Apr 2022 08:25:03 -0700 Subject: [PATCH] Lazily resolve the hermesc path rather than eagerly Summary: This commit moves the resolution of the hermesc inside the TaskAction block of the HermesBinaryTask. Therefore the hermesc path will be investigated only during the execution of the task, and not when the task is created. Changelog: [Internal] [Changed] - Lazily resolve the hermesc path rather than eagerly Reviewed By: motiz88 Differential Revision: D35930548 fbshipit-source-id: a517dda0fa9b10f53c25cd256ceb68d37d533d3b --- .../com/facebook/react/TaskConfiguration.kt | 5 ++--- .../facebook/react/tasks/HermesBinaryTask.kt | 10 ++++++---- .../com/facebook/react/utils/PathUtils.kt | 12 ++++++++++-- .../com/facebook/react/utils/PathUtilsTest.kt | 18 ------------------ 4 files changed, 18 insertions(+), 27 deletions(-) diff --git a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/TaskConfiguration.kt b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/TaskConfiguration.kt index aca77c8facd..60e7e60b0a6 100644 --- a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/TaskConfiguration.kt +++ b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/TaskConfiguration.kt @@ -15,7 +15,6 @@ import com.facebook.react.tasks.BundleJsAndAssetsTask import com.facebook.react.tasks.HermesBinaryTask import com.facebook.react.utils.detectedCliPath import com.facebook.react.utils.detectedEntryFile -import com.facebook.react.utils.detectedHermesCommand import java.io.File import org.gradle.api.Project import org.gradle.api.Task @@ -95,8 +94,8 @@ internal fun Project.configureReactTasks(variant: BaseVariant, config: ReactExte it.group = REACT_GROUP it.description = "bundle hermes resources for $targetName" - it.reactRoot = config.root.get().asFile - it.hermesCommand = detectedHermesCommand(config) + it.root = config.root.get().asFile + it.hermesCommand = config.hermesCommand.get() it.hermesFlags = config.hermesFlagsForVariant(variant) it.jsBundleFile = jsBundleFile it.composeSourceMapsCommand = nodeExecutableAndArgs + config.composeSourceMapsPath.get() diff --git a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/tasks/HermesBinaryTask.kt b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/tasks/HermesBinaryTask.kt index ab056981bb2..3f9e6359738 100644 --- a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/tasks/HermesBinaryTask.kt +++ b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/tasks/HermesBinaryTask.kt @@ -7,6 +7,7 @@ package com.facebook.react.tasks +import com.facebook.react.utils.detectOSAwareHermesCommand import com.facebook.react.utils.moveTo import com.facebook.react.utils.windowsAwareCommandLine import java.io.File @@ -19,7 +20,7 @@ import org.gradle.api.tasks.TaskAction open class HermesBinaryTask : DefaultTask() { - @get:Internal internal lateinit var reactRoot: File + @get:Internal internal lateinit var root: File @get:Input internal lateinit var hermesCommand: String @get:Input internal var hermesFlags: List = emptyList() @@ -33,8 +34,9 @@ open class HermesBinaryTask : DefaultTask() { @TaskAction fun run() { + val detectedHermesCommand = detectOSAwareHermesCommand(root, hermesCommand) val bytecodeTempFile = File("$jsBundleFile.hbc") - emitHermesBinary(outputFile = bytecodeTempFile) + emitHermesBinary(hermesCommand = detectedHermesCommand, outputFile = bytecodeTempFile) bytecodeTempFile.moveTo(jsBundleFile) if (hermesFlags.contains("-output-source-map")) { @@ -44,7 +46,7 @@ open class HermesBinaryTask : DefaultTask() { } } - private fun emitHermesBinary(outputFile: File) { + private fun emitHermesBinary(hermesCommand: String, outputFile: File) { project.exec { @Suppress("SpreadOperator") it.commandLine( @@ -60,7 +62,7 @@ open class HermesBinaryTask : DefaultTask() { private fun composeSourceMaps() { project.exec { - it.workingDir(reactRoot) + it.workingDir(root) @Suppress("SpreadOperator") it.commandLine( diff --git a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/PathUtils.kt b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/PathUtils.kt index 6915cc679c1..27f115db9bd 100644 --- a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/PathUtils.kt +++ b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/PathUtils.kt @@ -112,8 +112,16 @@ private fun detectCliPath( "This file typically resides in `node_modules/react-native/cli.js`") } -// Make sure not to inspect the Hermes config unless we need it, -// to avoid breaking any JSC-only setups. +/** + * Computes the `hermesc` command location. The Algo follows this order: + * 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 detectOSAwareHermesCommand(projectRoot: File, hermesCommand: String): String { // 1. If the project specifies a Hermes command, don't second guess it. if (hermesCommand.isNotBlank()) { diff --git a/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/utils/PathUtilsTest.kt b/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/utils/PathUtilsTest.kt index 34e10cb4cb9..a98c63c6888 100644 --- a/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/utils/PathUtilsTest.kt +++ b/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/utils/PathUtilsTest.kt @@ -142,24 +142,6 @@ class PathUtilsTest { detectedCliPath(project.projectDir, extension) } - @Test - fun detectedHermesCommand_withPathFromExtension() { - val extension = TestReactExtension(ProjectBuilder.builder().build()) - val expected = tempFolder.newFile("hermesc") - extension.hermesCommand.set(expected.toString()) - - val actual = detectedHermesCommand(extension) - - assertEquals(expected.toString(), actual) - } - - @Test(expected = IllegalStateException::class) - fun detectedHermesCommand_failsIfNotFound() { - val extension = TestReactExtension(ProjectBuilder.builder().build()) - - val actual = detectedHermesCommand(extension) - } - @Test fun projectPathToLibraryName_withSimplePath() { assertEquals("SampleSpec", projectPathToLibraryName(":sample"))