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

Setting PreserveCompilationContext=false trims Microsoft.AspNetCore.App shared runtime in deps files #2092

Closed
eerhardt opened this issue Mar 28, 2018 · 8 comments
Assignees
Milestone

Comments

@eerhardt
Copy link
Member

From @pranavkm on March 26, 2018 17:18

Steps to reproduce

  1. Create an webapi \ mvc \ emptyweb application using preview1 (or preview2) Sdk
  2. Build the project

The generated deps file will say that the application references both Microsoft.NETCore.App and Microsoft.AspNetCore.App:

"blah/1.0.0": {
    "dependencies": {
      "Microsoft.AspNetCore.App": "2.1.0-preview1-final",
      "Microsoft.NETCore.App": "2.1.0-preview1-26216-03"
    },
    "runtime": {
      "blah.dll": {}
    },
    "compile": {
      "blah.dll": {}
    }
},
  1. Delete the bin directory. Edit the csproj <PreserveCompilationContext>false</PreserveCompilationContext>. Rebuild the project
  2. The generated deps file will say that the app no longer references Microsoft.AspNetCore.App:
 "blah/1.0.0": {
   "dependencies": {
     "Microsoft.NETCore.App": "2.1.0-preview1-26216-03"
   },
   "runtime": {
     "blah.dll": {}
   }
 },

Environment data

.NET Command Line Tools (2.1.300-preview1-008174)

Product Information:
 Version:            2.1.300-preview1-008174
 Commit SHA-1 hash:  b8df89a54f

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.16299
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Users\Pranav\Downloads\dotnet-sdk-2.1.300-preview1-008174-win-x64\sdk\2.1.300-preview1-008174\

Microsoft .NET Core Shared Framework Host

  Version  : 2.1.0-preview1-26216-03
  Build    : f2c3216183d20416568a4bbf5bb7d153e826f153

More information:

The bulk of MVC's controller discovery model is based on finding libraries that directly or transitively depend on any one of Mvc's libraries. In 2.1.0-preview2, the Razor.Sdk only sets PreserveCompilationContext=true if the app has any files that require runtime compilation. For projects without views, such as webapi or empty web template, the produced deps file claims that the app does not reference Microsoft.AspNetCore.App -> Microsoft.AspNetCore.Mvc. Consequently no controllers are discovered.

Copied from original issue: dotnet/core-setup#3902

@eerhardt
Copy link
Member Author

From @pranavkm on March 26, 2018 17:57

Spoke to @eerhardt, the missing shared runtime isn't necessarily the cause for concern here - a console app's deps file also does not include a reference to the shared runtime Microsoft.NETCore.App. The issue is that DependencyContext.Load does not merge shared fx deps files. The suggested fix is to add a new overload to DependencyContext.Load that walks up the fx chain and merges the runtime deps files. MVC can consume this API once this is available.

cc @rynowak \ @steveharter \ @mkArtakMSFT

@eerhardt
Copy link
Member Author

Looking more, the DependencyContext.Load DOES merge in the shared fx deps files.

The issue is that the Dependency information is getting trimmed, so there is no link from the application's library to the MVC library. Or similarly, if the application references another project that contains controllers, that project won't have a Dependency on the MVC library either. This trimming logic is in the dotnet/sdk repo. Moving this issue there.

@eerhardt
Copy link
Member Author

See

if (IsFrameworkDependent)
{
allExclusionList.UnionWith(_lockFileTarget.GetPlatformExclusionList(PlatformLibrary, libraryLookup));
}
for where the PlatformLibrarys get trimmed.

@livarcocc livarcocc added this to the 2.1.3xx milestone Mar 28, 2018
@livarcocc
Copy link
Contributor

@dsplaisted please take a look.

@eerhardt
Copy link
Member Author

Some more thoughts about this:

Assuming the plan is to preserve the "Dependency" entries, even when the Platform library is trimmed:

  1. You will probably want to only enable this behavior with a switch that is disabled by default. ASP.NET can enable the switch as necessary, when they want this behavior. Leaving the "Dependency" entries pointing to trimmed libraries could be considered a breaking change. This is because someone could load the .deps.json file without also loading the shared fx .deps.json. When this happens, if there is code trying to resolve those dependencies it will fail to find the referenced library.
  2. The "Version" property of a Dependency may be problematic, depending on how the consumer's dependency resolution logic works. For example, if there was a Dependency for MVC v2.1.0 and the asp.net shared fx rolled forward to v2.1.6 or even v2.2.0, using the Dependency.Version to match on strings will not work.

@pranavkm
Copy link
Contributor

pranavkm commented Mar 28, 2018

Stepping back, the primary driver for turning off PreserveCompilationContext is to avoid publishing the refs directory when an application does not require it (e.g. when you're writing an application not using runtime compiled views). Right now, there's a single switch that determines the structure of the deps file and the published refs directory. Could we possibly introduce a new switch that controls publishing the refs directory?

// Microsoft.NET.Sdk.Razor.targets
<!-- Use PreserveCompilationContext's value if it was explicitly set in the user's project -->
<PreserveCompilationReferences Condition="'$(PreserveCompilationContext)' != ''">$(PreserveCompilationContext)</PreserveCompilationReferences>

<!-- Initialize PreserveCompilationContext -->
<PreserveCompilationReferences Condition="'$(PreserveCompilationReferences )' == ''">true</PreserveCompilationReferences>

Targets in Microsoft.NET.Sdk that control publishing would be modified to rely on this new switch:

<Target Name="ComputeRefAssembliesToPublish"
Condition="'$(PreserveCompilationContext)' == 'true'"

<Target Name="_CopyReferenceOnlyAssembliesForBuild"
Condition="'$(PreserveCompilationContext)' == 'true'"

It's a breaking change for users relying on the refs directory in non-web scenarios, but I'm not sure if that's a common scenario.

@nguerrera
Copy link
Contributor

Duplicate of #2122

@nguerrera nguerrera marked this as a duplicate of #2122 Jan 8, 2019
@nguerrera
Copy link
Contributor

Going with #2092 (comment), which is tracked by #2122.

JL03-Yue pushed a commit that referenced this issue Mar 19, 2024
…202.4 (#2092)

[main] Update dependencies from dotnet/arcade
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

No branches or pull requests

5 participants