fix: limit diagnostics width output by hermesc
#37531
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
|
@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
cortinico
approved these changes
May 23, 2023
|
@cortinico merged this pull request in 260bcf7. |
kelset
pushed a commit
that referenced
this pull request
May 25, 2023
Summary: Limit diagnostics width output by `hermesc` as they may cause slowdowns or even crashes in Gradle/Xcode when a minified bundle is used as input. This occurs because Hermes is unable to determine the terminal width when executed by Gradle/Xcode, and falls back to "unlimited". If the input is a minified bundle, Hermes will output the whole bundle for each warning. See issues filed: - microsoft/rnx-kit#2416 - microsoft/rnx-kit#2419 - microsoft/rnx-kit#2424 ## Changelog: [GENERAL] [FIXED] - Limit diagnostics width output by `hermesc` Pull Request resolved: #37531 Test Plan: See listed issues for repros. Reviewed By: cipolleschi Differential Revision: D46102686 Pulled By: cortinico fbshipit-source-id: 1b821cad7ef0d561a5e1c13a7aedf9b10164620a
facebook-github-bot
pushed a commit
that referenced
this pull request
Jul 6, 2023
Summary: Fixes a regression added [on merge of https://github.com/facebook/react-native/issues/37531](https://github.com/facebook/react-native/commit/260bcf7f1bf78022872eb2f40f33fb552a414809#diff-16a358d6a9dea8469bfdb899d0990df1c32b8c3b1149c86685bec81f50bd24beR179) (though oddly I don't see it in [#37531 diff](https://github.com/facebook/react-native/pull/37531/files#diff-16a358d6a9dea8469bfdb899d0990df1c32b8c3b1149c86685bec81f50bd24beR179), so perhaps added during conflict resolution?) where multiple arguments in `$EXTRA_COMPILER_ARGS` are now incorrectly wrapped in quotes, including RN's own sourcemap support: ``` runner@SeansMacBookGo sampleapp % /Users/runner/builds/y_x6gsp4/0/sampleapp/ios/Pods/hermes-engine/destroot/bin/hermesc -emit-binary -max-diagnostic-width=80 '-O -output-source-map' -out /Users/runner/builds/y_x6gsp4/0/sampleapp/ios/build/derived-data/Build/Intermediates.noindex/ArchiveIntermediates/sampleapp/BuildProductsPath/Release-iphoneos/sampleapp.app/main.jsbundle /Users/runner/builds/y_x6gsp4/0/sampleapp/ios/build/derived-data/Build/Intermediates.noindex/ArchiveIntermediates/sampleapp/BuildProductsPath/Release-iphoneos/main.jsbundle hermesc: Unknown command line argument '-O -output-source-map'. Try: '/Users/runner/builds/y_x6gsp4/0/sampleapp/ios/Pods/hermes-engine/destroot/bin/hermesc -help' hermesc: Did you mean '-output-source-map'? ``` ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [IOS] [FIXED] - Fix build error when there are multiple EXTRA_COMPILER_ARGS Pull Request resolved: #38147 Test Plan: Removing the quotes and running the command results a successful run of `hermesc` (exit code 0 and the expected files produced). Reviewed By: rshest Differential Revision: D47254412 Pulled By: cipolleschi fbshipit-source-id: 96b71bb05c7a6939088816e76a9a2d02e89ed768
kelset
pushed a commit
that referenced
this pull request
Jul 10, 2023
Summary: Fixes a regression added [on merge of https://github.com/facebook/react-native/issues/37531](https://github.com/facebook/react-native/commit/260bcf7f1bf78022872eb2f40f33fb552a414809#diff-16a358d6a9dea8469bfdb899d0990df1c32b8c3b1149c86685bec81f50bd24beR179) (though oddly I don't see it in [#37531 diff](https://github.com/facebook/react-native/pull/37531/files#diff-16a358d6a9dea8469bfdb899d0990df1c32b8c3b1149c86685bec81f50bd24beR179), so perhaps added during conflict resolution?) where multiple arguments in `$EXTRA_COMPILER_ARGS` are now incorrectly wrapped in quotes, including RN's own sourcemap support: ``` runner@SeansMacBookGo sampleapp % /Users/runner/builds/y_x6gsp4/0/sampleapp/ios/Pods/hermes-engine/destroot/bin/hermesc -emit-binary -max-diagnostic-width=80 '-O -output-source-map' -out /Users/runner/builds/y_x6gsp4/0/sampleapp/ios/build/derived-data/Build/Intermediates.noindex/ArchiveIntermediates/sampleapp/BuildProductsPath/Release-iphoneos/sampleapp.app/main.jsbundle /Users/runner/builds/y_x6gsp4/0/sampleapp/ios/build/derived-data/Build/Intermediates.noindex/ArchiveIntermediates/sampleapp/BuildProductsPath/Release-iphoneos/main.jsbundle hermesc: Unknown command line argument '-O -output-source-map'. Try: '/Users/runner/builds/y_x6gsp4/0/sampleapp/ios/Pods/hermes-engine/destroot/bin/hermesc -help' hermesc: Did you mean '-output-source-map'? ``` ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [IOS] [FIXED] - Fix build error when there are multiple EXTRA_COMPILER_ARGS Pull Request resolved: #38147 Test Plan: Removing the quotes and running the command results a successful run of `hermesc` (exit code 0 and the expected files produced). Reviewed By: rshest Differential Revision: D47254412 Pulled By: cipolleschi fbshipit-source-id: 96b71bb05c7a6939088816e76a9a2d02e89ed768
Kudo
pushed a commit
to expo/react-native
that referenced
this pull request
Jul 11, 2023
…38147) Summary: Fixes a regression added [on merge of https://github.com/facebook/react-native/issues/37531](https://github.com/facebook/react-native/commit/260bcf7f1bf78022872eb2f40f33fb552a414809#diff-16a358d6a9dea8469bfdb899d0990df1c32b8c3b1149c86685bec81f50bd24beR179) (though oddly I don't see it in [facebook#37531 diff](https://github.com/facebook/react-native/pull/37531/files#diff-16a358d6a9dea8469bfdb899d0990df1c32b8c3b1149c86685bec81f50bd24beR179), so perhaps added during conflict resolution?) where multiple arguments in `$EXTRA_COMPILER_ARGS` are now incorrectly wrapped in quotes, including RN's own sourcemap support: ``` runner@SeansMacBookGo sampleapp % /Users/runner/builds/y_x6gsp4/0/sampleapp/ios/Pods/hermes-engine/destroot/bin/hermesc -emit-binary -max-diagnostic-width=80 '-O -output-source-map' -out /Users/runner/builds/y_x6gsp4/0/sampleapp/ios/build/derived-data/Build/Intermediates.noindex/ArchiveIntermediates/sampleapp/BuildProductsPath/Release-iphoneos/sampleapp.app/main.jsbundle /Users/runner/builds/y_x6gsp4/0/sampleapp/ios/build/derived-data/Build/Intermediates.noindex/ArchiveIntermediates/sampleapp/BuildProductsPath/Release-iphoneos/main.jsbundle hermesc: Unknown command line argument '-O -output-source-map'. Try: '/Users/runner/builds/y_x6gsp4/0/sampleapp/ios/Pods/hermes-engine/destroot/bin/hermesc -help' hermesc: Did you mean '-output-source-map'? ``` ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [IOS] [FIXED] - Fix build error when there are multiple EXTRA_COMPILER_ARGS Pull Request resolved: facebook#38147 Test Plan: Removing the quotes and running the command results a successful run of `hermesc` (exit code 0 and the expected files produced). Reviewed By: rshest Differential Revision: D47254412 Pulled By: cipolleschi fbshipit-source-id: 96b71bb05c7a6939088816e76a9a2d02e89ed768
This was referenced Aug 10, 2023
This was referenced Jan 4, 2024
This was referenced Feb 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Limit diagnostics width output by
hermescas they may cause slowdowns or even crashes in Gradle/Xcode when a minified bundle is used as input. This occurs because Hermes is unable to determine the terminal width when executed by Gradle/Xcode, and falls back to "unlimited". If the input is a minified bundle, Hermes will output the whole bundle for each warning.See issues filed:
Changelog:
[GENERAL] [FIXED] - Limit diagnostics width output by
hermescTest Plan:
See listed issues for repros.