Fix _AfterILLinkAdditionalSteps breaking IlcCompile incrementalism#11509
Open
sbomer wants to merge 3 commits into
Open
Fix _AfterILLinkAdditionalSteps breaking IlcCompile incrementalism#11509sbomer wants to merge 3 commits into
sbomer wants to merge 3 commits into
Conversation
Split _AfterILLinkAdditionalSteps into _RunAfterILLinkAdditionalSteps (incremental, writes to afterlink/) and _AfterILLinkAdditionalSteps (always runs, updates itemgroups to afterlink/ paths). Previously, AssemblyModifierPipeline modified assemblies in-place in linked/, which updated file timestamps even for unchanged files (Cecil opens with ReadWrite, and FileStream.Dispose flushes the mtime). Since this target runs in the outer project after IlcCompile completes in the inner project, the linked/ timestamps ended up newer than native/*.o, causing IlcCompile to re-run on every subsequent build. Writing to a separate afterlink/ directory avoids touching linked/ and preserves IlcCompile's incremental state. Incremental no-op publish drops from ~17s to ~2.5s. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Assisted-by: Claude:claude-opus-4.6-1m
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a publish incrementalism regression where _AfterILLinkAdditionalSteps (via AssemblyModifierPipeline) was mutating linked assemblies in-place, updating mtimes and forcing IlcCompile to re-run on subsequent no-op builds. The proposed fix redirects post-ILLink modifications into a separate output directory and adds an always-run wrapper target to retarget downstream itemgroups to those outputs.
Changes:
- Introduces an
afterlink/intermediate output directory for post-ILLink modified assemblies. - Splits the work into an incremental
_RunAfterILLinkAdditionalStepstarget plus an always-run_AfterILLinkAdditionalStepswrapper that rewritesResolved*Assembliesitemgroups. - Adds
FileWritestracking for the new output directory.
Show a summary per file
| File | Description |
|---|---|
| src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets | Splits _AfterILLinkAdditionalSteps and redirects outputs to an afterlink/ directory, then rewrites assembly itemgroups to point to those outputs. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 2
Verify that _RunAfterILLinkAdditionalSteps is skipped on second build, the afterlink/ output directory contains assemblies, and the outer _AfterILLinkAdditionalSteps target always runs to update itemgroups. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Assisted-by: Claude:claude-opus-4.6-1m
The original PR flattened all ResolvedAssemblies into afterlink/ using %(Filename)%(Extension), which would cause assemblies from different ABIs to overwrite each other. Use %(DestinationSubPath) instead, matching the established pattern in _LinkAssembliesNoShrink. This produces per-ABI subdirectories (e.g. afterlink/arm64-v8a/Foo.dll). Update test to verify per-ABI subdirectories under afterlink/. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Assisted-by: Claude:claude-opus-4.6-1m
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.
This content was created with assistance from AI.
Summary
_AfterILLinkAdditionalStepsrunsAssemblyModifierPipelinewhich modifies assemblies in-place inlinked/. Even when files are unchanged ("Skipped unchanged file"), Cecil opens them withReadWriteandFileStream.Dispose()flushes the mtime. Since this target runs in the outer project after IlcCompile completes in the inner project, thelinked/timestamps end up newer thannative/*.o, causing IlcCompile to re-run on every subsequent build.Fix
Split
_AfterILLinkAdditionalStepsinto:_RunAfterILLinkAdditionalSteps— incremental target that writes toafterlink/instead of in-place_AfterILLinkAdditionalSteps— always-run wrapper that updatesResolvedAssemblies,ResolvedUserAssemblies, andResolvedFrameworkAssembliesitemgroups to point toafterlink/pathsThis follows the established pattern used by ILLink and ILC: write to a separate output location, then update itemgroups (which must happen even when the incremental target is skipped).
Results