Skip to content

Commit 7fdfc4e

Browse files
chrfalchfabriziocucci
authored andcommitted
Repair stale staged headers in the iOS prebuild (#58596)
Summary: The iOS prebuild (`node scripts/ios-prebuild`) stages React Native's headers into `packages/react-native/.build/headers` as hard links, and skipped any target that already existed. When changing the original source files, these hard links can become stale and errors like this can occur: ``` Libraries/LinkingIOS/RCTLinkingManager.mm:91:7: error: use of undeclared identifier 'RCTIsSceneDelegateApp' ``` This is a problem that contributors will see - not regular users, but the fix helps with strange error messages. ## Changelog: [INTERNAL] [FIXED] - Repair stale staged headers in the iOS prebuild instead of compiling against a previous checkout's copies Pull Request resolved: #58596 Test Plan: ✅ New unit tests, 16 cases in `packages/react-native/scripts/ios-prebuild/__tests__/setup-test.js`, using real temporary directories rather than an `fs` mock, since the defect is about inodes. End to end on an Xcode 27 checkout: - Replaced a staged header with a copy, so it kept the same contents but a different inode. `node scripts/ios-prebuild -s -f Debug` restored it to the source inode and logged `Linked React/Base → .build/headers/React`. The previous code skipped it. - Ran setup a second time with nothing changed: no file was relinked, and nothing was logged. - Confirmed the three colliding targets still resolve to the same source as before the change, by inode. - `node scripts/ios-prebuild -b -f Debug -p ios-simulator` → `** BUILD SUCCEEDED **`. Reviewed By: cipolleschi Differential Revision: D120740409 Pulled By: shwanton fbshipit-source-id: 41c1eb112c3395d6c7158d7151be7fdd92a21cef (cherry picked from commit b935e74)
1 parent 9465ef7 commit 7fdfc4e

2 files changed

Lines changed: 398 additions & 63 deletions

File tree

Lines changed: 296 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,296 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @flow strict-local
8+
* @format
9+
*/
10+
11+
'use strict';
12+
13+
const {createHeaderLinker} = require('../setup');
14+
const fs = require('node:fs');
15+
const os = require('node:os');
16+
const path = require('node:path');
17+
18+
describe('createHeaderLinker', () => {
19+
let tmp /*: string */ = '';
20+
let root /*: string */ = '';
21+
let linksFolder /*: string */ = '';
22+
let log /*: JestMockFn<[string], void> */ = jest.fn();
23+
let stage /*: (fromPath: string, includePath?: ?string) => void */ = () => {};
24+
25+
// Each pass gets its own linker, the way a prebuild run does.
26+
const newPass = () => {
27+
log = jest.fn();
28+
return createHeaderLinker(root, linksFolder, log);
29+
};
30+
31+
const write = (relPath /*: string */, contents /*: string */) => {
32+
const file = path.join(root, relPath);
33+
fs.mkdirSync(path.dirname(file), {recursive: true});
34+
fs.writeFileSync(file, contents);
35+
return file;
36+
};
37+
38+
const staged = (relPath /*: string */) => path.join(linksFolder, relPath);
39+
40+
const linkedMessage = (fromPath /*: string */, includePath /*: string */) =>
41+
`Linked ${fromPath} → ${path.relative(root, staged(includePath))}`;
42+
43+
beforeEach(() => {
44+
tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'header-links-test-'));
45+
root = path.join(tmp, 'source');
46+
linksFolder = path.join(tmp, 'headers');
47+
fs.mkdirSync(root, {recursive: true});
48+
fs.mkdirSync(linksFolder, {recursive: true});
49+
stage = newPass();
50+
});
51+
52+
afterEach(() => {
53+
jest.restoreAllMocks();
54+
fs.rmSync(tmp, {recursive: true, force: true});
55+
});
56+
57+
it('hard links a header into the staging folder', () => {
58+
const source = write('React/Base/RCTUtils.h', '// original\n');
59+
60+
stage('React/Base', 'React');
61+
62+
expect(fs.readFileSync(staged('React/RCTUtils.h'), 'utf8')).toBe(
63+
'// original\n',
64+
);
65+
expect(fs.statSync(staged('React/RCTUtils.h')).ino).toBe(
66+
fs.statSync(source).ino,
67+
);
68+
expect(log).toHaveBeenCalledTimes(1);
69+
expect(log).toHaveBeenCalledWith(linkedMessage('React/Base', 'React'));
70+
});
71+
72+
it('repairs a staged link whose source was replaced by a new inode', () => {
73+
write('React/Base/RCTUtils.h', '// june\n');
74+
stage('React/Base', 'React');
75+
76+
// git checkout writes a new file and renames it over the old one, so the
77+
// source gets a new inode and the staged link keeps serving the old one.
78+
const replacement = path.join(tmp, 'RCTUtils.h.new');
79+
fs.writeFileSync(replacement, '// august\n');
80+
const source = path.join(root, 'React/Base/RCTUtils.h');
81+
fs.renameSync(replacement, source);
82+
83+
stage = newPass();
84+
stage('React/Base', 'React');
85+
86+
expect(fs.readFileSync(staged('React/RCTUtils.h'), 'utf8')).toBe(
87+
'// august\n',
88+
);
89+
expect(fs.statSync(staged('React/RCTUtils.h')).ino).toBe(
90+
fs.statSync(source).ino,
91+
);
92+
expect(log).toHaveBeenCalledWith(linkedMessage('React/Base', 'React'));
93+
});
94+
95+
it('does not relink an up-to-date link on a later pass', () => {
96+
write('React/Base/RCTUtils.h', '// original\n');
97+
stage('React/Base', 'React');
98+
99+
const linkSync = jest.spyOn(fs, 'linkSync');
100+
const unlinkSync = jest.spyOn(fs, 'unlinkSync');
101+
102+
stage = newPass();
103+
stage('React/Base', 'React');
104+
105+
expect(linkSync).not.toHaveBeenCalled();
106+
expect(unlinkSync).not.toHaveBeenCalled();
107+
expect(log).not.toHaveBeenCalled();
108+
});
109+
110+
it('keeps the header staged first when two sources collide on one target', () => {
111+
const umbrella = write('callinvoker/React/CallInvoker.h', '// umbrella\n');
112+
write('callinvoker/ReactCommon/CallInvoker.h', '// interface\n');
113+
114+
stage('callinvoker/React', 'ReactCommon');
115+
stage('callinvoker/ReactCommon', 'ReactCommon');
116+
117+
expect(fs.readFileSync(staged('ReactCommon/CallInvoker.h'), 'utf8')).toBe(
118+
'// umbrella\n',
119+
);
120+
expect(fs.statSync(staged('ReactCommon/CallInvoker.h')).ino).toBe(
121+
fs.statSync(umbrella).ino,
122+
);
123+
expect(log).toHaveBeenCalledTimes(1);
124+
expect(log).toHaveBeenCalledWith(
125+
linkedMessage('callinvoker/React', 'ReactCommon'),
126+
);
127+
128+
const linkSync = jest.spyOn(fs, 'linkSync');
129+
const unlinkSync = jest.spyOn(fs, 'unlinkSync');
130+
131+
stage = newPass();
132+
stage('callinvoker/React', 'ReactCommon');
133+
stage('callinvoker/ReactCommon', 'ReactCommon');
134+
135+
expect(linkSync).not.toHaveBeenCalled();
136+
expect(unlinkSync).not.toHaveBeenCalled();
137+
expect(fs.readFileSync(staged('ReactCommon/CallInvoker.h'), 'utf8')).toBe(
138+
'// umbrella\n',
139+
);
140+
});
141+
142+
it('keeps the header visited first when one pass recurses into colliding siblings', () => {
143+
write('callinvoker/React/CallInvoker.h', '// umbrella\n');
144+
write('callinvoker/ReactCommon/CallInvoker.h', '// interface\n');
145+
146+
// Read the traversal order rather than assume it: the invariant is that the
147+
// first sibling visited wins, not that a particular sibling wins.
148+
const [firstVisited] = fs
149+
.readdirSync(path.join(root, 'callinvoker'), {withFileTypes: true})
150+
.filter(dirent => dirent.isDirectory())
151+
.map(dirent => String(dirent.name));
152+
const winner = path.join(
153+
root,
154+
'callinvoker',
155+
firstVisited,
156+
'CallInvoker.h',
157+
);
158+
159+
stage('callinvoker', 'ReactCommon');
160+
161+
expect(fs.readFileSync(staged('ReactCommon/CallInvoker.h'), 'utf8')).toBe(
162+
fs.readFileSync(winner, 'utf8'),
163+
);
164+
expect(fs.statSync(staged('ReactCommon/CallInvoker.h')).ino).toBe(
165+
fs.statSync(winner).ino,
166+
);
167+
});
168+
169+
it('replaces a staged symlink that points at the source file', () => {
170+
const source = write('React/Base/RCTUtils.h', '// original\n');
171+
fs.mkdirSync(staged('React'), {recursive: true});
172+
// A symlink resolves to the source inode but does not share it, so it would
173+
// not track in-place edits the way the staging tree assumes.
174+
fs.symlinkSync(source, staged('React/RCTUtils.h'));
175+
176+
stage('React/Base', 'React');
177+
178+
expect(fs.lstatSync(staged('React/RCTUtils.h')).isSymbolicLink()).toBe(
179+
false,
180+
);
181+
expect(fs.statSync(staged('React/RCTUtils.h')).ino).toBe(
182+
fs.statSync(source).ino,
183+
);
184+
});
185+
186+
it('stages the remaining headers when one file fails to link', () => {
187+
const consoleError = jest
188+
.spyOn(console, 'error')
189+
.mockImplementation(() => {});
190+
write('React/Base/RCTUtils.h', '');
191+
write('React/Base/RCTConversions.h', '');
192+
193+
const unlinkable = staged('React/RCTUtils.h');
194+
const realLinkSync = fs.linkSync;
195+
jest.spyOn(fs, 'linkSync').mockImplementation((sourceFile, targetFile) => {
196+
if (targetFile === unlinkable) {
197+
throw new Error('Operation not permitted');
198+
}
199+
realLinkSync(sourceFile, targetFile);
200+
});
201+
202+
expect(() => stage('React/Base', 'React')).not.toThrow();
203+
204+
expect(fs.readdirSync(staged('React'))).toEqual(['RCTConversions.h']);
205+
expect(consoleError).toHaveBeenCalledWith(
206+
expect.stringContaining(`Failed to create link for`),
207+
);
208+
});
209+
210+
it('replaces a staged copy that does not share the source inode', () => {
211+
const source = write('React/Base/RCTUtils.h', '// original\n');
212+
fs.mkdirSync(staged('React'), {recursive: true});
213+
fs.writeFileSync(staged('React/RCTUtils.h'), '// a copy, not a link\n');
214+
215+
stage('React/Base', 'React');
216+
217+
expect(fs.statSync(staged('React/RCTUtils.h')).ino).toBe(
218+
fs.statSync(source).ino,
219+
);
220+
});
221+
222+
it('replaces a staged symlink instead of following it', () => {
223+
const source = write('React/Base/RCTUtils.h', '// original\n');
224+
const elsewhere = write('elsewhere/RCTUtils.h', '// elsewhere\n');
225+
fs.mkdirSync(staged('React'), {recursive: true});
226+
fs.symlinkSync(elsewhere, staged('React/RCTUtils.h'));
227+
228+
stage('React/Base', 'React');
229+
230+
expect(fs.lstatSync(staged('React/RCTUtils.h')).isSymbolicLink()).toBe(
231+
false,
232+
);
233+
expect(fs.statSync(staged('React/RCTUtils.h')).ino).toBe(
234+
fs.statSync(source).ino,
235+
);
236+
expect(fs.readFileSync(elsewhere, 'utf8')).toBe('// elsewhere\n');
237+
});
238+
239+
it('stages only header files', () => {
240+
write('React/Base/RCTUtils.h', '');
241+
write('React/Base/RCTConversions.hpp', '');
242+
write('React/Base/RCTUtils.m', '');
243+
write('React/Base/RCTUtils.cpp', '');
244+
write('React/Base/BUCK.txt', '');
245+
246+
stage('React/Base', 'React');
247+
248+
expect(fs.readdirSync(staged('React')).sort()).toEqual([
249+
'RCTConversions.hpp',
250+
'RCTUtils.h',
251+
]);
252+
});
253+
254+
it('skips files of a folder without headers but still recurses into it', () => {
255+
write('React/README.md', '');
256+
write('React/Base/RCTUtils.h', '');
257+
258+
stage('React', 'React');
259+
260+
expect(fs.readdirSync(staged('React'))).toEqual(['RCTUtils.h']);
261+
expect(log).toHaveBeenCalledTimes(1);
262+
expect(log).toHaveBeenCalledWith(linkedMessage('React/Base', 'React'));
263+
});
264+
265+
it('does not log a pass that staged nothing', () => {
266+
write('React/Base/README.md', '');
267+
268+
stage('React/Base', 'React');
269+
270+
expect(log).not.toHaveBeenCalled();
271+
});
272+
273+
it('flattens nested subfolders under the same include path', () => {
274+
write('React/Base/RCTUtils.h', '');
275+
write('React/Base/Surface/RCTSurface.h', '');
276+
277+
stage('React/Base', 'React');
278+
279+
expect(fs.readdirSync(staged('React')).sort()).toEqual([
280+
'RCTSurface.h',
281+
'RCTUtils.h',
282+
]);
283+
});
284+
285+
it.each(['__tests__', 'tests', 'platform'])(
286+
'does not stage headers from a %s subfolder',
287+
folder => {
288+
write('React/Base/RCTUtils.h', '');
289+
write(`React/Base/${folder}/RCTUtilsTests.h`, '');
290+
291+
stage('React/Base', 'React');
292+
293+
expect(fs.readdirSync(staged('React'))).toEqual(['RCTUtils.h']);
294+
},
295+
);
296+
});

0 commit comments

Comments
 (0)