-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Local Live live builds #55
Conversation
<Project> | ||
|
||
<PropertyGroup> | ||
<HostArch>$([System.Runtime.InteropServices.RuntimeInformation]::ProcessArchitecture.ToString().ToLowerInvariant)</HostArch> |
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.
Do these static properties have to live in a targets file?
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.
The static properties don't have to. I just decided to keep them localized to where they're used. The path properties should be here since they depend on the configuration properties, which can be overridden in a subtree (libraries overrides them since there ConfigurationGroup
represents what Configuration
represents everywhere else).
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 would prefer if they were in a target so they're truly localized. But without seeing an MSBuild style guide anywhere ever, I'm not going to claim consistency here or anything like that. No objection to merging.
The way I do this in Core-Setup is with MSBuild tasks, nothing special. This is the same mechanism project references use. (When running an MSBuild task against the same project with the same properties from multiple projects, MSBuild only runs it once with safe parallelism and all that. This lets you build up the build graph with as fine-grained parallelism as you can stomach.) To make one chunk of the build depend on another, I used |
Unfortunate timing... my team's boxing up stuff to be moved to another part of the building over the weekend so I won't have access to my main hardware. I'll try to make some progress on these two if I can manage with laptops:
("Refactor out changes to GetCrossgenToolPaths" doesn't sound necessary for the initial dotnet/runtime opening, I can help with that in the future.) |
…dependencies are in the building subset/subset category.
One more TODO: Update documentation (e.g. https://github.com/dotnet/runtime/blob/master/docs/coreclr/building/testing-with-corefx.md) |
…ding a new BinplaceConfiguration. Update test_dependencies in the CoreCLR tree to use that to restore OOB libraries.
…he reference is only needed at runtime and Core_Root already has System.Drawing.Common).
…live world, we'll run this test configuration by pointing the CoreFX test host build step at the artifacts from a checked CoreCLR build (possible in both local and CI builds).
Update though today:
|
This PR now has support for live builds of coreclr and libraries being consumed in both product and test builds for everything except the dependency of the legacy Any additional work past these pieces that is not documentation (ie consuming the live hosting layer in the CoreFX tests) will wait until a future PR since that is not on the critical path. |
…ckaged Microsoft.NETCore.Platforms package.
…ntime into live-live-builds
<PropertyGroup> | ||
<CoreCLRArtifactsPath>$([MSBuild]::NormalizeDirectory('$(CoreCLRArtifactsPath)'))</CoreCLRArtifactsPath> | ||
</PropertyGroup> | ||
<PropertyGroup> |
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.
nit: can we merge these two property groups instead?
@@ -221,9 +234,18 @@ | |||
<ProjectAssetsFile></ProjectAssetsFile> | |||
</PropertyGroup> | |||
|
|||
<PropertyGroup Condition="'$(ReferenceSystemPrivateCoreLib)' == 'true' and '$(UsingMicrosoftNETSdk)' == 'true'"> | |||
<PropertyGroup Condition="'$(UsingMicrosoftNETSdk)' == 'true'"> |
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.
do we actually need this remaining condition? Usually we just disable the property entirely without checking if we use an SDK which honors 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.
I think I can remove the remaining part of the condition
<!-- enable trimming for any runtime project that's part of the shared framework and hasn't already set ILLinkTrimAssembly --> | ||
<SetProperties Condition="'$(BinPlaceRuntime)' == 'true' and '$(ILLinkTrimAssembly)' == ''">ILLinkTrimAssembly=true</SetProperties> | ||
</BinPlaceConfiguration> | ||
|
||
<!-- Binplace the OOB libraries built in the netcoreapp build. --> |
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.
Why don't you instead just use the artifacts/bin/ref/netcoreapp and artifacts/bin/runtime/netcoreapp folders instead of binplacing new ones?
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.
Yeah I think to build coreclr tests we can use ref/netcoreapp and runtime/netcoreapp. That way we only binplace the inbox assemblies which are needed by the installer to produce the shared framework package.
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.
From what I'm seeing, the artifacts/bin/runtime/netcoreapp
folder has more than just the implementation assemblies. It looks like it has the restored runtime from the CoreCLR build as well.
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 can change that if necessary. We don't use those artifacts there.
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.
Yeah. We just need to think about the ReferenceFromRuntime
scenario as we special case it for S.P.CoreLib
to take the assets from runtime.depproj
output, however for all others we get them from runtime/netcoreapp
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.
That's a good point. I think we should treat CoreLib as part of libraries.
Very impressive! Good work. |
Superseded by #494 |
) Do not generate the hml per default. This is a feature for xamarin-macios and has to be explicitly enabled. fixes: dotnet#55 Co-authored-by: Premek Vysoky <premek.vysoky@microsoft.com>
This PR implements local live-live builds. The
eng/liveBuilds.targets
MSBuild file is the central location for live dependency file discovery. There are various MSBuild property hooks in there to enable overriding the location of the various consumed live components.Work completed in this PR:
This PR depends on:
Future work for another PR once we are off the critical path:
src/installer/pkg/projects/netcoreapp/src/localnetcoreapp.override.targets
so we can send back a patch to Arcade and removelocalnetcoreapp.override.targets
.eng/liveBuilds.targets
to have a target for consuming a live build of the hosting layer.