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

MsBuild madness #176

Closed
Krzysztof-Cieslak opened this issue Jun 29, 2017 · 5 comments
Closed

MsBuild madness #176

Krzysztof-Cieslak opened this issue Jun 29, 2017 · 5 comments

Comments

@Krzysztof-Cieslak
Copy link
Member

Looks like it's windows only issue, mono as always works better.

So at the moment, FSAC doesn't work if MsBuild 14 is not installed - what's kinda annoying for users . Main 2 problems are:

  • if someone has just VS 2017 installed [that installs MsBuild 15] they need to install MsBuild 14 additionally;
  • MsBuild 15 is also latest version available using chocho so it forces users to provide specific version... and not everyone reads readme and it's constant source of problems ;)

The question is (and I guess I should know answer... ) why we even depend on specific MsBuild version?
My best bet would be that it's explicit ProjectCracker dependency - https://github.com/fsharp/FSharp.Compiler.Service/blob/master/src/fsharp/FSharp.Compiler.Service.ProjectCrackerTool/ProjectCrackerTool.fs#L127-L129

Couple of questions:
Will ProjectCracker work with MsBuild 15? Fix should be as simple as fixing this MsBuild lookup I've linked above
Or maybe @enricosada's tool for cracking new fsprojs could also work with old fsprojs? In my experience it's working better than ProjectCracker

CC: @dsyme

@rneatherway
Copy link
Contributor

Or maybe @enricosada's tool for cracking new fsprojs could also work with old fsprojs? In my experience it's working better than ProjectCracker

If this is possible, I think this is the best way to go, focusing on one method.

The question is (and I guess I should know answer... ) why we even depend on specific MsBuild version?

You are right, it is the ProjectCracker. The dependency is a compile-time one (https://github.com/fsharp/FSharp.Compiler.Service/blob/master/src/fsharp/FSharp.Compiler.Service.ProjectCrackerTool/FSharp.Compiler.Service.ProjectCrackerTool.fsproj#L55). We could update it to link against MSBuild 15 and then try to use 14 (and still 12?) as backups.

Testing that it still works on various combinations of MSBuild being installed is pretty difficult though. However, if the new cracker won't do the job I think that is the best solution.

@dsyme
Copy link
Contributor

dsyme commented Jun 29, 2017

As context:

  • Mono still ships with its MSBuild 14 implementation (xbuild) integrated. One distant day they will remove it, they already warn about it when you run xbuild from the command line in Mono 5.0. However I suspect that MSBuild 14 and xbuild won't actually go away from Mono for a long time. This is one reason why MSBuild 14 is not such a bad dependency to choose - it gives a zero-install experience on Linux, where installing things is hard.
  • Mono 5.0 also now ships with MSBuild 15, and uptake rates of Mono 5.0 seem fairly high.

One huge win would be if you could just encapsulate the dependency within FsAutoComplete or Ionide. For example ship your own encapsulated MSBuild 15. That must now be possible, since VS2017 ships its own isolated editions for Community, Professional etc. Since Ionide is an editing toolchain it would make sense to do the same isolated thing. And it would mean you are much less exposed to this dependency going forward.

@enricosada
Copy link
Contributor

where is msbuild?

some work from omnisharp team we can reuse
OmniSharp/omnisharp-roslyn#988 (comment)

@enricosada
Copy link
Contributor

As a note, dotnet.projinfo now parse old fsproj too:

  • require just the msbuild/xbuild path.
  • no libs loading/redirect, is just a command line invocation, like new sdk parsing.
  • no need to setup env var, like VisualStudioVersion

that should fix the issue, using the msbuild choosen by user or latest installed.
if build with msbuild, will work

@Krzysztof-Cieslak
Copy link
Member Author

Sounds good to me.

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

No branches or pull requests

4 participants