Address review: reset stamp on mismatch, compare framework names, use the built project

Author: chrfalch

packages/react-native/scripts/spm/__docs__/spm-scripts.md

@@ -756,5 +756,6 @@ script branches on the exit code:
 That split is the whole reason these cases have their own exit code: a transient
 sync hiccup should not break a build that could still succeed, while a missing
 manifest or an out-of-date project should not pass silently. Sync writes its
-stamp only after these checks pass, so every following build fails the same way
-until the problem is fixed.
+stamp only after these checks pass. When the project is out of date, sync also
+removes the earlier stamp, so every following build fails the same way until the
+project is updated.

packages/react-native/scripts/spm/__tests__/plugin-framework-mismatch-test.js

@@ -136,11 +136,11 @@ function projectFiles(xcodeprojPath) {
 
 // Runs the check and proves it wrote nothing under the .xcodeproj (pbxproj,
 // marker, scheme). Returns the printed lines and the thrown error, if any.
-function check(app) {
+function check(app, env = {}) {
   const before = projectFiles(app.xcodeprojPath);
   let error = null;
   try {
-    assertPluginFrameworksLinked(app.appRoot);
+    assertPluginFrameworksLinked(app.appRoot, env);
   } catch (e) {
     error = e;
   }
@@ -193,6 +193,38 @@ describe('assertPluginFrameworksLinked', () => {
     expect(lines[1]).toMatch(/^error: The Xcode project links ExpoCore\./);
   });
 
+  it('fails when a plugin renames its framework but keeps its id', () => {
+    const app = scaffoldApp();
+    inject(app, [REACT, EXPO]);
+    writeSidecar(app.appRoot, [{id: 'expo-core', frameworkName: 'NewCore'}]);
+
+    const {error, lines} = check(app);
+
+    expect(error).toBeInstanceOf(PluginFrameworkMismatchError);
+    expect(lines).toEqual([
+      'error: NewCore (expo-core) is a precompiled framework from an autolinking plugin, but the Xcode project does not link it. Run `npx react-native spm` to update the project.',
+      'error: The Xcode project links ExpoCore.framework, but no autolinking plugin provides it on this machine. Precompile ExpoCore, or run `npx react-native spm` to update the project.',
+    ]);
+  });
+
+  it('checks the project Xcode is building, not the first injected one', () => {
+    const app = scaffoldApp();
+    const stale = {
+      ...app,
+      xcodeprojPath: path.join(app.appRoot, 'A.xcodeproj'),
+    };
+    fs.mkdirSync(stale.xcodeprojPath);
+    fs.writeFileSync(path.join(stale.xcodeprojPath, 'project.pbxproj'), PLAIN);
+    inject(stale, [REACT]);
+    inject(app, [REACT, EXPO]);
+    writeSidecar(app.appRoot, [EXPO]);
+
+    expect(check(app, {PROJECT_FILE_PATH: app.xcodeprojPath})).toEqual({
+      error: null,
+      lines: [],
+    });
+  });
+
   it('passes when the project links exactly the paired plugin frameworks', () => {
     const app = scaffoldApp();
     inject(app, [REACT, HERMES, EXPO]);
@@ -201,6 +233,22 @@ describe('assertPluginFrameworksLinked', () => {
     expect(check(app)).toEqual({error: null, lines: []});
   });
 
+  it('compares only setting prefixes when the frameworks manifest is missing', () => {
+    const app = scaffoldApp();
+    inject(app, [REACT, HERMES, EXPO]);
+    writeSidecar(app.appRoot, [EXPO]);
+    fs.rmSync(
+      path.join(
+        app.appRoot,
+        'build',
+        'xcframeworks',
+        'flavored-frameworks.json',
+      ),
+    );
+
+    expect(check(app)).toEqual({error: null, lines: []});
+  });
+
   it('never reports built-in frameworks', () => {
     const app = scaffoldApp();
     inject(app, [REACT, HERMES]);

packages/react-native/scripts/spm/__tests__/sync-spm-autolinking-test.js

@@ -152,4 +152,17 @@ describe('sync-spm-autolinking main', () => {
     );
     expect(fs.existsSync(stampPath())).toBe(false);
   });
+
+  it('removes an earlier stamp when plugin frameworks and the project disagree', async () => {
+    fs.mkdirSync(path.dirname(stampPath()), {recursive: true});
+    fs.writeFileSync(stampPath(), 'earlier sync\n');
+    const mismatch = new PluginFrameworkMismatchError();
+    const deps = makeDeps({
+      assertPluginFrameworksLinked: jest.fn(() => {
+        throw mismatch;
+      }),
+    });
+    await expect(run(deps)).rejects.toBe(mismatch);
+    expect(fs.existsSync(stampPath())).toBe(false);
+  });
 });

packages/react-native/scripts/spm/generate-spm-xcodeproj.js

@@ -23,6 +23,7 @@
 
 const {
   BUILTIN_FRAMEWORKS,
+  FLAVORED_FRAMEWORKS_MANIFEST,
   readFlavoredFrameworksManifest,
   readPluginFrameworks,
 } = require('./flavored-frameworks');
@@ -2285,10 +2286,20 @@ function pbxListValues(list /*: string */) /*: Array<string> */ {
  * machine with the plugin frameworks the injected project links. Only `spm
  * add` / `spm update` change what the project links, so the build-time sync
  * reports a mismatch instead of fixing it: one `error:` line per framework,
- * then PluginFrameworkMismatchError. Never writes the project.
+ * then PluginFrameworkMismatchError. Never writes the project. Checks the
+ * project Xcode is building (`PROJECT_FILE_PATH`) when it is injected.
  */
-function assertPluginFrameworksLinked(appRoot /*: string */) /*: void */ {
-  const xcodeprojPath = findInjectedXcodeproj(appRoot);
+function assertPluginFrameworksLinked(
+  appRoot /*: string */,
+  env /*: {+[string]: ?string} */ = process.env,
+) /*: void */ {
+  const builtProjectPath = env.PROJECT_FILE_PATH;
+  const xcodeprojPath =
+    builtProjectPath != null &&
+    builtProjectPath.endsWith('.xcodeproj') &&
+    fs.existsSync(path.join(builtProjectPath, SPM_INJECTED_MARKER))
+      ? builtProjectPath
+      : findInjectedXcodeproj(appRoot);
   const rootUuid =
     xcodeprojPath != null ? readMarker(xcodeprojPath)?.rootUuid : null;
   if (xcodeprojPath == null || rootUuid == null) {
@@ -2316,36 +2327,47 @@ function assertPluginFrameworksLinked(appRoot /*: string */) /*: void */ {
   const outputs = pbxListValues(
     findField(text, embedPhase, 'outputPaths')?.value ?? '',
   );
-  const builtinPrefixes = new Set(
-    BUILTIN_FRAMEWORKS.map(framework => frameworkSettingPrefix(framework.id)),
+  const linkedPrefixes = new Set(
+    inputs.map(input => /^\$\((RN_SPM_\w+)_FRAMEWORK\)$/.exec(input)?.[1]),
   );
-  const linkedPluginNames /*: Map<string, string> */ = new Map();
-  inputs
-    .map(input => /^\$\((RN_SPM_\w+)_FRAMEWORK\)$/.exec(input)?.[1])
-    .filter(Boolean)
-    .forEach((prefix, index) => {
-      if (!builtinPrefixes.has(prefix)) {
-        linkedPluginNames.set(
-          prefix,
-          path.basename(outputs[index] ?? prefix, '.framework'),
-        );
-      }
-    });
+  // Built-in framework names come from the staged manifest. Without it, only
+  // setting prefixes are compared: a missing manifest is not a plugin mismatch.
+  let linkedPluginNames /*: ?Set<string> */ = null;
+  if (
+    fs.existsSync(
+      path.join(appRoot, 'build', 'xcframeworks', FLAVORED_FRAMEWORKS_MANIFEST),
+    )
+  ) {
+    const builtinIds = new Set(BUILTIN_FRAMEWORKS.map(({id}) => id));
+    const builtinNames = new Set(
+      readFlavoredFrameworksManifest(appRoot)
+        .frameworks.filter(framework => builtinIds.has(framework.id))
+        .map(framework => framework.frameworkName),
+    );
+    linkedPluginNames = new Set(
+      outputs
+        .map(output => path.basename(output, '.framework'))
+        .filter(name => !builtinNames.has(name)),
+    );
+  }
 
   const pluginFrameworks = readPluginFrameworks(appRoot);
-  const pairedPrefixes = new Set(
-    pluginFrameworks.map(framework => frameworkSettingPrefix(framework.id)),
+  const pairedNames = new Set(
+    pluginFrameworks.map(framework => framework.frameworkName),
   );
   const problems /*: Array<string> */ = [];
   for (const {id, frameworkName} of pluginFrameworks) {
-    if (!linkedPluginNames.has(frameworkSettingPrefix(id))) {
+    if (
+      !linkedPrefixes.has(frameworkSettingPrefix(id)) ||
+      linkedPluginNames?.has(frameworkName) === false
+    ) {
       problems.push(
         `error: ${frameworkName} (${id}) is a precompiled framework from an autolinking plugin, but the Xcode project does not link it. Run \`npx react-native spm\` to update the project.`,
       );
     }
   }
-  for (const [prefix, frameworkName] of linkedPluginNames) {
-    if (!pairedPrefixes.has(prefix)) {
+  for (const frameworkName of linkedPluginNames ?? []) {
+    if (!pairedNames.has(frameworkName)) {
       problems.push(
         `error: The Xcode project links ${frameworkName}.framework, but no autolinking plugin provides it on this machine. Precompile ${frameworkName}, or run \`npx react-native spm\` to update the project.`,
       );

packages/react-native/scripts/spm/sync-spm-autolinking.js

@@ -134,8 +134,21 @@ async function main(
     autolinkingArgv.push('--ios-deployment-target', iosDeploymentTarget);
   }
   deps.generateAutolinking(autolinkingArgv);
-  // Before the stamp, so every build fails until the project is updated.
-  deps.assertPluginFrameworksLinked(appRoot);
+  const stampPath = path.join(
+    appRoot,
+    'build',
+    'generated',
+    'autolinking',
+    '.spm-sync-stamp',
+  );
+  // Without a stamp every build re-runs sync and fails until the project is
+  // updated; an earlier stamp would let the next build skip it.
+  try {
+    deps.assertPluginFrameworksLinked(appRoot);
+  } catch (e) {
+    fs.rmSync(stampPath, {force: true});
+    throw e;
+  }
 
   // Rebuild the per-app generated-headers farm (vended as the ReactAppHeaders
   // SPM target inside the codegen package). React core headers need no trees
@@ -145,13 +158,6 @@ async function main(
   // so no path-locator JSON is written.
   deps.buildPerAppHeaderTree(appRoot, {log});
 
-  const stampPath = path.join(
-    appRoot,
-    'build',
-    'generated',
-    'autolinking',
-    '.spm-sync-stamp',
-  );
   fs.mkdirSync(path.dirname(stampPath), {recursive: true});
   fs.writeFileSync(stampPath, new Date().toISOString() + '\n', 'utf8');
   log('SPM autolinking sync complete.');