Add Crossgen pack support to shared framework tooling #4650
Conversation
| @@ -23,6 +23,16 @@ | |||
| </PropertyGroup> | |||
| </Target> | |||
|
|
|||
| <Target Name="GetCrossgenPackInstallerProperties" | |||
There was a problem hiding this comment.
Are these changes part of the work needed to create the crossgen2 packages? If so, shouldn't we reflect "2" in the names here?
There was a problem hiding this comment.
I would argue that since the pack name is "Crossgen pack" it should match that. It just so happens that the tool filename in a "crossgen pack" is crossgen2. Note the proposed package ID is Microsoft.NETCore.App.Crossgen.<RID> over at dotnet/runtime#906, definitely open to change but haven't gotten any feedback on it yet.
There was a problem hiding this comment.
Are these changes part of the work needed to create the crossgen2 packages?
Oh, and yes. 😄
There was a problem hiding this comment.
Note the proposed package ID is Microsoft.NETCore.App.Crossgen. over at dotnet/runtime#906,
Hmm... Would it be better to make this Crossgen2? There's a whole thread around the naming of this thing, and it would be much simpler after a name is picked to just findstr all "crossgen2" instances in all repos and change them. Mixing crossgen and crossgen2 names here is pretty confusing (especially that for a period of time, we'll be shipping both)
There was a problem hiding this comment.
Didn't realize it was still quite that changeable... in that case, sure, I'll name everything crossgen2. I think we should also keep this code in dotnet/runtime to keep the volatile names in the same place for now.
Copying it over to dotnet/runtime#1859
In theory this code can be added to the crossgen pack project itself, however it requires a specific placement due to MSBuild evaluation order. I used that to test out the change in an official build, but to keep things simple let's add support here.
For dotnet/runtime#906.
Supports PR dotnet/runtime#1859.