[LOCAL][CI] Require cocoapods to hermes-engine podspec #45134
Merged
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
the
CLA Signed
cipolleschi
force-pushed
the
cipolleschi/fix-hermes-scripts-for-GHA
branch
6 times, most recently
from
e37d284 to
6409036
Compare
cipolleschi
force-pushed
the
cipolleschi/fix-hermes-scripts-for-GHA
branch
4 times, most recently
from
9f16584 to
ac54bfe
Compare
Summary: This change is the first step in refactoring GHA so that they can be reused more easily across jobs. Its goal is also to be more reliable w.r.t. caches. That this change do: * moves `prepare_hermes_workspace` to a composite action * saves the `prepare_hermes_workspace` caches only on main * uploads the destination folder as an artifact so that we can use it later in the run * makes the `test-all`, `nightly` and `publish-release` workflow use the new composite action * updates the `setup-hermes-workspace` to download and use the artifact uploaded by `prepare_hermes_workspace` [Internal] - Factor out the prepare_hermes_workspace action Pull Request resolved: #45071 Test Plan: GHA in CI Reviewed By: cortinico Differential Revision: D58808087 Pulled By: cipolleschi fbshipit-source-id: 42c46bcf75fc73b2edfda9be62b5d0fe8a919a5d
cipolleschi
force-pushed
the
cipolleschi/fix-hermes-scripts-for-GHA
branch
from
ac54bfe to
88bee2c
Compare
cipolleschi
commented
Jun 24, 2024
| @@ -84,5 +84,5 @@ runs: | |||
| path: | | |||
| /tmp/hermes/download/ | |||
| /tmp/hermes/hermes/ | |||
| key: v1-hermes-${{ inputs.hermes-version }}-${{ github.run_number }} | |||
| key: v1-hermes-${{ inputs.hermes-version }} | |||
There was a problem hiding this comment.
cherry picked 9c6c637 for stabilize prepare-hermes-workspace
| @@ -0,0 +1,104 @@ | |||
| name: prepare-hermes-workspace | |||
There was a problem hiding this comment.
cherry picked 9c6c637 for stabilize prepare-hermes-workspace
| @@ -1,8 +1,13 @@ | |||
| name: setup_hermes_workspace | |||
| description: "Setup hermes workspace" | |||
| name: restore-hermes-workspace | |||
There was a problem hiding this comment.
cherry picked 9c6c637 for stabilize prepare-hermes-workspace
Comment on lines
+63
to
+74
| - name: Upload HermesC Artifact | ||
| uses: actions/[email protected] | ||
| with: | ||
| name: hermesc-apple | ||
| path: ./packages/react-native/sdks/hermes/build_host_hermesc | ||
| - name: Cache hermesc apple | ||
| uses: actions/cache/[email protected] | ||
| if: ${{ github.ref == 'refs/heads/main' }} # To avoid that the cache explode. | ||
| with: | ||
| path: ./packages/react-native/sdks/hermes/build_host_hermesc | ||
| key: v2-hermesc-apple-${{ needs.prepare_hermes_workspace.outputs.hermes-version }}-${{ needs.prepare_hermes_workspace.outputs.react-native-version }} | ||
| enableCrossOsArchive: true |
There was a problem hiding this comment.
apply the same cache-and-artifacts optimization we have in hermew-workspace to stabilize HermesC
Comment on lines
+83
to
+85
| IOS_DEPLOYMENT_TARGET: "13.4" | ||
| XROS_DEPLOYMENT_TARGET: "1.0" | ||
| MAC_DEPLOYMENT_TARGET: "10.15" |
There was a problem hiding this comment.
Added this to avoid reading cocoapods. That's in general better, we can find a way to read those from react-native rather than have a duplication.
However, these are fixed for the Release, so they should never change for 0.75
Comment on lines
+103
to
+109
| - name: Restore Hermes workspace | ||
| uses: ./.github/actions/restore-hermes-workspace | ||
| - name: Restore HermesC Artifact | ||
| uses: actions/[email protected] | ||
| with: | ||
| name: hermesc-apple | ||
| path: ./packages/react-native/sdks/hermes/build_host_hermesc |
There was a problem hiding this comment.
Restore hermesc artifact from previous job
| @@ -169,11 +141,17 @@ jobs: | |||
| exit 0 | |||
| fi | |||
|
|
|||
| export RELEASE_VERSION=${{ needs.prepare_hermes_workspace.outputs.react-native-version }} | |||
There was a problem hiding this comment.
Export React Native version so we don't need to read from cocoapods
cortinico
approved these changes
Jun 24, 2024
cipolleschi
added a commit
that referenced
this pull request
Jun 24, 2024
* [LOCAL][CI] Require cocoapods to hermes-engine podspec * Refactor Hermes workspace (#45071) Summary: This change is the first step in refactoring GHA so that they can be reused more easily across jobs. Its goal is also to be more reliable w.r.t. caches. That this change do: * moves `prepare_hermes_workspace` to a composite action * saves the `prepare_hermes_workspace` caches only on main * uploads the destination folder as an artifact so that we can use it later in the run * makes the `test-all`, `nightly` and `publish-release` workflow use the new composite action * updates the `setup-hermes-workspace` to download and use the artifact uploaded by `prepare_hermes_workspace` [Internal] - Factor out the prepare_hermes_workspace action Pull Request resolved: #45071 Test Plan: GHA in CI Reviewed By: cortinico Differential Revision: D58808087 Pulled By: cipolleschi fbshipit-source-id: 42c46bcf75fc73b2edfda9be62b5d0fe8a919a5d
Base commit: 61b3c95 |
This was referenced Jun 24, 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:
Building MacOS slices is failing because the Hermes build script tries to read the version for MacOS by evaluating the
hermes-engine.podspec. The Hermes script is actually executing thepodspec.However, the
podspecrelies on some constants that are defined in Cocoapods, but it is not explicitly depending on it. So, when executed without a cocoapods command, like we are doing in the hermes build scripts, it fails.To solve this, we are adding cocoapods as an explicit dependency of the
podspecChangelog:
[Internal] - Add explicit dependency to cocoapods in hermes-engine.podspec
Test Plan:
GHA should be green.