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 index 89393a28dc0..b06acbba246 100644 --- 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 index 4fe3d342fdc..7fa95223d54 100644 --- 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: /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 index 2ac7297d4ce..31508c6986e 100644 --- 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,50 @@ describe('addArrayStringValues', () => { expect(out).toContain('"-ObjC"'); }); + // Xcode writes the seed quoted, but the unquoted form is just as valid and + // appears in hand-edited projects. Both are the same value to the build + // system, so neither may be re-emitted alongside the seed. + it.each(['"$(inherited)"', '$(inherited)'])( + 'promotes a bare %s scalar without emitting the seed twice', + priorValue => { + const scalar = PLAIN_PBXPROJ.replace( + 'PRODUCT_NAME = "$(TARGET_NAME)";', + `OTHER_LDFLAGS = ${priorValue}; 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 index e75af429ce2..ead38f33f3f 100644 --- 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 */ = []; const appendedArrayValues /*: {[string]: Array} */ = {}; + const promotedArrayScalars /*: {[string]: string} */ = {}; const createdScalars /*: Array */ = []; 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 index 56f27e73ba8..cf0d79b07e5 100644 --- a/packages/react-native/scripts/spm/spm-pbxproj.js +++ b/packages/react-native/scripts/spm/spm-pbxproj.js @@ -406,10 +406,17 @@ function addArrayStringValues( const lines = fresh.map(v => `${memberIndent}${v},\n`).join(''); return text.slice(0, lineStart) + lines + text.slice(lineStart); } - // Existing scalar — promote to an array preserving the prior value. + // Existing scalar — promote to an array preserving the prior value. Skip it + // when it IS the `"$(inherited)"` the array is seeded with (emitted twice), + // or when it is empty (a bare `,` is not a valid plist element). The seed + // test unquotes first: pbxproj accepts `$(inherited)` bare, and Xcode's own + // quoted form is the same value to the build system. + const prior = field.value.trim(); + const carriesPrior = + prior !== '' && prior.replace(/^"(.*)"$/s, '$1') !== '$(inherited)'; const replacement = arrayBlock([ '"$(inherited)"', - field.value.trim(), + ...(carriesPrior ? [prior] : []), ...fresh, ]); return ( @@ -464,7 +471,12 @@ function setScalarField( // --------------------------------------------------------------------------- // Surgical removal — the inverse of the additive helpers above. `deinit` uses // these to undo exactly what injection added, leaving every other byte (incl. -// user edits made after injection) untouched. All are pure string transforms. +// user edits made after injection) untouched. The exception is a scalar that +// injection promoted to an array: reversing that rewrites the whole field (see +// removeRecordedBuildSettings), so members added to it afterwards are lost. +// That is not deinit-only — every re-sync reverts from the recorded baseline +// before re-injecting, so an `spm update` discards them just the same. +// All are pure string transforms. // --------------------------------------------------------------------------- /**