Repository navigation
Conversation
chrfalch
force-pushed
the
chrfalch/spm-drop-absolute-hermes-cli-path
branch
from
September 3, 2026 07:29
7164f61 to
6793d6f
Compare
chrfalch
force-pushed
the
chrfalch/spm-drop-absolute-hermes-cli-path
branch
3 times, most recently
from
September 3, 2026 08:17
c1077b1 to
cc6d8dd
Compare
`spm add` wrote the machine-specific absolute path of hermesc from the hermes-compiler npm package into every build configuration, and apps commit that pbxproj. react-native-xcode.sh already resolves hermesc at build time through react-native's own dependency graph, so drop the write: resolveHermesCliPathSetting() and the hermesCliPath parameter it fed go with it. Everything involved arrived with SwiftPM in #57332 and is still Preview-labelled, so removing the parameter from injectSpmIntoPbxproj and mergeReactBuildSettings is in scope even though it shipped in 0.87.0/0.87.1. Nothing a CocoaPods build uses is touched: the pod-derived HERMES_CLI_PATH default, the "hermesc could not be found" error and the hermesc -emit-binary call all pre-date SwiftPM. react-native-xcode.sh is unchanged here. Existing injected projects clean up on the next `spm add`/`update`, which strips every recorded scalar before re-injecting. Known gap, deliberately left for a follow-up: the shell fallback is gated on PODS_ROOT being absent, so a SwiftPM app that keeps side-by-side non-RN pods never reaches it and needs an explicit HERMES_CLI_PATH — set in an xcconfig, not the pbxproj, because the recorded-scalar cleanup is value-blind. Keying that gate on the hermes-engine pod directory would close it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
chrfalch
force-pushed
the
chrfalch/spm-drop-absolute-hermes-cli-path
branch
from
September 3, 2026 08:59
cc6d8dd to
6439916
Compare
cipolleschi
approved these changes
Sep 3, 2026
cipolleschi
left a comment
Contributor
There was a problem hiding this comment.
Thanks for fixing this.
|
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this in D118624244. |
cortinico
approved these changes
Sep 3, 2026
cortinico
left a comment
Contributor
There was a problem hiding this comment.
Review automatically exported from Phabricator review in Meta.
Copilot stopped work on behalf of
rachel11026725 due to an error
September 3, 2026 15:42
Copilot stopped work on behalf of
rachel11026725 due to an error
September 3, 2026 15:42
|
@cipolleschi merged this pull request in fcbeda1. |
shashikant-panchal
pushed a commit
to shashikant-panchal/react-native
that referenced
this pull request
Sep 10, 2026
…8292) Summary: `npx react-native spm add` writes `HERMES_CLI_PATH` into the app's committed `project.pbxproj` — the absolute path of `hermesc` in the `hermes-compiler` npm package, as resolved on whichever machine ran the command. So every SwiftPM-converted app commits one developer's disk layout. Nothing needs it: `react-native-xcode.sh` already resolves `hermesc` at build time through react-native's own dependency graph when the current value is not a file. A relative setting can't replace it either — `$(REACT_NATIVE_PATH)/../hermes-compiler` breaks on a symlinked `react-native`, and `$(SRCROOT)/../node_modules/…` breaks on hoisted monorepos. So this deletes `resolveHermesCliPathSetting()`, along with the `hermesCliPath` parameter it fed on `injectSpmIntoPbxproj` and `mergeReactBuildSettings`. Already-injected projects self-clean on the next `spm add`/`update`, which strips every scalar recorded in `.spm-injected.json` before re-injecting. ### Scope: SwiftPM only Both pieces involved arrived with SwiftPM in react#57332 and first shipped in 0.87.0 — `scripts/spm/generate-spm-xcodeproj.js` (the whole file, including the write) and the `NODE_HERMESC` build-time fallback in `react-native-xcode.sh`. Everything CocoaPods relies on is older and untouched, all present in 0.86.0: the pod-derived `HERMES_CLI_PATH` default, the "hermesc could not be found" error, and the `hermesc -emit-binary` call. **A CocoaPods build cannot be affected by this change.** That parameter shipped in 0.87.0 and 0.87.1, but SwiftPM is Preview-labelled, so removing it is in scope. Nothing in the repo passed it except the deleted resolver. ## Known limitation The fallback is gated on `PODS_ROOT` being absent, so a SwiftPM app that keeps side-by-side non-RN pods skips it and needs an explicit `HERMES_CLI_PATH`. Evaluating that gate verbatim out of the unchanged script: | app | gate | hermesc | | --- | --- | --- | | no `Pods/` (normal SwiftPM app) | fires | from `node_modules` | | `Pods/`, non-RN pods only | skipped | pod path — absent | | `Pods/` with `hermes-engine` | skipped | the pod's, unchanged | Set it in an xcconfig, not the pbxproj: `createdScalars` cleanup removes by key, so it would drop a hand-edited value too. Keying the gate on the `hermes-engine` directory would close this — left as a follow-up so this PR doesn't touch the bundling script. ## Changelog: [IOS] [FIXED] - SwiftPM: stop baking an absolute, machine-specific HERMES_CLI_PATH into the app's pbxproj; resolve hermesc at build time instead Pull Request resolved: react#58292 Test Plan: Red first — with the source reverted, the new entry-point test fails on a seeded `hermes-compiler` fixture: `✕ writes no HERMES_CLI_PATH, in any configuration or the marker` (1 failed, 76 passed). ``` yarn jest packages/react-native/scripts/spm --no-cache -i Test Suites: 18 passed, 18 total Tests: 835 passed, 835 total ``` `yarn flow-check` (0 errors), plus `yarn eslint --max-warnings 0` and `yarn prettier --check` on the four changed files. End to end: a 0.87.1 SwiftPM app (`shopify/react-native-skia` example, no Pods) with the `HERMES_CLI_PATH` lines removed from its pbxproj builds in Release, the fallback resolving `hermesc` from `node_modules/hermes-compiler`. That build ran against an earlier revision of this branch which also widened the shell gate; with no `Pods/` it is row 1, where both conditions behave identically. There is no shell test harness, and the script is unchanged here. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewed By: cortinico Differential Revision: D118624244 Pulled By: cipolleschi fbshipit-source-id: 5784a2de69af0c50c82129d96423801b0f5b0eba
tvinhas
pushed a commit
to tvinhas/react-native-macos
that referenced
this pull request
Oct 21, 2026
…8292) Summary: `npx react-native spm add` writes `HERMES_CLI_PATH` into the app's committed `project.pbxproj` — the absolute path of `hermesc` in the `hermes-compiler` npm package, as resolved on whichever machine ran the command. So every SwiftPM-converted app commits one developer's disk layout. Nothing needs it: `react-native-xcode.sh` already resolves `hermesc` at build time through react-native's own dependency graph when the current value is not a file. A relative setting can't replace it either — `$(REACT_NATIVE_PATH)/../hermes-compiler` breaks on a symlinked `react-native`, and `$(SRCROOT)/../node_modules/…` breaks on hoisted monorepos. So this deletes `resolveHermesCliPathSetting()`, along with the `hermesCliPath` parameter it fed on `injectSpmIntoPbxproj` and `mergeReactBuildSettings`. Already-injected projects self-clean on the next `spm add`/`update`, which strips every scalar recorded in `.spm-injected.json` before re-injecting. ### Scope: SwiftPM only Both pieces involved arrived with SwiftPM in react#57332 and first shipped in 0.87.0 — `scripts/spm/generate-spm-xcodeproj.js` (the whole file, including the write) and the `NODE_HERMESC` build-time fallback in `react-native-xcode.sh`. Everything CocoaPods relies on is older and untouched, all present in 0.86.0: the pod-derived `HERMES_CLI_PATH` default, the "hermesc could not be found" error, and the `hermesc -emit-binary` call. **A CocoaPods build cannot be affected by this change.** That parameter shipped in 0.87.0 and 0.87.1, but SwiftPM is Preview-labelled, so removing it is in scope. Nothing in the repo passed it except the deleted resolver. ## Known limitation The fallback is gated on `PODS_ROOT` being absent, so a SwiftPM app that keeps side-by-side non-RN pods skips it and needs an explicit `HERMES_CLI_PATH`. Evaluating that gate verbatim out of the unchanged script: | app | gate | hermesc | | --- | --- | --- | | no `Pods/` (normal SwiftPM app) | fires | from `node_modules` | | `Pods/`, non-RN pods only | skipped | pod path — absent | | `Pods/` with `hermes-engine` | skipped | the pod's, unchanged | Set it in an xcconfig, not the pbxproj: `createdScalars` cleanup removes by key, so it would drop a hand-edited value too. Keying the gate on the `hermes-engine` directory would close this — left as a follow-up so this PR doesn't touch the bundling script. ## Changelog: [IOS] [FIXED] - SwiftPM: stop baking an absolute, machine-specific HERMES_CLI_PATH into the app's pbxproj; resolve hermesc at build time instead Pull Request resolved: react#58292 Test Plan: Red first — with the source reverted, the new entry-point test fails on a seeded `hermes-compiler` fixture: `✕ writes no HERMES_CLI_PATH, in any configuration or the marker` (1 failed, 76 passed). ``` yarn jest packages/react-native/scripts/spm --no-cache -i Test Suites: 18 passed, 18 total Tests: 835 passed, 835 total ``` `yarn flow-check` (0 errors), plus `yarn eslint --max-warnings 0` and `yarn prettier --check` on the four changed files. End to end: a 0.87.1 SwiftPM app (`shopify/react-native-skia` example, no Pods) with the `HERMES_CLI_PATH` lines removed from its pbxproj builds in Release, the fallback resolving `hermesc` from `node_modules/hermes-compiler`. That build ran against an earlier revision of this branch which also widened the shell gate; with no `Pods/` it is row 1, where both conditions behave identically. There is no shell test harness, and the script is unchanged here. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewed By: cortinico Differential Revision: D118624244 Pulled By: cipolleschi fbshipit-source-id: 5784a2de69af0c50c82129d96423801b0f5b0eba
meta-codesync Bot
pushed a commit
that referenced
this pull request
Oct 23, 2026
Summary: Recent fixes to the iOS SwiftPM tooling (`scripts/spm`) all fixed bugs that CI did not catch. Adopters found them first. Each fix added a test for its own bug only. The bugs fall into a few classes: output that changes when nothing changed, generated names that collide or do not resolve, values that are hard-coded instead of read from the app, machine-specific paths in files that users commit, and malformed files that Xcode tolerates. This PR adds two test-only tools that look for each class in general. They run in the existing Linux Jest job (`test_js`). No production code changes. 1. **Invariant checker** (`spm-invariants.js`, not a test file itself). It reads the files that the tooling generates and returns a list of violations: - no machine paths (temp dir, `$HOME`, absolute build-setting values) in `project.pbxproj`, `.xcscheme`, or `.spm-injected.json`; - the scheme parses as XML, and the pre-action script keeps its line breaks; - in every generated `Package.swift`, target and product names are unique (also case-insensitive) and valid C99 identifiers, and every dependency resolves, including products of local `.package(path:)` dependencies; - every generated or scaffolded manifest uses the app's iOS deployment target; the codegen manifest, whose template fixes `.iOS(.v15)`, must not be above it; - `project.pbxproj` is balanced, every referenced object ID is defined, and the frameworks in `.spm-injected.json` match the project. Each rule also fails when the file or field it checks is missing, so a check cannot pass by finding nothing. 2. **Lifecycle test** (`spm-lifecycle-test.js` + `spm-fixture-app.js`). It builds a fake app on disk (deployment target 16.4), fake prebuilt artifacts for two React Native versions, and four fake libraries: one with its own `Package.swift`, one podspec-only library with `module_name` different from its npm name, one scoped `acme/…` package, and one autolinking plugin with a flavored framework. It then runs `scaffold → add → sync → sync → update (same version) → update (next version) → deinit` through `setup-apple-spm.js`, and runs the checker after each step. It also asserts: - a second sync changes no byte and replaces no `libs/` entry; - the build-time stale check reacts to the right changes and ignores files that Xcode writes; - a plugin framework mismatch makes sync exit with code 2; - `update` to the same version and `deinit` restore the project and scheme byte for byte. Network access, `plutil`, `otool`, codegen and `pod` are mocked, so the test runs offline on Linux. ## Changelog: [INTERNAL] [ADDED] - Add regression tests for the iOS SwiftPM tooling Pull Request resolved: #59217 Test Plan: ``` yarn jest --no-cache -i packages/react-native/scripts/spm/__tests__ ``` Result: 24 suites, 1124 tests, all passed, about 23 s (on main at dd9b6c5). The two new suites take about 4 s. Each earlier fix was undone one at a time with `git show <sha> -- ':!**/__tests__/**' | git apply -R`, and the new tests failed each time: | Fix reverted | New tests failing | |---|---| | #58826 (scheme pre-action newlines) | 9 | | #58292 (absolute `HERMES_CLI_PATH` in pbxproj) | 9 | | #58597 (`libs/` rewritten on every sync) | 2 | | #58828 (watched path absent at add time) | 1 | `yarn eslint` and `prettier --check` on the new files: clean. Not run: Linux locally. The `test_js` job on this PR is the first Linux run. Reviewed By: cortinico Differential Revision: D126304581 Pulled By: cipolleschi fbshipit-source-id: c1bacf9ef2c6955a262cac0053491d69535375e9
cipolleschi
pushed a commit
that referenced
this pull request
Oct 26, 2026
Summary: Recent fixes to the iOS SwiftPM tooling (`scripts/spm`) all fixed bugs that CI did not catch. Adopters found them first. Each fix added a test for its own bug only. The bugs fall into a few classes: output that changes when nothing changed, generated names that collide or do not resolve, values that are hard-coded instead of read from the app, machine-specific paths in files that users commit, and malformed files that Xcode tolerates. This PR adds two test-only tools that look for each class in general. They run in the existing Linux Jest job (`test_js`). No production code changes. 1. **Invariant checker** (`spm-invariants.js`, not a test file itself). It reads the files that the tooling generates and returns a list of violations: - no machine paths (temp dir, `$HOME`, absolute build-setting values) in `project.pbxproj`, `.xcscheme`, or `.spm-injected.json`; - the scheme parses as XML, and the pre-action script keeps its line breaks; - in every generated `Package.swift`, target and product names are unique (also case-insensitive) and valid C99 identifiers, and every dependency resolves, including products of local `.package(path:)` dependencies; - every generated or scaffolded manifest uses the app's iOS deployment target; the codegen manifest, whose template fixes `.iOS(.v15)`, must not be above it; - `project.pbxproj` is balanced, every referenced object ID is defined, and the frameworks in `.spm-injected.json` match the project. Each rule also fails when the file or field it checks is missing, so a check cannot pass by finding nothing. 2. **Lifecycle test** (`spm-lifecycle-test.js` + `spm-fixture-app.js`). It builds a fake app on disk (deployment target 16.4), fake prebuilt artifacts for two React Native versions, and four fake libraries: one with its own `Package.swift`, one podspec-only library with `module_name` different from its npm name, one scoped `acme/…` package, and one autolinking plugin with a flavored framework. It then runs `scaffold → add → sync → sync → update (same version) → update (next version) → deinit` through `setup-apple-spm.js`, and runs the checker after each step. It also asserts: - a second sync changes no byte and replaces no `libs/` entry; - the build-time stale check reacts to the right changes and ignores files that Xcode writes; - a plugin framework mismatch makes sync exit with code 2; - `update` to the same version and `deinit` restore the project and scheme byte for byte. Network access, `plutil`, `otool`, codegen and `pod` are mocked, so the test runs offline on Linux. ## Changelog: [INTERNAL] [ADDED] - Add regression tests for the iOS SwiftPM tooling Pull Request resolved: #59217 Test Plan: ``` yarn jest --no-cache -i packages/react-native/scripts/spm/__tests__ ``` Result: 24 suites, 1124 tests, all passed, about 23 s (on main at dd9b6c5). The two new suites take about 4 s. Each earlier fix was undone one at a time with `git show <sha> -- ':!**/__tests__/**' | git apply -R`, and the new tests failed each time: | Fix reverted | New tests failing | |---|---| | #58826 (scheme pre-action newlines) | 9 | | #58292 (absolute `HERMES_CLI_PATH` in pbxproj) | 9 | | #58597 (`libs/` rewritten on every sync) | 2 | | #58828 (watched path absent at add time) | 1 | `yarn eslint` and `prettier --check` on the new files: clean. Not run: Linux locally. The `test_js` job on this PR is the first Linux run. Reviewed By: cortinico Differential Revision: D126304581 Pulled By: cipolleschi fbshipit-source-id: c1bacf9ef2c6955a262cac0053491d69535375e9 (cherry picked from commit fba55e2)
tvinhas
pushed a commit
to tvinhas/react-native-macos
that referenced
this pull request
Oct 28, 2026
Summary: Recent fixes to the iOS SwiftPM tooling (`scripts/spm`) all fixed bugs that CI did not catch. Adopters found them first. Each fix added a test for its own bug only. The bugs fall into a few classes: output that changes when nothing changed, generated names that collide or do not resolve, values that are hard-coded instead of read from the app, machine-specific paths in files that users commit, and malformed files that Xcode tolerates. This PR adds two test-only tools that look for each class in general. They run in the existing Linux Jest job (`test_js`). No production code changes. 1. **Invariant checker** (`spm-invariants.js`, not a test file itself). It reads the files that the tooling generates and returns a list of violations: - no machine paths (temp dir, `$HOME`, absolute build-setting values) in `project.pbxproj`, `.xcscheme`, or `.spm-injected.json`; - the scheme parses as XML, and the pre-action script keeps its line breaks; - in every generated `Package.swift`, target and product names are unique (also case-insensitive) and valid C99 identifiers, and every dependency resolves, including products of local `.package(path:)` dependencies; - every generated or scaffolded manifest uses the app's iOS deployment target; the codegen manifest, whose template fixes `.iOS(.v15)`, must not be above it; - `project.pbxproj` is balanced, every referenced object ID is defined, and the frameworks in `.spm-injected.json` match the project. Each rule also fails when the file or field it checks is missing, so a check cannot pass by finding nothing. 2. **Lifecycle test** (`spm-lifecycle-test.js` + `spm-fixture-app.js`). It builds a fake app on disk (deployment target 16.4), fake prebuilt artifacts for two React Native versions, and four fake libraries: one with its own `Package.swift`, one podspec-only library with `module_name` different from its npm name, one scoped `acme/…` package, and one autolinking plugin with a flavored framework. It then runs `scaffold → add → sync → sync → update (same version) → update (next version) → deinit` through `setup-apple-spm.js`, and runs the checker after each step. It also asserts: - a second sync changes no byte and replaces no `libs/` entry; - the build-time stale check reacts to the right changes and ignores files that Xcode writes; - a plugin framework mismatch makes sync exit with code 2; - `update` to the same version and `deinit` restore the project and scheme byte for byte. Network access, `plutil`, `otool`, codegen and `pod` are mocked, so the test runs offline on Linux. ## Changelog: [INTERNAL] [ADDED] - Add regression tests for the iOS SwiftPM tooling Pull Request resolved: react#59217 Test Plan: ``` yarn jest --no-cache -i packages/react-native/scripts/spm/__tests__ ``` Result: 24 suites, 1124 tests, all passed, about 23 s (on main at dd9b6c5). The two new suites take about 4 s. Each earlier fix was undone one at a time with `git show <sha> -- ':!**/__tests__/**' | git apply -R`, and the new tests failed each time: | Fix reverted | New tests failing | |---|---| | react#58826 (scheme pre-action newlines) | 9 | | react#58292 (absolute `HERMES_CLI_PATH` in pbxproj) | 9 | | react#58597 (`libs/` rewritten on every sync) | 2 | | react#58828 (watched path absent at add time) | 1 | `yarn eslint` and `prettier --check` on the new files: clean. Not run: Linux locally. The `test_js` job on this PR is the first Linux run. Reviewed By: cortinico Differential Revision: D126304581 Pulled By: cipolleschi fbshipit-source-id: c1bacf9ef2c6955a262cac0053491d69535375e9 (cherry picked from commit fba55e2)
This file contains hidden or 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
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:
npx react-native spm addwritesHERMES_CLI_PATHinto the app's committedproject.pbxproj— the absolute path ofhermescin thehermes-compilernpm package, as resolved on whichever machine ran the command. So every SwiftPM-converted app commits one developer's disk layout.Nothing needs it:
react-native-xcode.shalready resolveshermescat build time through react-native's own dependency graph when the current value is not a file. A relative setting can't replace it either —$(REACT_NATIVE_PATH)/../hermes-compilerbreaks on a symlinkedreact-native, and$(SRCROOT)/../node_modules/…breaks on hoisted monorepos.So this deletes
resolveHermesCliPathSetting(), along with thehermesCliPathparameter it fed oninjectSpmIntoPbxprojandmergeReactBuildSettings. Already-injected projects self-clean on the nextspm add/update, which strips every scalar recorded in.spm-injected.jsonbefore re-injecting.Scope: SwiftPM only
Both pieces involved arrived with SwiftPM in #57332 and first shipped in 0.87.0 —
scripts/spm/generate-spm-xcodeproj.js(the whole file, including the write) and theNODE_HERMESCbuild-time fallback inreact-native-xcode.sh.Everything CocoaPods relies on is older and untouched, all present in 0.86.0: the pod-derived
HERMES_CLI_PATHdefault, the "hermesc could not be found" error, and thehermesc -emit-binarycall. A CocoaPods build cannot be affected by this change.That parameter shipped in 0.87.0 and 0.87.1, but SwiftPM is Preview-labelled, so removing it is in scope. Nothing in the repo passed it except the deleted resolver.
Known limitation
The fallback is gated on
PODS_ROOTbeing absent, so a SwiftPM app that keeps side-by-side non-RN pods skips it and needs an explicitHERMES_CLI_PATH. Evaluating that gate verbatim out of the unchanged script:Pods/(normal SwiftPM app)node_modulesPods/, non-RN pods onlyPods/withhermes-engineSet it in an xcconfig, not the pbxproj:
createdScalarscleanup removes by key, so it would drop a hand-edited value too. Keying the gate on thehermes-enginedirectory would close this — left as a follow-up so this PR doesn't touch the bundling script.Changelog:
[IOS] [FIXED] - SwiftPM: stop baking an absolute, machine-specific HERMES_CLI_PATH into the app's pbxproj; resolve hermesc at build time instead
Test Plan:
Red first — with the source reverted, the new entry-point test fails on a seeded
hermes-compilerfixture:✕ writes no HERMES_CLI_PATH, in any configuration or the marker(1 failed, 76 passed).yarn flow-check(0 errors), plusyarn eslint --max-warnings 0andyarn prettier --checkon the four changed files.End to end: a 0.87.1 SwiftPM app (
@shopify/react-native-skiaexample, no Pods) with theHERMES_CLI_PATHlines removed from its pbxproj builds in Release, the fallback resolvinghermescfromnode_modules/hermes-compiler. That build ran against an earlier revision of this branch which also widened the shell gate; with noPods/it is row 1, where both conditions behave identically. There is no shell test harness, and the script is unchanged here.🤖 Generated with Claude Code