Skip to content

simplify defines and move FCS to .NET Standard 2.0 #4368

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

Merged
merged 20 commits into from
Feb 23, 2018
Merged

Conversation

dsyme
Copy link
Contributor

@dsyme dsyme commented Feb 21, 2018

  • Based on a few review comments in Don't hide exceptions on the async cancellation-path  #3257 (comment) it's time to cleanup our defines and flush some last remains of Portable etc. out of the codebase. Removes about 400 lines of dead code and defines. Mostly this is straight-forward deletions. At a couple of points I clarified whether DOTNETCORE or coreclr means netstandard1.6 or netcoreapp1.0 or netcoreapp2.0

  • In this PR the COMPILER conditional now means specifically "are we compiling the core compiler component, i.e. FSharp.Compiler.Private.dll, or FSharp.Compiler.Service.dll, or fsc-proto.exe". This helps us simplify FX_NO_LOADER

  • This PR also moces FCS to .NET Standard 2.0. This was a bigger deal than I thought since it required moving to .NET SDK project files under fcs... in order to make the nuget packages (the nuget package merge tool we were using only supports .NET Standard 1.x). Anyway, that work had to be done sometime and it simplifies a lot of things, now removing 1300 lines of dead project files and several directories as well.

  • The PR also enables a lot more testing of the .NET Standard 2.0 FCS running as a .NET Core App 2.0. Only a subset of the tests were done previously.

@dsyme dsyme changed the title cleanup and simplify various defines simplify various defines and move FCS to .NET Standard 2.0 Feb 21, 2018
@dsyme
Copy link
Contributor Author

dsyme commented Feb 21, 2018

This also changes the build of the .NET Standard version of FCS (FSharp.Compiler.Service.dll) to be a .NET Standard 2.0 component. As a result, tools build using that version of FCS will support type provider loading. The .NET 4.5 build of FCS already supported this.

@dsyme dsyme changed the title simplify various defines and move FCS to .NET Standard 2.0 simplify defines and move FCS to .NET Standard 2.0 Feb 21, 2018
@dsyme
Copy link
Contributor Author

dsyme commented Feb 22, 2018

I may as well note that there are still differences between the FCS .NET Standard 2.0 and FCS .NET Framework 4.5 components. Notably, the defines in FSharp.Compiler.Service.fsproj.

These should, I think, now more clearly correspond to the same features that are no longer available in the .NET SDK F# compiler, and indicates that we could eventually build the .NET SDK compiler replacing FSharp.Compiler.Private.fsproj by the .NET Standard 2.0 FSharp.Compiler.Service.dll.

The differences are:

  • FX_NO_PDB_WRITER: legacy PDB format is not supported, only portable debug symbols
  • FX_NO_PDB_WRITER: legacy PDB format is not supported, only portable debug symbols
  • FX_NO_LINKEDRESOURCES: no adding linked native resources to binaries
  • FX_NO_APP_DOMAINS: some subtle differences in how F# interactive resolves assemblies
  • NETSTANDARD2_0: some differences in F# interactive assembly resolution

@KevinRansom I know you've known all this stuff for a long time, but it's the first time I've seen the differences so clearly in the context of FCS (moving FCS to .NET Standard 2.0 removes a lot of the FX_NO_XYZ's....). It's reassuring we're converging here

@dsyme
Copy link
Contributor Author

dsyme commented Feb 22, 2018

@KevinRansom @brettfo After this I can confidently say that it will be a huge win to move the whole compiler codebase to .NET Standard 2.0 and new-style SDK project files.

The amount of FX_NO_XYZ we can flush will be very large, and everything will become much, much clearer.

@dsyme dsyme mentioned this pull request Feb 22, 2018
@dsyme
Copy link
Contributor Author

dsyme commented Feb 22, 2018

@dotnet-bot test Ubuntu14.04 Release_fcs Build please

@dsyme
Copy link
Contributor Author

dsyme commented Feb 22, 2018

Moving to .NET SDK project files is causing problems on Ubuntu 14.04 because of the versions of Mono there, which is 5.2.0.224 on the Ubuntu 14.04 boxes. That's an old version that doesn't appear to support .NET SDK project files as fully as one might hope.

Unfortunately I can't find how to easily specify/control the version of Mono on the Jenkins CI system. (On Travis it's very simple to specify the version of Mono!)

It's possible that moving to CI Ubuntu 16.04 will help, or else we will need to package-upgrade Mono to latest

@dsyme
Copy link
Contributor Author

dsyme commented Feb 22, 2018

@dotnet-bot test ci please

1 similar comment
@dsyme
Copy link
Contributor Author

dsyme commented Feb 22, 2018

@dotnet-bot test ci please

@dsyme
Copy link
Contributor Author

dsyme commented Feb 22, 2018

@dotnet-bot test ci please

@dsyme
Copy link
Contributor Author

dsyme commented Feb 22, 2018

@dotnet-bot test Windows_NT Debug_default Build please
@dotnet-bot test Windows_NT Release_net40_no_vs Build please

@dsyme
Copy link
Contributor Author

dsyme commented Feb 22, 2018

@dotnet-bot Test Windows_NT Release_ci_part4 Build please

@dsyme
Copy link
Contributor Author

dsyme commented Feb 22, 2018

@dotnet-bot test this please

@dsyme
Copy link
Contributor Author

dsyme commented Feb 23, 2018

@dotnet-bot test this please

@dsyme
Copy link
Contributor Author

dsyme commented Feb 23, 2018

OK, this is FINALLY ready to merge.

@dsyme
Copy link
Contributor Author

dsyme commented Feb 23, 2018

Since this is nearly all related to FCS I will merge it now

@dsyme dsyme merged commit 509a8a2 into dotnet:master Feb 23, 2018
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.

1 participant