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

dotnet restore does not capture transitive framework assemblies from project references #4412

Open
srivatsn opened this issue Jan 27, 2017 · 22 comments
Labels
Functionality:Restore Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. Triage:Investigate
Milestone

Comments

@srivatsn
Copy link

From @NTaylorMullen on January 19, 2017 20:24

Trying to utilize a transitive project references framework assembly results in build errors. This looks to be the result of dotnet restore not capturing project references framework assemblies (doesn't exist in assets.json).

Note: This used to work in project.json land (built the repro in project.json then migrated).

See the simple repro here:
TransitiveProjectFrameworkRefs.zip

Go into ConsoleApp12 and dotnet restore; dotnet build

Copied from original issue: dotnet/sdk#691

@emgarten
Copy link
Member

emgarten commented Jan 27, 2017

@NTaylorMullen The current design is that reference items do not flow transitively. You will need to reference this in your projects instead.

With project.json framework assemblies were explicitly defined for NuGet in a special section of the file. With csproj both references to external dlls on disk and framework references are reference elements and there is no clear way to find the subset of items that are actual framework assemblies.

If this were changed and a subset of reference items flowed transitively I think it would be confusing as to why some automatically showed up in parent projects and some did not.

@srivatsn what would you expect here?

@NTaylorMullen
Copy link

That's unfortunate then. This used to work in project.json land. If we're to not support this scenario we should at the very least raise transitive framework assemblies during dotnet migrate.

@emgarten
Copy link
Member

@NTaylorMullen fixing this as part of migrate sounds like a better way to handle this. Afterwards the information project.json had as to what was a framework assembly reference that should flow vs a normal reference is gone.

@NTaylorMullen
Copy link

/cc @livarcocc

@livarcocc
Copy link

Migration used to do this, but once transitivity was to the SDK we removed this. So, I guess we need to add it back?

Is this related to dotnet/sdk#714?

@livarcocc
Copy link

@NTaylorMullen please, file an issue in the CLI for this.

@NTaylorMullen
Copy link

@livarcocc
Copy link

As Sri pointed out in the CLI issue. This would bring back problems where we would need to keep the project.json around until migration is fully done. This would be a problem for VS, as they migrate each project separately (including moving artifacts to backup).

This is copied over from the CLI issue:

  1. Make this work if the --skip-project-references is not passed. But that means that whenever the flag is passed (100% the case for VS), projects may be migrated missing this information.
  2. Make migration find project.json for dependencies in the backup folder. But in this case, I think we should also allow for the backup folder to be passed to migration, so that migration is not aware of how VS calculates the path to this folder. This would be potentially a significant change for migration so late in the game and I am a bit nervous about taking it.
  3. Fix this in NuGet and the SDK?
  4. Do nothing. Migrate these projects wrong and have people add the references themselves.

I am not sure why NuGet decided not to honor this in the MSBuild world. But I feel like none of the options above are ideal for the stage we are in the cycle.

@livarcocc
Copy link

So, after thinking about this, I don't think it is right for migration to handle this and as I stated above we really can't.

This is a gap in transitive dependencies that should be handled. In my opinion, NuGet should add these references to the asset.json file and the SDK should find and pass them forward. Now, given the timeline, I understand this not happening, but I don't see how we can handle it in migration either.

@emgarten
Copy link
Member

emgarten commented Feb 2, 2017

This is a gap in transitive dependencies that should be handled

I think this would make it very confusing for the user when some references flowed transitively and others didn't. For example:

<Reference Include="MyOwnGACReference" />
<Reference Include="System.Xml.Linq" />
<Reference Include="external.dll" />

The only framework assembly here is System.Xml.Linq, it seems like it would be hard to determine why this one flowed to parent projects but the others did not.

If we do try to go the route of flowing these then I think this issue should be tracked on the SDK side also, Restore would need something aware of the framework to provide the list of framework assemblies to put into the project.assets.json file before this could happen.

@rrelyea rrelyea modified the milestones: 4.0.1, 4.0 RTM Feb 3, 2017
@dougbu
Copy link

dougbu commented Feb 23, 2017

The lack of dependency information in .NET Core assemblies is a visible gap compared to .NET Framework assemblies. Using the same code to examine the dependencies when running Core or desktop applications will fail for Core and work for desktop. Attempting to avoid this gap during migration does not actually solve the problem, especially for dynamic load scenarios.

As things stand, every MVC web site must have $(PreserveCompilationContext) set to true (currently hidden in the Microsoft.NET.Sdk.Web targets) and in-memory testing i.e. loading of those projects requires the following extra target. This target (ok, hack) is not required unless testing with .NET Core.

  <Target Name="CopyDepsFiles" AfterTargets="Build" Condition="'$(TargetFramework)'!=''">
    <ItemGroup>
      <DepsFilePaths Include="$([System.IO.Path]::ChangeExtension('%(_ResolvedProjectReferencePaths.FullPath)', '.deps.json'))" />
    </ItemGroup>

    <Copy SourceFiles="%(DepsFilePaths.FullPath)" DestinationFolder="$(OutputPath)" Condition="Exists('%(DepsFilePaths.FullPath)')" />
  </Target>

@forrestab
Copy link

forrestab commented Mar 16, 2017

Sorry if this is the wrong place for this, but I have not seen any recommendations/suggested workarounds on how to handle this problem for building nuget packages that require framework assemblies in vs2017.

So, do we continue to manually add the framework assembly references that the nuget package requires to the consuming project or build the nuget package using a nuspec file in order to make use of the frameworkAssemblies tag/property?

@kevinchalet
Copy link

That's really a nasty surprise, and a very annoying breaking change! 👎

@emgarten
Copy link
Member

@forrestab this sounds difficult, would you open a new issue to track adding framework assemblies during pack?

This issue is specifically for the restore/consuming side of framework assemblies.

@kevinchalet
Copy link

kevinchalet commented Mar 20, 2017

@emgarten I went ahead and opened a new ticket: #4853.

@rynowak
Copy link

rynowak commented Apr 28, 2017

Can we get an ETA on this or reprioritize it?

This keeps coming up for our users when they try to integration test MVC sites or develop a multi-project MVC site.

Example here: aspnet/Razor#1212 We've had many bugs like this 😞 Our best mitigation right now is to spread the hack that's described here.

@giggio
Copy link

giggio commented Oct 16, 2017

Is there a fix planned for this bug on the roadmap? Just as stated before, this problem seems to be the source for several other bugs.
I am also having to do the .deps file hack.
It's been six months already without any feedback on this issue.

@emgarten emgarten modified the milestones: Future-0, Backlog Oct 16, 2017
@emgarten
Copy link
Member

I am also having to do the .deps file hack.

@giggio can you share what this is? Is there a reason you are fixing it in deps instead of referencing the framework assemblies from the projects directly?

Feedback on why this issue is important would help the NuGet team prioritize this.

@giggio
Copy link

giggio commented Oct 16, 2017

@emgarten In my case I have an integration test, referencing an aspnet core project (a) that references another aspnet core project (b). My views are in project (b) and unless I use the .deps hack I get an error on the compilation of any of the views. The views are using an embedded file system.
I am on the latest stable version of everything, VS, dotnet, etc.

@emgarten
Copy link
Member

@giggio what exactly do you change in the .deps file? I'm not clear on what is missing and why in your scenario.

@giggio
Copy link

giggio commented Oct 17, 2017

I don't change anything, I just need to copy the deps file, as shown here: #4412 (comment)

@voltcode
Copy link

voltcode commented Dec 3, 2018

Guys, any update on this ? I have a .NET FX csproj that references .NET Standard csproj. That .NET Standard csproj packagereferences a package.
The problem is that the project.assets.json is not generated on build for .NET FX csproj.

How can this be generated so that transitive project->package references are properly collected, without resorting to manually redeclaring all nested package references in the top level project? That defeats the sole purpose of package reference.

@jeffkl jeffkl added Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. and removed Pipeline:Icebox labels Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Functionality:Restore Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. Triage:Investigate
Projects
None yet
Development

No branches or pull requests