Crossgen Task by fadimounir · Pull Request #2997 · dotnet/sdk

fadimounir · 2019-03-01T00:15:02Z

Intitial implementation of the crossgen task, and plumbing in the SDK targets to invoke the task. The crossgen task invokes crossgen from the runtime pack.

Task triggered when is set to true in the main app's project file (opt-in feature)
Added new condition to download runtime packs: when the ReadyToRun property is set.
Adding R2R exclusion list capability
Symbol emission is optional (opt-in, when ReadyToRunEmitSymbols is set to true)

fadimounir · 2019-03-01T00:16:16Z

@nguerrera Here's the crossgen task. It's still WIP since I haven't tested on OSX yet (tested on Linux and Windows).

src/Tasks/Common/Resources/xlf/Strings.de.xlf

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.TargetingPackResolution.targets

src/Tasks/Microsoft.NET.Build.Tasks/RunCrossgen.cs

fadimounir · 2019-03-06T20:34:27Z

Tested on OSX

nguerrera

I am not finished reviewing everything, but didn't want to hold back these comments.

src/Tasks/Microsoft.NET.Build.Tasks/ResolveFrameworkReferences.cs

src/Tasks/Microsoft.NET.Build.Tasks/RunCrossgen.cs

src/Tasks/Microsoft.NET.Build.Tasks/ResolveFrameworkReferences.cs

src/Tasks/Microsoft.NET.Build.Tasks/RunCrossgen.cs

fadimounir · 2019-03-11T19:11:26Z

@nguerrera Doesn't look like I have access to merge this. Could you please take another look at the new changes I posted, and merge if it looks good?

src/Tasks/Common/TaskToolBase.cs

src/Tasks/Microsoft.NET.Build.Tasks/CompileReadyToRunImages.cs

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets

nguerrera

Sorry for the delay in doing a full review. I have some more concerns noted above.

Also, tests need to be added.

fadimounir · 2019-03-18T21:18:48Z

@nguerrera I changed the platform validation logic a bit to consume the runtime graph as we discussed, to validate target/host platform compatibility. It now does so by comparing the best matching RID of the target to the best matching RID of the host. Tested it on Ubuntu, OSX, Alpine, and RHEL.6.

I need some pointers on how to add a test to the CI. I can add it as a separate PR.

src/Tasks/Microsoft.NET.Build.Tasks/CompileReadyToRunImages.cs

nguerrera · 2019-03-19T21:16:28Z

For the tests, look a GuvenThatWeWantToCrossPublish for simple example.

You probably want to mix this with PackageReferences and ProjectReferences to get more than one file to crossgen and to test more cases. GivenThatWeWantDesignerSupport shows this.

You can enable ReadyToRun via TestProject.AdditionalProperties.

It would be nice to have negative coverage for the mismatched host, and coverage of all the options (PDBs, exclude list).

nguerrera · 2019-03-19T21:21:15Z

I need some pointers on how to add a test to the CI. I can add it as a separate PR.

We always include tests in the original PR. I've put some pointers above. Feel free to grab some time with me if you want to pair on this or have more questions about it.

src/Tests/Microsoft.NET.TestFramework/ProjectConstruction/TestProject.cs

src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToRunCrossgen.cs

src/Tasks/Microsoft.NET.Build.Tasks/RuntimeGraphCache.cs

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets

… targets to invoke the task. Added new condition to download runtime packs: when the ReadyToRun property is set. Adding R2R exclusion list capability

