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.

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

Fix/hermes in release #34223

Merged
merged 1 commit into from
Jul 20, 2022
Merged

Fix/hermes in release #34223

merged 1 commit into from
Jul 20, 2022

Conversation

cipolleschi
Copy link
Contributor

@cipolleschi cipolleschi commented Jul 19, 2022

Summary

The prepare_hermes_workspace has a Download Hermes Tarball that downloads the tarball to build hermes from source only in some circumstances. When we have a PR that runs against X.YY-stable, the download does not happens. However, the CI expect to have a couple of folder and tries to move them.
When the download does not happens, these folders are not there and therefore the CI fails.

This PR should fix the issue: when the download does not happen, we don't try to copy these folders.

Changelog

Avoid copying the folders when they are not there.

[General] [Changed] - When preparing the Hermes workspace, we don't copy the folders that are not present.

Test Plan

The CI should be green.

@facebook-github-bot 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 19, 2022
@cipolleschi cipolleschi changed the base branch from main to 0.69-stable July 19, 2022 15:48
@cipolleschi cipolleschi force-pushed the fix/hermes_in_release branch from c120db2 to 4338826 Compare July 19, 2022 15:57
@analysis-bot
Copy link

analysis-bot commented Jul 19, 2022

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,794,949 -25,535
android hermes armeabi-v7a 7,196,376 -17,996
android hermes x86 8,103,481 -30,723
android hermes x86_64 8,083,326 -28,983
android jsc arm64-v8a 9,667,984 -30,019
android jsc armeabi-v7a 8,438,444 -15,683
android jsc x86 9,616,904 -32,896
android jsc x86_64 10,214,293 -33,730

Base commit: d0b1d49
Branch: main

cipolleschi pushed a commit to cipolleschi/react-native that referenced this pull request Jul 19, 2022
Summary:
This Diff is a copy of this [PR](facebook#34223) that we have against 0.69-stable.

This Diff introduces some checks to prevent that we try to copy folders that are not present.

## Changelog

Avoid copying the folders when they are not there.

[General] [Changed] - When preparing the Hermes workspace, we don't copy the folders that are not present.

Differential Revision: D37961092

fbshipit-source-id: f52f73a53a93dcb549d5c5cbf714e3ba242ada9a
@cipolleschi cipolleschi force-pushed the fix/hermes_in_release branch from 4338826 to d6be1af Compare July 19, 2022 16:33
@kelset
Copy link
Contributor

kelset commented Jul 19, 2022

@cipolleschi do you rekon we need to wait for this to land to do 0.69.2 or we could proceed without? We are planning to push it out tomorrow morning

@cipolleschi
Copy link
Contributor Author

@kelset I'm working on this to unblock @Kudo on #34214. If we need #34214, then yes, this should be included, otherwise we can't know whether the other PR break the CI or not.
Otherwise, no, we can live without this and wait for #34224 which is this same PR but on main.

The bad thing is that the CI will take 2 hours to finish, but given that we are targeting 0.69-stable, we don't have to import this in fbsource, so as soon as the CI is green (a part from the test_ios_unit job) we can merge this.

@cipolleschi cipolleschi marked this pull request as ready for review July 19, 2022 17:15
@cipolleschi cipolleschi requested a review from hramos as a code owner July 19, 2022 17:15
@kelset
Copy link
Contributor

kelset commented Jul 19, 2022

ok, no rush - worst case we can do a separate 0.69.3 :)

@facebook-github-bot 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 19, 2022
@cipolleschi
Copy link
Contributor Author

@kelset: I think this is ok. If you want, we can merge this to unlock @Kudo and #34214

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants