Packaging for crossgen2 as a framework dependent application. #27296

fadimounir · 2019-10-18T20:28:45Z

This requires us to manually emit a runtimeconfig json for now, since the build produces a self-contained version of crossgen2.

Also includes some minor cleanups to the way we build crossgen2:

Removed from build.proj since we explicitly build it as a self-contained application from build.cmd/build.sh
Removed the output path property from the csproj files. We use an explicit -o argument when publishing to specify the output folder

The crossgen2 tools folder will include the following files:

clrjitilc.dll
crossgen2.exe
crossgen2.dll
crossgen2.runtimeconfig.json
ILCompiler.DependencyAnalysisFramework.dll
ILCompiler.ReadyToRun.dll
ILCompiler.TypeSystem.ReadyToRun.dll
jitinterface.dll
Microsoft.DiaSymReader.dll
System.CommandLine.dll

This requires us to manually emit a runtimeconfig json for now, since the build produces a self-contained version of crossgen2. Also includes some minor cleanups to the way we build crossgen2: - Removed from build.proj since we explicitly build it as a self-contained application from build.cmd/build.sh - Removed the output path property from the csproj files. We use an explicit -o argument when publishing to specify the output folder

fadimounir · 2019-10-18T20:33:17Z

src/.nuget/Microsoft.NETCore.Runtime.CoreCLR/Microsoft.NETCore.Runtime.CoreCLR.pkgproj

+        two simple scenarios we want to support in the short term: win_x64 to win_x64 and linux_x64 to linux_x64 compilations.
+        Once we have more complex targets, especially cross-platform and cross-architecture, crossgen2 should move to its own
+        package, and become a self-contained package. -->
+    <Crossgen2RuntimeConfigFile>$(ArtifactsObjDir)crossgen2\crossgen2.runtimeconfig.json</Crossgen2RuntimeConfigFile>


This will go away in dotnet 6.0 when crossgen2 moves to its own package

src/.nuget/Microsoft.NETCore.Runtime.CoreCLR/Microsoft.NETCore.Runtime.CoreCLR.pkgproj

src/tools/crossgen2/crossgen2/crossgen2.csproj

src/build.proj

fadimounir · 2019-10-21T17:16:22Z

BTW, is crossgen2 the name we want to keep long term?

jkotas · 2019-10-21T17:20:17Z

is crossgen2 the name we want to keep long term?

I do not think so. It is ugly name.

MichalStrehovsky · 2019-10-21T17:27:45Z

is crossgen2 the name we want to keep long term?

Last time this was discussed ("CPAOT directory structure on the CoreCLR side" internal email thread), the plan was to finalize the name before preview. Simon also threw an idea we could just call it crossgen (sans 2) once the native crossgen is retired.

Like they say - there's only two hard problems in computer science...

Renaming crossgen2 to just crossgen (still keeping crossgen2 for the publish folder name)

Fix minor path issue

fadimounir · 2019-10-22T23:29:50Z

I renamed the crossgen2 to just crossgen. I kept the publish folder name unchanged for now.
cc @MichalStrehovsky @nattress

fadimounir · 2019-10-22T23:32:36Z

@dagood Could you please review this?
The goal is to publish crossgen2 into its own package. There's most likely a piece of work that'll need to be done in dotnet/core-setup too, which i'll do once this is merged.
The goal is to have the Microsoft.NETCore.<RID> packages that the SDK downloads and uses to compile apps into R2R.
For dotnet5.0, this will only be a scenario for linux/windows x64, with no cross-architecture compilations.

Also, is there anything else I need to do to have this new package published to the feed?

MichalStrehovsky · 2019-10-23T08:32:36Z

I renamed the crossgen2 to just crossgen. I kept the publish folder name unchanged for now.

Did something change since we last discussed this? In the "CPAOT directory structure on the CoreCLR side" internal email thread, Sergiy said "We should keep ‘2’ in the exe name for the move in order to avoid confusion at least in the nearest future. We can finalize the name before preview.".

src/tools/crossgen2/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj

src/.nuget/Microsoft.NETCore.Crossgen/Microsoft.NETCore.Crossgen.pkgproj

dagood

I don't have experience with the recent stuff like Microsoft.Build.Traversal, but some comments.

src/.nuget/Microsoft.NETCore.Crossgen/Microsoft.NETCore.Crossgen.pkgproj

src/.nuget/Microsoft.NETCore.Crossgen/runtime.Linux.Microsoft.NETCore.Crossgen.props

fadimounir

Did something change since we last discussed this? In the "CPAOT directory structure on the CoreCLR side" internal email thread, Sergiy said "We should keep ‘2’ in the exe name for the move in order to avoid confusion at least in the nearest future. We can finalize the name before preview.".

The problem is that any renaming down the line now will cause some headache because it will require:

Fixing packaging in this repo
Fixing packaging in the dotnet/core-setup repo (although this might go away)
Fixing the dotnet/sdk repo to reflect name changes.

After a chat with @davidwrighton yesterday, we realized that it would make more sense to just assume crossgen2 will be renamed to crossgen in the future when the current crossgen retires, and doing the rename right now aligns us with the most likely long term plan. Plus, we wouldn't have to fix the SDK again in the future when we rename.
To address the confusion issue, i kept the output folder name unchanged, and just renamed the binary itself.

Fixes to the Linux arm/arm64 builds. Fix CI test failure.

fadimounir · 2019-10-23T21:59:42Z

cc @sergiy-k

dagood

Packaging infra LGTM.

fadimounir · 2019-10-24T19:36:51Z

To address the confusion issue, i kept the output folder name unchanged, and just renamed the binary itself.

