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 add package should use use correct defaults at evaluation time #7972

Closed
peterhuene opened this issue Apr 8, 2019 · 8 comments
Closed
Labels
Product:dotnet.exe Resolution:ByDesign This issue appears to be ByDesign

Comments

@peterhuene
Copy link

From @sharwell on Monday, 08 April 2019 18:34:35

Steps to reproduce

dotnet add package coverlet.msbuild

Expected behavior

The default values for IncludeAssets and PrivateAssets are determined automatically during the build:

<PackageReference Include="coverlet.msbuild" Version="2.5.0" />

Actual behavior

The default values for IncludeAssets and PrivateAssets clutter the project file:

<PackageReference Include="coverlet.msbuild" Version="2.5.0"> 
  <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets> 
  <PrivateAssets>all</PrivateAssets> 
</PackageReference> 

Environment data

Unknown (filed in response to dotnet/winforms#702 (comment))

Copied from original issue: dotnet/cli#11101

@peterhuene
Copy link
Author

From @peterhuene on Monday, 08 April 2019 19:34:27

The dotnet add package functionality actually responsible for modifying the project file lives in NuGet (in NuGet.CommandLine.XPlat.dll).

I'm going to move this there.

@nkolev92
Copy link
Member

nkolev92 commented Apr 8, 2019

I'd imagine this is a dev dependency package? If so that's by design (as a popular ask by the community)
Those values are not the defaults.

#4125

@nkolev92 nkolev92 added Resolution:ByDesign This issue appears to be ByDesign and removed Triage:NeedsTriageDiscussion Type:DCR Design Change Request labels Apr 8, 2019
@nkolev92 nkolev92 closed this as completed Apr 8, 2019
@sharwell
Copy link

sharwell commented Apr 8, 2019

@nkolev92 The values are not the defaults for all PackageReference items, but they are the defaults for this PackageReference item. This bug indirectly blocks the use of dotnet add package in projects I work on, leaving users with manual editing of the project file as the only allowed way to update references.

@sharwell
Copy link

sharwell commented Apr 8, 2019

The specific situation where this arises is unit test projects. The projects have no downstream consumers, so the elements here are unnecessary noise in the project file for this case (they have no observable impact in the solution aside from clutter). Therefore, pull requests following the use of dotnet add package for these projects get rejected, and users are instructed to not use this command for NuGet operations.

If NuGet automatically applied the correct properties during the build when they are not set in the project file, the values can be omitted from the project file and users can use dotnet add package to update packages without the clutter.

/cc @KathleenDollard

@nkolev92
Copy link
Member

nkolev92 commented Apr 8, 2019

The values are not the defaults for all PackageReference items, but they are the defaults for this PackageReference item.

Spec at https://github.com/NuGet/Home/wiki/DevelopmentDependency-support-for-PackageReference

I understand that, but that is the best solution we have.
Having the package be private assets implicitly is a non-starter. The user needs to be able to understand why certain assets were excluded/included. Furthermore the user needs to be able to change that.

The other options was having different item type in MSBuild for dependencies. That got rejected in the long thread.

The projects have no downstream consumers, so the elements here are unnecessary noise in the project file for this case (they have no observable impact in the solution aside from clutter).

They do have an impact. It's a dev dependency and it's expressed as such in the project file. No compile assets & no transitive flow.

This is not specific to the dotnet add package, it's the same experience in VS as well.

@rrelyea

@voroninp
Copy link

@nkolev92 , is it possible to set defaults on the package side?

Currently we have developmentDependency flag that affects the defaults. But can I somehow define the values for Include/Exlclude/PrivateAssets that will be used by default when package is referenced?

@nkolev92
Copy link
Member

That's not really possible, but often times, it's not really necessary.

If you have assets that you don't want your package consumers to use, don't add them to the package.
if you have transitives that you don't want your consumers to use, just exclude in the declaration for those packages.

@voroninp
Copy link

@nkolev92 yet sometimes it should give a choice just with different defaults.

For example, Mapperly uses conditionals for its attributes and authors recommend to reference the package this way:

<PackageReference Include="Riok.Mapperly" Version="4.0.0" ExcludeAssets="runtime" PrivateAssets="all" />

thus erasing any trace of the library. Yet it's possible to save attributes, if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Product:dotnet.exe Resolution:ByDesign This issue appears to be ByDesign
Projects
None yet
Development

No branches or pull requests

5 participants