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.

Sign up for GitHub

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

Jump to bottom

Restore to project with single TargetFramework must not condition props #3588

Closed
nguerrera opened this issue Oct 4, 2016 · 6 comments
Closed

Restore to project with single TargetFramework must not condition props #3588

nguerrera opened this issue Oct 4, 2016 · 6 comments
Assignees
@emgarten
Labels
Functionality:Restore Priority:1 High priority issues that must be resolved in the current sprint. Type:DCR Design Change Request
Milestone
4.0 RC

Comments

@nguerrera
Copy link

A project declaring a single TFM, e.g <TargetFramework>netstandard1.0</TargetFramework> will do so after props are imported. As such, if the .g.props file has a $(TargetFramework) condition as it does today, the condition will never be true, and the props will never be imported.

Nuget needs to distinguish the single TFM case and not emit conditions to the .g.props file (and probably not to the .g.targets either for the sake of consistency).
@nguerrera nguerrera mentioned this issue Oct 4, 2016
Closed
@emgarten
Copy link
Member

emgarten commented Oct 4, 2016

What is the expected behavior for buildCrossTargeting? Are they left out or are they also put in without a condition?

@nguerrera
Copy link
Author

They should be left out of the single TFM case. There is no outer build there.

@emgarten
Copy link
Member

emgarten commented Oct 4, 2016

They should be left out of the single TFM case. There is no outer build there.

That makes sense. For Pack then it seems like the targets file will need to be under both buildCrossTargeting and build, one will be used for multiple TFMs, and the other for a single TFM. //cc @rohit21agrawal

@rrelyea rrelyea added this to the 3.6 RC milestone Oct 7, 2016
@rrelyea rrelyea added Type:DCR Design Change Request Functionality:Restore Priority:1 High priority issues that must be resolved in the current sprint. labels Oct 7, 2016
@emgarten
Copy link
Member

@nguerrera what is the expected behavior if a single TFM is passed in <TargetFrameworks>?

@eerhardt
Copy link

what is the expected behavior if a single TFM is passed in ?

The same behavior as if multiple TFMs are specified in $(TargetFrameworks). There is an "outer" build, and NuGet would add the single condition on the Import.

@emgarten
Copy link
Member

The same behavior as if multiple TFMs are specified in $(TargetFrameworks). There is an "outer" build, and NuGet would add the single condition on the Import.

Thanks for confirming. NuGet will need to keep track of how it got the single framework to decide if buildCrossTargeting and TFM conditionals are used then.

@eerhardt eerhardt mentioned this issue Oct 11, 2016
Merged
@emgarten emgarten mentioned this issue Oct 11, 2016
Merged
emgarten added a commit to NuGet/NuGet.Client that referenced this issue Oct 11, 2016
@emgarten
Conditions for targets/props imports
3fc5aff 
Framework conditions for targets/props are added only when a project uses cross targeting. For scenarios with a single framework under $(TargetFramework) buildCrossTargeting will be left out and there will be no framework conditions to block the imports.

In addition to this a new property has been added to allow control over package imports: ExcludeRestorePackageImports. This flag is used at restore time to avoid imports from packages changing the inputs to restore, without this it is possible to get different results between the first and second restore.

This change also contains support for nooping when writing out targets/props files. Currently the check is a simple string compare on the files since these files are small. Nooping is now a major benefit since all NETCore projects reference the SDK package, without nooping on the targets/props files MSBuild will always end up rebuilding everything for NETCore solutions.

Fixes NuGet/Home#3588
Fixes NuGet/Home#3637
Fixes NuGet/Home#3604
Fixes NuGet/Home#3641
Fixes NuGet/Home#3199
emgarten added a commit to NuGet/NuGet.Client that referenced this issue Oct 11, 2016
@emgarten
Conditions for targets/props imports
e021efc 
Framework conditions for targets/props are added only when a project uses cross targeting. For scenarios with a single framework under $(TargetFramework) buildCrossTargeting will be left out and there will be no framework conditions to block the imports.

In addition to this a new property has been added to allow control over package imports: ExcludeRestorePackageImports. This flag is used at restore time to avoid imports from packages changing the inputs to restore, without this it is possible to get different results between the first and second restore.

This change also contains support for nooping when writing out targets/props files. Currently the check is a simple string compare on the files since these files are small. Nooping is now a major benefit since all NETCore projects reference the SDK package, without nooping on the targets/props files MSBuild will always end up rebuilding everything for NETCore solutions.

Fixes NuGet/Home#3588
Fixes NuGet/Home#3637
Fixes NuGet/Home#3604
Fixes NuGet/Home#3641
Fixes NuGet/Home#3199
@nguerrera nguerrera mentioned this issue Oct 12, 2016
Closed
@eerhardt eerhardt mentioned this issue Oct 12, 2016
Closed
@srivatsn srivatsn mentioned this issue Oct 12, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@emgarten emgarten

Labels
Functionality:Restore Priority:1 High priority issues that must be resolved in the current sprint. Type:DCR Design Change Request
Projects
None yet
Milestone
4.0 RC
Development

No branches or pull requests
4 participants
@nguerrera @emgarten @eerhardt @rrelyea