Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unix msbuild probing enhancements #1488

Merged
merged 2 commits into from
Mar 13, 2017

Conversation

baronfel
Copy link
Contributor

This is a refactor of the logic that retrieves the path to MSBuild on various platforms, primarily inspired by the fact that MSBuild now works cross platform on Mono, yay!

First off, I apologize for the whitespace changes, it seems that the file saves/git interactions on OSX were pretty zealous on this one. Consider using the ?w=1 query string when reviewing.

Copying my comment here to talk about the change:

/// Tries to detect the right version of MSBuild.
///   - On all OS's, we check a `MSBuild` environment variable which is either
///     * a direct path to a file to use, or
///     * a directory that contains a file called `msbuild` on non-Windows systems, or `MSBuild.exe` on Windows systems
///   - In addition, on non-Windows systems we check the current PATH for the following binaries, in this order:
///     * `msbuild`
///     * `xbuild`
///   - In addition, on Windows systems we
///     * try to read the MSBuild tool location from the AppSettings file using a parameter named `MSBuild`, and finally
///     * if a `VisualStudioVersion` environment variable is specified, we try to use the specific MSBuild version, matching that Visual Studio version.

The biggest change here is that we're sharing the environment variable configuration searching xplat now, and then we're searching the PATH on non-windows for msbuild and then xbuild. This allows for the kind of PATH-manipulation shenanigans that *nix users are used to so define which binaries should be run.

@baronfel
Copy link
Contributor Author

Unrelated but I ran into a host of issues building this locally on Mono 4.8 that I'll try to run down in more detail and send PRs for if I find anything more concrete.

@forki forki merged commit add59b8 into fsprojects:master Mar 13, 2017
@forki
Copy link
Member

forki commented Mar 13, 2017

Mhm I updated FAKE itself to the new alpha version with this change and now it doesn't build for me any more:

image

@forki
Copy link
Member

forki commented Mar 13, 2017

Ok I think it's unrelated

@dsyme
Copy link
Collaborator

dsyme commented Apr 29, 2017

@baronfel I'm getting a problem using the latest FAKE on Linux/macOS with Mono 4.8 and Mono latest (currently 4.8.1)

The latest FAKE seems to be invoking MSBuild instead of XBuild on these platforms. That's fine, but using the Unix msbuild (at least from Mono 4.8.*) seems to fail when importing the targets files for F#, particularly if resources are involved. Specifically when compiling FSharp.Compiler.Service with (latest FAKE and FSharp.,Compiler.Tools](https://travis-ci.org/fsharp/FSharp.Compiler.Service/jobs/227043065) using this:

  /Library/Frameworks/Mono.framework/Versions/Current/Commands/msbuild  /Users/travis/build/fsharp/FSharp.Compiler.Service/FSharp.Compiler.Service.sln /t:Build    /p:RestorePackages="False" /p:Configuration="Release" /p:TargetFrameworkVersion="v4.5" /logger:Fake.MsBuildLogger+ErrorLogger,"/Users/travis/build/fsharp/FSharp.Compiler.Service/packages/FAKE/tools/FakeLib.dll"

I get

/Users/travis/build/fsharp/FSharp.Compiler.Service/packages/FSharp.Compiler.Tools/tools/Microsoft.FSharp.Targets(125,13): 
error MSB4131: The "ResourceFilesWithManifestResourceNames" parameter is not supported by 
the "CreateFSharpManifestResourceName" task. Verify the parameter exists on the task, and it 
is a gettable public instance property. 
[/Users/travis/build/fsharp/FSharp.Compiler.Service/src/fsharp/FSharp.Compiler.Service/FSharp.Compiler.Service.fsproj]

I have no idea why this would suddenly start failing and my guess is that this seems likely to be an MSBuild issue - perhaps it is fixed in 5.0.0.

But the problem is that I believe the latest FAKE exposes people to this by switching them to MSBuildd instead of xbuild? Should we avoid using MSBuild instead of XBuild on MOno 4.8.x and before?

@dsyme
Copy link
Collaborator

dsyme commented Apr 29, 2017

@baronfel I've raised this as a separate issue here: #1539

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants