Wrong path for build tools when multitargeting

[daveaglick]

Seeing this error on CI builds:

C:\Program Files\dotnet\sdk\2.0.2\Sdks\Microsoft.NET.Sdk\Sdk\Sdk.props(29,3): error MSB4019: The imported project "C:\Program Files\dotnet\sdk\2.0.2\2.0\Microsoft.Common.props" was not found. Confirm that the path in the <Import> declaration is correct, and that the file exists on disk.

The import in Sdk.props looks like this:

<Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" />

It appears that MSBuildToolsVersion is getting set to "2.0" instead of "15.0" for some reason.

Found a related issue in DocFx that perfectly describes this behavior here: dotnet/docfx#1752 (comment) (Buildalyzer already sets the MSBuildExtensionsPath).

[daveaglick]

Interestingly, this only appears to affect loading of one project. One of the only differences between that project and the others is that it's multi-targeted:

<TargetFrameworks>net46;netstandard1.6</TargetFrameworks>

So some combination of multi-targeting and the CI server appears to cause the issue.

[jp2masa]

I think that's the error message I was getting when I reported #16, so I'll close it.

[daveaglick]

@jp2masa Thanks - good to consolidate issues whenever we can. I've been working on this and have a failing test for it so hopefully I'll figure it out soon.

[ReubenBond]

Currently I'm working around this with a particularly nasty hack inspired by one of the linked threads:

// HACK: https://github.com/daveaglick/Buildalyzer/issues/29

var dotnetSdkVersion = "2.0.0";
var extensionsPath = $@"C:\Program Files\dotnet\sdk\{dotnetSdkVersion}";
var msbuildSdkPath = $@"{extensionsPath}\SDKs";
Environment.SetEnvironmentVariable("VisualStudioVersion", "15.0");
Environment.SetEnvironmentVariable("MSBuildExtensionsPath", extensionsPath);
Environment.SetEnvironmentVariable("MSBuildSDKsPath", msbuildSdkPath);
Environment.SetEnvironmentVariable("VSINSTALLDIR", @"C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\");

// END

[daveaglick]

Interesting clue: when running from the test runner under .NET Core I get the following error

C:\Program Files\dotnet\sdk\2.1.2\Sdks\Microsoft.NET.Sdk\Sdk\Sdk.targets(41,3): error MSB4019: The imported project "E:\NuGet\microsoft.testplatform.testhost\15.5.0\lib\netstandard1.5\Microsoft.CSharp.targets" was not found. Confirm that the path in the <Import> declaration is correct, and that the file exists on disk.

It's the same message but the path where it's looking for Microsoft.CSharp.targets looks like it's relative to the working path.

[daveaglick]

Getting closer - dumped a binary log of the failing build and noticed this:

However, MSBuildToolsVersion is set when the build begins:

[daveaglick]

And this seems to confirm my initial thought - it's specifically multitargeting that resets the tools version. It's set correctly to "15.0" in the outer build but then the inner builds for each target use "2.0":

[daveaglick]

Opened an upstream issue at dotnet/msbuild#2846

[daveaglick]

A thought: maybe it makes sense to convert a multi-targeted project to a single target for Buildalyzer purposes. Is there a benefit to running the build with all the targets - if we're mainly looking for MSBuild properties like source files and csc arguments maybe we don't care about all the targets.

[daveaglick]

Found this in Microsoft.Common.CrossTargeting.targets:

  <Target Name="DispatchToInnerBuilds"
          DependsOnTargets="_ComputeTargetFrameworkItems"
          Returns="@(InnerOutput)">
    <!-- If this logic is changed, also update Clean -->
    <MSBuild Projects="@(_InnerBuildProjects)"
             Condition="'@(_InnerBuildProjects)' != '' "
             Targets="$(InnerTargets)"
             BuildInParallel="$(BuildInParallel)">
      <Output ItemName="InnerOutput" TaskParameter="TargetOutputs" />
    </MSBuild>
  </Target>

The MSBuild task is described here: https://docs.microsoft.com/en-us/visualstudio/msbuild/msbuild-task. It doesn't look like the optional Properties parameter is being used to pass the current properties down to the inner build(s). That means anything we tweak in the outside build by setting properties via the MSBuild APIs probably isn't making it to the inner builds.

[jp2masa]

I don't think that the properties need to be passed to the task, I already had some problems related to that. At least TargetFramework is set as a global property by the MSBuild task, so I assume that all properties are "inherited" as global properties from the project that calls the MSBuild task.

[daveaglick]

Resolved with the latest commit - ended up stripping multiple targets from project files and just building the first target.