Avoid loading NuGet.targets if already loaded

[chipitsine]

Signed-off-by: Ilya Shipitsin chipitsine@gmail.com

[rainersigwald]

@chipitsine Can you provide some details about why this is needed?

cc @rrelyea, @jeffkl

[chipitsine]

in complex environments NuGet.targets might be occasioanely imported many times

     xxx\Current\bin\Microsoft.Common.CurrentVersion.targets(6086,3): warning MSB4011: "xxx\Current\bin\NuGet.targets" cannot be imported again. It was already imported at "xxx\yyy.props (63,3)". This is most likely a build authoring error. This subsequent import will be ignored. [xxx\yyy.proj]

IsRestoreTargetsFileLoaded is property that means NuGet.targets is already imported

[rrelyea]

Including @nkolev92

What is impact of loading multiple times in one project? Is there a repro of how this happens?

[rainersigwald]

@rrelyea the impact is that warning (which could be treated as an error if a user has their build configured that way), and confusion around import ordering. This would only address the former.

I would also like to know more details about exactly what projects are causing this warning--what are the "complex environments" like?

[chipitsine]

I'm afraid I cannot provide particular details due to some NDA.

but google shows similar issues

[chipitsine]

yep, the impact is to mute warnings

[rainersigwald]

@nkolev92, do you have a preference on this? The MSBuild team is willing to take it.

The only downside is that if you have a situation where you import NuGet.targets early, then in the normal location, it might be harder to set properties to configure NuGet behavior. That's true today if you're in that situation, but you get the warning.

One possible workaround: -warnAsMessage:MSB4011 on the command line should quiet this.

[nkolev92]

I do not recall getting a lot of feedback (read: any) around duplicate imports of the NuGet targets file.
So I am inclined to say, I'm ok with this change.

Is there even a default template that could lead you to this situation?

[rainersigwald]

Sounds like there weren't any real objections to doing this check, so let's go forward with it. Object ASAP if you care to, please. We'll merge ~tomorrow.

[rainersigwald]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).