Enable compiling with crossgen2 #31778
Conversation
ghost
added
the
area-infrastructure
…ng linux-musl from the build
davidwrighton
force-pushed
the
Crossgen2Support
branch
from
a119d9a to
04fbd5c
Compare
src/Framework/App.Runtime/src/Microsoft.AspNetCore.App.Runtime.csproj
Outdated
Show resolved
Hide resolved
| <Reference Include="Microsoft.NETCore.App.Crossgen2.$(BuildOsName)-$(BuildArchitecture)" | ||
| ExcludeAssets="All" | ||
| PrivateAssets="All" | ||
| GeneratePathProperty="true" /> |
There was a problem hiding this comment.
I suggest Microsoft.NETCore.App.Crossgen2.* packages should contain a same-named .props file that sets a property for the tool path. This didn't make sense for Microsoft.NETCore.App.Runtime.* packages but definitely does now. Wouldn't need the path property nor some of the later complications finding the tool if package were more self-describing.
There was a problem hiding this comment.
Any possibility of removing the need for workarounds (hacks) like https://github.com/dotnet/aspnetcore/pull/31778/files#diff-eb8fd31a7971d712606a187ea074d0bd5d6ea85aae5a707107fd998216986ea5R117, https://github.com/dotnet/aspnetcore/pull/31778/files#diff-eb8fd31a7971d712606a187ea074d0bd5d6ea85aae5a707107fd998216986ea5R365-R368, and related bits in the future❔
|
@dougbu Can you take a look? I also attempted to add some tests of the generated product by removing the disabling of crossgen2 in a spot in the official build, but I don't have confidence this actually did anything useful. My plan is to remove that change to the yaml before checkin. |
| <Reference Include="Microsoft.NETCore.App.Crossgen2.$(BuildOsName)-$(BuildArchitecture)" | ||
| ExcludeAssets="All" | ||
| PrivateAssets="All" | ||
| GeneratePathProperty="true" /> |
There was a problem hiding this comment.
Any possibility of removing the need for workarounds (hacks) like https://github.com/dotnet/aspnetcore/pull/31778/files#diff-eb8fd31a7971d712606a187ea074d0bd5d6ea85aae5a707107fd998216986ea5R117, https://github.com/dotnet/aspnetcore/pull/31778/files#diff-eb8fd31a7971d712606a187ea074d0bd5d6ea85aae5a707107fd998216986ea5R365-R368, and related bits in the future❔
| @@ -65,7 +65,7 @@ jobs: | |||
| - script: ./eng/build.cmd -ci -nobl -noBuildRepoTasks -noRestore -test -all -noBuildJava -noBuildNative | |||
| -projects eng\helix\helix.proj /p:RunQuarantinedTests=true /p:IsRequiredCheck=true /p:IsHelixJob=true | |||
| /p:BuildInteropProjects=true /p:RunTemplateTests=true | |||
| /p:CrossgenOutput=false /p:ASPNETCORE_TEST_LOG_DIR=artifacts/log | |||
| /p:ASPNETCORE_TEST_LOG_DIR=artifacts/log | |||
There was a problem hiding this comment.
This isn't a particularly interesting change given we expect failures in the quarantined runs. Suggest instead removing /p:CrossgenOutput=false from the 'Tests: Helix x64' job's build steps in ci.yml. If the crossgen2'd assemblies are platform-specific with the command arguments you're currently using, I'd expect to see a lot of failures on Linux and macOS work items (ProjectTemplates.Test in particular) submitted from that job without the property setting.
Reverting that change makes sense but I'd like to see test failures showing we're producing the best images for each platform i.e. that we hit problems when Note we only use the shared Fx assemblies for a limited number of tests. Search for |
|
Doug is taking over this with #32508, so I'm closing this PR. |
|
Hi @davidwrighton. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context. |
Enable using crossgen2 instead of crossgen in build. Building block for enabling pgo for this repo.
Addresses #31040 (in this specific format)