Skip to content

Commit 09ba4cd

Browse files
chrfalchclaude
andcommitted
SPM: unquote before the promotion seed test, and note re-sync in the tradeoff
Review feedback on the promoted-scalar restore: - The seed guard compared the prior scalar against the quoted `"$(inherited)"` only, so an unquoted `$(inherited)` — equally valid, and present in the suite's own untrimmed-scalar fixture — was re-emitted alongside the seed. Deinit still restored it byte-identically (the record is raw), so this was duplication rather than breakage, but it defeated the guard. Compare unquoted, and parametrize the guard's unit test over both spellings so the injected shape is asserted, not just the post-deinit bytes. - The reversal tradeoff is not deinit-only: every re-sync reverts from the recorded baseline before re-injecting, so an `spm update` discards hand-added members just the same. Say so in the banner. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent d6ff222 commit 09ba4cd

2 files changed

Lines changed: 32 additions & 21 deletions

File tree

packages/react-native/scripts/spm/__tests__/spm-pbxproj-test.js

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -195,25 +195,31 @@ describe('addArrayStringValues', () => {
195195
expect(out).toContain('"-ObjC"');
196196
});
197197

198-
it('promotes a bare "$(inherited)" scalar without emitting it twice', () => {
199-
const scalar = PLAIN_PBXPROJ.replace(
200-
'PRODUCT_NAME = "$(TARGET_NAME)";',
201-
'OTHER_LDFLAGS = "$(inherited)"; PRODUCT_NAME = "$(TARGET_NAME)";',
202-
);
203-
const out = addArrayStringValues(
204-
scalar,
205-
targetDebugDict(scalar),
206-
'OTHER_LDFLAGS',
207-
['"-ObjC"'],
208-
);
209-
const members = /OTHER_LDFLAGS = \(\n([\s\S]*?)\t+\);/
210-
.exec(out)[1]
211-
.split('\n')
212-
.map(line => line.trim().replace(/,$/, ''))
213-
.filter(member => member.length > 0);
214-
// The scalar's value IS the seed the array is created with.
215-
expect(members).toEqual(['"$(inherited)"', '"-ObjC"']);
216-
});
198+
// Xcode writes the seed quoted, but the unquoted form is just as valid and
199+
// appears in hand-edited projects. Both are the same value to the build
200+
// system, so neither may be re-emitted alongside the seed.
201+
it.each(['"$(inherited)"', '$(inherited)'])(
202+
'promotes a bare %s scalar without emitting the seed twice',
203+
priorValue => {
204+
const scalar = PLAIN_PBXPROJ.replace(
205+
'PRODUCT_NAME = "$(TARGET_NAME)";',
206+
`OTHER_LDFLAGS = ${priorValue}; PRODUCT_NAME = "$(TARGET_NAME)";`,
207+
);
208+
const out = addArrayStringValues(
209+
scalar,
210+
targetDebugDict(scalar),
211+
'OTHER_LDFLAGS',
212+
['"-ObjC"'],
213+
);
214+
const members = /OTHER_LDFLAGS = \(\n([\s\S]*?)\t+\);/
215+
.exec(out)[1]
216+
.split('\n')
217+
.map(line => line.trim().replace(/,$/, ''))
218+
.filter(member => member.length > 0);
219+
// The scalar's value IS the seed the array is created with.
220+
expect(members).toEqual(['"$(inherited)"', '"-ObjC"']);
221+
},
222+
);
217223

218224
it('promotes an empty scalar without emitting a bare `,` member', () => {
219225
const scalar = PLAIN_PBXPROJ.replace(

packages/react-native/scripts/spm/spm-pbxproj.js

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -408,9 +408,12 @@ function addArrayStringValues(
408408
}
409409
// Existing scalar — promote to an array preserving the prior value. Skip it
410410
// when it IS the `"$(inherited)"` the array is seeded with (emitted twice),
411-
// or when it is empty (a bare `,` is not a valid plist element).
411+
// or when it is empty (a bare `,` is not a valid plist element). The seed
412+
// test unquotes first: pbxproj accepts `$(inherited)` bare, and Xcode's own
413+
// quoted form is the same value to the build system.
412414
const prior = field.value.trim();
413-
const carriesPrior = prior !== '' && prior !== '"$(inherited)"';
415+
const carriesPrior =
416+
prior !== '' && prior.replace(/^"(.*)"$/s, '$1') !== '$(inherited)';
414417
const replacement = arrayBlock([
415418
'"$(inherited)"',
416419
...(carriesPrior ? [prior] : []),
@@ -471,6 +474,8 @@ function setScalarField(
471474
// user edits made after injection) untouched. The exception is a scalar that
472475
// injection promoted to an array: reversing that rewrites the whole field (see
473476
// removeRecordedBuildSettings), so members added to it afterwards are lost.
477+
// That is not deinit-only — every re-sync reverts from the recorded baseline
478+
// before re-injecting, so an `spm update` discards them just the same.
474479
// All are pure string transforms.
475480
// ---------------------------------------------------------------------------
476481

0 commit comments

Comments
 (0)