Repository navigation
Implement Hermes version check for our custom artifacts - #103317
mateuuszzzzz wants to merge 2 commits into
Conversation
|
|
There was a problem hiding this comment.
Seems that the command in buildIOS.yml on line 125 could use flag
--hermes-version-file="$GITHUB_WORKSPACE/node_modules/react-native/sdks/.hermesv1version".
It seems that it currently does not account the new metadata, and without it args['hermes-version-file'] in resolve-artifacts.ts is undefined, so the cache key no longer matches the artifact version.
| # The published POM records this value so that the artifacts resolver can compare it with the Hermes the consuming | ||
| # app links. The artifacts do not bundle a Hermes engine, but their React code is compiled against the ABI of one, so | ||
| # an artifact built against a different Hermes is unusable. We cannot predict how upcoming upstream versions will | ||
| # pick the Hermes version (which file, whether V1 stays opt-in), so this tag is the safeguard against silently | ||
| # consuming artifacts built against the wrong ABI: on a mismatch the consumer falls back to a source build and | ||
| # names both versions in its log, instead of failing at link time or at runtime. |
There was a problem hiding this comment.
I'd either shorten this part or even move it higher up. This is very specific, and the first part seems to already explain what the script does.
Explanation of Change
We had troubles with Hermes ABI mismatch between our artifacts and local builds. This PR adds extra metadata to artifact POM files to allow us to detect ABI mismatches during the build.
This cannot be implemented reliably, as it depends on upstream changes that we cannot predict. However, with this change, ABI mismatches should be detected in most cases. In the worst case, if any part of upstream build infrastructure changes the way it selects the Hermes version, we will immediately know about it, because an artifact built with the wrong Hermes version will be rejected by the build system and the build will fall back to building from source.
Fixed Issues
$ #100503
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari