SPM: restore a promoted scalar build setting on deinit

chrfalch · claude · chrfalch · commit 15fefc2cf781 · 2026-07-28T19:28:46.000+02:00
`spm add` merges its array build settings (`HEADER_SEARCH_PATHS`, `OTHER_LDFLAGS`,
`FRAMEWORK_SEARCH_PATHS`, `LD_RUNPATH_SEARCH_PATHS`) via `addArrayStringValues`,
which has three add paths: create the field, append to an existing array, or
promote an existing SCALAR into a `( … )` array. Only the first two were
reversible. A promotion was recorded as `appendedArrayValues`, whose reversal
only strips the injected members — so `spm deinit` left the promoted array and
its `"$(inherited)"` seed behind, and the original scalar was never recorded
anywhere to restore from. A scalar is the ordinary shape in a real project
(`HEADER_SEARCH_PATHS = "$(inherited)";` in the Debug config), so this broke the
byte-identical restore `deinit` promises on a common input.

Record the pre-injection value in a new `promotedArrayScalars` field on the
marker's `BuildSettingChange` and restore it in place. Notes on the details:

- The recorded value is kept RAW. `findField`'s token for a bare scalar runs to
  the `;`, so it carries any whitespace before it, and deinit has to write those
  bytes back. The value emitted as an array MEMBER is still trimmed — a member
  with trailing whitespace would be malformed. The two differ deliberately.
- The record is gated on the merge having actually changed the text.
  `addArrayStringValues` no-ops when its value list is empty (as
  `FRAMEWORK_SEARCH_PATHS` is with no flavored frameworks) or when every value
  is already present, and a recorded-but-untouched field would have deinit
  clobber whatever the user has there by then.
- Restoration is skipped when the field is absent at deinit time: it was deleted
  after injection, and re-adding it would resurrect it at the top of the
  dictionary, matching neither the original nor the user's intent.
- Promotion no longer re-emits a prior value that is itself `"$(inherited)"`
  (the seed) or empty (a bare `,` is not a valid plist element).

Reversing a promotion rewrites the whole field, because the injected members and
the seed are indistinguishable from the user's own once folded together. Members
hand-added to a promoted array afterwards are therefore lost; the removal-helper
banner in spm-pbxproj.js now names that exception.

Co-Authored-By: Claude Opus 5 &lt;noreply@anthropic.com&gt;
diff --git a/packages/react-native/scripts/spm/__tests__/inject-spm-xcodeproj-test.js b/packages/react-native/scripts/spm/__tests__/inject-spm-xcodeproj-test.js
@@ -29,6 +29,16 @@ const PODS = PLAIN.replace(
   'AA0000000000000000000901 /* Debug */ = {\n\t\t\tisa = XCBuildConfiguration;\n\t\t\tbaseConfigurationReference = BB0000000000000000000001 /* Pods-MyApp.debug.xcconfig */;\n\t\t\tbuildSettings = {',
 );
 
+// Derive a variant whose app-target configs already carry HEADER_SEARCH_PATHS
+// as a plain scalar (ordinary, valid pbxproj) — the state injection promotes to
+// an array.
+function withScalarHeaderSearchPaths(value) {
+  return PLAIN.replaceAll(
+    'PRODUCT_BUNDLE_IDENTIFIER = com.example.MyApp;',
+    `HEADER_SEARCH_PATHS = ${value};\n\t\t\t\tPRODUCT_BUNDLE_IDENTIFIER = com.example.MyApp;`,
+  );
+}
+
 const RN_PATH = '../node_modules/react-native';
 
 // Absolute, mirroring resolveHermesCliPathSetting (a `..`-relative path through
@@ -227,6 +237,36 @@ describe('injectSpmIntoPbxproj — Tier 2 (build settings + phase)', () => {
     expect(syncIdx).toBeLessThan(sourcesIdx);
   });
 
+  it.each([
+    [
+      '"$(inherited)"',
+      ['"$(inherited)"', '"$(SRCROOT)/build/generated/autolinking/headers"'],
+    ],
+    [
+      '"$(inherited) $(SRCROOT)/vendor/include"',
+      [
+        '"$(inherited)"',
+        '"$(inherited) $(SRCROOT)/vendor/include"',
+        '"$(SRCROOT)/build/generated/autolinking/headers"',
+      ],
+    ],
+  ])(
+    'promotes a pre-existing HEADER_SEARCH_PATHS scalar (%s) to an array, keeping its value and one $(inherited)',
+    (scalar, expectedMembers) => {
+      const {text} = inject(withScalarHeaderSearchPaths(scalar));
+      const arrays = [
+        ...text.matchAll(/HEADER_SEARCH_PATHS = \(\n([\s\S]*?)\t+\);/g),
+      ].map(m =>
+        m[1]
+          .split('\n')
+          .map(line => line.trim().replace(/,$/, ''))
+          .filter(member => member.length > 0),
+      );
+      // Both app-target configs (Debug + Release).
+      expect(arrays).toEqual([expectedMembers, expectedMembers]);
+    },
+  );
+
   it('adds one generated embed phase immediately after Frameworks', () => {
     const {text} = inject(PLAIN);
     expect(text).not.toContain('Fix SPM Embedded Flavor');
diff --git a/packages/react-native/scripts/spm/__tests__/remove-spm-injection-test.js b/packages/react-native/scripts/spm/__tests__/remove-spm-injection-test.js
@@ -34,15 +34,42 @@ afterEach(() => {
   scaffoldedAppRoots = [];
 });
 
+// Pre-existing values for an injected array setting (HEADER_SEARCH_PATHS),
+// seeded into both app-target configs. The plain fixture has none, so it only
+// ever exercises the create-from-absent path; deinit must restore each of
+// these forms — a plain scalar is ordinary, valid pbxproj.
+const PRE_EXISTING_HEADER_SEARCH_PATHS = {
+  'a bare $(inherited) scalar': '"$(inherited)"',
+  'a scalar with real content': '"$(inherited) $(SRCROOT)/vendor/include"',
+  'an array': '(\n\t\t\t\t"$(inherited)",\n\t\t\t)',
+};
+
+// Seed a whole `KEY = value;` field (comments and stray whitespace included)
+// into both app-target configs.
+function withSetting(field /*: string */) {
+  return PLAIN.replaceAll(
+    'PRODUCT_BUNDLE_IDENTIFIER = com.example.MyApp;',
+    `${field}\n\t\t\t\tPRODUCT_BUNDLE_IDENTIFIER = com.example.MyApp;`,
+  );
+}
+
+function withHeaderSearchPaths(value /*: string */) {
+  return withSetting(`HEADER_SEARCH_PATHS = ${value};`);
+}
+
 // Build a throwaway app dir: <tmp>/MyApp.xcodeproj/project.pbxproj seeded with
 // the plain (SPM-only) fixture, and a node_modules/react-native sibling so the
 // relative reactNativePath resolves.
-function scaffoldApp() {
+function scaffoldApp(pbxproj = PLAIN) {
   const appRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'spm-deinit-'));
   scaffoldedAppRoots.push(appRoot);
   const xcodeprojPath = path.join(appRoot, 'MyApp.xcodeproj');
   fs.mkdirSync(xcodeprojPath, {recursive: true});
-  fs.writeFileSync(path.join(xcodeprojPath, 'project.pbxproj'), PLAIN, 'utf8');
+  fs.writeFileSync(
+    path.join(xcodeprojPath, 'project.pbxproj'),
+    pbxproj,
+    'utf8',
+  );
   const rnRoot = path.join(appRoot, 'node_modules', 'react-native');
   fs.mkdirSync(rnRoot, {recursive: true});
   const artifactRoot = path.join(appRoot, 'build', 'xcframeworks');
@@ -189,6 +216,144 @@ describe('removeSpmInjection — the surgical inverse of add', () => {
   });
 });
 
