Skip to content
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

Create targets and tasks for authoring a shared framework #2704

Closed
natemcmaster opened this issue May 6, 2019 · 27 comments · Fixed by #5714
Closed

Create targets and tasks for authoring a shared framework #2704

natemcmaster opened this issue May 6, 2019 · 27 comments · Fixed by #5714
Assignees

Comments

@natemcmaster
Copy link
Contributor

natemcmaster commented May 6, 2019

There are 3 different shared frameworks built by .NET teams - NETCore.App, AspNetCore.App, and WindowsDesktop.App. We share similar requirements and similar code. I think we should commonize these into a shared package to reduce the maintenance burden of building shared frameworks 'the right way.'

Desired usage

Projects which need to author a framework can create two project files, one for the targeting pack and another for the runtime packs. Both projects use a custom MSBuild SDK, "Microsoft.NET.SharedFramework.Sdk" and set some well-known properties which will inform the custom SDK how to build a shared framework.

Requirements

  • Generate a shared framework (gather binaries into a single folder)
  • Generate a shared framework layout ($dotnet_root$/shared/$name$/$version$)
  • The SDK can generate the manifests which all shared frameworks are supposed to include
    • .deps.json
    • .runtimeconfig.json
    • PackageOverrides.txt
    • PlatformManifest.txt
  • The SDK can produce .nupkgs with the correct layout and content for
    • The ref pack
    • The runtime pack
  • The SDK can crossgen assemblies in the shared framework
    • By default, enabled only when Configuration == Release

Out of scope

  • Generalizing support for building msi/deb/rpm/zip/tgz installers and archives.
  • Generating the AppHost packages (out of scope for now)
  • FrameworkList.xml - spec for this still unclear

Project file API

Example usage for the runtime project

<Project Sdk="Microsoft.NET.SharedFramework.Sdk">
  <PropertyGroup>
     <!-- Required properties -->
    <TargetFramework>netcoreapp3.0</TargetFramework>
    <RuntimeIdentifier>win-x64</RuntimeIdentifier>
    <SharedFrameworkName>Microsoft.Banana.App</SharedFrameworkName>
    <PlatformPackageType>RuntimePack</PlatformPackageType>
    
    <!--  This will be the default, but be overridden by the project file. -->
    <PackageId>Microsoft.Banana.App.Runtime.$(RuntimeIdentifier)</PackageId>
  </PropertyGroup>
  
  <ItemGroup>
    <FrameworkReference Include="Microsoft.NETCore.App" RuntimeFrameworkVersion="3.0.0" />
    
    <ProjectReference Include="..\..\MyAssembly1\MyAssembly1.csproj" />
    <ProjectReference Include="..\..\MyAssembly2\MyAssembly2.csproj" />
    
    <NativeRuntimeAsset Include="linux-x64/libmydep.so" Condition=" '$(RuntimeIdentifier)' == 'linux-x64' " />
  </ItemGroup>
  
</Project>

Example usage for the targeting pack project

<Project Sdk="Microsoft.NET.SharedFramework.Sdk">
  <PropertyGroup>
     <!-- Required properties -->
    <TargetFramework>netcoreapp3.0</TargetFramework>
    <SharedFrameworkName>Microsoft.Banana.App</SharedFrameworkName>
    <PlatformPackageType>TargetingPack</PlatformPackageType>
    
    <!--  This will be the default, but be overridden by the project file. -->
    <PackageId>Microsoft.Banana.App.Ref</PackageId>
  </PropertyGroup>
  
  <ItemGroup>
    <ProjectReference Include="..\..\MyAssembly1\MyAssembly1.csproj" />
    <ProjectReference Include="..\..\MyAssembly2\MyAssembly2.csproj" />
    
    <!--  Optimization for assemblies that are provided in both a targeting pack and an OOB NuGet package.  -->
    <ProvidesPackage Include="MyAssembly1" Version="1.0.0" />
  </ItemGroup>
</Project>

cc @ericstj @dagood

@natemcmaster
Copy link
Contributor Author

This is still a draft. Feel free to comment on naming, requirements, scope, etc.

@natemcmaster
Copy link
Contributor Author

cc @nguerrera @dsplaisted

@markwilkie
Copy link
Member

Cool idea. I see overlap (in concept) with setup authoring. My general comment is that this kind of thing tends to be a "slippery slope" and it starts to take on a life of its own. However, if there's high confidence that the requirements are truly known - than hooray! :)

@natemcmaster
Copy link
Contributor Author

