Restore3 couples PackageReferences with TargetFrameworks. #3504

Closed
srivatsn opened this Issue Sep 23, 2016 · 6 comments

Comments

Projects
None yet
5 participants
@srivatsn

Restore3 today only looks for PackageReference items in a project if the project has the TargetFrameworks property. While that is being used as a proxy for core projects, that introduces unwanted coupling. For eg, to enable packagereferences in UWP projects we will need to introduce this property there.

My proposal for restore3 behavior is:
Look for the existence of a project.json and if it exists, go into project.json mode. Otherwise assume that the project has PackageReferences. That also leads to the behavior that post Dev15, if someone creates a new project, PackageReference will be the default.

@rrelyea rrelyea added this to the 3.6 Beta2 milestone Sep 26, 2016

@nguerrera

This comment has been minimized.

Show comment
Hide comment
@nguerrera

nguerrera Oct 4, 2016

We also need to address this (quoting @srivatsn) when supporting a project with only TargetFramework singular:

There’s another issue I mentioned to Nick yesterday. NuGet today generates a projectName.NuGet.props file which imports props from packages conditioned on TargetFramework.

However, we would normally define TargetFramework after importing Common.props (which would pull in the projectName.NuGet.props through the MSBuildProjectExtensionsPath mechanism). So if we just have TargetFramework, then props files from packages won’t get imported.

One solution could be that if there’s just a TargetFramework, then the generated props\targets could unconditionally import the props\targets for that TargetFramework.

+1 to the suggested solution.

We also need to address this (quoting @srivatsn) when supporting a project with only TargetFramework singular:

There’s another issue I mentioned to Nick yesterday. NuGet today generates a projectName.NuGet.props file which imports props from packages conditioned on TargetFramework.

However, we would normally define TargetFramework after importing Common.props (which would pull in the projectName.NuGet.props through the MSBuildProjectExtensionsPath mechanism). So if we just have TargetFramework, then props files from packages won’t get imported.

One solution could be that if there’s just a TargetFramework, then the generated props\targets could unconditionally import the props\targets for that TargetFramework.

+1 to the suggested solution.

@nguerrera

This comment has been minimized.

Show comment
Hide comment
@nguerrera

nguerrera Oct 4, 2016

It is also worth noting that we have a related problem for props in buildCrossTargeting: #3532. Fixing the issue for single TargetFramework is much higher priority, but it might help to think about the general problem while fixing this.

It is also worth noting that we have a related problem for props in buildCrossTargeting: #3532. Fixing the issue for single TargetFramework is much higher priority, but it might help to think about the general problem while fixing this.

@emgarten

This comment has been minimized.

Show comment
Hide comment
@emgarten

emgarten Oct 4, 2016

Contributor

@nguerrera would you open a different issue for tracking the use of TargetFramework in the imports targets/props with an explanation of what is expected there.

This issue relates to the precendence for how NuGet determines which type of Restore to perform, ex: create project.lock.json or obj/project.assets.json when a project contains both TargetFrameworks and project.json.

Contributor

emgarten commented Oct 4, 2016

@nguerrera would you open a different issue for tracking the use of TargetFramework in the imports targets/props with an explanation of what is expected there.

This issue relates to the precendence for how NuGet determines which type of Restore to perform, ex: create project.lock.json or obj/project.assets.json when a project contains both TargetFrameworks and project.json.

@nguerrera

This comment has been minimized.

Show comment
Hide comment
@nguerrera

nguerrera Oct 4, 2016

@emgarten OK. #3588. Is that clear enough?

@emgarten OK. #3588. Is that clear enough?

@emgarten

This comment has been minimized.

Show comment
Hide comment
@emgarten

emgarten Oct 4, 2016

Contributor

@nguerrera thanks!

Contributor

emgarten commented Oct 4, 2016

@nguerrera thanks!

@rohit21agrawal rohit21agrawal removed their assignment Oct 13, 2016

@rrelyea

This comment has been minimized.

Show comment
Hide comment
@rrelyea

rrelyea Oct 27, 2016

Contributor

This blocks legacy csproj from command line (msbuild /t:restore + nuget.exe)

Contributor

rrelyea commented Oct 27, 2016

This blocks legacy csproj from command line (msbuild /t:restore + nuget.exe)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment