Remove unnecessary _MSBuildProjectReferenceExistent references

[joelverhagen]

Having _MSBuildProjectReferenceExistent is incorrect since we use the ProjectReference set to get project references. We should only have batching operations %(...) on a single project reference collection.

Pass the parent project's TFM down to the client project as _ParentTargetFramework. I did this because the child project should not use this MSBuild property in conditionals.

Fixes NuGet/Home#3865.

/cc @emgarten @mishra14 @nguerrera @rainersigwald

[nguerrera]

This is better, but there is one case where you'll miss project references. If the project that is referred to is x-targeted and has project references conditioned on $(TargetFramework). If you want to see those, you need to evaluate with best compatible TFM, which the code I sent in email earlier would do. Given the urgency, I could understand worrying about that separately, but I suggest at least filing a bug to add a test for that case and deal with it. Conditional project references should be rare enough for this not to be high priority.

[nguerrera]

Also, if you're depending on CustomAfterMicrosoftCommonTargets for this, you likely also need CustomAfterMicrosoftCommonCrossTargetingTargets (check spelling).

[joelverhagen]

We don't care about dependencies on the child projects.

[nguerrera]

Ah, I misunderstood what this is doing. The only exposure would be if anything _AddMetadataToProjectReferences returned were conditional on TargetFramework, and from the list below that seems like a non-scenario. :)

        <PackageId>$(PackageId)</PackageId>
        <PackageVersion>$(PackageVersion)</PackageVersion>
        <IncludeAssets>$(IncludeAssets)</IncludeAssets>
        <ExcludeAssets>$(ExcludeAssets)</ExcludeAssets>
        <PrivateAssets>$(PrivateAssets)</PrivateAssets>

FWIW, I was confused by the target name. From the POV of the project that actually executes the target, it has nothing to do with enumerating project references.

The point about CustomAfter for cross-targeting stands though. I think you would not get the data from x-targeted libs if that is how they get these targets. Actually, wouldn't they have them already though if they reference the SDK and wouldn't this cause duplicate imports?

[nguerrera]

My suggestion to make this easier to follow would be:

1. Don't pass the parent target framework back and forth, just add the target framework in the caller after you get the other data back.
2. Rename _AddMetadataToProjectReferences to _GetPackageInfo or something and don't name the single item it returns _ProjectToProjectReferences.

[nguerrera]

Ditto for IncludeAssets, ExcludeAssets, PrivateAssets: no need to pass them back and forth. Now that I've gone through this mental refactoring, I see all we really want from the reference that we don't already have are PackageId and PackageVersion. :)

[rainersigwald]

Strong agree with @nguerrera. This is basically:

projectReferences.Select(GetPackageInfo);

where

def GetPackageInfo(p):
    projectInfo = new ProjectInfo(this.path)
    projectInfo.PackageId = this.PackageId
    projectInfo.PackageVersion = this.PackageVersion

    return projectInfo

[rainersigwald]

After this review, let's sync up about changing these targets so they can work in parallel in a /m build. But I think getting them right in this batched version first is good.

[rainersigwald]

Strong agree with @nguerrera. This is basically:

projectReferences.Select(GetPackageInfo);

where

def GetPackageInfo(p):
    projectInfo = new ProjectInfo(this.path)
    projectInfo.PackageId = this.PackageId
    projectInfo.PackageVersion = this.PackageVersion

    return projectInfo

[emgarten]

This should be a semi-colon delimited list

[joelverhagen]

Sure, the existing parsing code was expecting ,, but I guess that is a vestige of project.json format.

[joelverhagen]

Fixed.