Fortunately, this is mostly implemented already. The work for this would mostly be to put code into a shared location, update core-setup and aspnetcore to consume the shared code, and make sure the build output doesn't change as a result.

FYI - I'm volunteering myself to do the work to put common code in this repo and update aspnetcore.

@dsplaisted
Copy link
Member

I think @dagood and @ericstj are the right folks to comment on this from the Microsoft.NETCore.App and Microsoft.WindowsDesktop.App side of things.

@dagood
Copy link
Member

dagood commented May 8, 2019

We discussed it in a meeting already and it seems pretty reasonable. 😄 A few things that I see after reading:

  1. I think "generate a shared framework" is in scope but not listed as a requirement. "Generate a shared framework layout" I think would cover it without getting into the scope of installers/Deb/RPM/etc. My understanding is shared frameworks will keep being their own assets alongside the framework packs.

    • In general the description seems to blend shared frameworks with the framework packs, maybe it's just an issue of clarity or our definitions being different.
  2. AppHost pack is missing. I think it's fine to leave out from initial implementation and I'll add it when Core-Setup gets on this. AFAIK netcoreapp is the only framework with an AppHost pack.

  3. <RuntimeIdentifier>win-x64</RuntimeIdentifier>. This suggests a new project for each RID. RuntimeIdentifiers?

  4. On a similar note... I don't see a good reason to support PlatformPackageType but not (say) PlatformPackageTypes. (In any case, a way to build multiple packs from one project that contains all the info.) I don't think it's unreasonable to have one project for each pack type but listing some justification would be nice.

  5. FrameworkList.xml isn't fully spec'd yet to my knowledge, but belongs here too. (It's already generated for the packs built in Core-Setup.)

@natemcmaster
Copy link
Contributor Author

Updated the requirements. Thanks for the feedback.

RuntimeIdentifier

Didn't mean to imply a separate project per file. In AspNetCore, we always set RuntimeIdentifier and infer it from the host machine (unless it is explicitly set by build script parameters).

I think RuntimeIdentifier should still be required because there is no such thing as an "AnyCPU/AnyOS" shared framework. RuntimeIdentifiers could additionally be set, but not sure if requiring it is important.

AppHost

Yeah, I think that's a unique requirement of netcoreapp. I'm expecting there will be more than a few things like that because it is the base framework. Can those be out of scope for now? I'm not sure yet if those should remain one-off things that netcoreapp does, or if they need to be supported into the shared targets as opt-in knobs.

FrameworkList.xml isn't fully spec'd yet to my knowledge, but belongs here too. (It's already generated for the packs built in Core-Setup.)

Must have missed something. What is this file? AspNetCore is not producing it.

@dagood
Copy link
Member

dagood commented May 9, 2019

I'm not very familiar with the built in SDK targets, I'll leave it to you for RuntimeIdentifier. I'm not sure what it really means to set it in this context.

Agreed on AppHost and FrameworkList.xml and anything else Core-Setup-only being out of scope for now.

For context, I got some hints of FrameworkList.xml in early email threads and specific related feature requests/plans. https://github.com/dotnet/cli/issues/10537 tracks the overall work. For context on usage, https://github.com/dotnet/cli/issues/10536 is good. For now I'll just plan on adding it to Arcade when porting over--it's not complex.

@natemcmaster natemcmaster self-assigned this May 22, 2019
@natemcmaster
Copy link
Contributor Author

I'm going to get started on this tomorrow. To avoid having a single massive PR with a bunch of stuff, I was thinking of doing this in a feature branch with PRs for smaller chunks of the work. Sound okay @dagood?

@dagood
Copy link
Member

dagood commented May 24, 2019

I was thinking of doing this in a feature branch with PRs for smaller chunks of the work. Sound okay @dagood?

Good idea, thanks! For posterity: https://github.com/dotnet/arcade/tree/feature/sharedfx-sdk

@natemcmaster
Copy link
Contributor Author

I'm going to be out on leave starting in a few days. I think I was overly ambitious in assigning myself to do this work. I think it should still be done, but I won't be able to tackle this one until at least August, maybe later. I'll leave myself assigned, but consider this one up for grabs for the next 2 months.

@natemcmaster
Copy link
Contributor Author

FYI - I saved some partially-done work here: https://github.com/natemcmaster/arcade/tree/generate-deps

@natemcmaster
Copy link
Contributor Author

Just learned that the Preview 6 SDK includes the ability to crossgen a .NET Core 3 app by setting PublishReadyToRun. I wonder if we can take advantage of the SDKs targets for cross-gening shared frameworks...

https://devblogs.microsoft.com/dotnet/announcing-net-core-3-0-preview-6/

@nguerrera
Copy link
Contributor

I think it should be possible if you are driving the shared framework production as a publish.

You will likely need dotnet/sdk#3326 (still in PR)

@natemcmaster
Copy link
Contributor Author

I think it should be possible if you are driving the shared framework production as a publish.

I'm not. I've reused a few things from the SDK, like the targets for generating a runtimeconfig.json file and ResolveReferences, but pretty much everything else was custom. Here is how we implemented crossgen for the aspnetcore framework: https://github.com/aspnet/AspNetCore/blob/af812f2d898457f3cdf3161703768467c552c9a7/src/Framework/src/Microsoft.AspNetCore.App.Runtime.csproj#L283-L353.

@dagood
Copy link
Member

dagood commented Jun 12, 2019

Core-Setup uses Publish to make its shared frameworks, but the publish uses the nupkgs that we ship so the crossgen has to happen earlier.

Changing this around has been in the back of my mind though--creating a single layout that is crossgen'd and packaged up in various ways. Maybe that initial layout could be produced by Publish and crossgen could run through that. (The layout is probably basically the shared framework with other things mixed in that need to be filtered out in various configurations for various types of packages. I haven't thought it through very far.)

@nguerrera
Copy link
Contributor

Still might be possible, but not as straight forward. I'm working on sharing more between crossgen feature and dotnet store and that might help too.

@ViktorHofer
Copy link
Member

ViktorHofer commented Jun 23, 2019

@natemcmaster just pointed me to this. As neither @ericstj nor I knew about this, please make sure to cc us on such issues moving forward :) EDIT: Oh I see, @ericstj was tagged on this.

