-
Notifications
You must be signed in to change notification settings - Fork 317
Use Forge to parse .fsproj files #500
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
40bbfab to
690bd21
Compare
|
@alfonsogarciacaro I see you updated the FCS version to 8.0, please note that for Fable-netcore you need to pull this PR too (until it gets merged, hopefully soon). It adds a missing reference when parsing .fsx scripts. |
|
Ah, ok, thanks! Maybe I could pull the project from your fork, though I cannot compile FCS on my Mac anymore :/ BTW, I also removed CoreLib from the default references when reading project options with Forge (mainly because I didn't know what was that 😅). Should I put it back again? |
|
@alfonsogarciacaro For .netcore, yes, you need that reference for System.Object etc. let (-->) = type-forwards to Another option (temporary, easier than custom FCS build) would be to add that reference in Fable after parsing the .fsx project options (again, .netcore only). |
3958d77 to
9741a99
Compare
9741a99 to
6b1a523
Compare
6fe19e5 to
778b651
Compare
|
@alfonsogarciacaro I have rebased this PR to FCS latest so you can use my FCS fork if that helps. Besides that, and the slightly different reference resolution, what else is problematic with the netcore Fable version? I'm not aware of any other pending issue atm, but do let me know if you have some. |
|
Thanks @ncave, I'll do! Ideally I'd like to have mostly the same tests between .NET/Mono and Netcore versions. That is, using the same project file and including most of the tests. Why is Regex failing? Is that the same as #535? I quickly tested the netcore version on Windows this weekend and I couldn't make it work but I didn't look at it in detail. Also, latest FCS is not building the Netcore version for me on Mac so I can only test the Netcore version when I'm on Windows :( Because of this, I'd rather wait until .NET Standard 2.0 is released and the new .fsproj is compatible with editors to minimize the maintenance burden. But if you want, I can give you publish rights for fable-compiler-netcore so you can update the package faster. You just need to create an npm account and tell me the username 👍 Quick question, you added the reference to CoreLib after parsing project files with Forge, right? Is still necessary to use your PR even so? Is that for parsing references in script files? Thanks a lot for all your help @ncave! |
|
|
@alfonsogarciacaro don't be so keen about getting rid of the MSBuild dependency, eventually Forge is going to go back to using MSBuild. But it'll be using MSBuild via nuget packages, so the installation issue will be mitigated. https://github.com/fsharp-editing/ProtoWorkspace is eventually going to be the core of FSharp.Editing, but since it already has a bunch of functionality that Forge doesn't there isn't a good reason to maintain them both. I'm not sure what the best role for Forge will be at that point, it might just become the CLI built on top of the workspace to provide an option for commandline project and solution management. It might also be interesting to turn Forge into something like the A big advantage of this roslyn workspace to editor tooling is it will make it much easier to integrate a new project format (fstoml) with cross project references to normal fsproj and the like. at the moment it's only net46, but once MSBuild 15 is released I'll be able to move the whole thing to netcore. @ncave if you're pretty familiar with netcore @enricosada and I could use some backup getting the F# tooling ready. The preview3 tooling seems pretty adequate for building F# netcore projects https://github.com/dotnet/netcorecli-fsc/pull/22/files, but unfortunately we don't have any good editor tooling for writing them yet. |
|
@alfonsogarciacaro I added a small PR to fix the FCS netcore build script, works under Linux now. I can't try it on osx, but it should work there too, i.e. |
|
@ncave Cool, thank you! Yes, if you don't mind I'd prefer you publish the netcore versions for now, as you're working with that version much more than me. Just send me your npm account username and I'll make you a collaborator 👍 @cloudRoutine Thanks for the info! When I talk about MSBuild as a dependency, I actually mean requiring a specific version installed in the System. Of course, adding MSBuild as a library would be no problem :) |
Now that FCS doesn't rely on MSBuild being installed, it's good timing to harness the power of Forge to parse .fsproj files and totally remove MSBuild dependency. This will avoid the problems sometimes reported about Fable not working on clean machines, and should also make the transition to netcore easier (actually Fable should already be able to read new .fsproj files).
The downside of this is Forge parsing and FCS reference resolution capabilities are not as powerful as MSBuild, but it should be enough for Fable, as projects usually have a very simple structure.
This PR also allows compiling multiple projects at the same time. Note that in this case Fable will merge all the files and compile them as if they were a single project, so they need to be specified in the proper order.
UPDATE
This PR also makes output a
.fablemapfile when generating adll. This file maps the type filenames in the assembly with the JS files compiled by Fable, so the proper imports can be generated when compiling with Fable another project that references this assembly.