-
Notifications
You must be signed in to change notification settings - Fork 158
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
Crash when upgrading to FSharp.Formatting 1.4.0.4 #337
Comments
FCS 1.4.0.4 is not binary compatible with previous versions because some things have been made internal. FSharp.Formatting uses a version constraint ">= 0.0.87" for FCS which is inherently dangerous since binary compat has never really been promised for FCS. I think a constraint ">= 0.0.87" and "<= 1.3.0.2" will work OK though in practice |
I guess we should just update FSharp.Formatting to use the latest version then. But we are also relying on Visual F# PowerTools, so we might need to wait for them first... In the meantime, is there a way to specify range in a nuspec file? |
Note 1.4 is for F# 4.x support. That might not be what you want when processing scripts as it may induce dependencies on having FSHarp.Core 4.4.0.0 available, at least check for that. (The default behaviour should be to look at the FSharp.Core referenced by the ultimate host .EXE of the DLL, which sounds sensible, but be careful!) |
I see, didn't realize this is for F# 4.x. I'm testing a fix (specifying range in the NuGet package) |
Ok, new version of F# formatting fixes this, but there is a big but.... If you are using Paket, you need to explicitly tell it that you want at least the current version of F# Formatting:
As we did not have the constraint before, there is another valid resolution which is to get the latest F# Compiler Service and older release of F# Formatting. This will not work - but I think there is no way to prevent this - because the new incompatible version of FCS has a higher version :-( |
Actually, maybe you only run into the issues if you also explicitly require FCS, which was the case in my project, which had:
So, this should be OK for normal uses... |
I am resolving into this:
However I am still getting the same error
What do I exactly need to do to fix this? |
not sure, but would releasing #345 help? |
Here are the relevant files which cause it: That repo |
@forki perhaps, if I ran with this it worked:
but I wanted to upgrade FSharp.Formatting :) |
yeah but paket can't do magic. if stuff is in compatible we need to express this in the dependencies. |
Does this issue need to be reopened then? Since the current dependencies actually don't work? |
Yes it's breaking other things too. Like fsreveal
|
For completeness sake, this is the last thing I managed to get working:
Whenever I play with increasing version to something higher, things break at various places Best case I would simply need |
If my understanding is correct, this is fundamentally because we get a version of FSharpVSPowerTools and a version of FSharp.Compiler.Service that are not compatible. So perhaps we need some help from @dungpa and @vasily-kirichenko to better understand how to correctly reference VSPowerTools? (So that new releases of FSharp.Compiler.Service do not break FSharp.Formatting...??) |
I tried forcing FSharp.Formatting 2.8.0 with the latest Powertools and latest compiler services (since the dependencies on powertools allow that) but strangely enough in that case I had a problem coming from powertools. You can try with:
|
The problem here is https://github.com/exira/ges/blob/ca747a75b544259b714af65cf6535b776360296a/docs/tools/generate.fsx#L30 We cannot do much here at FSharp.Formatting and our dependencies are in fact correct as far as I can see. I think this issue can be closed. TLDR: |
Ah, this was coming from ProjectScaffold https://github.com/fsprojects/ProjectScaffold/blob/master/docs/tools/generate.template#L30-L31 |
Confirmed to solve it. My
My
The fix:
Probably should be ported back to the guys from ProjectScaffold |
If you can send a fix to ProjectScaffold, that would be awesome! |
Thanks! I see what's happening now - there are two different copies of FSharp.Compiler.Service (one coming with FAKE and one coming with F# Formatting) and the ordering determines which one gets loaded... |
ah so maybe we should internalize that into FSharp.Formatting. I'd like to 2015-10-14 10:34 GMT+02:00 Tomas Petricek [email protected]:
|
or wait. maybe I can still do this FAKE 2015-10-14 10:39 GMT+02:00 Steffen Forkmann [email protected]:
|
In F# Formatting, we could have two NuGet packages - one with everything internalized (for "tool-like" use) and one with nothing internalized (for "library-like" use). Though even then, I can imagine scenarios where things would not be ideal. I guess internalizing FSharp.Compiler.Service in FAKE is safer, because it is always used as a tool. But what happens when the F# Compiler Service running inside FAKE tries to load another F# Compiler Service that remains a mystery to me (I guess it might work though...) |
FAKE 4.6 comes with internalized FCS. Please check it out. |
yeah. the other thing didn't work. When I ILRepack into FakeLib.dll the I get an invalid CLR program ... |
@Krzysztof-Cieslak just reported an error with FsLab journals, which was caused by this too. I tried understanding what's going on, but I have absolutely no idea what is compatible with what. I get the above error with the following combo:
But at the moment, that's the only allowed mix... Do we need to: a) Just update to FCS 1.4 (now that there is a matching VFPT release)? What is better option? |
This also does not work:
Is there any combination at the moment that actually works?? |
I guess this might also be happening because FsLab silently loads the version from FAKE, but I can't seem to figure out how to avoid that. |
Does FCS 1.4 require a different version of FSharp.Core? (which would make upgrading a nightmare...) |
I have absolutely no idea what's going on here. But it seems that the version of FAKE I'm using has some effect here...
Is this just me or can others reproduce this? |
This combo works:
On Oct 17, 2015 19:16, "Tomas Petricek" [email protected] wrote:
|
Working with Not working with any higher- I've checked latest ( |
Yes it's related to the FAKE update to FSharp.Core-4 and FSharp.Compiler-Service-1.4. However I'm not completely sure why it WORKS within a FAKE environment (FAKE.exe) but NOT outside. The documentation generation script depends on FakeLib.dll which itself depends on the latest FSC.dll. On the other hand the script depends on FSharp.Formatting which depends on an older FSC.dll. Definitely a (high level) problem there. By the way this seems to work as well:
So after updating to FSharp.Core 4 and PowerTools 2.1 it will probably start to work OK again. However understanding why it DOES work in a FAKE.exe even with a FSharpVSPowerTools.Core version which SHOULD NOT work still puzzles me (can somebody reproduce this with running the script with FAKE.exe instead of fsi.exe or is it just some weird thing happening on my machine?). I think in the future we should add some more tracing to figure out exactly which assemblies are loaded (but they are loaded lazily ...). |
My tests result are all based on FsLab template and run using FAKE script. |
Thanks everyone for going through the additional test cases. Now I'm really puzzled what change in FAKE is doing this... I tried printing the version of FCS loaded in current |
Can confirm but not explain this ... Maybe FAKE.exe runs in a security context where it is allowed to access internal methods, that would at least explain how it can sometimes work at all... Maybe the context sometimes changes depending on how assemblies are build (the only difference in 4.5.4 and 4.5.5 is adding groups and To close this issue we should release a version compatible with latest FAKE (ie update to powertools 2.1.0 and latest The only safe option to prevent this error for now are (at least as far as I can understand this currently):
Maybe we should somehow detect a valid fake version and fail early if we are not compatible? |
Yeah, we should definitely update F# Formatting to latest versions.. but I would still like to understand why this is happening. Looks like there was no relevant change in FAKE, so perhaps this is something that has been caused by an update of FCS from version 1.4.0.1 to version 1.4.0.6. Perhaps there was some change there that affected how assemblies are loaded in FSI evaluator....? |
Looks like FCS does not have a tag on GitHub for 1.4.0.1, but here are the changes between 1.4.0.2 and 1.4.0.6: fsharp/fsharp-compiler-docs@1.4.0.2...1.4.0.6 |
definitely... 1.4.0.3 was the first version where the methods where made internal (https://github.com/fsharp/FSharp.Compiler.Service/blob/master/RELEASE_NOTES.md#1403--). This is what @dsyme was mentioning on his first post. So I think that's the solution to the mystery? It works until FAKE-4.5.4 because it uses FSharp.Compiler.Service < 1.4.0.3 and doesn't work on later versions. However this bug also shows that locking our versions in the package doesn't really solve anything for us when running within FAKE. Maybe we should figure out a way to solve this eventually, when we are hit by this again (Spawning our own AppDomain?). |
Ah, I see... so really, when running inside FAKE, we always end up running against the FAKE version of F# Compiler Service... Well that's a bummer. I guess that's what @forki was trying to solve by internalizing F# Compiler Service, so that we can load our own version. This is all a bit silly, but at least I get it now :) |
Another idea would be to move @forki would you accept such a change in FAKE? That's not really in the spirit of paket groups and Edit: It's ironic that this couldn't happen before introducing paket groups :) |
Wait - I'm not sure I understand - how is this related to Paket groups? (I thought the issue is just that FAKE comes with FCS and now it happens to come with one that's not compatible with F# Formatting...) |
Yes, but by putting fsf into the main group FAKE's own dependency file
|
I'm actually ok with that and would accept pr
|
I have one more idea: What about adding FAKE as a dependency of FSharp.Formatting? That should work as well and is a better solution imho (as we really do depend on FAKE in the projectscaffold use-case). Ideally we would not add the dependency to the library package and only to a "scripting" package. But it should not do any harm either (an additional download in the worst case). For FAKE it would lead to slower adoption rates for new versions (as everyone needs to wait for a FSF update). This solution assumes that people have FSharp.Formatting and FAKE in the same paket group... |
2015-10-19 18:01 GMT+02:00 Matthias Dittrich [email protected]:
|
I came across the following crash when upgrading FSharp.Formatting in the mbrace-docs repository:
The text was updated successfully, but these errors were encountered: