build: only regenerate Cocoa bindings when their inputs change - #5594
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5594 +/- ##
==========================================
- Coverage 74.85% 74.84% -0.01%
==========================================
Files 515 515
Lines 18963 18963
Branches 3694 3694
==========================================
- Hits 14194 14193 -1
- Misses 3891 3892 +1
Partials 878 878 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 94e1da9. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| <!-- No Carthage/Headers glob: Inputs doesn't expand wildcards, and an ItemGroup glob is empty on a fresh clone, which skips the target. | ||
| .built-from-sha is written after the headers and suffices for the checks. --> |
There was a problem hiding this comment.
| <!-- No Carthage/Headers glob: Inputs doesn't expand wildcards, and an ItemGroup glob is empty on a fresh clone, which skips the target. | |
| .built-from-sha is written after the headers and suffices for the checks. --> |
I think we can remove this comment. The Inputs are self explanatory and the comment is fairly cryptic (probably made sense to the LLM at the time it wrote it - possibly makes sense to us now, in the context of reviewing this PR - likely won't make sense to our future selves reading this code independent of this PR).
jamescrosswell
left a comment
jamescrosswell
left a comment
There was a problem hiding this comment.
Looks good - I'd just remove the cryptic comment.
The generated files were the target's Outputs, but Objective Sharpie writes ApiDefinition.cs before patch-cocoa-bindings.cs makes it valid C#. A parallel build sampling the timestamps in between skipped generation and compiled the unpatched output. Gate the check on a stamp touched once generation and patching are both done, so a late instance blocks on the in-flight TargetFramework=once request. The generated files stay in Outputs so a deleted one still forces regeneration. Latent until #5594 removed a non-existent glob input that had been forcing the target to run every time. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 participants
Summary
Discovered in #5583:
_GenerateSentryCocoaBindings listed Carthage/Headers/**/*.h in its Inputs.
MSBuild does not glob-expand the Inputs attribute, so that entry was treated as a literal filename, never found, and the target was out of date on every build:
Notes
Moving the glob into an ItemGroup instead would not work as item globs are evaluated before any target runs. On a fresh clone (where _BuildCocoaSDK has not yet staged the headers) the list is empty and MSBuild skips the
target ("Skipping target ... because it has no inputs"). That would also skip dirty-check.ps1, silently disabling the CI guard against stale committed bindings.
build-sentry-cocoa.sh deletes and re-creates the Carthage/.built-from-sha stamp last, so the stamp is never older than the
headers it describes.
Verification
Verified on macOS with a wiped Carthage, regenerates and reaches dirty-check; a repeat build skips; touching each of the three remaining inputs triggers exactly one regeneration; bindings stay byte-identical across regenerations.
#skip-changelog