Using /p:RestoreUseStaticGraphEvaluation=true Results in Value Cannot Be Null

[aolszowka]

As per #8791 we have been testing with the optional /p:RestoreUseStaticGraphEvaluation=true to speed up restores. However we've encountered an issue when this is sent to a project which does not contain any PackageReferences to restore.

The command line is:

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

The redacted error is:

Microsoft (R) Build Engine version 16.5.0+d4cbfca49 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.

Build started 3/9/2020 8:44:50 AM.
     1>Project "C:\REDACTED\Utilities.sln" on node 1 (Restore target(s)).
     1>ValidateSolutionConfiguration:
         Building solution configuration "Release|Any CPU".
       Restore:
         Determining projects to restore...
         Evaluated 3 project(s) in 131ms (3 builds, 0 failures).
     1>C:\Program Files (x86)\Microsoft Visual Studio\2019\Preview\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.RestoreEx.targets(10,5): error : Value cannot be null. [C:\REDACTED\Utilities.sln]
C:\Program Files (x86)\Microsoft Visual Studio\2019\Preview\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.RestoreEx.targets(10,5): error : Parameter name: path2 [C:\REDACTED\Utilities.sln]
     1>Done Building Project "C:\REDACTED\Utilities.sln" (Restore target(s)) -- FAILED.

Build FAILED.

       "C:\REDACTED\Utilities.sln" (Restore target) (1) ->
       (Restore target) ->
         C:\Program Files (x86)\Microsoft Visual Studio\2019\Preview\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.RestoreEx.targets(10,5): error : Value cannot be null. [C:\REDACTED\Utilities.sln]
       C:\Program Files (x86)\Microsoft Visual Studio\2019\Preview\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.RestoreEx.targets(10,5): error : Parameter name: path2 [C:\REDACTED\Utilities.sln]

    0 Warning(s)
    1 Error(s)

Time Elapsed 00:00:00.70

Removing the /p:RestoreUseStaticGraphEvaluation=true fixes the issue. As mentioned above this solution actually doesn't have any projects that require a package restore, however out of habit I type /restore which means I've started to type /p:RestoreUseStaticGraphEvaluation=true as well. To work around the issue simply avoid the above.

[jeffkl]

Can you please attach a minimal repro that I can investigate?

[aolszowka]

I have been unable to make a minimal repo that reproduces the issue yet (the solution in question has ~3500 projects).

I have been able to make it further in debugging and can point to this stack trace as being the culprit:

 	mscorlib.dll!System.IO.Path.Combine(string path1, string path2) Line 1199	C#
 	NuGet.Build.Tasks.Console.exe!NuGet.Build.Tasks.Console.MSBuildStaticGraphRestore.GetRestoreOutputPath(NuGet.Build.Tasks.Console.IMSBuildProject project)	Unknown
>	NuGet.Build.Tasks.Console.exe!NuGet.Build.Tasks.Console.MSBuildStaticGraphRestore.GetProjectRestoreMetadataAndTargetFrameworkInformation(NuGet.Build.Tasks.Console.IMSBuildProject project, System.Collections.Generic.IReadOnlyDictionary<NuGet.Frameworks.NuGetFramework, NuGet.Build.Tasks.Console.IMSBuildProject> projectsByTargetFramework, NuGet.Configuration.ISettings settings)	Unknown
 	NuGet.Build.Tasks.Console.exe!NuGet.Build.Tasks.Console.MSBuildStaticGraphRestore.GetPackageSpec(NuGet.Build.Tasks.Console.IMSBuildProject project, System.Collections.Generic.IReadOnlyDictionary<string, NuGet.Build.Tasks.Console.IMSBuildProject> allInnerNodes)	Unknown
 	NuGet.Build.Tasks.Console.exe!NuGet.Build.Tasks.Console.MSBuildStaticGraphRestore.GetDependencyGraphSpec.AnonymousMethod__0(NuGet.Build.Tasks.Console.ProjectWithInnerNodes project)	Unknown
 	mscorlib.dll!System.Threading.Tasks.Parallel.ForWorker.AnonymousMethod__1() Line 1193	C#
 	mscorlib.dll!System.Threading.Tasks.Task.InnerInvokeWithArg(System.Threading.Tasks.Task childTask) Line 2899	C#
 	mscorlib.dll!System.Threading.Tasks.Task.ExecuteSelfReplicating.AnonymousMethod__0(object value) Line 2624	C#
 	mscorlib.dll!System.Threading.Tasks.Task.Execute() Line 2498	C#
 	mscorlib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx) Line 954	C#
 	mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx) Line 902	C#
 	mscorlib.dll!System.Threading.Tasks.Task.ExecuteWithThreadLocal(ref System.Threading.Tasks.Task currentTaskSlot) Line 2827	C#
 	mscorlib.dll!System.Threading.Tasks.Task.ExecuteEntry(bool bPreventDoubleExecution) Line 2767	C#
 	mscorlib.dll!System.Threading.ThreadPoolWorkQueue.Dispatch() Line 820	C#
 	[Native to Managed Transition]	

The source in question is here:

https://github.com/NuGet/NuGet.Client/blob/9f9870a6b47f7918524d68e52ce21c422ce1af12/src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs#L428-L433

The project does not have either of these properties:

string outputPath = project.GetProperty("RestoreOutputPath") ?? project.GetProperty("MSBuildProjectExtensionsPath");

Meaning outputPath comes back as null hence the exception. What did the vendor miss here?

Here's a redacted version of the project file, but I suspect without all the targets its useless to you:

<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="15.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <Import Project="..\Synergy.Traditional.PropertyGroups.msbuild" />
  <PropertyGroup>
    <AssemblyName>Repository</AssemblyName>
    <CommonPropertiesFileLocation>..\Synergy.Traditional.PropertyGroups.msbuild</CommonPropertiesFileLocation>
    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
    <EnableCommonProperties>True</EnableCommonProperties>
    <IncludeDebugInformation>False</IncludeDebugInformation>
    <Name>Repository</Name>
    <OutputName>Repository</OutputName>
    <OutputType>RPS</OutputType>
    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
    <Prefer32Bit>False</Prefer32Bit>
    <ProjectGuid>{CAE85A2A-F058-464F-9D34-CC16643EC756}</ProjectGuid>
    <ProjectTypeGuids>{1BD24377-84D3-44B8-B8F3-81C1EB3E22B4};{BBD0F5D1-1CC4-42FD-BA4C-A96779C64378}</ProjectTypeGuids>
    <RepositoryOutput Condition=" '$(RepositoryOutput)' == ''">..\..\bin\rpsdat\</RepositoryOutput>
    <RootNamespace>Repository</RootNamespace>
  </PropertyGroup>
  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
    <LanguageCompatibilityLevel>10030303</LanguageCompatibilityLevel>
    <MOutputName>rpsmain.eng</MOutputName>
    <OutputPath>$(RepositoryOutput)</OutputPath>
    <PlatformTarget>x86</PlatformTarget>
    <TargetRuntimeLevel>10030303</TargetRuntimeLevel>
    <TOutputName>rpstext.eng</TOutputName>
    <UnevaluatedOutputPath>$(RepositoryOutput)</UnevaluatedOutputPath>
  </PropertyGroup>
  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
    <LanguageCompatibilityLevel>10030303</LanguageCompatibilityLevel>
    <MOutputName>rpsmain.eng</MOutputName>
    <OutputPath>$(RepositoryOutput)</OutputPath>
    <PlatformTarget>x86</PlatformTarget>
    <TargetRuntimeLevel>10030303</TargetRuntimeLevel>
    <TOutputName>rpstext.eng</TOutputName>
    <UnevaluatedOutputPath>$(RepositoryOutput)</UnevaluatedOutputPath>
  </PropertyGroup>
  <ItemGroup>
    <Compile Include="temp.schema" Priority="20" />
  </ItemGroup>
  <Import Project="$(MSBuildExtensionsPath)\Synergex\dbl\Synergex.SynergyDE.Repository.targets" />
  <PropertyGroup>
    <PostBuildEvent></PostBuildEvent>
    <PreBuildEvent></PreBuildEvent>
  </PropertyGroup>
  <Target Name="FixSDI2015Bug" BeforeTargets="CoreCompile">
    <Message Text="Fixing Bug Caused By SDI 2501 Not Deleting the Temp Schema" />
    <ItemGroup>
      <TempSchemaFile Include="$(IntermediateOutputPath)\**\temp.scm" />
    </ItemGroup>
    <Delete Files="@(TempSchemaFile)" />
  </Target>
</Project>

Strangely enough attempting to build just this project with the same switches throws no errors?

[jeffkl]

Perhaps the best workaround is to set MSBuildProjectExtensionsPath to a value like its set in the SDK.

https://github.com/dotnet/msbuild/blob/master/src/Tasks/Microsoft.Common.props#L50

The project is ignored if it doesn't have a _IsProjectRestoreSupported target as well. How is that target set for the project that fails?

[aolszowka]

Perhaps the best workaround is to set MSBuildProjectExtensionsPath to a value like its set in the SDK.

Near as I can tell this works; I will let the vendor know

The project is ignored if it doesn't have a _IsProjectRestoreSupported target as well. How is that target set for the project that fails?

Normally I would be able to see this in https://github.com/KirillOsenkov/MSBuildStructuredLog however because this is happening outside of pure MSBuild I can't tell you for sure where this comes from (in the sense that I cannot tell you how it is inherited).

Looking at the debugger it looks like at very least the target exists and is being pulled in from NuGet.targets (C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.targets); perhaps an oversight when they implemented this functionality? I can ask the vendor and report back if its important enough?

What about having some sane default fall back for scenarios like this? or do you think this is uncommon enough?

[jeffkl]

If the project imports NuGet.targets, it should also import Microsoft.Common.props which would declare MSBuildProjectExtensionsPath and fix this issue as well.

[aolszowka]

Thanks @jeffkl I am going to close this issue as resolved. Feel free to re-open this or create a new issue if there is code changes to make this more defensive in the above linked method.

[zivkan]

I think we should keep this open until the error message is fixed. I'm fine with the restore failing/crashing, but it should say the issue is with MSBuildProjectExtensionsPath, not path2.

[jeffkl]

Agreed, I'll add a guard against this scenario