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

ResolveAssemblyReference is incorrectly marking references from NuGet packages as referenceassembly #1738

Closed
pranavkm opened this issue Nov 15, 2017 · 22 comments
Assignees
Labels
Milestone

Comments

@pranavkm
Copy link
Contributor

Reproduced in VS 2017 15.4.4.

  1. Create an Mvc application targeting net471
  2. Build and run the application.

Views fail to compile with an error stating from DependencyModel stating reference assemblies cannot be resolved:

System.InvalidOperationException: Cannot find reference assembly 'System.AppContext.dll' file for package System.AppContext.Reference
   at Microsoft.Extensions.DependencyModel.Resolution.ReferenceAssemblyPathResolver.TryResolveAssemblyPaths(CompilationLibrary library, List`1 assemblies)
   at Microsoft.Extensions.DependencyModel.Resolution.CompositeCompilationAssemblyResolver.TryResolveAssemblyPaths(CompilationLibrary library, List`1 assemblies)
   at Microsoft.Extensions.DependencyModel.CompilationLibrary.ResolveReferencePaths(ICompilationAssemblyResolver resolver, List`1 assemblies)
   at Microsoft.Extensions.DependencyModel.CompilationLibrary.ResolveReferencePaths()
...

Notes from @eerhardt's investigation

I quickly looked at this scenario, and it appears to be a dotnet/sdk bug to me. The SDK is getting confused about a bunch of references:

eric

All these references are being marked as “Reference Assembly” references, when in actuality they are coming from NuGet packages.

The reason they are being marked as “Reference Assembly” references (as far as I can tell) is because RAR is seeing the same file in the ‘C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework.NETFramework\v4.7.1’ RedistList, which is causing them to get the “FrameworkFile=true” metadata on them. The dotnet/sdk then thinks that this is a “Reference Assembly” (i.e. it comes from C:\Program Files (x86)\Reference Assemblies) because “FrameworkFile=true” on them.

I’ve added a quick check locally to that SDK code that appears to fix the issue:

<_FrameworkReferenceAssemblies Include="@(ReferencePath)"
                             Condition="(%(ReferencePath.FrameworkFile) == 'true' or
                                        %(ReferencePath.ResolvedFrom) == 'ImplicitlyExpandDesignTimeFacades') and
                                        %(ReferencePath.NuGetSourceType) == ''" />
@pranavkm
Copy link
Contributor Author

cc @eerhardt \ @dsplaisted \ @weshaggard

@livarcocc
Copy link
Contributor

@joperezr This seems to be a regression because of the changes we recently made. Any chance you can lend a hand and take a look at this?

@livarcocc
Copy link
Contributor

@pranavkm I just noticed that this is for 15.4.4. Have you tried this scenario on 15.5? We have made a series of fixes there supporting net471. Can you give it a try?

@joperezr
Copy link
Member

I believe this is the scenario for which we had an internal thread about, and we did confirm that the issue doesn't repro anymore on 15.5

@eerhardt
Copy link
Member

eerhardt commented Nov 16, 2017

From the email thread about 15.5:

I investigated why it works in VS 15.5, and it looks to be purely by accident. The .deps.json file is still getting generated incorrectly. So #1738 should still be fixed.
With VS 15.5, a “refs” folder is getting created (for a different reason) which is helping the DependencyModel locate System.AppContext.dll as a compile assembly.

This is a general problem that will happen again in the future anytime a System assembly goes from OOB to inbox - just like System.AppContext did in this case.

@pranavkm
Copy link
Contributor Author

pranavkm commented Feb 2, 2018

Coming back to this because I've had a couple of users file reports for this now. Is there a workaround they could do to unblock themselves? There was the promise of a target that could be added to the project, (involved rebuilding _FrameworkReferenceAssemblies) that might work: Is there an alternative to this as yet?

@dsplaisted
Copy link
Member

@pranavkm Is there a repro of this for VS 15.5+? From the comments it sounds like it no longer repros, but could again in the future if an assembly moves from OOB to inbox. Are there any other examples of this?

@pranavkm
Copy link
Contributor Author

@dsplaisted I couldn't reproduce the original issue with VS 15.5.6. I'm not familiar with what it would take to move an assembly in to the framework, so I can't get you a new sample to reproduce this with. Would the guidance be to update VS \ .NET Framework for now and we can revive this issue the next time something gets moved in to the fx?

@livarcocc
Copy link
Contributor

@nguerrera is this solved by #2133?

@nguerrera
Copy link
Contributor

@livarcocc No.

@nguerrera nguerrera self-assigned this Apr 17, 2018
@nguerrera
Copy link
Contributor

@eerhardt

I investigated why it works in VS 15.5, and it looks to be purely by accident. The .deps.json file is still getting generated incorrectly.

I've been looking at this for a while and that doesn't match what I'm seeing. The change in behavior in 15.5 seems to deliberately follow from:

#1618 In conflict resolution, treat items from FrameworkList.xml as platform assemblies

We end up getting :

Encountered conflict between 'Reference:C:\Users\nicholg\.nuget\packages\system.appcontext\4.3.0\ref\net463\System.AppContext.dll' 
and 'Platform:System.AppContext.dll'.  Choosing 'Platform:System.AppContext.dll' because AssemblyVersion '4.1.2.0' is greater than '4.1.1.0'.

And System.AppContext from the nupkg is discarded in favor of the in-box reference assembly, which is then accurately designated as such.

While being defensive about FrameworkFile=true getting set on a nuget reference seems like a good idea based on the past bug here,
I am so far unable to produce a test case where this would happen.

If the nupkg wins conflict resolution to platform, then by virtue of higher assembly version in nupkg, it will not get FrameworkFile=true:

https://github.com/Microsoft/msbuild/blob/7c1f7a8cd8f2b844046892036912db28a7ec4c37/src/Tasks/AssemblyDependency/ReferenceTable.cs#L2641-L2644
https://github.com/Microsoft/msbuild/blob/7c1f7a8cd8f2b844046892036912db28a7ec4c37/src/Tasks/AssemblyDependency/ReferenceTable.cs#L2382-L2385

If nupkg loses conflict resolution, then the ReferencePath inside the nupkg will not show up at all.

The one unclear case for me would be if there is a nupkg with equal assembly version to what has been moved in box. However, I have not been able to pin down such a combination.

cc @ericstj

@nguerrera nguerrera modified the milestones: 2.1.3xx, 2.1.4xx Apr 18, 2018
@nguerrera
Copy link
Contributor

@eerhardt and I spoke offline and we think that conflict resolution handles the real scenarios now, but it would still be good to add an extra layer of defense against ever marking something from nuget as "referenceassembly" in deps.json, because that is always wrong. However, since we can't come up with a real scenario where this defense mechanism would trigger, I'm moving this to 2.1.4xx. I will submit a PR without a test (because we can't actually trigger the case we're defending against with real packages and frameworks) to the 2.1.4xx branch.

@eriksendc
Copy link

I just updated VS to 15.6.7 and I'm getting

InvalidOperationException: Cannot find reference assembly 'System.AppContext.dll' file for package System.AppContext.Reference

Our .csproj alread has

<PackageReference Include="Microsoft.AspNetCore.Mvc.Razor.ViewCompilation" Version="2.0.1" PrivateAssets="All" />

Is there a workaround?

Thanks!
-Brian Eriksen

@eriksendc
Copy link

FYI, we're targeting full framework, 4.7.1.

Also note, I closed my solution, went to Tools/Options/NuGet Package Manager/General, selected Clear All NuGet Cache(s), reopened my solution, waited for NuGet packages to be restored, built, ran and now everything's working again. Somehow some dependency stuff with NuGet is still jacked up.

@joperezr
Copy link
Member

joperezr commented May 2, 2018

@eriksendc your issue doesn't seem to be related to this thread. Do you mind creating a new issue with a bit more details so that we can take a look at what you are hitting? By more info I mean ideally a minimal repro so that we can check what is going on. We could also take a diagnostic log or a binlog from your build. In order to produce a binlog, simply call from your command line msbuild yourProject.csproj /t:rebuild /bl which should create a msbuild.binlog file you can share so we may investigate.

@eriksendc
Copy link

@joperezr sorry, I was reading aspnet/Mvc#6993 and it said that it was a dupe of this one, so I put my comment here. Normally I try to be a good Community Member and I'd go back and recreate if I could, but that would require me to roll back my laptop to a point in time and retry. If the information I've provided thus far isn't actionable, then that's that this time, I'm afraid. Sorry 'bout that.

@pranavkm
Copy link
Contributor Author

pranavkm commented May 3, 2018

@eriksendc do you have any preview builds of the Sdk installed? The reported issue manifested with an intermediary build of rc1 Sdk and was resolved in the time frame. Take that back. I was thinking of a different issue

@eriksendc
Copy link

Hi @pranavkm I do not have any preview builds of the SDK installd, no. FYI, this thread is happening elsewhere: https://developercommunity.visualstudio.com/comments/244877/view.html and it says the issue is fixed pending release.

@nguerrera
Copy link
Contributor

@eriksendc Which version of the sdk are you using? What is dotnet --info run from your project directory?

I have #2154 out for review, but it was deemed to be hygiene at this point because I cannot reproduce the issue in any SDK that has #1618. There's some process to get that PR through and I've yielded to higher priority work on the assumption that the issue is already fixed and the PR is just a defensive code change. If that is mistaken, then I will push harder on it, and add tests to the PR to trigger the actual issue.

@eriksendc
Copy link

This is what dotnet --info returns:

D:\GetYourPet\GetYourPet\src\GetYourPet>dotnet --info
.NET Command Line Tools (2.1.200)

Product Information:
 Version:            2.1.200
 Commit SHA-1 hash:  2edba8d7f1

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.16299
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\2.1.200\

Microsoft .NET Core Shared Framework Host

  Version  : 2.0.7
  Build    : 2d61d0b043915bc948ebf98836fefe9ba942be11

@nguerrera
Copy link
Contributor

nguerrera commented May 9, 2018

@eriksendc Reading more carefully, it looks like you had a different issue and you are unable to go back and reproduce it. Based on that, I'm going to assume it is fixed. If you encounter it again, please do open a new issue with repro steps.

@nguerrera
Copy link
Contributor

Fixed by #2154

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

No branches or pull requests

8 participants