Skip to content

Commit 0627ac7

Browse files
Saadnajmiclaude
andcommitted
refactor: address PR review comments (diff tags, code cleanup)
- Add missing [macOS] diff tags across Package.swift, hermes.js, and shell build scripts per the diff tag guide - Move macosx case to bottom of get_deployment_target elif chain to minimize upstream diff in build-ios-framework.sh - Remove hermes.js re-exports; import directly from macosVersionResolver - Extract {stdio: 'inherit'} to const in buildFromHermesCommit - Add HERMES_PATH override comment in build-apple-framework.sh - Increase CI artifact retention-days from 1 to 30 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent dee0b6a commit 0627ac7

5 files changed

Lines changed: 29 additions & 34 deletions

File tree

.github/workflows/microsoft-build-spm.yml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ jobs:
3434
id: resolve
3535
working-directory: packages/react-native
3636
run: |
37-
COMMIT=$(node -e "const {hermesCommitAtMergeBase} = require('./scripts/ios-prebuild/hermes'); console.log(hermesCommitAtMergeBase().commit);" 2>&1 | grep -E '^[0-9a-f]{40}$')
37+
COMMIT=$(node -e "const {hermesCommitAtMergeBase} = require('./scripts/ios-prebuild/macosVersionResolver'); console.log(hermesCommitAtMergeBase().commit);" 2>&1 | grep -E '^[0-9a-f]{40}$')
3838
echo "hermes-commit=$COMMIT" >> "$GITHUB_OUTPUT"
3939
echo "Resolved Hermes commit: $COMMIT"
4040
@@ -51,7 +51,7 @@ jobs:
5151
with:
5252
name: hermes-artifacts
5353
path: hermes-destroot
54-
retention-days: 1
54+
retention-days: 30
5555

5656
build-hermesc:
5757
name: "Build hermesc"
@@ -89,7 +89,7 @@ jobs:
8989
with:
9090
name: hermesc
9191
path: hermes/build_host_hermesc
92-
retention-days: 1
92+
retention-days: 30
9393

9494
build-hermes-slice:
9595
name: "Hermes ${{ matrix.slice }}"
@@ -151,7 +151,7 @@ jobs:
151151
with:
152152
name: hermes-slice-${{ matrix.slice }}
153153
path: hermes/destroot
154-
retention-days: 1
154+
retention-days: 30
155155

156156
assemble-hermes:
157157
name: "Assemble Hermes xcframework"
@@ -207,7 +207,7 @@ jobs:
207207
with:
208208
name: hermes-artifacts
209209
path: hermes/destroot
210-
retention-days: 1
210+
retention-days: 30
211211

212212
build-spm:
213213
name: "SPM ${{ matrix.platform }}"

