Avoid building JITs for cross tools when unnecessary #116332
Merged
Conversation
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
Makes the builds that happen in the SPMI pipelines a bit faster. Maybe helps in some other cases too.
github-actions
bot
added
the
needs-area-label
There was a problem hiding this comment.
Pull Request Overview
The PR updates the cross-tool JIT build logic to only include JITs when the selected cross-component subsets explicitly require crossgen2 or ILC, improving overall build speed.
- Narrow JIT builds to subsets containing
clr.toolsorclr.nativecorelib - Updated comment to reflect the more precise build condition
Comments suppressed due to low confidence (1)
eng/Subsets.props:361
- [nitpick] Consider clarifying this comment by explicitly naming the subset keys (
clr.toolsandclr.nativecorelib) used in the Condition to make the rationale and matching criteria clearer.
<!-- crossgen2/ILC have dependencies on the JITs, so build them if the cross component includes crossgen2/ILC. -->
eng/Subsets.props
Outdated
Comment on lines
358
to
362
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <!-- When building cross components, always build the JITs. We will need them for running crossgen2 and ILC in the build. --> | ||
| <_CrossToolSubset Condition="'$(_BuildCrossComponents)' == 'true'" Include="ClrAllJitsSubset=true" /> | ||
| <!-- crossgen2/ILC have dependencies on the JITs, so build them if the cross component includes crossgen2/ILC. --> | ||
| <_CrossToolSubset Condition="'$(_BuildCrossComponents)' == 'true' and ($(_subset.Contains('+clr.tools+')) or $(_subset.Contains('+clr.nativecorelib+')))" Include="ClrAllJitsSubset=true" /> |
There was a problem hiding this comment.
[nitpick] To improve readability and reuse, consider extracting the combined MSBuild Condition into a named property (e.g., ShouldBuildCrossJits) and then referencing that property here.
Copilot uses AI. Check for mistakes.
jakobbotsch
added
area-Infrastructure-coreclr
and removed
needs-area-label
|
Tagging subscribers to this area: @hoyosjs |
|
Can you enable this to also build when you specify clr.crossarchtools? |
jkoritzinsky
approved these changes
Jun 5, 2025
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.
Makes the builds that happen in the SPMI pipelines a bit faster. Maybe helps in some other cases too.