Traversal targets for cross-targeting

[nguerrera]

This implements the portion of cross-targeting that we agreed must happen first in msbuild proper and as soon as possible to uncover any unforseen breaking impact on existing projects.

1. Redirect Microsoft.VisualBasic/CSharp.targets to new Microsoft.Common.Traversal.targets if $(TargetFrameworks) is set, but $(TargetFramework) is empty.
2. Implement the protocol where every project will pass its TFM down to project references and get the properties that configure the destination to the best compatible target.
3. For now, the common traversal targets just import based on a variable that we will set in a props hook, but ultimately some general logic should probably be live there, and there should be a way for arbitrary nuget packages to participate. This needs design we have not yet done so going with a minimal, unblocking start.

[rainersigwald]

This is a nice improvement 👍

[rainersigwald]

What defines this?

[nguerrera]

I will define it in our after-props hook. We are only redirecting targets, not props so it will work. We need to discuss the outer build extensibility and how nuget packages participate, etc. This is just enough to (1) unblock my follow up work (2) smoke test that the common targets hijacking doesn't interfere with existing projects.

[nguerrera]

Ready for review, @AndyGerlicher @cdmihai @srivatsn @davkean.

[nguerrera]

FYI, @dotnet/project-system

[rainersigwald]

I'm not a fan of calling this "traversal". Inside Microsoft, there's a long tradition of "traversal projects" (traditionally dirs.proj) that are lightweight directory-hierarchy traversers. MSBuild has this concept as well, for instance in BuildRequestConfiguration.

That said, I'm not sure what the right name for this concept is. "Inner" and "outer" as we've been casually calling it isn't very descriptive. Maybe something like CrossTargetingCoordinating? That's certainly not elegant.

[nguerrera]

I'm happy to rename but we need a quick decision if this is really going in tonight. Traversal was what we had said in Friday's design meeting and I reiterated in my notes sent immediately after that. CrossTargetCoordinating is too long. Maybe we just go with what we've been saying and use Outer. I know @srivatsn doesn't like Outer though.

[srivatsn]

I'll defer to the MSBuild folks wrt naming here.

[cdmihai]

If there's a design doc on some github SDK page about what a cross-tagetting traversal build is, this comment would be a good spot to reference it in case users want to understand the bigger picture. :)

[AndyGerlicher]

How about Microsoft.Common.CrossTargeting.targets?

[nguerrera]

How about Microsoft.Common.CrossTargeting.targets?

OK.

[nguerrera]

And IsCrossTargetingBuild to mean that we're in the outer build. Inner build is non cross-targeting since it is one TFM. I think this is a fine way to go.

[AndyGerlicher]

LGTM once IsTraversalBuild is renamed.

[nguerrera]

@AndyGerlicher Thanks and done.

[nguerrera]

@AndyGerlicher @cdmihai @rainersigwald Thanks again! Can you also help me get this ported to xplat and published to a package that the cli can consume?

[cdmihai]

@nguerrera

I think this is how you get into xplat:

• cherry pick the squash commit that went into master into a new branch
• do a new commit adding the targets to the following nuspec:
  https://github.com/Microsoft/msbuild/blob/xplat/src/nuget/Microsoft.Build.Targets.nuspec
• create a PR to xplat

@jeffkl, does that look alright?

[nguerrera]

@cdmihai @jeffkl I've done this as #1025.

[rainersigwald]

For posterity: this change caused an error in an internal project that had a custom target defined as:

<Target Name="ValidateReferences"
        DependsOnTargets="
            $(ResolveReferencesDependsOn);
            ValidateExportsFilesExist; 
            ValidatePackageReferences; 
            ValidateBinplace"
        AfterTargets="$(ResolveReferencesDependsOn)">

Note that it hooks after (any of) $(ResolveReferencesDependsOn) but then requires (all of) $(ResolveReferencesDependsOn) to run. Adding this target created a cycle on Clean:

Clean depends on CleanReferencedProjects depends on PrepareProjectReferences depends on AssignProjectConfiguration.

ValidateReferences has an AfterTargets for AssignProjectConfiguration, so ValidateReferences attempts to run, but first we have to run its DependsOnTargets, so we try to add them to the target stack, but when we consider PrepareProjectReferences, it’s already on the stack, so trying to add it would create a cycle, and we fail.

The error was

error MSB4006: There is a circular dependency in the target dependency graph involving target "PrepareProjectReferences".

This can be fixed by changing the custom target's hook:

             ValidateExportsFilesExist; 
             ValidatePackageReferences; 
             ValidateBinplace"
-          AfterTargets="$(ResolveReferencesDependsOn)">
+          BeforeTargets="ResolveReferences">
 
     <Error Text="@(ValidationError->'&#xa;%(Identity): %(Text)','')"
            Condition="@(ValidationError) != '' and $(ValidateProjectSettings) == 'true'"/>