Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(android): Append .exe to hermesc binary path for Windows users #34151

Closed
wants to merge 3 commits into from
Closed

fix(android): Append .exe to hermesc binary path for Windows users #34151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
84aea1a
fix(android): Append `.exe` to hermesc binary path for Windows users
JoseLion Jul 7, 2022
de9623b
Address @cortinico feedback
JoseLion Jul 7, 2022
cd912ac
Fix Kotlin changes
JoseLion Jul 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/PathUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ internal fun detectOSAwareHermesCommand(projectRoot: File, hermesCommand: String

// 3. If the react-native contains a pre-built hermesc, use it.
val prebuiltHermesPath =
HERMESC_IN_REACT_NATIVE_PATH.replace("%OS-BIN%", getHermesOSBin())
HERMESC_IN_REACT_NATIVE_DIR.plus(getHermesCBin())
.replace("%OS-BIN%", getHermesOSBin())
// Execution on Windows fails with / as separator
.replace('/', File.separatorChar)

Expand All @@ -162,19 +163,21 @@ internal fun detectOSAwareHermesCommand(projectRoot: File, hermesCommand: String

/**
* Gets the location where Hermesc should be. If nothing is specified, built hermesc is assumed to
* be inside [HERMESC_BUILT_FROM_SOURCE_PATH]. Otherwise user can specify an override with
* be inside [HERMESC_BUILT_FROM_SOURCE_DIR]. Otherwise user can specify an override with
* [pathOverride], which is assumed to be an absolute path where Hermes source code is
* provided/built.
*
* @param projectRoot The root of the Project.
*/
internal fun getBuiltHermescFile(projectRoot: File, pathOverride: String?) =
if (!pathOverride.isNullOrBlank()) {
File(pathOverride, "build/bin/hermesc")
File(pathOverride, "build/bin/${getHermesCBin()}")
} else {
File(projectRoot, HERMESC_BUILT_FROM_SOURCE_PATH)
File(projectRoot, HERMESC_BUILT_FROM_SOURCE_DIR.plus(getHermesCBin()))
}

internal fun getHermesCBin() = if (Os.isWindows()) "hermesc.exe" else "hermesc"

internal fun getHermesOSBin(): String {
if (Os.isWindows()) return "win64-bin"
if (Os.isMac()) return "osx-bin"
Expand All @@ -190,7 +193,7 @@ internal fun projectPathToLibraryName(projectPath: String): String =
.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/ReactAndroid/hermes-engine/build/hermes/bin/hermesc"
private const val HERMESC_IN_REACT_NATIVE_DIR =
"node_modules/react-native/sdks/hermesc/%OS-BIN%/"
private const val HERMESC_BUILT_FROM_SOURCE_DIR =
"node_modules/react-native/ReactAndroid/hermes-engine/build/hermes/bin/"
8 changes: 5 additions & 3 deletions react.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,20 +102,22 @@ def getHermesCommand = {
}
}

def hermescBin = Os.isFamily(Os.FAMILY_WINDOWS) ? 'hermesc.exe' : 'hermesc'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great stuff. Can I ask you to do the same logic here:

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

Lines 125 to 161 in c37f719

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 =
getBuiltHermescFile(projectRoot, System.getenv("REACT_NATIVE_OVERRIDE_HERMES_DIR"))
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
}
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")
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! Thanks for the review BTW 🙂

Copy link
Contributor Author

@JoseLion JoseLion Jul 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! I took a slightly different approach since the Kotlin was a bit different, but it's still the same concept 😁


// 2. If the project is building hermes-engine from source, use hermesc from there
// Also note that user can override the hermes source location with
// the `REACT_NATIVE_OVERRIDE_HERMES_DIR` env variable.
def hermesOverrideDir = System.getenv("REACT_NATIVE_OVERRIDE_HERMES_DIR")
def builtHermesc = hermesOverrideDir ?
new File(hermesOverrideDir, "build/bin/hermesc") :
new File(reactRoot, "node_modules/react-native/ReactAndroid/hermes-engine/build/hermes/bin/hermesc")
new File(hermesOverrideDir, "build/bin/$hermescBin") :
new File(reactRoot, "node_modules/react-native/ReactAndroid/hermes-engine/build/hermes/bin/$hermescBin")

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"
def prebuiltHermesPath = "node_modules/react-native/sdks/hermesc/%OS-BIN%/$hermescBin"
.replaceAll("%OS-BIN%", getHermesOSBin())
.replace('/' as char, File.separatorChar);
def prebuiltHermes = new File(reactRoot, prebuiltHermesPath)
Expand Down