avoid warning for importing pack targets twice

[rohit21agrawal]

The fix for #888 was incomplete in the sense that if $(NuGetBuildTasksPackTargets) is defined in a target contained via the nupkg, it is already imported as a result of restore. SDK shouldn't be trying to import those targets again.

@dsplaisted @nguerrera

CC: @kzu to make sure it doesn't break nugetizer's behavior since he did the original fix.

[dsplaisted]

So to clarify, the NuGet package sets the ImportNuGetBuildTasksPackTargets property? So this change means that if the property is set, it won't be used to import anything?

[rohit21agrawal]

@dsplaisted no, the NuGet package sets the NuGetBuildTasksPackTargets property. If that property is not set, then sdk targets evaluate ImportNuGetBuildTasksPackTargets to true (https://github.com/dotnet/sdk/pull/1185/files#diff-0f39add6afb079d5b2b9c0af850df7d7R49) and import the targets that are baked in with the sdk.

[dsplaisted]

It seems a bit backwards that if you set the property, that causes it not to get used.

Could the NuGet package set ImportNuGetBuildTasksPackTargets to false instead of setting the NuGetBuildTasksPackTargets property, and then could this PR be updated to only set the ImportNuGetBuildTasksPackTargets property if it's not already defined?

[rohit21agrawal]

@dsplaisted have made the suggested changes. here is corresponding PR for the nuget side changes:

NuGet/NuGet.Client#1368

[kzu]

Doesn't make sense to set the NuGetBuildTasksPackTargets at all if the ImportNuGetBuildTasksPackTargets controls whether it's used or not, @rohit21agrawal

Makes it very confusing to set it and not see it imported explicitly anywhere. Better just leave it unassigned

[rohit21agrawal]

@dotnet-bot test Windows_NT_FullFramework Release please.

[rohit21agrawal]

CC: @livarcocc @dsplaisted @nguerrera ..all tests passed, can we merge this?