-
Notifications
You must be signed in to change notification settings - Fork 5.1k
With build -subset help
output "macro" subset components
#64578
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
With build -subset help
output "macro" subset components
#64578
Conversation
Tagging subscribers to this area: @dotnet/area-infrastructure-libraries Issue DetailsSubsets like "clr", "mono", "host" are "macro" subset names that imply several other subset names. To try to explain how these relate to all the other names in e.g.,
|
Is this actually correct? Are there other "macros" that I missed? Does anyone else think this is helpful? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As part of this, we should also update any subsets that are not included in these subsets to have the OnDemand="true"
metadata.
eng/Subsets.props
Outdated
@@ -101,7 +101,7 @@ | |||
|
|||
<ItemGroup> | |||
<!-- CoreClr --> | |||
<SubsetName Include="Clr" Description="The CoreCLR runtime, LinuxDac, CoreLib (+ native), tools and packages." /> | |||
<SubsetName Include="Clr" Description="The full CoreCLR runtime: $(DefaultCoreClrSubsets)" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MichalStrehovsky @jkotas Does it make sense that build.cmd clr
builds nativeaotlibs but not nativeaotruntime?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clr includes clr.native and that's all native code under src/coreclr including clr.nativeaotruntime, clr.iltools, etc.
I just realized I put clr.spmi
into the DefaultCoreClrSubsets
but that one is also redundant with clr.native
. We could remove it; it's a NOP.
eng/Subsets.props
Outdated
@@ -101,7 +101,7 @@ | |||
|
|||
<ItemGroup> | |||
<!-- CoreClr --> | |||
<SubsetName Include="Clr" Description="The CoreCLR runtime, LinuxDac, CoreLib (+ native), tools and packages." /> | |||
<SubsetName Include="Clr" Description="The full CoreCLR runtime: $(DefaultCoreClrSubsets)" /> | |||
<SubsetName Include="Clr.NativePrereqs" Description="Managed tools that support building the native components of the runtime (such as DacTableGen)." /> | |||
<SubsetName Include="Clr.ILTools" Description="The CoreCLR IL tools." /> | |||
<SubsetName Include="Clr.Runtime" Description="The CoreCLR .NET runtime." /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BruceForstall do you feel it makes sense that the default coreclr set includes SPMI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably not required; we'd just need to make sure all the JIT automation that requires it does actually build it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we'd just need to make sure all the JIT automation that requires it does actually build it.
Although maybe #64035 already did everything that is necessary w.r.t. CI changes required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dotnet/jit-contrib There's a proposal here to remove "clr.spmi" (the build of the SuperPMI tools) from the default "clr" component build.
(I'm not sure how the "clr.native" component works. It looks like if you use "clr.native" then src\coreclr\runtime.proj calls build-runtime.cmd/sh with no "-component" arguments, so the __CMakeTarget
ends up being install
, which builds everything? So using "clr.native" will still build everything? But "clr" uses "clr.native", so not sure how we actually "remove" clr.spmi from the "clr" component.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'd have to make clr.native in the default be clr.runtime+clr.iltools basically. I was just thinking that most people rarely use SPMI. Not sure libs dev should learn the long incantation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think @hoyosjs's question is: Should clr.runtime
include the clr.spmi
subset? I think having clr
build all of the native code is still the correct thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One could make the same argument about whether the clr
should include the Linux DAC. I've never in my life used it and it takes longer to build than spmi. We also build the JIT 6 times because crossgen2/NativeAOT want to be able to crosstarget, but most people do not crosstarget in their day-to-day (crossgen2 devs included).
I think most devs who do daily work in the clr tree already have their own invocations of the build script and don't build all of clr
. The clr
subset is good for people new to the project that don't know what they want. So they just get everything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dotnet/runtime-infrastructure Do we see build clr
as the product build, or as the inner loop? yml can easily have the longer invocation as needed, but I feel the short one should really be catering to the common case scenario (runtime + corelib) + crossgening maybe even optional. Probably shouldn't be part of this PR, but that could enable an easier workflow for a lot of people
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the model where build clr
is the "easy; build everything (related to coreclr)" model, and as you get more sophisticated, you should be able to only build what you care about by using some longer incantation.
Remove `clr.spmi` from default list; it's already there due to `clr.native`.
I couldn't find any (in CLR) that were not correctly marked (by my understanding). |
All: I updated the help text, and added documentation text for clr.native and clr.runtime, which include other subsets due to how the build-runtime script (and runtime.proj) are implemented. This is ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM from the coreclr perspective. Also, OnDemand seems up to date for the src/coreclr components.
Tagging subscribers to this area: @dotnet/runtime-infrastructure Issue DetailsSubsets like "clr", "mono", "host" are "macro" subset names that imply several other subset names. To try to explain how these relate to all the other names in e.g.,
|
Subsets like "clr", "mono", "host" are "macro" subset names that imply several other subset names. To try to explain how these relate to all the other names in
build -subset help
, actually output what they "expand" into.e.g.,