-
Notifications
You must be signed in to change notification settings - Fork 842
reverse integrate all fsharp/fsharp changes #2442
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
Conversation
|
<snip> |
yeah they kind of suck
That would be better but historically we never tried to improve anything in http://github.com/fsharp/fsharp since it was only building up more and more misalignment. Likewise if we try to improve things on this PR then that will create yet another round of conflict when we finally integrate the history back to totally align everything. Best to align by whatever means then improve from there |
|
@KevinRansom @dsyme I made some improvements to the surface area tests a while back while working on |
|
It was easier to find my changes than I thought, so I've submitted a PR with them: #2445 |
|
@jack-pappas |
|
@KevinRansom You're welcome, glad it's useful. @dsyme Is it worth continuing with #2114, or will the macOS builds be fixed by merging these changes in? |
|
@jack-pappas It is worth continuing with #2114 - we don't use Travis in this repo - and it would be great to have Mono+OSX coverage
|
|
This is now green and ready to merge |
KevinRansom
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work.
This is a catalog of all changes that are in http://github.com/fsharp/fsharp that are not in this repo. (This doesn't include added files)
Note these changes are about what constitutes "The Mono F# Compiler". This packaging of the F# compiler is also the one found in the FSharp.Compiler.Tools nuget package, which is being more widely used in automation these days (and is used as part of the LKG process for bootstrapping this repo), so is kind of important.
I'm running through the changes assessing which ones should be brought back into this repo
Change in NuGet.targets for OSX. It's possible this is no longer needed if we update to latest NuGet.targets, but is harmless and we can remove it later when we update if needed.
The Mono F# compiler still assumes MSBuild 12.0 for stability and to avoid any nonsense about MSBuild not being installed. Every time we've tried to change this it's been horrible (I find mucking with MSBuild versions really tedious and it's unclear if we care to try to progress the Mono F# compiler on to Mono 14.0 or 15.0) This leads to some project file changes and the use of "MicroBuild" is turned off for Mono (it doesn't work with MSBuild 12.0).
FsLexYacc 7.0.4 is needed instead of 7.0.3
FSharp.Compiler.Tools.4.0.1.21 instead of FSharp.Compiler.Tools.4.0.1.19
The Mono F# Compiler uses strong named binaries using
STRONG_NAME_FSHARP_COMPILER_WITH_TEST_KEYandsrc\fsharp\test.snkfor everything except FSharp.Core. In all cases the binaries use version number the same as$(FSCoreVersion)The Mono F# compiler defines
CROSS_PLATFORM_COMPILER. We obviously don't want this on all the time, and should probably change it toMONO_FSHARP_COMPILERin any case, but some of the changes are relevant to Mono delivery, I'll list them out belowSome things were commented out of FSharpSource.targets because Mono was barfing on them. These have been made conditional. Mono seems OK with them like that
When running on Mono the default assembly search path includes
mono/4.5andmono/4.5-apiandmono/4.5-api/Facadesif the directories exist. This is harmless for this repo since it's protected byrunningOnMonoThere are changes in
src/fsharp/FSharp.Build/CreateFSharpManifestResourceName.fsthat have been made mono-specific. These can be integrated since they are mono-conditional. The Mono version of the F# compiler and targets has strange issues with resources in subdirectories and that problem may be related to these changes. The targets/task/... logic implementing the treatment of resources in MSBuild is horrible messy with a mess of MSBuild and tasks that differs in the Mono and .NET implementations. We should be careful about integrating this change.FSharp.Build passes its own location as a hint to
FSharpEnvironment.BinFolderOfDefaultFSharpCompilerto prevent the need for registry scraping. This is reasonable and necessary for the Mono packaging.src/fsharp/FSharp.Build/Microsoft.Portable.FSharp.targetsis adjusted to account for a difference in how portable targets work on Monosrc/fsharp/FSharp.Compiler.Server.Shared/AssemblyInfo.fsis adjusted to allow this DLL to be compiled without the VS SDK using#if HAVE_VS_SDKHarmless adjustments in some
src/fsharp/FSharp.Core.Unittests/FSharp.CoretestsMono BigInteger had some bugs at one point, these are likely fixed but perhaps not on some of the older versions of Mono we still CI against. These lead to some
#if CROSS_PLATFORM_COMPILER. We can fix these up at a later point.Dependency on
packages\Microsoft.FSharp.TupleSample.1.0.0-alpha-161121is dropped - fsharp/fsharp doesn't have dependencies on "myget" and all source needs to be available for Debian.Harmless
try/withwas added tosrc/fsharp/ast.fsto catch against an exception that Mono was throwing in XML processing in some rare cases. Don't know if the exception is still happening but we may as well integrate the changeUnix-friendly console key binding feature has been added to
src/fsharp/fsi/console.fsWe may a well integrate this, perhaps conditional underMONO_FSHARP_COMPILERthe
--guiflag is assumedfalseon Mono. This is because System.Windows.Forms just doesn't exist in good health on Mono and there is no default windowing-friendly event loop that can be assumed.There are bunch of pretty harmless changes in
tests/fsharp/core/*for compiling the tests as part of a single program, rather than lots of little programs. That's how http://github.com/fsharp/fsharp currently runs these tests as "smoke tests" on Mono - party because the FSharp.Tests current test harness assumes tools likepeverifyexist and are in good shape.These diffs have not been included in this PR:
NO: The Mono FSharpSource.targets includes info for building the versions of FSharp.Core used for
monodroid,monotouch,xamarinwatchos,xamarintvos,xamarinmacmobile,xamarinmacfull. @nosami, @directhex and @migueldeicaza may be able to advise if these are all still used by Xamarin tooling, or what will be needed going forward. They are not included in this PR since they are not under test.NO: The Mono F# compiler was adjusted to avoid a dependency on
System.ValueTuple. The change won't be integrated - it's not clear why that's necessary - we should get to the bottom of itNO: The Mono F# compiler was also adjusted not to reference
System.ValueTupleby default for scripts. The change won't be integrated - it's not clear why that's necessary - we should get to the bottom of itNO:
The Mono F# targets file removed <CoreCompileDependsOn>_ComputeNonExistentFileProperty</CoreCompileDependsOn>is removed. I don't think that change is needed.After this PR we should
Work out what to do with those very few remaining diffs listed above
Adjust the Mono F# compiler to enable -gui if running on Windows
Work out what's going on with resources for the Mono F# Compiler
Integrate the changes for authoring the two community nuget packages from http://github.com/fsharp/fsharp FSharp.Core and FSharp.Compiler.Tools
Consider integrating the remaining changes from http://github.com/fsharp/FSharp.Compiler.Services and the authoring of the nuget package built there.