[dotnet-fake] msbuild doesn't work within dotnet-fake because of MSBUILD_EXE_PATH

[matthid]

Description

I just got some weird build-errors when trying to use dotnet-fake as dotnet-cli tool on AppVeyor. Maybe it was because of dotnet-sdk version issues or something else:

fsprojects/FSharp.Formatting#460

Repro steps

Run fsprojects/FSharp.Formatting@e5901e7 on AppVeyor (https://ci.appveyor.com/project/matthid/fsharp-formatting-32np3/build/1.0.75).

Edit: The error was

C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin\MSBuild.exe  FSharp.Formatting.sln /t:Rebuild /m  /nodeReuse:False  /v:m   /p:RestorePackages="True" /p:VisualStudioVersion="15.0" /p:Verbosity="Minimal" /p:Configuration="Release" /logger:"C:\Program Files\AppVeyor\BuildAgent\Appveyor.MSBuildLogger.dll"
C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin\MSBuild.exe  FSharp.Formatting.sln /t:Rebuild /m  /nodeReuse:False  /v:m   /p:RestorePackages="True" /p:VisualStudioVersion="15.0" /p:Verbosity="Minimal" /p:Configuration="Release" /logger:"C:\Program Files\AppVeyor\BuildAgent\Appveyor.MSBuildLogger.dll"
Microsoft (R) Build Engine version 15.5.180.51428 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.
C:\Program Files\dotnet\sdk\2.1.4\Microsoft.Common.CurrentVersion.targets(1601,5): error MSB4062: The "NuGet.Build.Tasks.GetReferenceNearestTargetFrameworkTask" task could not be loaded from the assembly C:\Program Files\dotnet\sdk\2.1.4\NuGet.Build.Tasks.dll. Could not load file or assembly 'NuGet.Build.Tasks, Version=4.5.0.4, Culture=neutral, PublicKeyToken=31bf3856ad364e35' or one of its dependencies. An attempt was made to load a program with an incorrect format. Confirm that the <UsingTask> declaration is correct, that the assembly and all its dependencies are available, and that the task contains a public class that implements Microsoft.Build.Framework.ITask. [C:\projects\fsharp-formatting-32np3\src\FSharp.Markdown\FSharp.Markdown.fsproj]
C:\Program Files\dotnet\sdk\2.1.4\Microsoft\Microsoft.NET.Build.Extensions\Microsoft.NET.Build.Extensions.NETFramework.targets(56,5): error MSB4062: The "GetDependsOnNETStandard" task could not be loaded from the assembly C:\Program Files\dotnet\sdk\2.1.4\Microsoft\Microsoft.NET.Build.Extensions\\tools\net46\Microsoft.NET.Build.Extensions.Tasks.dll. Could not load file or assembly 'file:///C:\Program Files\dotnet\sdk\2.1.4\Microsoft\Microsoft.NET.Build.Extensions\tools\net46\Microsoft.NET.Build.Extensions.Tasks.dll' or one of its dependencies. The system cannot find the file specified. Confirm that the <UsingTask> declaration is correct, that the assembly and all its dependencies are available, and that the task contains a public class that implements Microsoft.Build.Framework.ITask. [C:\projects\fsharp-formatting-32np3\src\CSharpFormat\CSharpFormat.csproj]
C:\Program Files\dotnet\sdk\2.1.4\Microsoft.Common.CurrentVersion.targets(1601,5): error MSB4062: The "NuGet.Build.Tasks.GetReferenceNearestTargetFrameworkTask" task could not be loaded from the assembly C:\Program Files\dotnet\sdk\2.1.4\NuGet.Build.Tasks.dll. Could not load file or assembly 'NuGet.Build.Tasks, Version=4.5.0.4, Culture=neutral, PublicKeyToken=31bf3856ad364e35' or one of its dependencies. An attempt was made to load a program with an incorrect format. Confirm that the <UsingTask> declaration is correct, that the assembly and all its dependencies are available, and that the task contains a public class that implements Microsoft.Build.Framework.ITask. [C:\projects\fsharp-formatting-32np3\tests\FSharp.Markdown.Tests\FSharp.Markdown.Tests.fsproj]

which indicates maybe some dotnet-cli stuff was overwritten/broken (as the working version executes exactly same command, see https://ci.appveyor.com/project/matthid/fsharp-formatting-32np3/build/1.0.69 for example).

[matthid]

Ok travis seems to have a similar behavior. When using dotnet fake calling msbuild fails (https://travis-ci.org/fsprojects/FSharp.Formatting/builds/337216898?utm_source=github_status&utm_medium=notification):

  /usr/bin/msbuild  FSharp.Formatting.sln /t:Rebuild  /v:m   /p:RestorePackages="True" /p:VisualStudioVersion="15.0" /p:Verbosity="Minimal" /p:Configuration="Release" 
/usr/bin/msbuild  FSharp.Formatting.sln /t:Rebuild  /v:m   /p:RestorePackages="True" /p:VisualStudioVersion="15.0" /p:Verbosity="Minimal" /p:Configuration="Release" 
Copyright (C) Microsoft Corporation. All rights reserved.
MSBUILD : warning MSB4010: The "*.tasks" files could not be successfully loaded from their expected location "/usr/lib/mono/xbuild/15.0/bin". Default tasks will not be available. Could not find a part of the path '/usr/lib/mono/xbuild/15.0/bin'. [/home/travis/build/fsprojects/FSharp.Formatting/FSharp.Formatting.sln]
/home/travis/build/fsprojects/FSharp.Formatting/FSharp.Formatting.sln.metaproj : error MSB4036: The "Message" task was not found. Check the following: 1.) The name of the task in the project file is the same as the name of the task class. 2.) The task class is "public" and implements the Microsoft.Build.Framework.ITask interface. 3.) The task is correctly declared with <UsingTask> in the project file, or in the *.tasks files located in the "/usr/lib/mono/xbuild/15.0/bin" directory. [/home/travis/build/fsprojects/FSharp.Formatting/FSharp.Formatting.sln]
Finished Target: Build
Running build failed.
Error:
Building FSharp.Formatting.sln failed with exitcode 1.

While the exact same commands work in "full" fake (https://travis-ci.org/fsprojects/FSharp.Formatting/builds/337219438?utm_source=github_status&utm_medium=notification)

 /usr/bin/msbuild  FSharp.Formatting.sln /t:Rebuild  /v:m   /p:RestorePackages="True" /p:VisualStudioVersion="15.0" /p:Verbosity="Minimal" /p:Configuration="Release" 
/usr/bin/msbuild  FSharp.Formatting.sln /t:Rebuild  /v:m   /p:RestorePackages="True" /p:VisualStudioVersion="15.0" /p:Verbosity="Minimal" /p:Configuration="Release" 
Copyright (C) Microsoft Corporation. All rights reserved.

I guess when running in dotnet fake (or any dotnet-cli) some environment variables are set to something broken?
/cc @enricosada

[matthid]

The difference between fake run myscript.fsx and dotnet fake run myscript.fsx is:

--- fake_run.txt        2018-02-04 16:09:55.899220100 +0100
+++ dotnet_fake_run.txt 2018-02-04 16:10:02.430051700 +0100
@@ -1,4 +1,4 @@
-$ fake run myscript.fsx
+$ dotnet fake run myscript.fsx
 loading dependencies ...
 'ACLOCAL_PATH' -> 'C:\Program Files\Git\mingw64\share\aclocal;C:\Program Files\Git\usr\share\aclocal'
 'ALLUSERSPROFILE' -> 'C:\ProgramData'
@@ -6,7 +6,7 @@
 'ANSICON_DEF' -> '7'
 'APPDATA' -> 'C:\Users\matth\AppData\Roaming'
 'CLINK_DIR' -> 'C:\Program Files (x86)\clink\0.4.8'
-'COMMONPROGRAMFILES' -> 'C:\Program Files (x86)\Common Files'
+'COMMONPROGRAMFILES' -> 'C:\Program Files\Common Files'
 'COMPUTERNAME' -> 'DESKTOP-FQBAN56'
 'COMSPEC' -> 'C:\Windows\system32\cmd.exe'
 'CONFIG_SITE' -> 'C:/Program Files/Git/mingw64/etc/config.site'
@@ -54,6 +54,7 @@
 'MINGW_CHOST' -> 'x86_64-w64-mingw32'
 'MINGW_PACKAGE_PREFIX' -> 'mingw-w64-x86_64'
 'MINGW_PREFIX' -> 'C:/Program Files/Git/mingw64'
+'MSBUILD_EXE_PATH' -> 'C:\Program Files\dotnet\sdk\2.1.4\MSBuild.dll'
 'MSYSTEM' -> 'MINGW64'
 'MSYSTEM_CARCH' -> 'x86_64'
 'MSYSTEM_CHOST' -> 'x86_64-w64-mingw32'
@@ -70,12 +71,11 @@
 'PATHEXT' -> '.COM;.EXE;.BAT;.CMD;.VBS;.VBE;.JS;.JSE;.WSF;.WSH;.MSC'
 'PKG_CONFIG_PATH' -> 'C:\Program Files\Git\mingw64\lib\pkgconfig;C:\Program Files\Git\mingw64\share\pkgconfig'
 'PLINK_PROTOCOL' -> 'ssh'
-'PROCESSOR_ARCHITECTURE' -> 'x86'
-'PROCESSOR_ARCHITEW6432' -> 'AMD64'
+'PROCESSOR_ARCHITECTURE' -> 'AMD64'
 'PROCESSOR_IDENTIFIER' -> 'Intel64 Family 6 Model 158 Stepping 9, GenuineIntel'
 'PROCESSOR_LEVEL' -> '6'
 'PROCESSOR_REVISION' -> '9e09'
-'PROGRAMFILES' -> 'C:\Program Files (x86)'
+'PROGRAMFILES' -> 'C:\Program Files'
 'PS1' -> '\[\033]0;$TITLEPREFIX:$PWD\007\]\n\[\033[32m\]\u@\h \[\033[35m\]$MSYSTEM \[\033[33m\]\w\[\033[36m\]`__git_ps1`\[\033[0m\]\n$ '
 'PSModulePath' -> 'C:\Program Files\WindowsPowerShell\Modules;C:\Windows\system32\WindowsPowerShell\v1.0\Modules'
 'PUBLIC' -> 'C:\Users\Public'
@@ -98,4 +98,4 @@
 'USERNAME' -> 'matth'
 'USERPROFILE' -> 'C:\Users\matth'
 'WINDIR' -> 'C:\Windows'
-'_' -> 'C:/ProgramData/chocolatey/bin/fake'
\ No newline at end of file
+'_' -> 'C:/Program Files/dotnet/dotnet'
\ No newline at end of file

retrieved with

seq { for d in System.Environment.GetEnvironmentVariables() do let de = d :?> System.Collections.DictionaryEntry in yield (de.Key.ToString(), de.Value.ToString()) }
    |> Seq.sortBy fst
    |> Seq.iter (fun (key, value) -> printfn "'%O' -> '%O'" key value)

The 'MSBUILD_EXE_PATH' looks promising

[matthid]

The following workaround at the start of the build script seems to work:

// Workaround https://github.com/fsharp/FAKE/issues/1776
System.Environment.SetEnvironmentVariable("MSBUILD_EXE_PATH", null)

I verified that this workaround indeed works by checking that fsprojects/FSharp.Formatting@ec73945 is green as well.

[matthid]

Question now is: Is this a fake or a dotnet-sdk bug?

[matthid]

I have asked the core-developers about this: https://github.com/dotnet/cli/issues/8530

[dasMulli]

for reference, omnisharp and a few other tools reset both MSBUILD_EXE_PATH as well as MSBuildExtensionsPath when calling a different instance of msbuild than described by MSBUILD_EXE_PATH or when calling into the .net cli tools.

This environment variable may also be set when doing the inverse and trying to set up a build environment with custom msbuild locations..

Not sure what's a bug or feature here (.net cli, fake, msbuild,..), but the safe way is always to either use the msbuild version of MSBUILD_EXE_PATH or reset those two env vars..

[matthid]

 Unhandled Exception: System.BadImageFormatException: An attempt was made to load a program with an incorrect format. (Exception from HRESULT: 0x8007000B)

Is another instance of this.

[matthid]

@dasMulli thanks a lot I'll add that logic to FAKE, interesting that we never have noticed this in all the years. I hope that we don't have some users who want to set those environment variables.... This will be impossible/hard with any patch in FAKE.

[cdrnet]

Would this prevent me from using Mono's MsBuild through FAKE?

[matthid]

@cdrnet Are you setting one of these Environment variables by hand?

[enricosada]

in the library for FSAC, i do the same, resetting msbuild vars

https://github.com/enricosada/dotnet-proj-info/blob/1cc3a7343327603d44f3b93b51027f3e20dd98ba/src/dotnet-proj-info/Program.fs#L81-L89

but that's because i explicit invoke msbuild tools. FSAC need to add a config for that too, atm use msbuild and dotnet msbuild (no path)

probably you can just add a setting to:

• use current env vars
• use specifici msbuild path (so override env vars).

[matthid]

Ok thanks, so in addition to the change we can add an option to the msbuild parameters in FAKE.
Default will be to remove the two environment variables (as indicated in the patch).
But I'll add an option to set those variables by hand.

This way even if it breaks someone there is a workaround.

[volaticus]

Sorry for commenting on a closed issue but I'm still able to reproduce this issue while building f sharp project with netcoreapp2.0 as target framework. All FAKE libraries updated to 5.0.0-beta014.
Exception:
C:\Program Files\dotnet\sdk\2.1.4\FSharp\Microsoft.FSharp.Targets(181,9): error MSB4062: The "FSharpEmbedResXSource" task could not be loaded from the assembly C:\Program Files\dotnet\sdk\2.1.4\FSharp\FSharp.Build.dll. Could not load file or assembly 'FSharp.Build, Version=4.4.1.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' or one of its dependencies.

Only maual reset of MSBUILD_EXE_PATH to null helps e.g. System.Environment.SetEnvironmentVariable("MSBuildExtensionsPath", null) as the step before calling MsBuild.Run.

Thanks.

[matthid]

Maybe I failed somewhere?

[matthid]

@volaticus Can you tell me how I can reproduce this?

[0x53A]

I set it for the Paket build: https://github.com/fsprojects/Paket/blob/master/build.cmd#L10

Nevermind, these are different vars

[volaticus]

I think the problem is in module Fake.Core.Process with the removeEnvironmentVariable method.
As I understand it just removes MSBUILD_EXE_PATH from the process env variables map and when the process starts it just unable to find this override and uses global env variable.
When I changing this method to update MSBUILD_EXE_PATH value to empty string all works as expected.

[matthid]

@volaticus Yes, thanks I kind of failed there :/
Relevant ongoing discussion: 012aa9c

[matthid]

After trying a bunch of different implementation and looking at https://github.com/dotnet/corefx/blob/master/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs I think the only reasonable way is a breaking change in the Process Module by changing Environment to an Map<string, string> and pre-fill it with the current environment variables (Just like ProcessStartInfo is doing it). The same goes for the MsBuild Paramters (they will then just replace the one from the ProcessStartInfo).

Everything boils down to: Currently the Process module doesn't support this use-case.
Everything else will be a hack or unnecessary complex

[matthid]

I even experimented with

type EnvironmentVariableState =
    | SetEnvironmentVariable of string
    | UnsetEnvironmentVariable

type ProcStartInfo = {
    Environment : Map<string, EnvironmentVariableState > }

[0x53A]

How would the user code look like for Map<string,string>? This is the best I could come up with:

type ProcStartInfo = {
    Environment : Map<string, string > }

let upd (psi:ProcStartInfo) =
    { psi with
        Environment =
            (psi.Environment)
            |> Map.add "foo" "bar"
            |> Map.add "x" "y"
            |> Map.add "a" "b"

}

I guess it isn't too bad ...

But the danger is that people will create a new map with just their new values, instead of updating the existing map.

[matthid]

Either that or

let upd (psi:ProcStartInfo) =
    psi
    |> Process.setEnvironmentVariable "foo" "bar"
    |> Process.setEnvironmentVariable "x" "y"
    |> Process.setEnvironmentVariable "a" "b"

or

let upd (psi:ProcStartInfo) =
    psi
    |> Process.setEnvironmentVariables [ "foo","bar"; "x","y"; "a","b" ]

[0x53A]

I'm not sure I like it myself, but what do you think of

type ProcStartInfo = {
    Environment : (Map<string, string> -> Map<string, string>) option }

let upd (psi:ProcStartInfo) =
    { psi with
        Environment = Some (fun env ->
            env
            |> Map.add "foo" "bar"
            |> Map.add "x" "y"
            |> Map.add "a" "b")
}

?

At least it would be (imo) pretty obvious that this is an update operation, not a replacement.

[matthid]

Therefore, my next suggestion will be 3bdf101

[matthid]

type ProcStartInfo = {
    Environment : (Map<string, string> -> Map<string, string>) option }

let upd (psi:ProcStartInfo) =
    { psi with
        Environment = Some (fun env ->
            env
            |> Map.add "foo" "bar"
            |> Map.add "x" "y"
            |> Map.add "a" "b")
}

My problem with that is that we have too much callbacks here, The ProcStartInfo is already in a callback where the user should modify things. I don't think that makes it any easier mentally.
I might be wrong ...

[0x53A]

nah, I guess you are right. I was just thinking about ways to make it obvious to the user that they need to update it, not replace it.

[matthid]

I added a comment to not set this field directly but use the helper methods ...

[0x53A]

At least it will probably fail relatively early if there is no temp, no path, etc

[0x53A]

Or - wild suggestion - check a few predefined (by windows) env variables, and throw if they are missing.

E.g., check if the Map contains at least path and temp. If both are missing, don't even Process.Start, just fail hard.

[matthid]

We could maybe add some hidden default variable (which is ignored later) and check if that one is still set :)

[matthid]

@0x53A Can you review a5bd92f I hope that works and the error message is reasonable :)

[0x53A]

@matthid yeah, looks good.

So, if I, as a user, want to reuse the dotnet-cli MSBuild, I would

1. manually read MSBUILD_EXE_PATH from inside fake
2. pass that to MSBuildParams.ToolPath

If you wanted to keep source-compatibility, you could keep the old Property, but make it Obsolete.

[matthid]

So, if I, as a user, want to reuse the dotnet-cli MSBuild, I would

No that wouldn't work as the path is C:\Program Files\dotnet\sdk\2.1.4\MSBuild.dll and you cannot start a .dll?

[matthid]

Currently you would use dotnet build (and the helpers there) instead.

[enricosada]

So, if I, as a user, want to reuse the dotnet-cli MSBuild, I would

run dotnet msbuild, is a valid command. there are helper for dotnet command

[matthid]

I think this is finally fixed now with beta015. Note that beta015 has a broken Cli.DotNetPack which I'm trying to fix right now, but the workaround is to use Cli.Dotnet with the correct parameters by hand. Sadly I just run into dotnet/fsharp#4357 (comment) which means it will take some minutes until I have 016 out.

[matthid]

Wow this is coming back again for the Dotnet helpers as well :/