packages/react-native/Package.swift

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ let reactJsErrorHandler = RNTarget(
249249
let reactGraphicsApple = RNTarget(
250250
name: .reactGraphicsApple,
251251
path: "ReactCommon/react/renderer/graphics/platform/ios",
252-
linkedFrameworks: ["CoreGraphics"],
252+
linkedFrameworks: ["CoreGraphics"], // [macOS] UIKit removed; linked conditionally via platformLinkerSettings below
253253
// [macOS] Package.swift evaluates on the host (macOS), not the target, so #if os(macOS) doesn't work for cross-compilation.
254254
// not the target. Use .when(platforms:) for cross-compilation support.
255255
platformLinkerSettings: [
@@ -400,8 +400,7 @@ let reactFabric = RNTarget(
400400
"components/view/tests",
401401
"components/view/platform/android",
402402
"components/view/platform/windows",
403-
// "components/view/platform/cxx", // [macOS] excluded on macOS, included on iOS/visionOS (see reactFabricViewPlatformExcludes)
404-
// "components/view/platform/macos", // [macOS] excluded on iOS/visionOS, included on macOS (see reactFabricViewPlatformExcludes)
403+
// "components/view/platform/macos", // [macOS] moved to reactFabricViewPlatformExcludes for conditional exclusion
405404
"components/scrollview/tests",
406405
"components/scrollview/platform/android",
407406
"mounting/tests",

packages/react-native/scripts/ios-prebuild/hermes.js

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ async function prepareHermesArtifactsAsync(
105105
const sourceType = await hermesSourceType(
106106
resolvedVersion,
107107
buildType,
108-
allowBuildFromSource,
108+
allowBuildFromSource, // [macOS]
109109
);
110110
localPath = await resolveSourceFromSourceType(
111111
sourceType,
@@ -156,7 +156,7 @@ type HermesEngineSourceType =
156156
| 'local_prebuilt_tarball'
157157
| 'download_prebuild_tarball'
158158
| 'download_prebuilt_nightly_tarball'
159-
| 'build_from_hermes_commit'
159+
| 'build_from_hermes_commit' // [macOS]
160160
*/
161161

162162
const HermesEngineSourceTypes = {
@@ -258,16 +258,11 @@ async function hermesArtifactExists(
258258

259259
/**
260260
* Determines the source type for Hermes based on availability
261-
*
262-
* @param version - The resolved version string
263-
* @param buildType - Debug or Release
264-
* @param allowBuildFromSource - If true (macOS main branch), fall back to BUILD_FROM_HERMES_COMMIT
265-
* when no prebuilt artifacts exist. If false, fall back to nightly download (original behavior).
266261
*/
267262
async function hermesSourceType(
268263
version /*: string */,
269264
buildType /*: BuildFlavor */,
270-
allowBuildFromSource /*: boolean */ = false,
265+
allowBuildFromSource /*: boolean */ = false, // [macOS]
271266
) /*: Promise<HermesEngineSourceType> */ {
272267
if (hermesEngineTarballEnvvarDefined()) {
273268
hermesLog('Using local prebuild tarball');
@@ -443,20 +438,22 @@ async function buildFromHermesCommit(
443438
const HERMES_GITHUB_URL = 'https://github.com/facebook/hermes.git';
444439
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-build-'));
445440
const hermesDir = path.join(tmpDir, 'hermes');
441+
const inheritStdio = {stdio: 'inherit'};
446442

447443
try {
448444
// Clone Hermes at the identified commit using the most efficient
449445
// single-fetch pattern (see https://github.com/actions/checkout)
450446
hermesLog(`Cloning Hermes at commit ${commit}...`);
451-
execSync(`git init "${hermesDir}"`, {stdio: 'inherit'});
452-
execSync(`git -C "${hermesDir}" remote add origin ${HERMES_GITHUB_URL}`, {
453-
stdio: 'inherit',
454-
});
447+
execSync(`git init "${hermesDir}"`, inheritStdio);
448+
execSync(
449+
`git -C "${hermesDir}" remote add origin ${HERMES_GITHUB_URL}`,
450+
inheritStdio,
451+
);
455452
execSync(
456453
`git -C "${hermesDir}" fetch --no-tags --depth 1 origin +${commit}:refs/remotes/origin/main`,
457-
{stdio: 'inherit', timeout: 300000},
454+
{...inheritStdio, timeout: 300000},
458455
);
459-
execSync(`git -C "${hermesDir}" checkout main`, {stdio: 'inherit'});
456+
execSync(`git -C "${hermesDir}" checkout main`, inheritStdio);
460457

461458
const reactNativeRoot = path.resolve(__dirname, '..', '..');
462459
const buildScript = path.join(
@@ -482,8 +479,8 @@ async function buildFromHermesCommit(
482479

483480
hermesLog(`Building Hermes frameworks (${buildType})...`);
484481
execSync(`bash "${buildScript}"`, {
482+
...inheritStdio,
485483
cwd: hermesDir,
486-
stdio: 'inherit',
487484
timeout: 3600000, // 60 minutes
488485
env: buildEnv,
489486
});
@@ -492,9 +489,7 @@ async function buildFromHermesCommit(
492489
const tarballName = `hermes-ios-${buildType.toLowerCase()}.tar.gz`;
493490
const tarballPath = path.join(artifactsPath, tarballName);
494491
hermesLog('Creating Hermes tarball from build output...');
495-
execSync(`tar -czf "${tarballPath}" -C "${hermesDir}" destroot`, {
496-
stdio: 'inherit',
497-
});
492+
execSync(`tar -czf "${tarballPath}" -C "${hermesDir}" destroot`, inheritStdio);
498493

499494
hermesLog(`Hermes built from source and packaged at ${tarballPath}`);
500495
return tarballPath;
@@ -538,6 +533,4 @@ function abort(message /*: string */) {
538533

539534
module.exports = {
540535
prepareHermesArtifactsAsync,
541-
findMatchingHermesVersion, // [macOS] re-exported from macosVersionResolver.js
542-
hermesCommitAtMergeBase, // [macOS] re-exported from macosVersionResolver.js
543536
};

packages/react-native/sdks/hermes-engine/utils/build-apple-framework.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ CURR_SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd -P)"
1212
IMPORT_HERMESC_PATH=${HERMES_OVERRIDE_HERMESC_PATH:-$PWD/build_host_hermesc/ImportHermesc.cmake}
1313
BUILD_TYPE=${BUILD_TYPE:-Debug}
1414

15+
# [macOS] Allow HERMES_PATH override for building from a standalone Hermes checkout
1516
HERMES_PATH=${HERMES_PATH:-"$CURR_SCRIPT_DIR/.."}
1617
REACT_NATIVE_PATH=${REACT_NATIVE_PATH:-$CURR_SCRIPT_DIR/../../..}
1718

packages/react-native/sdks/hermes-engine/utils/build-ios-framework.sh

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ function get_architecture {
2020
echo "arm64"
2121
elif [[ $1 == "appletvsimulator" ]]; then
2222
echo "x86_64;arm64"
23-
elif [[ $1 == "catalyst" || $1 == "macosx" ]]; then
23+
elif [[ $1 == "catalyst" || $1 == "macosx" ]]; then # [macOS] added macosx
2424
echo "x86_64;arm64"
2525
else
2626
echo "Error: unknown architecture passed $1"
@@ -29,10 +29,12 @@ function get_architecture {
2929
}
3030

3131
function get_deployment_target {
32-
if [[ $1 == "macosx" ]]; then
33-
echo "$(get_mac_deployment_target)"
34-
elif [[ $1 == "xros" || $1 == "xrsimulator" ]]; then
32+
if [[ $1 == "xros" || $1 == "xrsimulator" ]]; then
3533
echo "$(get_visionos_deployment_target)"
34+
# [macOS
35+
elif [[ $1 == "macosx" ]]; then
36+
echo "$(get_mac_deployment_target)"
37+
# macOS]
3638
else # tvOS and iOS use the same deployment target
3739
echo "$(get_ios_deployment_target)"
3840
fi
@@ -55,7 +57,7 @@ function build_framework {
5557
# group the frameworks together to create a universal framework
5658
function build_universal_framework {
5759
if [ ! -d destroot/Library/Frameworks/universal/hermes.xcframework ]; then
58-
create_universal_framework "macosx" "iphoneos" "iphonesimulator" "catalyst" "xros" "xrsimulator" "appletvos" "appletvsimulator"
60+
create_universal_framework "macosx" "iphoneos" "iphonesimulator" "catalyst" "xros" "xrsimulator" "appletvos" "appletvsimulator" # [macOS] added macosx
5961
else
6062
echo "Skipping; Clean \"destroot\" to rebuild".
6163
fi
@@ -65,7 +67,7 @@ function build_universal_framework {
6567
# this is used to preserve backward compatibility
6668
function create_framework {
6769
if [ ! -d destroot/Library/Frameworks/universal/hermes.xcframework ]; then
68-
build_framework "macosx"
70+
build_framework "macosx" # [macOS]
6971
build_framework "iphoneos"
7072
build_framework "iphonesimulator"
7173
build_framework "appletvos"

0 commit comments

Comments
 (0)