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

ExcludeRestorePackageImports=true should not exclude package path properties #8840

Closed
timhermann opened this issue Nov 19, 2019 · 5 comments
Closed

Comments

@timhermann
Copy link

timhermann commented Nov 19, 2019

Details about Problem

Visual Studio 15.9.17 and msbuild cli
NuGet version: 5.3.1.6268

OS version (i.e. win10 v1607 (14393.321)):
Win10 v1909 (18363.476)

Detailed repro steps so we can see the same problem

  1. Set ExcludeRestorePackageImports=true
  2. Add <PackageReference Include="Test.PathProperty" Version="x.x.x" GeneratePathProperty="true" />
  3. Attempt to use $(PkgTest_PathProperty) as a variable

Expected behavior:

$(PkgTest_PathProperty) is defined, but no targets or props are imported from it.

Actual behavior:

$(PkgTest_PathProperty) is not avaiable because of the propertygroup condition <PropertyGroup Condition=" '$(ExcludeRestorePackageImports)' != 'true' ">

@timhermann
Copy link
Author

This issue in VS 15.9 means those paths aren't generated in VS2017 (which IMO should be fixed/backported): #8374 - but even from the CLI msbuild this is a problem. The package has targets that I must not have included in the project, but doing so means I cannot get the path definitions because the same condition excludes those properties..

@nkolev92
Copy link
Member

Why do you need to set ExcludeRestorePackageImports?

This is meant to be used for restore purposes so that packages' props/targets do not affect restore and potentially create a behavior that's not repeatable.

I get that this is not in the same breath as props/targets coming from packages, but the motivation is equivalent.

@nkolev92 nkolev92 added Functionality:Restore Resolution:ByDesign This issue appears to be ByDesign Style:PackageReference WaitingForCustomer Applied when a NuGet triage person needs more info from the OP labels Nov 23, 2019
@timhermann
Copy link
Author

timhermann commented Nov 26, 2019

Many packages include targets that inject behavior into the build and/or have not updated their packages to be packagereference-friendly. This is both a compliance nightmare as well as a headache to track down when something isn't working right. I need to exclude the targets that actually mess up my build, but I need to reference, copy, or transform files that are in the path.

The property should exclude the imports - that's what it says it does. Instead, it's excluding generated properties that I have explicitly opted into creating and intend to use.

I want to use it for reliable/repeatable restore purposes, but the current design prevents that.

@nkolev92
Copy link
Member

Many packages include targets that inject behavior into the build and/or have not updated their packages to be packagereference-friendly.

You should use Include/Exclude assets for those purposes.
https://docs.microsoft.com/en-us/nuget/consume-packages/package-references-in-project-files#controlling-dependency-assets

The property should exclude the imports - that's what it says it does. Instead, it's excluding generated properties that I have explicitly opted into creating and intend to use.

I get the name is not perfect, but this is really a NuGet internal that basically means packages' targets cannot not affect restore.

@nkolev92
Copy link
Member

Closing this as by design per the above comments.

@nkolev92 nkolev92 removed the WaitingForCustomer Applied when a NuGet triage person needs more info from the OP label Dec 31, 2019
@nkolev92 nkolev92 added this to the 5.5 milestone Dec 31, 2019
@zkat zkat reopened this Mar 18, 2020
@zkat zkat closed this as completed Mar 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants