Using /graphBuild May Result in Projects Silently Being Excluded

[aolszowka]

Steps to reproduce

Attempting to use the new /graphBuild switch may result in Projects being excluded from MSBuild with no warning to indicate projects have been excluded.

For example

"C:\Program Files (x86)\Microsoft Visual Studio\2019\Preview\MSBuild\Current\Bin\amd64\MSBuild.exe" C:\REDACTED\Utilities.sln /m /graphBuild /t:Build /p:Configuration=Release

Yields

Microsoft (R) Build Engine version 16.5.0-preview-20113-03+04ed36359 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.

Build started 3/4/2020 3:41:34 PM.

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:00.10

Excluding the /graphBuild command yields the expected build.

"C:\Program Files (x86)\Microsoft Visual Studio\2019\Preview\MSBuild\Current\Bin\amd64\MSBuild.exe" C:\REDACTED\Utilities.sln /m /t:Build /p:Configuration=Release

Microsoft (R) Build Engine version 16.5.0-preview-20113-03+04ed36359 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.

Build started 3/4/2020 3:43:33 PM.
     1>Project "C:\REDACTED\Utilities.sln" on node
        1 (Build target(s)).
     1>ValidateSolutionConfiguration:
         Building solution configuration "Release|Any CPU".
     1>Project "C:\REDACTED\Utilities.sln" (1) is building "C:\REDACTED\Utilities.synproj" (2) on node 1 (default targets).
     2>PrepareForBuild:
         Creating directory "C:\REDACTED\..\bin\Client\".
         Creating directory "C:\REDACTED\obj\Release\".
     1>Project "C:\REDACTED\Utilities.sln" (1) is building "C:\REDACTED\Structures.synproj" (3) on node 2 (default targets).
     3>PrepareForBuild:
         Creating directory "C:\REDACTED\obj\Release\".
     1>Project "C:\REDACTED\Utilities.sln" (1) is
       building "C:\REDACTED\Schemas\Repository.synproj" (4) on node 3 (default targets).
     4>BeforeBuild:
         Creating directory "C:\REDACTED\Schemas\obj\Release\AnyCPU".
       FixSDI2015Bug:
         Fixing Bug Caused By SDI 2501 Not Deleting the Temp Schema
       CopyFilesToOutputDirectory:
         Creating directory "..\..\bin\rpsdat".
         Creating directory "..\..\bin\rpsdat".
         Creating directory "..\..\bin\rpsdat".
         Creating directory "..\..\bin\rpsdat".
         Copying file from "C:\REDACTED\Schemas\obj\Release\AnyCPU\rpstext.eng" to "C:\REDACTED\bin\rpsdat\rpstext.eng".
         Copying file from "C:\REDACTED\Schemas\obj\Release\AnyCPU\rpsmain.en1" to "C:\REDACTED\bin\rpsdat\rpsmain.en1".
         Copying file from "C:\REDACTED\Schemas\obj\Release\AnyCPU\rpstext.en1" to "C:\REDACTED\bin\rpsdat\rpstext.en1".
         Copying file from "C:\REDACTED\Schemas\obj\Release\AnyCPU\rpsmain.eng" to "C:\REDACTED\bin\rpsdat\rpsmain.eng".
         Repository ->
     4>Done Building Project "C:\REDACTED\Schemas\Repository.synproj" (default targets).
     3>CopyFilesToOutputDirectory:
         Copying file from "C:\REDACTED\obj\Release\SynergyStructures.elb" to "C:\REDACTED\bin\Client\SynergyStructures.elb".
         SynergyStructures -> C:\REDACTED\bin\Client\SynergyStructures.elb
     3>Done Building Project "C:\REDACTED\SynergyStructures.synproj" (default targets).
     2>CopyFilesToOutputDirectory:
         Copying file from "C:\REDACTED\obj\Release\utilities.elb" to "C:\REDACTED\bin\Client\utilities.elb".
         Utilities -> C:\REDACTED\bin\Client\utilities.elb
     2>Done Building Project "C:\REDACTED\Utilities.synproj" (default targets).
     1>Done Building Project "C:\REDACTED\Utilities.sln" (Build target(s)).

Build succeeded.

    0 Error(s)

Time Elapsed 00:01:08.90

