Give an app extension its build settings, so UIKit links - #5578
Merged
Conversation
Mirrors the cloud builder fix. The extension target's settings block is parsed out of a pbxproj-shaped string, and the parser sliced the last character of the value INSTEAD of dropping it, so every "KEY = YES;" line produced ";". Xcode reads that as off: CLANG_ENABLE_MODULES never took, clang compiled the extension without -fmodules, and without modules there is no autolinking. The target's frameworks phase carries Foundation alone, so an extension importing UIKit reached ld with nothing to resolve _OBJC_CLASS_$_UIView against. CLANG_ENABLE_OBJC_ARC was off for the same reason, so the extension built as MRC and leaked. Dropping the semicolon exposes the second half, which never ran before: a quoted value like "gnu++14" is re-emitted inside a Ruby string literal in the project fixup script, where the kept quotes are a syntax error. So unwrap those too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Compared 151 screenshots: 151 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
Collaborator
Author
|
Compared 148 screenshots: 148 matched. Benchmark Results
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 181 screenshots: 181 matched. |
Collaborator
Author
|
Compared 144 screenshots: 144 matched. |
Collaborator
Author
|
Compared 149 screenshots: 149 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 217 screenshots: 217 matched. |
Collaborator
Author
|
Compared 143 screenshots: 143 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Mirrors the cloud builder fix. Past the frameworks problem the archive dies in the app's own target with "Embedded binary's bundle identifier is not prefixed with the parent app's bundle identifier -- Embedded Binary Bundle Identifier: (null)". Not prefixed wrongly: absent. A modern Xcode target keeps CFBundleIdentifier and the version strings in build settings and generates them into the plist, so an extension folder exported from such a project ships an Info.plist without those keys, and builtin-infoPlistUtility only expands $(...) references that are already there -- it does not add the key. Every extension the builder generates itself writes CFBundleIdentifier = $(PRODUCT_BUNDLE_IDENTIFIER) into its plist; the generic .ios.appext path trusted whatever the archive carried. It now adds the same reference when the key is missing and aligns CFBundleShortVersionString / CFBundleVersion with the app, which Apple requires of an embedded extension. A correct value, and one written as a $(...) reference, are left alone; anything changed is logged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee7eb951e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Mirrors the cloud builder fix. The archive was unpacked into dist/ and deleted from the resources directory at that point -- after the translator has already copied that directory into <main>-src, where every file becomes an app resource. An unbuilt second copy of the extension therefore rode along inside the .app beside the .appex it was unpacked into, adding its weight to the IPA and putting the extension's sources in the bundle. Unpacking cannot move earlier: it wires Xcode targets, and the project does not exist yet. The move out of resDir can, and that is all that was needed -- every other archive kind consumed out of resDir (.lproj.zip, .placeindist.zip, .framework.zip) deletes itself in the same early pass for exactly this reason. The archives are now staged into tmp/appext before the resources are walked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 687ec047e0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Mirrors the cloud builder fix. The stamper took buildVersion and the ios.bundleVersion hint, but the app's own Info.plist does not: ios.plistInject wins for CFBundleShortVersionString and CFBundleVersion, and the injection only falls back to buildVersion when it says nothing. An app that injects its version ships that value, while the extension was stamped with the raw one -- so an extension whose version already MATCHED its app could be rewritten into one that does not, which is the embedded-bundle validation failure the stamping exists to prevent. The Matter extension already resolved both keys correctly, inline. That resolution is now two named helpers shared by both callers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mirrors the cloud builder fix for two review catches, both cases where
the stamping edits the wrong thing.
INFOPLIST_FILE: an archive's buildSettings.properties may point the
target at a plist other than <folder>/Info.plist, and those properties
are folded into the target's build settings further down, so Xcode
processes that file into the .appex. Stamping the default left the plist
that actually ships without the identifier and versions. The effective
path is now resolved first (relative to the project directory, with
$(SRCROOT) and $(PROJECT_DIR) understood); a reference this build cannot
resolve returns null and the build says so rather than editing a file
nothing reads.
The value scan: indexOf("<string>") from the key found the next string
ANYWHERE after it, so a CFBundleVersion given <integer>7</integer>, or
the valid empty form <string/>, sent the rewrite into an unrelated later
value -- CFBundleName, or a field inside the NSExtension dict, stamped
with a version number. This is the trap injectedPlistString's comment
records, so the fix is its machinery: the key's own value element, with
whitespace, comments and CDATA skipped. A non-string value is left
alone; <string/> is this key's own empty value and gets filled.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mirrors the cloud builder fix. The key lookup was a whole-file search, so a key of the same name inside a nested dictionary -- NSExtensionAttributes, CFBundleURLTypes and CFBundleDocumentTypes all carry dictionaries of their own -- answered first when it came earlier in the file. That reads as "the bundle already has an identifier" while the real key is still missing, or sends the version rewrite into an unrelated nested value. The three keys are now looked up among the DIRECT children of the root dict: a small walker that steps over each value whole (depth counted on the element's own name) and skips comments and CDATA rather than reading a < inside either as a tag. And CFBundleIdentifier is filled when it is present but empty. It is set with overwrite off, because an explicit identifier is the extension's own business -- but <string/> is not an explicit identifier, it is no identifier, and it fails the embedded-binary check exactly like a missing one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mirrors the cloud builder fix. Stripping a leading $(SRCROOT)/ handled
the prefix and nothing else, so INFOPLIST_FILE =
$(SRCROOT)/$(TARGET_NAME)/Info.plist -- how an Xcode project writes the
plist that sits in the extension's own folder, i.e. the common case --
was left holding $(TARGET_NAME) and refused as unresolvable, and the
stamper skipped a plist it could have found.
Every setting in that path is known here: SRCROOT and PROJECT_DIR are the
directory the extension folders are extracted into, and TARGET_NAME is
the folder's name, because that is the name the target is created with.
PRODUCT_NAME follows TARGET_NAME unless the archive overrode it with a
literal. Both spellings are substituted, $(NAME) and ${NAME}. A path
still holding a $ afterwards is refused, because a half-resolved path
names some file and editing whichever one it lands on is worse than
saying so. The properties reader is now shared with the other callers.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da6420bb2b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Mirrors the cloud builder fix, and the exemption was wrong in both directions. A version written as $(MARKETING_VERSION) was left standing on the grounds that a build-setting reference is the project saying it knows what it is doing. It is not: the archive's buildSettings.properties are copied into this target's build configurations further down, so the reference resolves to whatever they say -- a stale 1.0 under an app at 5.4. And a reference to a setting they do NOT define resolves to nothing at all, because the generated target carries no version settings of its own. A reference is now resolved against those same properties and judged by the result: one that already lands on the app's version is left alone, anything else is replaced with the literal, and the log says what it resolved to. The identifier is untouched by this -- it is written with overwrite off, so an explicit $(PRODUCT_BUNDLE_IDENTIFIER) still stands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mirrors the cloud builder fix. Emptiness was tested on the raw text between the tags, so <string><!-- note --></string> and <string><![CDATA[]]></string> counted as values that are already there: a nonzero run of characters and an empty value. The identifier was then preserved and the extension shipped without one. The content is now resolved with plistStringContent first -- CDATA read, comments stripped, entities decoded -- and emptiness, equality and the build-setting resolution all run on that, so a version written <![CDATA[5.4]]> under an app at 5.4 is recognised as already right and left as the archive wrote it. Padding is not accepted as right: a plist parser keeps the spaces in <string> 5.4 </string>, so Apple compares " 5.4 " with the app's "5.4" and rejects the pair. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mirrors the cloud builder fix. A setting's value may name another setting and Xcode keeps expanding until none is left; one traversal of the map does that only when the iteration order happens to be the dependency order, and Properties hands them over in hash order. With VERSION_SUFFIX = 1 and MARKETING_VERSION = 5.4$(VERSION_SUFFIX), visiting MARKETING_VERSION first left $(VERSION_SUFFIX) behind, the strip deleted it as though nothing defined it, and a version the device resolves to 5.41 read as the app's own 5.4. Now it expands until nothing changes or nothing is left to expand, capped so a cycle settles instead of spinning; what survives is treated as resolving to nothing, which is what Xcode does with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mirrors the cloud builder fix. INFOPLIST_FILE arrives inside the developer's .ios.appext and the stamper WRITES to whatever it names, so an absolute path or a ../../ traversal had the builder rewriting a file outside the project. The resolved path is now compared, canonically, against the project directory, and anything landing outside is refused with a log line rather than edited. Canonical because an archive can carry symlinks: a path that sits inside the project can still point out of it. The default <folder>/Info.plist goes through the same check, since a zip may plant a symlink at that very name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db507a7946
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Mirrors the cloud builder comment. Two review rounds proposed the same fix in different spellings -- that <string /> is not recognised, that <string> </string> survives -- and both were already handled, for a reason that sat one line away and was not written down: plistStringContent trims on both of its paths, so whitespace, a comment, an empty CDATA section and any mix arrive as "", and the self-closing test above catches both <string/> and <string /> because XML puts the slash against the '>' whatever precedes it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…n in Mirrors the cloud builder fix for two review catches. A qualified setting names a plist too. Xcode honours INFOPLIST_FILE[sdk=iphoneos*], the archive's properties are copied into the target verbatim, and a qualified value beats the base one for the builds it matches, so the device build shipped the one plist the stamper had not touched. All of them are stamped now; which applies depends on the sdk, configuration and arch, and stamping is idempotent. Only the ESCAPED spelling gets this far -- an unescaped one splits on the = inside the brackets and leaves a key Xcode does not recognise -- so the filter requires the closing bracket. And the plist is read as its own bytes declare -- byte order mark first, then the encoding in its XML declaration -- and written back the same way. Reading with the platform default charset left a UTF-16 plist as noise that would not parse, and turned a Latin-1 one's accented characters into replacements that would then have been written back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mirrors the cloud builder fix. A build that succeeds and an archive that
exports can still be rejected on upload:
Invalid bundle structure. The ".appex/WalletNonUIExtension" binary
file is not permitted. Your app cannot contain standalone executables
or libraries, other than a valid CFBundleExecutable of supported
bundles.
The .appex never claimed its own binary: its plist has no
CFBundleExecutable, so validation reads the executable inside it as a
loose program rather than the bundle's own. Every extension this builder
generates itself writes CFBundleExecutable, CFBundlePackageType,
CFBundleName, CFBundleInfoDictionaryVersion and CFBundleDevelopmentRegion
into its plist; a brought-in archive that leaves those to
GENERATE_INFOPLIST_FILE arrives without them, so the generic path now
fills the same set when they are missing and keeps whatever the extension
declares itself.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mirrors the cloud builder fix. CFBundleDevelopmentRegion was filled with a literal "en", so an extension whose development language is not English advertised the wrong fallback localization -- and it need not have, because the archive's buildSettings.properties are copied into this target, so $(DEVELOPMENT_LANGUAGE) lands on whatever that extension set. It is also how the builder writes this key for the extensions it generates itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mirrors the cloud builder fix. Every generic .ios.appext target was created with '10.0', hard-coded. Xcode writes that into the .appex as MinimumOSVersion, so an extension calling iOS 14 APIs shipped claiming iOS 10 and App Store validation rejected the upload -- "Please ensure the MinimumOSVersion value of your extension is 14 or later" -- after a build that succeeded. 10.0 is also below the floor the current SDK builds against at all. The target now takes, in order: what the archive's buildSettings.properties says; 14.0 when its entitlements ask for payment-pass-provisioning, since PKIssuerProvisioningExtensionHandler is an iOS 14 API; otherwise the app's own deployment target, never below 12.0. A bare major from the app's hint is normalised to major.minor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a62805064c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Mirrors the cloud builder fix. The declared deployment target won unconditionally, so an archive exported from an old project carrying IPHONEOS_DEPLOYMENT_TARGET = 10.0 reproduced the rejection the change exists to prevent. The floor is now a floor -- 14.0 for a payment-pass extension, 12.0 otherwise -- and the declared value wins above it. SWIFT_VERSION: the project's Swift settings are applied to the app target alone, so a brought-in extension holding .swift reached the compiler with none and died on "SWIFT_VERSION '' is unsupported", after its sources had been added to the target. Set from ios.swiftVersion (5.0 by default) with ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES when there is Swift to compile. TARGETED_DEVICE_FAMILY and SKIP_INSTALL: every generated extension sets both and the generic path set neither. And extraction now refuses an archive whose symlinks leave the extension folder: unzip creates links happily, and everything under that folder is handed to Xcode, copied into the bundle and swept into the sources tarball. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25ef6179c2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The two-argument appExtensionBundleId asks what identifier an extension declares with no archive in hand, and it passes a genuine null context. Selecting the winner now needs one to match conditions against, so the first qualified key reached conditionApplies and threw -- a crash where the answer was merely unknown. Caught by a test written for it, which failed with the NullPointerException before the fix. conditionApplies reads a null context the way it already reads a context whose fields are null: this build cannot tell, so every condition counts. The variant arm needed the same guard, and it is the same rule. Note this is NOT the earlier "null context" report, which was mistaken: the three-argument repair passes an unknown sdk, configuration and arch through ArchiveContext.of, which always returns an object. That comment and its test still stand; this one is a real null, from a different caller. 1171 tests in the module, all green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
💡 Codex Review Enumerate architecture-specific plist helper paths
When an unqualified path such as ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
[variant=normal] and [variant=profile] are equally specific, so carrying both into a single context reduced them by map order: an INFOPLIST_FILE = $(PLIST_PATH) written through variant-qualified helpers had only one variant's plist discovered and stamped, and the other kept a stale identity for a build Xcode really makes. The enumeration already varied SDK and configuration; it splits the variant list too now, the way the entitlements walk does, and the extra candidates carry a [variant=...] qualifier so each identity stays where it belongs. 1172 tests in the module, all green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8902110c48
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
EXECUTABLE_PREFIX and EXECUTABLE_SUFFIX are settings Xcode gives every target -- and both are blank for an extension's Mach-O executable. An identifier written through one therefore expands to "com.example.app." exactly as an undefined name does, and the allowlist waved it through: the same malformed identifier the guard exists to prevent, from the other direction. Membership is now "Xcode always supplies this, AND it is never empty". 1172 tests in the module, all green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efd8c328e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A key that names its own configuration is not enumerated across configurations -- it applies to one and says so -- but it was a reason to enumerate nothing at all, so an INFOPLIST_FILE[config=Release] written through variant-qualified helpers had one variant's plist chosen by map order while the other variant of that same Release archive kept an unstamped identity. Only the configuration dimension is fixed for such a key now; SDK and variant are still enumerated, and the qualifier only names the dimensions that actually differ. And the wildcard-variant branch kept the whole variant list because one of them matched. [variant=prof*] describes this build when profile is among its variants, but the entry belongs to profile alone -- leaving normal in the context let an EXTENSION_ID[variant=normal] answer for it by map order. 1173 tests in the module, all green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b202e2c2f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…s target BUILD_VARIANTS can itself be qualified, so the variants of a Debug build are not necessarily this archive's; capturing the active list once meant a configuration that selects another variant never had its plist discovered. Recomputed per configuration now. And architectures were fixed to the archive's, so an arm64 build never looked at an [arch=x86_64] helper and the Intel simulator's plist went undiscovered -- enumerated the same way SDKs are, over what the archive wrote down and nothing invented. Separately, $(inherited) was substituted with the ios.deployment_target HINT, which is absent unless the developer set one -- the default case -- while the generated project always has a target this builder computes from the app's own floors. An extension inheriting the project minimum was therefore written down to the extension floor. The project's target is passed for inheritance now, with the hint still answering when there is nothing better. appExtensionDeploymentFloor(List) took a null list as a crash where its File sibling has always taken it as "signed with nothing". 1175 tests in the module, all green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49a26764e3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
SRCROOT, PROJECT_DIR, BUILT_PRODUCTS_DIR and TARGET_BUILD_DIR expand to filesystem paths full of slashes, and ARCHS to a space-separated list. They pass the two tests the allowlist already applied -- Xcode always supplies them, and they are never empty -- and an identifier built from one is invalid anyway, so preserving the expression only shipped the invalid identifier instead of the valid derived one. Membership is now "Xcode always supplies this, it is never empty, and it can be part of an identifier". 1175 tests in the module, all green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb1a4c37c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Enumerated values are filtered through the KEY's own conditions now. A key that names a dimension applies to that value and no other, and enumerating the rest built candidates that contradict themselves -- [variant=profile][variant=normal] -- which nothing downstream rejects, so a plist the extension never uses would have been rewritten. The variant is named in a synthesized key whenever it is not the archive's own, rather than only when several are in play: a configuration whose BUILD_VARIANTS selects a single OTHER variant still needs saying, or the candidate's context inherits this archive's variant and misses the target identifier Xcode picks for it. And an identifier this build cannot finish resolving was kept on the strength of its references being Xcode's own. That is not enough: $(EXECUTABLE_NAME) expands to the extension's executable name, an identifier with no relation to the containing app, and the namespace refusal never sees it precisely because it did not resolve. The literal head is what can be established, so that is what is required. 1177 tests in the module, all green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8185fd2eb6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
An inactive [config=Deb*] leaves the context's configuration as the PATTERN, and enumerating that supplies no CONFIGURATION at all -- so Debug/Info.plist was never discovered, stamped or assigned its identifier, and a later Debug build used a target identifier its own plist disagrees with. The project's concrete configurations are filtered through the condition now, which answers the exact case identically and the wildcard case correctly, and is one rule instead of two. And architectures were read only from [arch=...] qualifiers, while ARCHS names them outright with no qualified key anywhere: an archive building "arm64 x86_64" through $(CURRENT_ARCH) has two plists and only one of them was ever seen. 1179 tests in the module, all green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ntext The repairs skipped any key whose condition does not describe this archive. That is right for a deployment target -- the floor came from the entitlements THIS archive is signed with -- and wrong for an identifier: every entry here is copied onto the generated target verbatim, so a [config=Debug] identifier from another project survived into the project and failed the Debug build later. An identifier outside the host's namespace cannot be embedded in this app in ANY configuration, so that repair runs on inactive keys and the floor repair still does not. A test pinned the opposite for identifiers; it encoded the weaker rule and is reversed, with the reasoning beside it. And ARCHS can be qualified, so the architectures of a Debug build are not necessarily this archive's: asking with the fixed context never enumerated them and that configuration's plist went undiscovered. 1179 tests in the module, all green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10f34f39f7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 243b12da36
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Candidates that resolved to the same physical file as the archive's own were dropped as duplicates. They are not: the stamper processes one file once per CONTEXT, because what is inside it can vary even when the path does not -- a Shared.plist holding $(MARKETING_VERSION) beside a MARKETING_VERSION[config=Debug] is right for one configuration and stale for the other. Candidate keys carry their qualifiers so nothing collides, and the stamper already dedups by file AND context. Two tests asserted the candidate COUNT for the ordinary case; they now assert what they were really about -- which files were discovered. And ARCHS was read as raw text with every $-token ignored, so ARCHS[config=Debug] = $(DEBUG_ARCHS) enumerated nothing for that configuration and its plist went unstamped. 1179 tests in the module, all green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
startsUnderHost expanded what it could and read the result, but resolveSettingsInValue DELETES what it cannot expand -- so a leading $(EXECUTABLE_NAME) vanished and "$(EXECUTABLE_NAME)com.example.app.Ext" read as being under the host. Xcode puts the executable name back and the identifier is not under the host at all. Only the head before the first thing still unexpanded is known, and that is what decides it now. And a condition does not have to sit on PRODUCT_BUNDLE_IDENTIFIER to decide the identifier. An unqualified $(EXTENSION_ID) over a host-prefixed base and an EXTENSION_ID[config=Debug] from the project the archive came from passes every check this build makes, and both settings are copied onto the target -- so the Debug build expands the same identifier to the foreign value and cannot be signed. The qualified HELPER is what makes it foreign, so that is what goes. 1182 tests in the module, all green. The ported test is what caught the second fix missing from this copy on the first attempt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64a3db90dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Dropping a helper that takes the identifier out of the namespace removed every QUALIFIED setting describing that build, not the ones the identifier is built from. A CODE_SIGN_ENTITLEMENTS[config=Debug] sitting beside the culprit was deleted with it, and that target then signed with the wrong entitlements. Only the identifier's reference closure is a candidate now. And a wildcard architecture qualifier was enumerated as the pattern itself, which is useless: extensionSettingsWithBuiltIns will not define CURRENT_ARCH for a family, so an [arch=x86*] path never resolved and that plist went unstamped. Architecture names are a closed set -- unlike an SDK's version, which is why that one is still refused -- so expanding a family invents nothing. 1185 tests in the module, all green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A family pattern does not say WHICH simulator SDK and does say exactly which platform, so PLATFORM_NAME is supplied for one where SDK_NAME still is not. That makes the counterpart platform worth enumerating: a path written as $(PLATFORM_NAME)/Info.plist needs no version, and the simulator's plist was invisible to a device archive unless the archive happened to declare an SDK qualifier of its own. A path that does need $(SDK_NAME) still does not resolve there. And dropping a helper that leaves the namespace took whichever foreign leaf the map handed over first. With EXTENSION_ID[config=Debug] = $(DEBUG_ID) over a foreign DEBUG_ID[config=Debug], deleting the leaf left the override resolving to nothing -- Xcode then builds that configuration with no identifier at all instead of falling back to the valid base. The chain is walked outermost first now. 1188 tests in the module, all green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The repair derived its chain from the unqualified identifier alone, so a literal base beside a PRODUCT_BUNDLE_IDENTIFIER[config=Debug] = $(EXTENSION_ID) never looked at the helper the qualified form uses -- and a Debug simulator build expanded to a foreign identifier it could not be signed with. Every form of the identifier contributes to the chain now, and each context is judged by the identifier that GOVERNS it. And when several qualified forms of a helper apply, the first one visited was removed: a broad [config=Debug] went while the narrower foreign [config=Debug][sdk=iphonesimulator*] stayed and still governed that build. The winning key is what goes, by the same specificity rule Xcode uses. 1190 tests in the module, all green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…from Xcode supplies "-iphoneos" or "-iphonesimulator" for it, it follows the platform -- so it is knowable for a family too -- and it is identifier-safe, which is why an extension distinguishing its platforms writes it straight into PRODUCT_BUNDLE_IDENTIFIER. Unmodelled, the identifier could not be resolved and did not look like one written through Xcode's own settings either, so the whole setting was deleted before the merge and the target silently reverted to its folder-derived name. Supplied where the other built-ins are, and on the allowlist for the same reason the others are. 1191 tests in the module, all green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Removing the foreign helper can strand what named it: a PRODUCT_BUNDLE_IDENTIFIER[config=Debug] = $(EXTENSION_ID) whose only EXTENSION_ID was the one just dropped stays in the map, and Xcode expands it to the empty string -- which OVERRIDES the valid base rather than falling back to it. That is the blank identifier the whole repair exists to avoid, reached by the repair itself. Each qualified consumer is dropped until the context resolves again, and what it resolves to is the base. 1191 tests in the module, all green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found by red-teaming this branch rather than by review. Two repairs decided "out of namespace" from resolveSettingsInValue, which DELETES what it cannot expand -- so they saw a truncation and could not tell three very different situations apart. One question with three answers now, in identifierAsBuilt: - It resolves completely. That is the answer. - What is left is Xcode's own to supply. This build cannot say what it comes to, so the literal head decides instead: com.other.$(EXECUTABLE_NAME) is unknowable at the end and plainly foreign at the front, and answering "cannot say" let it through. - What is left is a name nothing will define. The generated target carries only what buildSettings.properties wrote -- no .xcconfig comes across -- so Xcode expands it to nothing and the TRUNCATION is what ships. All three are pinned in one test, for both the identifier and the helper repair, since keeping them apart is the whole difficulty. 1192 tests in the module, all green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mirrors codenameone/BuildDaemon#191 in the open-source builder, which is what a
local iOS build uses.
An iOS archive with a
.ios.appextapp extension fails at link time:The extension's
Ldline carries-framework Foundationand nothing else, whilethe main app's compile line has
-fmodulesand the extension's does not. Nomodules means no clang autolinking, so
#import <UIKit/UIKit.h>never emits-framework UIKit, and the generated target's frameworks phase has only what thebuilder attaches by hand.
The settings that should have turned modules on are parsed out of a
pbxproj-shaped block, and the parser sliced the last character of the value
INSTEAD of dropping it:
So every
KEY = YES;line produced";", which Xcode reads as off.CLANG_ENABLE_OBJC_ARCwent the same way, so the extension has been building asMRC and leaking.
Dropping the semicolon exposes the second half, which never ran before. A quoted
value like
"gnu++14"is re-emitted inside a Ruby string literal in the projectfixup script, where the kept quotes are a syntax error. So unwrap those too.
Note this flips ARC on for
.ios.appexttargets that have silently been MRC.An extension written with explicit
retain/releasewill now fail to compile andneeds
CLANG_ENABLE_OBJC_ARC = NOin itsbuildSettings.properties(that file isread with
Properties.loadand was never affected by the bug). Swift extensionsare unaffected.
mvn -pl codenameone-maven-plugin compileon JDK 8 is green.🤖 Generated with Claude Code