I had an offline discussion with @MichalStrehovsky regarding the renaming and he pointed out that the SuperILC tool needs to be fixed too. It started to look like the value of a rename at this point was less than the confusion that could result.
We agreed that it would be better to do all the renames in a separate PR, so for the sake of reducing confusion and making it easy to findstr all "crossgen2" instances later when doing the work, i'll keep "crossgen2" as the name for now and undo some of my changes.

MichalStrehovsky · 2019-10-24T19:47:58Z

src/tools/crossgen2/ILCompiler.TypeSystem.ReadyToRun/ILCompiler.TypeSystem.ReadyToRun.csproj

    <AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>

    <!-- We're binplacing these into an existing publish layout so that F5 build in VS updates
         the same bits tests expect to see in bin/crossgen2. That way we never need to wonder which
         binaries are up to date and which are stale. -->
-    <OutputPath>$(BinDir)/crossgen2</OutputPath>


Can we leave the conditioned OutputPath in the other projects (like in crossgen2.csproj)?

There is no need for those. Building the main crossgen2 project takes care of putting these dependent files in the correct place (one of the scenarios I tested from VS)

fadimounir requested review from jkotas and MichalStrehovsky October 18, 2019 20:28

fadimounir commented Oct 18, 2019

View reviewed changes

src/.nuget/Microsoft.NETCore.Runtime.CoreCLR/Microsoft.NETCore.Runtime.CoreCLR.pkgproj Outdated Show resolved Hide resolved

jkotas reviewed Oct 18, 2019

View reviewed changes

src/tools/crossgen2/crossgen2/crossgen2.csproj Show resolved Hide resolved

MichalStrehovsky reviewed Oct 19, 2019

View reviewed changes

src/build.proj Show resolved Hide resolved

Fadi Hanna added 2 commits October 22, 2019 15:59

Move crossgen2 to a separate package.

8fcccb7

Renaming crossgen2 to just crossgen (still keeping crossgen2 for the publish folder name)

Check crossgen2 build errorcode for success.

9293cf0

Fix minor path issue

fadimounir requested a review from dagood October 22, 2019 23:33

Make PlatformTarget=AnyCPU to fix linux arm/arm64 publishing errors

79c414f

MichalStrehovsky reviewed Oct 23, 2019

View reviewed changes

src/tools/crossgen2/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj Outdated Show resolved Hide resolved

MichalStrehovsky reviewed Oct 23, 2019

View reviewed changes

src/.nuget/Microsoft.NETCore.Crossgen/Microsoft.NETCore.Crossgen.pkgproj Outdated Show resolved Hide resolved

dagood reviewed Oct 23, 2019

View reviewed changes

fadimounir commented Oct 23, 2019

View reviewed changes

Code review feedback fixes.

e7468fa

Fixes to the Linux arm/arm64 builds. Fix CI test failure.

Fadi Hanna added 3 commits October 23, 2019 15:45

Set outputpath when building from VS to make F5 scenario work again

31b572d

Fix version to 3.0.100

35cc07c

Change version to 5.0.0-alpha1.0 and remove rollForward=major

faa84b4

dagood approved these changes Oct 24, 2019

View reviewed changes

Undo renames of crossgen2

5fe72ee

MichalStrehovsky approved these changes Oct 24, 2019

View reviewed changes

fadimounir merged commit 02169de into dotnet:master Oct 25, 2019

dagood mentioned this pull request Dec 16, 2019

Create "crossgen pack" for new crossgen2 compiler dotnet/runtime#906

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Packaging for crossgen2 as a framework dependent application. #27296

Packaging for crossgen2 as a framework dependent application. #27296

fadimounir commented Oct 18, 2019 •

edited

Loading

fadimounir Oct 18, 2019

fadimounir commented Oct 21, 2019

jkotas commented Oct 21, 2019

MichalStrehovsky commented Oct 21, 2019

fadimounir commented Oct 22, 2019 •

edited

Loading

fadimounir commented Oct 22, 2019 •

edited

Loading

MichalStrehovsky commented Oct 23, 2019

dagood left a comment •

edited

Loading

fadimounir left a comment

fadimounir commented Oct 23, 2019

dagood left a comment

fadimounir commented Oct 24, 2019

MichalStrehovsky Oct 24, 2019

fadimounir Oct 24, 2019 •

edited

Loading

Packaging for crossgen2 as a framework dependent application. #27296

Packaging for crossgen2 as a framework dependent application. #27296

Conversation

fadimounir commented Oct 18, 2019 • edited Loading

fadimounir Oct 18, 2019

Choose a reason for hiding this comment

fadimounir commented Oct 21, 2019

jkotas commented Oct 21, 2019

MichalStrehovsky commented Oct 21, 2019

fadimounir commented Oct 22, 2019 • edited Loading

fadimounir commented Oct 22, 2019 • edited Loading

MichalStrehovsky commented Oct 23, 2019

dagood left a comment • edited Loading

Choose a reason for hiding this comment

fadimounir left a comment

Choose a reason for hiding this comment

fadimounir commented Oct 23, 2019

dagood left a comment

Choose a reason for hiding this comment

fadimounir commented Oct 24, 2019

MichalStrehovsky Oct 24, 2019

Choose a reason for hiding this comment

fadimounir Oct 24, 2019 • edited Loading

Choose a reason for hiding this comment

fadimounir commented Oct 18, 2019 •

edited

Loading

fadimounir commented Oct 22, 2019 •

edited

Loading

fadimounir commented Oct 22, 2019 •

edited

Loading

dagood left a comment •

edited

Loading

fadimounir Oct 24, 2019 •

edited

Loading