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

JoseLion · 2022-07-07T04:02:39Z

Summary

Resolves #34116.

In a nutshell, the problem was a missing .exe extension on the hermesc binary path when running on Windows OS. The missing extension causes the method .exists() of the File instance to always return false, so none of the conditions ever met and an error was thrown whenever a release build with Hermes enabled was run on Windows. More details can be found in the comments on the above issues.

Changelog

[Android] [Fixed] - Fix error of release builds with Hermes enabled for Windows users

Test Plan

Reproduce

Changes on Gradle scrips are better tested on an actual application. To reproduce the issue you can:

Create or reuse a React Native application with version v0.69.1 on a Windows machine
Enable Hermes on Android following the steps on the documentation
Clean the build folder: cd android && ./gradlew clean
Bundle the JS and assets for a release version: ./gradlew bundleReleaseJsAndAssets
The build fails with the following error:

Execution failed for task ':app:bundleReleaseJsAndAssets'.
> java.lang.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

Test the changes

Follow the same steps above using the fix on this PR and the error should disappear 🙂

facebook-github-bot · 2022-07-07T04:02:42Z

Hi @JoseLion!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

JoseLion · 2022-07-07T04:11:00Z

.gitignore

@@ -122,6 +122,7 @@ package-lock.json
 # Visual studio
 .vscode
 .vs
+bin/


Added this line because vscode-java extension creates this folder on Java projects 🙂

I have the same problem. Still I won't add bin/ here (I added it in my global .gitignore).

The problem is on VS Code's end, as they used a folder name which is too generic and potentially used in production. This has the risk of accidentally make us skip artifacts during NPM packaging or git commits.

analysis-bot · 2022-07-07T04:32:30Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	7,799,665	-2,828
android	hermes	armeabi-v7a	7,194,370	-892
android	hermes	x86	8,110,306	-2,191
android	hermes	x86_64	8,090,015	-2,022
android	jsc	arm64-v8a	9,696,735	+0
android	jsc	armeabi-v7a	8,451,995	+0
android	jsc	x86	9,647,485	+0
android	jsc	x86_64	10,245,252	+0

Base commit: c37f719
Branch: main

cortinico

Thank you very much for doing this @JoseLion

cortinico · 2022-07-07T06:13:54Z

.gitignore

@@ -122,6 +122,7 @@ package-lock.json
 # Visual studio
 .vscode
 .vs
+bin/


I have the same problem. Still I won't add bin/ here (I added it in my global .gitignore).

The problem is on VS Code's end, as they used a folder name which is too generic and potentially used in production. This has the risk of accidentally make us skip artifacts during NPM packaging or git commits.

cortinico · 2022-07-07T06:15:59Z

react.gradle

@@ -102,20 +102,22 @@ def getHermesCommand = {
        }
    }

+    def hermescBin = Os.isFamily(Os.FAMILY_WINDOWS) ? 'hermesc.exe' : 'hermesc'


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")

}

Sure! Thanks for the review BTW 🙂

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

facebook-github-bot · 2022-07-07T06:24:59Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

analysis-bot · 2022-07-07T09:20:01Z

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: c37f719
Branch: main

facebook-github-bot · 2022-07-11T16:18:06Z

@philIip has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

cortinico

Sorry this took longer. This can be merged 👍
I've also added tests which will appear in the final commit

react-native-bot · 2022-08-04T15:40:04Z

This pull request was successfully merged by @JoseLion in 7fcdb9d.

^{When will my fix make it into a release? | Upcoming Releases}

…34151) Summary: Resolves #34116. In a nutshell, the problem was a missing `.exe` extension on the `hermesc` binary path when running on Windows OS. The missing extension causes the method `.exists()` of the File instance to always return false, so none of the conditions ever met and an error was thrown whenever a release build with Hermes enabled was run on Windows. More details can be found in the comments on the above issues. ## Changelog  [Android] [Fixed] - Fix error of release builds with Hermes enabled for Windows users Pull Request resolved: #34151 Test Plan: ### Reproduce Changes on Gradle scrips are better tested on an actual application. To reproduce the issue you can: 1. Create or reuse a React Native application with version `v0.69.1` on a Windows machine 2. Enable Hermes on Android following the steps on the [documentation](https://reactnative.dev/docs/hermes#enabling-hermes) 3. Clean the build folder: `cd android && ./gradlew clean` 4. Bundle the JS and assets for a release version: `./gradlew bundleReleaseJsAndAssets` 5. The build fails with the following error: ```shell Execution failed for task ':app:bundleReleaseJsAndAssets'. > java.lang.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 ``` ### Test the changes Follow the same steps above using the fix on this PR and the error should disappear 🙂 Reviewed By: NickGerleman Differential Revision: D37755468 Pulled By: cortinico fbshipit-source-id: 2ad0ced583555b907259df116f64a45da6d153f3

…acebook#34151) Summary: Resolves facebook#34116. In a nutshell, the problem was a missing `.exe` extension on the `hermesc` binary path when running on Windows OS. The missing extension causes the method `.exists()` of the File instance to always return false, so none of the conditions ever met and an error was thrown whenever a release build with Hermes enabled was run on Windows. More details can be found in the comments on the above issues. ## Changelog  [Android] [Fixed] - Fix error of release builds with Hermes enabled for Windows users Pull Request resolved: facebook#34151 Test Plan: ### Reproduce Changes on Gradle scrips are better tested on an actual application. To reproduce the issue you can: 1. Create or reuse a React Native application with version `v0.69.1` on a Windows machine 2. Enable Hermes on Android following the steps on the [documentation](https://reactnative.dev/docs/hermes#enabling-hermes) 3. Clean the build folder: `cd android && ./gradlew clean` 4. Bundle the JS and assets for a release version: `./gradlew bundleReleaseJsAndAssets` 5. The build fails with the following error: ```shell Execution failed for task ':app:bundleReleaseJsAndAssets'. > java.lang.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 ``` ### Test the changes Follow the same steps above using the fix on this PR and the error should disappear 🙂 Reviewed By: NickGerleman Differential Revision: D37755468 Pulled By: cortinico fbshipit-source-id: 2ad0ced583555b907259df116f64a45da6d153f3

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

84aea1a

react-native-bot added Bug Platform: Android Android applications. labels Jul 7, 2022

JoseLion commented Jul 7, 2022

View reviewed changes

JoseLion mentioned this pull request Jul 7, 2022

Error build release (hermes enable) #34116

Closed

cortinico reviewed Jul 7, 2022

View reviewed changes

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 7, 2022

facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Jul 7, 2022

JoseLion added 2 commits July 7, 2022 01:44

Address @cortinico feedback

de9623b

Fix Kotlin changes

cd912ac

JoseLion mentioned this pull request Jul 11, 2022

[Windows] Task :ReactAndroid:hermes-engine:buildCMakeRelease[arm64-v8a][libhermes] FAILED #34084

Closed

deng81416754 mentioned this pull request Jul 27, 2022

buildCMakeRelease Error #34283

Closed

cortinico approved these changes Aug 4, 2022

View reviewed changes

kelset added the Platform: Windows Building on Windows. label Aug 4, 2022

facebook-github-bot closed this in 7fcdb9d Aug 4, 2022

react-native-bot added the Merged This PR has been merged. label Aug 4, 2022

JoseLion deleted the fix/windows-hermesc-path branch August 4, 2022 18:12

snyk-bot mentioned this pull request Sep 27, 2022

[Snyk] Upgrade react-native from 0.63.3 to 0.70.0 NOUIY/react-native-macos#752

Open

cipolleschi mentioned this pull request Nov 1, 2023

[RN][CI]Update node installation on debian (0.70) #41278

Closed

cipolleschi mentioned this pull request Jan 8, 2024

[0.70][RN][General] Migrate boost download url away from JFrog #42178

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

JoseLion commented Jul 7, 2022

facebook-github-bot commented Jul 7, 2022

JoseLion Jul 7, 2022

cortinico Jul 7, 2022

analysis-bot commented Jul 7, 2022 •

edited

Loading

cortinico left a comment

cortinico Jul 7, 2022

cortinico Jul 7, 2022

JoseLion Jul 7, 2022

JoseLion Jul 7, 2022 •

edited

Loading

facebook-github-bot commented Jul 7, 2022

analysis-bot commented Jul 7, 2022

facebook-github-bot commented Jul 11, 2022

cortinico left a comment

react-native-bot commented Aug 4, 2022

	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")
	}

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

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

Conversation

JoseLion commented Jul 7, 2022

Summary

Changelog

Test Plan

Reproduce

Test the changes

facebook-github-bot commented Jul 7, 2022

Action Required

Process

JoseLion Jul 7, 2022

Choose a reason for hiding this comment

cortinico Jul 7, 2022

Choose a reason for hiding this comment

analysis-bot commented Jul 7, 2022 • edited Loading

cortinico left a comment

Choose a reason for hiding this comment

cortinico Jul 7, 2022

Choose a reason for hiding this comment

cortinico Jul 7, 2022

Choose a reason for hiding this comment

JoseLion Jul 7, 2022

Choose a reason for hiding this comment

JoseLion Jul 7, 2022 • edited Loading

Choose a reason for hiding this comment

facebook-github-bot commented Jul 7, 2022

analysis-bot commented Jul 7, 2022

facebook-github-bot commented Jul 11, 2022

cortinico left a comment

Choose a reason for hiding this comment

react-native-bot commented Aug 4, 2022

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

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

analysis-bot commented Jul 7, 2022 •

edited

Loading

JoseLion Jul 7, 2022 •

edited

Loading