This can be especially dangerous if you have a large mixed-technology solution file (once that contains a significant amount of C# in addition to the unsupported SDK) because the build will "appear to work", and unless you are explicitly checking for the binaries produced by the build this issue may go undetected.

Based on https://github.com/microsoft/msbuild/blob/master/documentation/specs/static-graph.md it seems to indicate that Existing functionality must still work. This new behavior is opt-in only. We assume this means that the third party SDK does not properly support the new graphBuild behavior; but that msbuild should fall back to the previous behavior in times of difficulty.

The vendor of the SDK Extending MSBuild has been notified to see if they can conform to the Static Graph standard listed above.

Environment data

msbuild /version output:

Microsoft (R) Build Engine version 16.5.0-preview-20113-03+04ed36359 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.

16.5.0.11303

OS info: Windows 10 1909

[rainersigwald]

@cdmihai can you take a look? Is this just that /graph doesn't support solutions?

[cdmihai]

/graph supports solutions (or rather, the solutions that VS creates for managed projects). The description in #4463 summarizes what's supported and what's not.

I see you are using a custom project, synproj. At the very least it has to adhere to the msbuild p2p protocol. If you want to make sure you've got it right, also add /isolate in addition to /graph. /isolate will fail the build if anything not predicted by the graph is built. Without /isolate, /graph will infer whatever it can and build it, regardless of how different it is from a vanilla msbuild invocation.

Since no projects are found in your graph based solution build, I suspect that synproj is not using ProjectReference items to specify project dependencies.

[madkat]

@cdmihai @rainersigwald It looks like the /graph feature actually excludes non-C# projects from being read from the solution.

If I identify my synproj as a {FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} GUID in the solution file, my build will get run. If I switch back to my actual project type GUID definition of {BBD0F5D1-1CC4-42FD-BA4C-A96779C64378}, we get skipped.

Edit for clarity: We do support msbuild p2p. There are no references involved in my test case.

[cdmihai]

It's most likely this line: https://github.com/microsoft/msbuild/blob/master/src/Build/Graph/GraphBuilder.cs#L255-L258

At the very least, a warning should be written saying there's ignored projects from the solution.

[aolszowka]

@madkat Thanks for chiming in (this is the third party vendor).

In our test case we have ProjectReferences but as @cdmihai just pointed out this does not show as a SolutionProjectType.KnownToBeMSBuildFormat (I've gotten burned by that in the utilities I've written to operate on these files). Is there anything we can do to register as a "Known" MSBuildFormat? They are really good citizens of the ecosystem and its improved our productivity!

[madkat]

There is no warning happening at normal, detailed or diagnostic verbosity. Maybe that would be different if I had a C# project present.

Edit: No, there is no indicator that a project was skipped. Even on diagnostic.

[madkat]

If I have a C# project reference a Synergy .NET project, there is no problem. We are evaluated correctly by the graph.

F:\temp\msftsample>msbuild /m /isolate /graphBuild MSBuildGraphTestSynNet.sln /v:minimal
Microsoft (R) Build Engine version 16.4.0+e901037fe for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.

MSBuildGraphTestSynNet -> F:\temp\msftsample\bin\Debug\MSBuildGraphTestSynNet.exe
MSBuildGraphTestCS -> F:\temp\msftsample\bin\Debug\MSBuildGraphTestCS.exe

[aolszowka]

If I have a C# project reference a Synergy .NET project, there is no problem

I noticed that in our testing too; In addition the Repository project type also worked (which is also referenced by a C# Project). However Traditional Synergy Projects are not recognized.

Does a Repository Project type alone work?

[cdmihai]

If I have a C# project reference a Synergy .NET project, there is no problem. We are evaluated correctly by the graph.

It works because the C# project passes the filter criteria, and then graph construction finds the reference to the .synproject project.

I'm thinking a better fix would be to just mimic what vanilla msbuild does when it interprets solutions and finds a project that's not msbuild syntax (if it fails this check https://github.com/Microsoft/msbuild/blob/07c3b3392dc05e0e3aba18bdec235a374aa72301/src/Build/Construction/Solution/ProjectInSolution.cs#L279-L362). Which means anything that looks like an msbuild project file will get accepted, and for the rest I'll reverse engineer what vanilla msbuild does.

In the meantime, as @madkat observed, a workaround is to add the C# GUID to the .synproj.

Let us know if using /graph improves your build times (when it's building a non empty set of projects :)), and whether the build is correct w.r.t. vanilla msbuild. /isolate is a good proxy for that correctness.

[aolszowka]

We are SUPER Excited for this; as I mentioned in that other thread we've got a large solution (~3500 Projects) split around 2200 SYNPROJ and 1300 CSPROJ with a VERY complex dependency tree. We've been given permission to throw some pretty significant hardware at it in an attempt to speed up build times.

In testing in Azure with 64 Cores / 128GB RAM / 64GB RAM Drive (Shared) [F64s_v2 instance] we have seen build times of around 17 minutes. Comparatively on a local Hyper-V instance on my dev machine VM with 16 Cores (i7-8770) / 36GB RAM / NVMe SSD we see around 30 minutes. Our target is 10 minutes from clean build.

We are hoping that we will avoid seeing the bottlenecking behavior late in the build and will report back.

[aolszowka]

To follow up on this:

While the work around of changing the Project Type GUID in the solution file is working we are encountering a new issue when building via /graphBuild that we do not encounter when building without it. We are working with the vendor through their issue tracking system and will open a new issue based on our findings if we feel it is Microsoft related.

To be clear:

The ask on this issue is a fix to NOT Exclude non-CSPROJ Types when building via /graphBuild

[cdmihai]

The ask on this issue is a fix to NOT Exclude non-CSPROJ Types when building via /graphBuild

That's correct, given my current understanding right now, I think the ideal solution is to mimic vanilla msbuild's behaviour as reasonable as possible. And at the very least, provide warnings for any projects that do get skipped from the solution.

[aolszowka]

@cdmihai With the merge of #5698 this is working (just tested by building Master) for our particular use case. I am OK closing it, but realistically this should probably remain open for any other ISV who has extended MSBuild/Visual Studio and is expecting to get a free ride via the Solution.

[iskiselev]

Looks like same happens with WIX installer project: https://wixtoolset.org/
It also silently excluded from build when -graph mode used, unless it is referenced by some other project.

[aolszowka]

@iskiselev Yup and without the escape hatch (still unimplemented see #5931) you might be out of luck without that reference. For us we added the type on #5698 but that's not a great long term solution, maybe someone over in the WiX Project might be willing to submit a PR?