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

PackageReference does not treat build and contentFiles same to lib/ref - leading to unintuitive asset selection #6908

Open
rido-min opened this issue May 5, 2018 · 3 comments
Labels
Functionality:Restore Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. Style:PackageReference Type:DCR Design Change Request
Milestone

Comments

@rido-min
Copy link

rido-min commented May 5, 2018

I'm validating a package install with a dependency to net462.

Using packages.config, installing in a project targeting net461 fails (as expected) with:

You are trying to install this package into a project that targets '.NETFramework,Version=v4.6.1', 
but the package does not contain any assembly references or content files that are compatible 
with that framework. For  more information, contact the package author.

However when installing the same package in the same project using PackageRef the package is installed without any error or warning.

@nkolev92
Copy link
Member

nkolev92 commented May 7, 2018

@rido-min
What's the specific package you're trying to install?
It could that there are other assets which are compatible.

@nkolev92 nkolev92 added Functionality:Restore Style:PackageReference WaitingForCustomer Applied when a NuGet triage person needs more info from the OP labels May 7, 2018
@nkolev92
Copy link
Member

nkolev92 commented May 9, 2018

Discussed via e-mail with @rido-min
Can repro the behavior:

The package structure is as follows:

A\
   A.nuspec
   \build\net462\random.dll
   \build\net462\A.targets

The root cause is that in the compatibility checker we consider every package without assemblies to be compatible.

From
https://github.com/NuGet/NuGet.Client/blob/c82241cf7f9d27eba58550c5f86baa5d3fe2c7bf/src/NuGet.Core/NuGet.Commands/RestoreCommand/CompatibilityChecker.cs#L49-L56

If any package provides a "ref/foo/Thingy.dll", there MUST be a matching "lib/foo/Thingy.dll" or  "runtimes/bar/lib/foo/Thingy.dll" provided by a package in the graph.
All packages that contain Managed Assemblies must provide assemblies for 'foo'. If a package
contains any of 'ref/' folders, 'lib/' folders, or framework assemblies, it must provide at least
one of those for the 'foo' framework. Otherwise, the package is intending to provide managed assemblies
but it does not support the target platform. If a package contains only 'content/', 'build/', 'tools/' or
other NuGet convention folders, it is exempt from this check. Thus, content-only packages are always considered
compatible, regardless of if they actually provide useful content.

This indicates that this is by design.

The workaround/recommended approach for such packages is to have a placeholder in the lib folder

lib/net462/_._

It's a common pattern seen in a lot of packages.

//cc @rrelyea @jainaashish @emgarten
Can one you confirm that my reasoning is correct?
If so, we need to ensure that this is well documented.

@nkolev92 nkolev92 removed the WaitingForCustomer Applied when a NuGet triage person needs more info from the OP label May 9, 2018
@emgarten
Copy link
Member

emgarten commented May 9, 2018

That is correct and the right workaround. I believe this was first done since a compat failure for meta packages would be incorrect.

In my opinion it would be correct for build and contentFiles to be treated the same as the lib folder and fail. This code was ported over from DNU which didn't have other asset types like this and the issue wasn't caught until much later after shipping (few people complain about not getting failures).

In the past we've opted to continue leaving build/contentFiles out of the compat check here to avoid breaking existing packages.

@nkolev92 nkolev92 added the Type:DCR Design Change Request label Jun 21, 2018
@nkolev92 nkolev92 added this to the Backlog milestone Jun 21, 2018
@nkolev92 nkolev92 added the Priority:2 Issues for the current backlog. label Jun 21, 2018
@nkolev92 nkolev92 changed the title PackageRef allows to install packages not compatible with current target framework. PackageReference does not treat build and contentFiles same to lib/ref - leading to unintuitive asset selection Jun 21, 2018
@nkolev92 nkolev92 added Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. and removed Priority:2 Issues for the current backlog. labels Dec 2, 2021
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. Style:PackageReference Type:DCR Design Change Request
Projects
None yet
Development

No branches or pull requests

5 participants