…ring to have a ToolTaskBase) Fixing logic with RuntimeIdentifier values used by the task (now it's read from the RuntimePack)

…ferenceAssemblyAttribute

… validation to use the runtime graph data

…that calls ExecuteTool multiple times. The split also enables msbuild to automatically skip entries that are already up to date. Removed ToolTaskBase since it's not really adding much value

….1 (#2997) - Microsoft.DotNet.Cli.Runtime - 5.0.100-alpha1.19480.1

fadimounir commented Mar 1, 2019

View reviewed changes

src/Tasks/Common/Resources/xlf/Strings.de.xlf Outdated Show resolved Hide resolved

nguerrera reviewed Mar 1, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.TargetingPackResolution.targets Show resolved Hide resolved

fadimounir commented Mar 1, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/RunCrossgen.cs Outdated Show resolved Hide resolved

nguerrera added this to the 3.0.1xx milestone Mar 1, 2019

nguerrera reviewed Mar 7, 2019

View reviewed changes

nguerrera reviewed Mar 14, 2019

View reviewed changes

src/Tasks/Common/TaskToolBase.cs Outdated Show resolved Hide resolved

nguerrera reviewed Mar 14, 2019

View reviewed changes

src/Tasks/Common/TaskToolBase.cs Outdated Show resolved Hide resolved

nguerrera reviewed Mar 14, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/CompileReadyToRunImages.cs Outdated Show resolved Hide resolved

nguerrera reviewed Mar 14, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/CompileReadyToRunImages.cs Outdated Show resolved Hide resolved

nguerrera reviewed Mar 14, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/CompileReadyToRunImages.cs Outdated Show resolved Hide resolved

nguerrera reviewed Mar 14, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/CompileReadyToRunImages.cs Outdated Show resolved Hide resolved

nguerrera reviewed Mar 14, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/CompileReadyToRunImages.cs Outdated Show resolved Hide resolved

nguerrera reviewed Mar 14, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/CompileReadyToRunImages.cs Outdated Show resolved Hide resolved

nguerrera reviewed Mar 14, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/CompileReadyToRunImages.cs Outdated Show resolved Hide resolved

nguerrera reviewed Mar 14, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets Show resolved Hide resolved

nguerrera suggested changes Mar 14, 2019

View reviewed changes

fadimounir changed the base branch from release/3.0.1xx to master March 18, 2019 21:54

fadimounir force-pushed the crossgentask branch 3 times, most recently from 75b4b85 to a7933fa Compare March 18, 2019 22:19

nguerrera reviewed Mar 19, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/CompileReadyToRunImages.cs Outdated Show resolved Hide resolved

nguerrera reviewed Mar 19, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/CompileReadyToRunImages.cs Outdated Show resolved Hide resolved

nguerrera reviewed Mar 19, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/CompileReadyToRunImages.cs Outdated Show resolved Hide resolved

nguerrera reviewed Mar 19, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/CompileReadyToRunImages.cs Outdated Show resolved Hide resolved

nguerrera reviewed Mar 19, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/CompileReadyToRunImages.cs Outdated Show resolved Hide resolved

nguerrera reviewed Mar 19, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/CompileReadyToRunImages.cs Outdated Show resolved Hide resolved

nguerrera reviewed Mar 19, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/CompileReadyToRunImages.cs Show resolved Hide resolved

nguerrera reviewed Mar 19, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/CompileReadyToRunImages.cs Outdated Show resolved Hide resolved

nguerrera reviewed Mar 19, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/CompileReadyToRunImages.cs Outdated Show resolved Hide resolved

nguerrera reviewed Mar 19, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/CompileReadyToRunImages.cs Outdated Show resolved Hide resolved

nguerrera reviewed Mar 19, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/CompileReadyToRunImages.cs Outdated Show resolved Hide resolved

nguerrera reviewed Mar 26, 2019

View reviewed changes

src/Tests/Microsoft.NET.TestFramework/ProjectConstruction/TestProject.cs Show resolved Hide resolved

nguerrera reviewed Mar 26, 2019

View reviewed changes

src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToRunCrossgen.cs Outdated Show resolved Hide resolved

nguerrera reviewed Mar 26, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/RuntimeGraphCache.cs Outdated Show resolved Hide resolved

nguerrera reviewed Mar 26, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets Outdated Show resolved Hide resolved

fadimounir force-pushed the crossgentask branch from c30e095 to 5a5dad2 Compare March 27, 2019 00:17

fadimounir added 8 commits March 29, 2019 11:43

Intitial implementation of the crossgen task, and plumbing in the SDK…

fa45b5d

… targets to invoke the task. Added new condition to download runtime packs: when the ReadyToRun property is set. Adding R2R exclusion list capability

Making the crossgen task a ToolTask based task (and necessary refacto…

37f4e7b

…ring to have a ToolTaskBase) Fixing logic with RuntimeIdentifier values used by the task (now it's read from the RuntimePack)

Exclude ref assemblies from compilation based on the presence of a Re…

571aea0

…ferenceAssemblyAttribute

Rewriting some of the logic around host/target platform compatibility…

6773094

… validation to use the runtime graph data

Split R2R compilation task into two tasks, to not have a single task …

78e784e

…that calls ExecuteTool multiple times. The split also enables msbuild to automatically skip entries that are already up to date. Removed ToolTaskBase since it's not really adding much value

Using NETCoreSdkRuntimeIdentifier to read host machine's RID

baecf84

Adding crossgen CI tests

1d07056

CR feedback

4c64d23

fadimounir force-pushed the crossgentask branch from 5a5dad2 to 5e43db7 Compare March 29, 2019 19:21

Resolve merge conflicts

ec53a2a

fadimounir force-pushed the crossgentask branch from 5e43db7 to ec53a2a Compare March 29, 2019 20:51

nguerrera approved these changes Mar 29, 2019

View reviewed changes

nguerrera merged commit 7d6b26d into dotnet:master Mar 29, 2019

jkotas mentioned this pull request Mar 30, 2019

[Question] CoreRT future plans dotnet/corert#7200

Open

fadimounir deleted the crossgentask branch April 4, 2019 18:16

dsplaisted mentioned this pull request Apr 6, 2019

Rewrite deps file generation #3086

Merged

wli3 pushed a commit that referenced this pull request Feb 7, 2020

Update dependencies from https://github.com/dotnet/cli build 20190930…

ef7617b

….1 (#2997) - Microsoft.DotNet.Cli.Runtime - 5.0.100-alpha1.19480.1

Conversation

fadimounir commented Mar 1, 2019

Uh oh!

fadimounir commented Mar 1, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fadimounir commented Mar 6, 2019

Uh oh!

nguerrera left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fadimounir commented Mar 11, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nguerrera left a comment

Choose a reason for hiding this comment

Uh oh!

fadimounir commented Mar 18, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nguerrera commented Mar 19, 2019

Uh oh!

nguerrera commented Mar 19, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants