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

Update fsharp deployments to include simple Microsoft.NET.Sdk.FSharp.props #2993

Merged
merged 11 commits into from
May 7, 2017
Merged

Update fsharp deployments to include simple Microsoft.NET.Sdk.FSharp.props #2993

merged 11 commits into from
May 7, 2017

Conversation

KevinRansom
Copy link
Member

  1. Add it to Compiler Redist for VS and MSBuild deployment
  2. Add it to Microsoft.Compiler nuget package for dotnet cli deployment

Approach:

  • The SDK and VS will both ship shim targets and props files that tell msbuild where to find the compiler.
  • When building using the Desktop MSbuild (VS and MSBuild both do this) use the Desktop fsc.exe
  • When building using the dotnet cli use the coreclr fsc.exe.
  • Detects when the FSharp.SDK is used and gets out of it's way, to ensure they continue to build.

The VS experience with legacy projects will be unchanged, I.e. everything that used to work will still work because nothing has changed.

This adds support for building dotnet SDK projects using the desktop compiler.

  • Microsoft.NET.Sdk.FSharp.props is currently a minimal implementation it will probably need some fleshing out.

It's purpose is for settings that are specific to FSharp on the SDK.


<PropertyGroup Condition=" '$(MSBuildRuntimeType)' == 'core' ">
<FscToolPath Condition="'$(FscToolPath)' == ''">$(MSBuildThisFileDirectory)</FscToolPath>
<FscToolExe Condition="'$(OS)' != 'Unix' and '$(FscToolExe)' == ''">RunFsc.cmd</FscToolExe>
Copy link
Contributor

@dsyme dsyme May 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where/what are RunFcs.cmd and RunFsc.sh? Do we really have to shell out via a command shell to invoke a compiler? It feels like something is flawed if that's the case and using extra shell scripts always brings pain - and especially CMD scripts. Something always goes wrong, e.g. spaces in paths or strange characters in file names.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dsyme, this is the exact way that C# and VB do it. We have an extra property on our build task, but I figured following C# and VB would be preferable.e

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, thanks

@KevinRansom
Copy link
Member Author

@dotnet-bot test this pleas

@dsyme
Copy link
Contributor

dsyme commented May 5, 2017

I took a look at src/fsharp/FSharp.Build/Microsoft.NET.Sdk.FSharp.props being added here and compared it to what I think is the corresponding file in FSharp.NET.Sdk

These are the differences I see

  • FSharp.NET.Sdk sets DefaultProjectTypeGuid
  • FSharp.NET.Sdk sets FSharpLanguageTargets to FSharp.NET.Current.Sdk.targets.
  • This file conditionally sets FscToolPath

Please consider each of these to confirm we aren't missing anything here, thanks.

@KevinRansom
Copy link
Member Author

@dsyme

  1. I DefaultProjectTypeGuid is for when dotnet cli generates solutions for projects. I haven't considered that yet.

  2. We don't have an equivalent of: FSharp.NET.Current.Sdk.targets. There is a bunch of dotnetsdk related build logic in: Microsoft.NET.sdk.FSharp.targets in the targets file. Currently it also has some stuff which I now consider should probable go into Microsoft.FSharp.targets

  3. FscToolPath is how the build task finds the program to execute, fsc.exe or dotnet.exe or in this case runfsc.cmd and runfsc.sh. Allowing FscToolPath to be overridden, which the conditionality gives us, allows us to point to a different fsharp compiler implementation. And does it in a way that should be familiar to fsharp developers since it's always been a configuration point. I expect to use it in the OSS build for proto / lkg invocation amongst other things.

@dsyme
Copy link
Contributor

dsyme commented May 6, 2017

@KevinRansom There seems to be a massive backlog on the CI system today - any ideas why?

@KevinRansom
Copy link
Member Author

@dsyme,

No, I don't

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