+describe.each(Object.entries(PRE_EXISTING_HEADER_SEARCH_PATHS))(
+  'removeSpmInjection with HEADER_SEARCH_PATHS already set to %s',
+  (_label, value) => {
+    it('restores the pre-existing value byte-for-byte', () => {
+      const {appRoot, xcodeprojPath, rnRoot} = scaffoldApp(
+        withHeaderSearchPaths(value),
+      );
+      const before = pbxprojOf(xcodeprojPath);
+
+      injectSpmIntoExistingXcodeproj({
+        appRoot,
+        reactNativeRoot: rnRoot,
+        xcodeprojPath,
+      });
+      expect(pbxprojOf(xcodeprojPath)).not.toBe(before);
+
+      expect(removeSpmInjection({appRoot, xcodeprojPath}).status).toBe(
+        'removed',
+      );
+      expect(pbxprojOf(xcodeprojPath)).toBe(before);
+    });
+
+    it('re-syncing is byte-for-byte identical', () => {
+      const {appRoot, xcodeprojPath, rnRoot} = scaffoldApp(
+        withHeaderSearchPaths(value),
+      );
+      injectSpmIntoExistingXcodeproj({
+        appRoot,
+        reactNativeRoot: rnRoot,
+        xcodeprojPath,
+      });
+      const first = pbxprojOf(xcodeprojPath);
+      injectSpmIntoExistingXcodeproj({
+        appRoot,
+        reactNativeRoot: rnRoot,
+        xcodeprojPath,
+      });
+      expect(pbxprojOf(xcodeprojPath)).toBe(first);
+    });
+  },
+);
+
+// findField's token for a BARE scalar ends AT the `;`, so it includes any
+// whitespace before it. Deinit must put those bytes back exactly, not a
+// tidied-up version of them.
+describe.each([
+  'HEADER_SEARCH_PATHS = $(inherited)   ; /* note */',
+  'HEADER_SEARCH_PATHS =   ;',
+])('removeSpmInjection with the untrimmed scalar `%s`', field => {
+  it('restores it byte-for-byte', () => {
+    const {appRoot, xcodeprojPath, rnRoot} = scaffoldApp(withSetting(field));
+    const before = pbxprojOf(xcodeprojPath);
+
+    injectSpmIntoExistingXcodeproj({
+      appRoot,
+      reactNativeRoot: rnRoot,
+      xcodeprojPath,
+    });
+    expect(pbxprojOf(xcodeprojPath)).not.toBe(before);
+
+    expect(removeSpmInjection({appRoot, xcodeprojPath}).status).toBe('removed');
+    expect(pbxprojOf(xcodeprojPath)).toBe(before);
+  });
+});
+
+describe('a scalar array setting injection has nothing to add to', () => {
+  const SCALAR = 'FRAMEWORK_SEARCH_PATHS = "$(inherited)";';
+  const EDITED = 'FRAMEWORK_SEARCH_PATHS = "$(inherited) $(SRCROOT)/Vendor";';
+
+  // The fixture's flavored-frameworks manifest is empty, so
+  // FRAMEWORK_SEARCH_PATHS is injected with no values at all.
+  it('is left untouched, unrecorded, and survives a later user edit', () => {
+    const {appRoot, xcodeprojPath, rnRoot} = scaffoldApp(withSetting(SCALAR));
+
+    injectSpmIntoExistingXcodeproj({
+      appRoot,
+      reactNativeRoot: rnRoot,
+      xcodeprojPath,
+    });
+
+    const injected = pbxprojOf(xcodeprojPath);
+    expect(injected).toContain(SCALAR);
+    expect(injected).not.toMatch(/FRAMEWORK_SEARCH_PATHS = \(/);
+    for (const change of readMarker(xcodeprojPath).buildSettingChanges) {
+      expect(change.promotedArrayScalars ?? {}).not.toHaveProperty(
+        'FRAMEWORK_SEARCH_PATHS',
+      );
+    }
+
+    fs.writeFileSync(
+      path.join(xcodeprojPath, 'project.pbxproj'),
+      injected.replaceAll(SCALAR, EDITED),
+      'utf8',
+    );
+    removeSpmInjection({appRoot, xcodeprojPath});
+
+    const after = pbxprojOf(xcodeprojPath);
+    expect(after).toContain(EDITED);
+    expect(after).not.toContain(SCALAR);
+  });
+});
+
+describe('a promoted array setting the user deleted after add', () => {
+  const SCALAR = '"$(inherited) $(SRCROOT)/vendor/include"';
+
+  it('is not resurrected by deinit', () => {
+    const {appRoot, xcodeprojPath, rnRoot} = scaffoldApp(
+      withHeaderSearchPaths(SCALAR),
+    );
+    const before = pbxprojOf(xcodeprojPath);
+
+    injectSpmIntoExistingXcodeproj({
+      appRoot,
+      reactNativeRoot: rnRoot,
+      xcodeprojPath,
+    });
+
+    const deleted = pbxprojOf(xcodeprojPath).replace(
+      /\n\t+HEADER_SEARCH_PATHS = \(\n[\s\S]*?\n\t+\);/g,
+      '',
+    );
+    expect(deleted).not.toContain('HEADER_SEARCH_PATHS');
+    fs.writeFileSync(
+      path.join(xcodeprojPath, 'project.pbxproj'),
+      deleted,
+      'utf8',
+    );
+
+    removeSpmInjection({appRoot, xcodeprojPath});
+
+    // Everything else is back to its pre-injection bytes; only the setting the
+    // user deleted stays gone.
+    expect(pbxprojOf(xcodeprojPath)).toBe(
+      before.replaceAll(`\n\t\t\t\tHEADER_SEARCH_PATHS = ${SCALAR};`, ''),
+    );
+  });
+});
+
 describe('generated-sources reconciliation on update', () => {
   it('removes exactly the UUIDs of an entry dropped from the manifest, keeping the rest', () => {
     const {appRoot, xcodeprojPath, rnRoot} = scaffoldApp();
diff --git a/packages/react-native/scripts/spm/__tests__/spm-pbxproj-test.js b/packages/react-native/scripts/spm/__tests__/spm-pbxproj-test.js
@@ -195,6 +195,44 @@ describe('addArrayStringValues', () => {
     expect(out).toContain('"-ObjC"');
   });
 
+  it('promotes a bare "$(inherited)" scalar without emitting it twice', () => {
+    const scalar = PLAIN_PBXPROJ.replace(
+      'PRODUCT_NAME = "$(TARGET_NAME)";',
+      'OTHER_LDFLAGS = "$(inherited)"; PRODUCT_NAME = "$(TARGET_NAME)";',
+    );
+    const out = addArrayStringValues(
+      scalar,
+      targetDebugDict(scalar),
+      'OTHER_LDFLAGS',
+      ['"-ObjC"'],
+    );
+    const members = /OTHER_LDFLAGS = \(\n([\s\S]*?)\t+\);/
+      .exec(out)[1]
+      .split('\n')
+      .map(line => line.trim().replace(/,$/, ''))
+      .filter(member => member.length > 0);
+    // The scalar's value IS the seed the array is created with.
+    expect(members).toEqual(['"$(inherited)"', '"-ObjC"']);
+  });
+
+  it('promotes an empty scalar without emitting a bare `,` member', () => {
+    const scalar = PLAIN_PBXPROJ.replace(
+      'PRODUCT_NAME = "$(TARGET_NAME)";',
+      'OTHER_LDFLAGS =   ; PRODUCT_NAME = "$(TARGET_NAME)";',
+    );
+    const out = addArrayStringValues(
+      scalar,
+      targetDebugDict(scalar),
+      'OTHER_LDFLAGS',
+      ['"-ObjC"'],
+    );
+    const block = /OTHER_LDFLAGS = \(\n([\s\S]*?)\t+\);/.exec(out)[1];
+    // Asserted on the raw block: a member list filtered for emptiness (as the
+    // test above does) would hide the malformed element this guards against.
+    expect(block.split('\n').filter(line => /^\s*,$/.test(line))).toEqual([]);
+    expect(block).toContain('"-ObjC"');
+  });
+
   it('dedups by EXACT token, not substring (adds "-ObjC" even when "-ObjCFoo" is present)', () => {
     const withArray = PLAIN_PBXPROJ.replace(
       'PRODUCT_NAME = "$(TARGET_NAME)";',
diff --git a/packages/react-native/scripts/spm/generate-spm-xcodeproj.js b/packages/react-native/scripts/spm/generate-spm-xcodeproj.js
@@ -136,6 +136,11 @@ type BuildSettingChange = {
   // value), e.g. a ${PODS_ROOT}-anchored REACT_NATIVE_PATH that dangles once
   // CocoaPods is deintegrated. Deinit restores the original.
   replacedScalars?: {[string]: string},
+  // Array settings that existed as a SCALAR and were promoted to a `( … )`
+  // array (key → the pre-injection raw value text, quotes included). Deinit
+  // restores that value verbatim; removing the injected members would leave
+  // the promoted array and its `"$(inherited)"` seed behind.
+  promotedArrayScalars?: {[string]: string},
 };
 // A plugin-contributed source, normalized for pbxproj emission. `path` is
 // SRCROOT-relative when under the app root, else absolute; `sourceTree` is the
@@ -1492,6 +1497,7 @@ function mergeReactBuildSettings(
   };
   const createdArrayKeys /*: Array<string> */ = [];
   const appendedArrayValues /*: {[string]: Array<string>} */ = {};
+  const promotedArrayScalars /*: {[string]: string} */ = {};
   const createdScalars /*: Array<string> */ = [];
   const arraySettings = [
     ...INJECTED_ARRAY_SETTINGS,
@@ -1503,15 +1509,32 @@ function mergeReactBuildSettings(
       continue;
     }
     const existing = findField(text, d, key);
+    // Non-null only for a scalar addArrayStringValues would promote to an array
+    // (same array-vs-scalar test it uses). Kept RAW: findField's token for a
+    // bare scalar ends at the `;`, so it carries any whitespace before it, and
+    // deinit has to write those bytes back verbatim.
+    const priorScalar =
+      existing != null && !existing.value.trimStart().startsWith('(')
+        ? existing.value
+        : null;
     if (existing == null) {
       createdArrayKeys.push(key);
-    } else {
+    } else if (priorScalar == null) {
       const fresh = values.filter(v => !existing.value.includes(v));
       if (fresh.length > 0) {
         appendedArrayValues[key] = fresh;
       }
     }
+    const beforeAdd = text;
     text = addArrayStringValues(text, d, key, values);
+    // Record only a promotion that actually happened: addArrayStringValues
+    // no-ops when `values` is empty or every value is already a member, and a
+    // recorded-but-untouched field would have deinit clobber whatever the user
+    // has there by then. Restoring the scalar subsumes removing the injected
+    // members, so the two records stay mutually exclusive per key.
+    if (priorScalar != null && text !== beforeAdd) {
+      promotedArrayScalars[key] = priorScalar;
+    }
   }
   const replacedScalars /*: {[string]: string} */ = {};
   for (const {key, value} of scalars) {
@@ -1570,6 +1593,9 @@ function mergeReactBuildSettings(
       appendedArrayValues,
       createdScalars,
       replacedScalars,
+      ...(Object.keys(promotedArrayScalars).length > 0
+        ? {promotedArrayScalars}
+        : {}),
     },
   };
 }
@@ -2097,6 +2123,25 @@ function removeRecordedBuildSettings(
         );
       }
     }
+    const promotedArrayScalars /*: {[string]: string} */ =
+      change.promotedArrayScalars ?? {};
+    for (const key of Object.keys(promotedArrayScalars)) {
+      const current = dict();
+      const originalValue = promotedArrayScalars[key];
+      // A field that is gone was deleted by the user after injection; restoring
+      // it would resurrect it, at the top of the dict, matching neither state.
+      if (
+        current != null &&
+        typeof originalValue === 'string' &&
+        findField(text, current, key) != null
+      ) {
+        // Rewriting the whole value is what makes the promotion reversible at
+        // all — its members and its `"$(inherited)"` seed are indistinguishable
+        // from the user's own once folded together. The tradeoff: members the
+        // user hand-added to the promoted array afterwards are discarded.
+        text = setScalarField(text, current, key, originalValue);
+      }
+    }
     for (const key of change.createdArrayKeys ?? []) {
       const current = dict();
       if (current != null) {
diff --git a/packages/react-native/scripts/spm/spm-pbxproj.js b/packages/react-native/scripts/spm/spm-pbxproj.js
