fix(build): Delegate incremental state to xcodemake#485
Draft
cameroncooke wants to merge 6 commits into
Draft
Conversation
Update the pinned xcodemake revision so absolute DerivedData paths produce safe capture log filenames. Match the new logs by their argument hash before deciding whether to run make directly. Fixes #466
Always invoke the pinned wrapper so it owns Makefile argument and freshness validation before delegating to make. Refs #466
Execute the checksum-verified pinned wrapper with fake build tools and verify capture, reuse, argument invalidation, and project freshness behavior. Refs #466
commit: |
There was a problem hiding this comment.
Pull request overview
This PR updates XcodeBuildMCP’s incremental-build path to always invoke the pinned xcodemake wrapper (instead of locally deciding when to run make), and refreshes the pinned wrapper commit/checksum to address failures when -derivedDataPath is an absolute external path.
Changes:
- Updated the pinned
xcodemakecommit + SHA-256, and added a checksum-verified fixture plus a wrapper lifecycle regression test. - Simplified incremental build execution to always route through
xcodemake, delegating Makefile reuse / invalidation decisions to the wrapper. - Added unit coverage ensuring
executeXcodeBuildCommanddelegates toxcodemake(via mocking) for external absolute DerivedData paths, and recorded the fix in the changelog.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/xcodemake.ts | Updates the pinned xcodemake commit/checksum and keeps download + integrity verification flow. |
| src/utils/build-utils.ts | Removes local Makefile/log heuristics and always runs xcodemake for incremental builds. |
| src/utils/tests/xcodemake.test.ts | Drops coverage for removed local log-file detection helper. |
| src/utils/tests/xcodemake-wrapper.test.ts | Adds an execution-based regression test for the pinned wrapper behavior using hermetic fake build tools. |
| src/utils/tests/fixtures/xcodemake/xcodemake-7d20… | Adds the exact pinned wrapper script as a fixture for checksum-verified regression execution. |
| src/utils/tests/fixtures/xcodemake/README.md | Documents fixture pinning and checksum requirements. |
| src/utils/tests/build-utils-xcodemake.test.ts | Adds a focused unit test verifying build-utils always delegates incremental builds to xcodemake. |
| CHANGELOG.md | Adds an Unreleased entry for the incremental xcodemake absolute DerivedData fix (Issue #466). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Repin the bundled wrapper to the upstream lifecycle fixes and add regression coverage for long configuration arguments, direct fallback, and Makefile reuse.
Keep the wrapper lifecycle test independent of the process working directory.
Compare the observed Makefile timestamp with the explicit sentinel using a filesystem-resolution tolerance.
Comment on lines
+181
to
+185
| result = await executeXcodemakeCommand( | ||
| projectDir, | ||
| command.slice(1), | ||
| platformOptions.logPrefix, | ||
| ); |
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
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.
Incremental builds now use the repaired
xcodemakefork when DerivedData is an external absolute path, instead of failing while constructing a capture-log filename. The dependency remains pinned by commit and checksum.XcodeBuildMCP delegates Makefile reuse, argument changes, and project freshness decisions to
xcodemakeon every incremental call. This removes the unsafe local shortcut where a retained historical log could select a global Makefile generated for different arguments.The pin now includes the lifecycle fixes merged in
cameroncooke/xcodemake#3: Makefile reuse checks no longer regenerate on every invocation, and both-configand-configurationremain stable through capture and direct fallback.Regression coverage executes the exact checksum-verified pinned Perl wrapper with hermetic fake build tools and verifies initial capture, matching reuse without Makefile regeneration, changed-argument recapture, project-newer-than-log recapture, long-form configuration preservation, and direct fallback.
Fixes #466.