This is a great idea! Composing a shared framework version isn't straight-forward. You usually plug-in additional required stuff when something breaks in the sdk. In example we recently enabled project restore for our test projects and had to work around a couple of issue as we don't restore the targeting pack.

@ericstj
Copy link
Member

ericstj commented Jun 24, 2019

FWIW I was expecting this work to happen early in 3.0 based on the spec that @rakeshsinghranchi put together last year that covered most of what's listed here, but I think folks ended up with a more reactive approach in their respective repos. I believe right now @dagood is working on generalizing as much of the shared-fx building in core-setup and then will eventually move as much to arcade so that it can be shared. @natemcmaster's template above is a reasonable strategy but will need to reconcile all the requirements/behavior of existing solutions. First priority is make it work for NETCoreApp, ASP.NET, and WindowsDesktop. Where we can make it help CoreFx testing, we should, but that's a slightly different problem since it needs to support inner-loop testing.

@markwilkie
Copy link
Member

@dagood - should we start tracking the work you're doing, close this issue, and reference so that @natemcmaster input is maintained?

@dagood
Copy link
Member

dagood commented Jul 17, 2019

The work I'm planning to do is tracked at https://github.com/dotnet/core-setup/issues/4787. I don't see a reason to close this issue--it's a good, well-documented incremental step towards completing https://github.com/dotnet/core-setup/issues/4787, which has much broader scope.

If you'd like to get this issue out of the Arcade repo, I wouldn't object to transferring it to Core-Setup. (And transfer again if it turns out not to make sense.)

@natemcmaster natemcmaster removed their assignment Jul 18, 2019
@natemcmaster
Copy link
Contributor Author

Unassigning myself. I'm not going to get to this in the near future, but I think this is still worth doing.

@markwilkie
Copy link
Member

Makes sense @dagood . I think the goal is to make sure this doesn't get lost and stays top of mind. Is core-setup the best place for that or arcade in your opinion?

@dagood
Copy link
Member

dagood commented Jul 18, 2019

It seems to me people are more likely to stumble upon it in Arcade than Core-Setup, since it makes sense to look in dotnet/arcade for Arcade feature tracking issues. Moving it to Core-Setup would only put it in front of my own face more often, and I'm already keenly aware of it. 😄

@markwilkie markwilkie self-assigned this Aug 30, 2019
@markwilkie markwilkie assigned dagood and unassigned markwilkie Aug 30, 2019
@markwilkie
Copy link
Member

Assigned this to you @dagood for whenver you get to it.

@jkoritzinsky
Copy link
Member

I've started working on this, using lessons learned from the shared framework SDK used by dotnet/runtime and dotnet/windowsdesktop and the proposed UX in the start of this issue as a starting point.

@jkoritzinsky
Copy link
Member

If anyone wants to review the PR for this, it's at #5714

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants