Automagically fix up choose block when importing FSharp.Compiler.Tools

[0x53A]

FSharp.Compiler.Tools includes it's own copy of Microsoft.FSharp.Targets, but due to how the current previous (the latest one are ok) fsproj templates are defined, it will always use the system-defined one.

We would need to replicate the changes from dotnet/fsharp#2575.

I'm not a big fan of including package specific logic in paket, but IMO this is one of the few places where it would be worth it.

What do you think?

[matthid]

Oh come on wtf

[0x53A]

Can I take that as a no? :D

[matthid]

Nah. I just see a bunch of issues, reverts and a bunch of broken users in my imaginary eye.

So it is recommended to use FSharp.Compiler.Tools now instead of the installed version? Or whats the reasoning there?

[matthid]

My point is we can't even get targets and props files correct in the current state. Adding special logic around some packages will not improve that :) But if we need to do it for some reason we probably will (as always)...

[matthid]

Or asked differently: Why can users using the package not manually edit their msbuild files? Or have a different template?

[0x53A]

FSharp.Compiler.Tools is great because it means you can use the latest compiler on a buildserver without requiring visual studio 2017.

E.g. our buildservers are still on VS 2015, but with this I can use all the latest F# features.

Sure, users can always hand-edit their fsproj, and with the latest templates (F# nightly, will be included in VS15.3 I think), that won't even be necessary.

So doing nothing is absolutely ok, and as you point out, may even be the best choice.

I just wanted to throw this out and get a few different opinions. =)

[matthid]

FSharp.Compiler.Tools is great because it means you can use the latest compiler on a buildserver without requiring visual studio 2017.

I completely agree and we might find a way to embrace that! Maybe we can have some separated parameter of "install" which does the setup to FSharp.Compiler.Tools and checks at the same time if it's part of the solution.
This would have a couple of benefits to doing it automatically (or maybe we can integrate later when its stable?).

[0x53A]

My reason for doing it is also a great reason against it: MSBuild is a bitch, and no normal user wants/is able to mess with it.

So on one hand, it would be nice to do this relatively simple-and-safe transform so the user doesn't need to bother, on the other hand, if this causes an issue later, it will be a bitch to track down for the user ...

Maybe a first step would be a warning? I don't think a separate command is useful, because no one would probably use it ...

[matthid]

Shouln't this be an active decision? I don't want my build behavior changed only because FSharp.Compiler.Tools was pulled as a transitive dependency somewhere (via FSharp.SDK <- paket.core remember ;) ).

On the other side maybe it won't hurt...

[0x53A]

I don't think this should be an active decision, because if you know that this may be necessary, then you can just hand-edit it. I wanted to automate it because most users probably don't (and shouldn't need to) know about this.

If it was pulled in transitively, then it probably has a conditional include (for netstandard, or whatever), which means even after the transformation, the system compiler and system targets file are used.

Atm I can't think of any scenarios where this transformation would create a breaking change, but you never know =)

If we take this to the extreme, we may also want to add logic to ngen fsc.exe inside the package :D

[0x53A]

Thinking about this some more, and also about your suggestion for a special command, I would like a way to mark a package as global for a specific project type.

E.g. I want to mark FSharp.Compiler.Tools as global for all fsproj, and Microsoft.Net.Compilers as global for all csproj.

Then this special command could take care of the transformation, and in this case it should be safe because there is explicit user intention.

[0x53A]

@matthid Ok, forget it, I found another issue with installing targets / props, so don't want to open a second can of worms now with this ...