mirror of
https://github.com/react/react-native.git
synced 2026-09-28 05:13:06 +08:00
Stop re-creating library package roots on every sync (#58597)
Summary: Building an app in Xcode fails when it autolinks a library that ships its own `Package.swift`, with one error per such library: ``` Missing package product 'reactnativeskia_ReactNativeSkia.ReactNativeSkia' Missing package product 'reactnativesafeareacontext_ReactNativeSafeAreaContext.ReactNativeSafeAreaContext' ``` The same project builds fine from the command line. Two problems combine. **The sync destroys package roots Xcode has already loaded.** Autolinking writes one symlink per self-managed library into `build/generated/autolinking/libs/<Name>`, and Xcode treats each as a local Swift package root. The generator deleted that whole directory and recreated it on every run, even when the generated output was byte-for-byte identical. Measured on a real app, across one no-op sync: the `libs/` inode changed from 1913513750 to 1913930645, `libs/ReactNativeSkia` from 1913513937 to 1913930649, while the generated `Package.swift` kept the same MD5. Recreating a package root that Xcode has already resolved is what produces the error above. **Xcode's own bookkeeping made the sync run every time.** The "Sync SPM Autolinking" build phase re-syncs when a watched input looks newer than its stamp, and it checked each library's whole directory with `find <dir> -newer <stamp>`. Xcode writes its per-user scheme state inside that directory, at `<lib>/.swiftpm/xcode/xcuserdata/<user>.xcuserdatad/xcschemes/xcschememanagement.plist`. So Xcode's own write marked the next build stale, which triggered the destructive re-sync, which broke that build. `xcodebuild` does not write that file, which is why command-line builds were never affected. This change makes the `libs/` tree idempotent — unchanged entries keep their inode, and entries that are no longer autolinked are pruned instead of wiped — and makes the staleness check skip `.swiftpm`. ### How to verify In an app that autolinks a library shipping its own `Package.swift`, build in Xcode twice in a row. Both builds should succeed. Before this change the second build fails with `Missing package product`. ## Changelog: [IOS][FIXED] - Stop SwiftPM autolinking from recreating library package roots on every sync, which broke Xcode builds of apps using libraries that ship their own Package.swift Pull Request resolved: https://github.com/react/react-native/pull/58597 Test Plan: **Unit tests.** `yarn test packages/react-native/scripts/spm` — 21 suites, 1009 tests pass. New tests, written and seen failing before the fix: - Two consecutive generation runs over an unchanged self-managed dependency keep both the `libs/` inode and each entry's inode. Failed before the fix with the same inode churn measured on the real app. - A dependency removed between two runs leaves no symlink under `libs/`. - The emitted staleness snippet is extracted from the generated script and executed under `/bin/bash -c` with `set -euo pipefail` against a temporary tree. A write under `.swiftpm/` is ignored; a real source change is still detected. **Real app.** A React Native 0.87.1 app on Xcode 27 autolinking `shopify/react-native-skia` and `react-native-safe-area-context`, both self-managed. Before: `xcodebuild` succeeded, Xcode failed in about 5 seconds with the two errors above, on nearly every build. **Not run:** the full CI matrix, and no Android-side check — this touches iOS SwiftPM tooling only. ## Scope Deliberately minimal. One case still re-syncs: the first time Xcode creates `.swiftpm` inside a library, that bumps the library directory's own mtime, so the build after a fresh checkout reports stale once per library. That is harmless now that the sync is idempotent. One related item is left alone: the aggregate `Package.swift` is still rewritten on every sync even when its content is unchanged, which bumps its mtime and can make Xcode re-resolve. That is wasteful but not destructive, and it is no longer reached on an ordinary IDE build now that `.swiftpm` writes do not mark the sync stale. Reviewed By: cipolleschi Differential Revision: D120740029 Pulled By: shwanton fbshipit-source-id: 3783d4cee4791ab13517d8f0dcac71f752cb7c1b
This commit is contained in:
committed by
meta-codesync[bot]
parent
900b717e83
commit
97cc934dc7
@@ -1982,6 +1982,103 @@ describe('main() — .spm-sync-watch-paths emission', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// main() — libs/ symlinks for self-managed deps
|
||||
//
|
||||
// Xcode loads each libs/<SwiftName> symlink as a local package root. Replacing
|
||||
// one that did not change invalidates the package graph Xcode already holds,
|
||||
// and the build then fails with "Missing package product". So a sync that
|
||||
// changes nothing must leave every inode under libs/ — and libs/ itself —
|
||||
// untouched, while a dep that is gone must lose its symlink.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('main() — libs/ symlinks for self-managed deps', () => {
|
||||
const {created} = useTempApps();
|
||||
|
||||
function buildApp(depNames) {
|
||||
const appRoot = fs.realpathSync(
|
||||
fs.mkdtempSync(path.join(os.tmpdir(), 'spm-libs-sync-')),
|
||||
);
|
||||
created.push(appRoot);
|
||||
const rnRoot = path.join(appRoot, 'rn');
|
||||
fs.mkdirSync(rnRoot, {recursive: true});
|
||||
fs.writeFileSync(
|
||||
path.join(appRoot, 'package.json'),
|
||||
JSON.stringify({name: 'app'}),
|
||||
);
|
||||
|
||||
const dependencies = {};
|
||||
for (const npmName of depNames) {
|
||||
// A hand-authored root Package.swift (no AUTOGEN marker) is what makes a
|
||||
// dep self-managed.
|
||||
const depDir = path.join(appRoot, 'node_modules', npmName);
|
||||
fs.mkdirSync(depDir, {recursive: true});
|
||||
fs.writeFileSync(
|
||||
path.join(depDir, 'Package.swift'),
|
||||
'// swift-tools-version:5.9\n// hand-authored\n',
|
||||
);
|
||||
fs.writeFileSync(path.join(depDir, 'Source.swift'), '// src\n');
|
||||
dependencies[npmName] = {root: depDir, platforms: {ios: {}}};
|
||||
}
|
||||
|
||||
const autolinkDir = path.join(appRoot, 'build', 'generated', 'autolinking');
|
||||
fs.mkdirSync(autolinkDir, {recursive: true});
|
||||
const writeAutolinkingJson = names =>
|
||||
fs.writeFileSync(
|
||||
path.join(autolinkDir, 'autolinking.json'),
|
||||
JSON.stringify({
|
||||
dependencies: Object.fromEntries(
|
||||
names.map(n => [n, dependencies[n]]),
|
||||
),
|
||||
}),
|
||||
);
|
||||
writeAutolinkingJson(depNames);
|
||||
|
||||
return {
|
||||
libsDir: path.join(autolinkDir, 'libs'),
|
||||
writeAutolinkingJson,
|
||||
sync: () => main(['--app-root', appRoot, '--react-native-root', rnRoot]),
|
||||
};
|
||||
}
|
||||
|
||||
const inodesOf = libsDir =>
|
||||
Object.fromEntries(
|
||||
['.', ...fs.readdirSync(libsDir)].map(entry => [
|
||||
entry,
|
||||
fs.lstatSync(path.join(libsDir, entry)).ino,
|
||||
]),
|
||||
);
|
||||
|
||||
it('keeps every inode when nothing changed', () => {
|
||||
const app = buildApp(['react-native-foo', 'react-native-bar']);
|
||||
|
||||
app.sync();
|
||||
const before = inodesOf(app.libsDir);
|
||||
expect(Object.keys(before).sort()).toEqual([
|
||||
'.',
|
||||
'ReactNativeBar',
|
||||
'ReactNativeFoo',
|
||||
]);
|
||||
|
||||
app.sync();
|
||||
expect(inodesOf(app.libsDir)).toEqual(before);
|
||||
});
|
||||
|
||||
it('drops the symlink of a dep that is no longer autolinked', () => {
|
||||
const app = buildApp(['react-native-foo', 'react-native-bar']);
|
||||
|
||||
app.sync();
|
||||
expect(fs.readdirSync(app.libsDir).sort()).toEqual([
|
||||
'ReactNativeBar',
|
||||
'ReactNativeFoo',
|
||||
]);
|
||||
|
||||
app.writeAutolinkingJson(['react-native-foo']);
|
||||
app.sync();
|
||||
expect(fs.readdirSync(app.libsDir)).toEqual(['ReactNativeFoo']);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// main() — the name a dep's podspec declares reaching a real manifest.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -234,6 +234,70 @@ describe('sync scripts', () => {
|
||||
);
|
||||
});
|
||||
|
||||
// Xcode writes per-user scheme state into <watched dir>/.swiftpm/ on every
|
||||
// IDE build. Counting that as a change made every IDE build re-sync.
|
||||
describe('the watched-directory staleness probe', () => {
|
||||
// Runs the generated `find` in isolation, with $P/$STAMP bound as the
|
||||
// build phase binds them.
|
||||
function probe(watchedDir, stampFile) {
|
||||
const findCommand = /\$\((find "\$P"[^()]*)\)/.exec(script)?.[1];
|
||||
expect(findCommand).toBeDefined();
|
||||
return execFileSync(
|
||||
'/bin/bash',
|
||||
[
|
||||
'-c',
|
||||
`set -euo pipefail\nP="$1"\nSTAMP="$2"\n${String(findCommand)}\n`,
|
||||
'probe',
|
||||
watchedDir,
|
||||
stampFile,
|
||||
],
|
||||
{encoding: 'utf8'},
|
||||
);
|
||||
}
|
||||
|
||||
let root;
|
||||
let watchedDir;
|
||||
let stampFile;
|
||||
let schemeState;
|
||||
let source;
|
||||
|
||||
beforeEach(() => {
|
||||
root = fs.mkdtempSync(path.join(os.tmpdir(), 'spm-sync-stale-'));
|
||||
watchedDir = path.join(root, 'node_modules', 'react-native-foo');
|
||||
schemeState = path.join(
|
||||
watchedDir,
|
||||
'.swiftpm/xcode/xcuserdata/someone.xcuserdatad/xcschemes/xcschememanagement.plist',
|
||||
);
|
||||
source = path.join(watchedDir, 'Foo.swift');
|
||||
fs.mkdirSync(path.dirname(schemeState), {recursive: true});
|
||||
fs.writeFileSync(schemeState, '<plist/>\n');
|
||||
fs.writeFileSync(source, '// src\n');
|
||||
// The stamp is written after the tree, so nothing is newer until a test
|
||||
// makes it so.
|
||||
stampFile = path.join(root, '.spm-sync-stamp');
|
||||
fs.writeFileSync(stampFile, '');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
fs.rmSync(root, {recursive: true, force: true});
|
||||
});
|
||||
|
||||
const touch = file => {
|
||||
const future = new Date(Date.now() + 10_000);
|
||||
fs.utimesSync(file, future, future);
|
||||
};
|
||||
|
||||
it('ignores Xcode-owned state under .swiftpm', () => {
|
||||
touch(schemeState);
|
||||
expect(probe(watchedDir, stampFile)).toBe('');
|
||||
});
|
||||
|
||||
it('still reports a changed source file', () => {
|
||||
touch(source);
|
||||
expect(probe(watchedDir, stampFile).trim()).toBe(source);
|
||||
});
|
||||
});
|
||||
|
||||
it('is deterministic, shared with the pre-action, and valid POSIX shell', () => {
|
||||
expect(buildSyncAutolinkingScript(baked)).toBe(script);
|
||||
expect(buildSchemePreActionScript(baked)).toBe(script);
|
||||
|
||||
@@ -1515,11 +1515,14 @@ function main(argv /*:: ?: Array<string> */) /*: void */ {
|
||||
// is the Swift module name (guaranteed unique per dep), so SPM's
|
||||
// path-basename-based package identity never collides — even when two
|
||||
// libs ship their own Package.swift inside `ios/` (a common convention).
|
||||
// Wiped on every run; populated below as self-managed deps are visited.
|
||||
// Populated below as self-managed deps are visited, then pruned. Entries
|
||||
// that do not change keep their inode: Xcode holds each one as a loaded
|
||||
// package root, and recreating one it already resolved fails the build with
|
||||
// "Missing package product".
|
||||
const libsDir = path.join(outputDir, 'libs');
|
||||
const wantedLibAliases /*: Set<string> */ = new Set();
|
||||
fs.mkdirSync(packagesDir, {recursive: true});
|
||||
fs.mkdirSync(headersDir, {recursive: true});
|
||||
fs.rmSync(libsDir, {recursive: true, force: true});
|
||||
fs.mkdirSync(libsDir, {recursive: true});
|
||||
|
||||
const wrapperDirs /*: Map<string, string> */ = new Map();
|
||||
@@ -1652,6 +1655,7 @@ function main(argv /*:: ?: Array<string> */) /*: void */ {
|
||||
const realPackageDir = selfManagedDirs.get(target.name) ?? absSource;
|
||||
const aliasPath = path.join(libsDir, target.name);
|
||||
ensureSymlink(aliasPath, realPackageDir);
|
||||
wantedLibAliases.add(target.name);
|
||||
aggregatorPackageDeps.push({
|
||||
swiftName: target.name,
|
||||
packagePath: `libs/${target.name}`,
|
||||
@@ -1779,23 +1783,30 @@ function main(argv /*:: ?: Array<string> */) /*: void */ {
|
||||
});
|
||||
}
|
||||
|
||||
// Prune stale wrappers + header dirs for entries no longer autolinked.
|
||||
// Preserve both wrapper-managed and self-managed names; only entries that
|
||||
// are no longer autolinked at all get removed. Note: `packages/` only has
|
||||
// wrapper-managed names (self-managed deps live in their own source dirs),
|
||||
// but `headers/` has both since we populate the central tree for everyone.
|
||||
// Prune stale wrappers, header dirs and lib aliases for entries no longer
|
||||
// autolinked. Preserve both wrapper-managed and self-managed names; only
|
||||
// entries that are no longer autolinked at all get removed. Note:
|
||||
// `packages/` only has wrapper-managed names (self-managed deps live in
|
||||
// their own source dirs), but `headers/` has both since we populate the
|
||||
// central tree for everyone. `libs/` keeps only the aliases written above,
|
||||
// so a dep that stopped being self-managed loses its alias too.
|
||||
const activeNames /*: Set<string> */ = new Set([
|
||||
...wrapperDirs.keys(),
|
||||
...selfManagedDirs.keys(),
|
||||
]);
|
||||
for (const subdir of ['packages', 'headers']) {
|
||||
const pruneTargets /*: Array<[string, Set<string>]> */ = [
|
||||
['packages', activeNames],
|
||||
['headers', activeNames],
|
||||
['libs', wantedLibAliases],
|
||||
];
|
||||
for (const [subdir, keptNames] of pruneTargets) {
|
||||
const dir = path.join(outputDir, subdir);
|
||||
try {
|
||||
const existing /*: Array<{name: string, isSymbolicLink(): boolean, isDirectory(): boolean}> */ =
|
||||
// $FlowFixMe[incompatible-type] Dirent typing
|
||||
fs.readdirSync(dir, {withFileTypes: true});
|
||||
for (const entry of existing) {
|
||||
if (activeNames.has(entry.name)) continue;
|
||||
if (keptNames.has(entry.name)) continue;
|
||||
const stale = path.join(dir, entry.name);
|
||||
if (entry.isSymbolicLink() || !entry.isDirectory()) {
|
||||
fs.unlinkSync(stale);
|
||||
|
||||
@@ -767,7 +767,9 @@ if [ "$STALE" -eq 0 ] && [ -f "$WATCH_FILE" ]; then
|
||||
while IFS= read -r P; do
|
||||
[ -z "$P" ] && continue
|
||||
if [ -d "$P" ]; then
|
||||
if [ -n "$(find "$P" -newer "$STAMP" -print -quit 2>/dev/null)" ]; then
|
||||
# .swiftpm holds Xcode's own per-user scheme state, which it rewrites
|
||||
# during a build — reading it as a change makes every IDE build re-sync.
|
||||
if [ -n "$(find "$P" -name .swiftpm -prune -o -newer "$STAMP" -print -quit 2>/dev/null)" ]; then
|
||||
STALE=1
|
||||
break
|
||||
fi
|
||||
|
||||
Reference in New Issue
Block a user