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

Cleanup RazorEngine files. #273

Closed
matthid opened this issue Mar 24, 2015 · 0 comments · Fixed by #274
Closed

Cleanup RazorEngine files. #273

matthid opened this issue Mar 24, 2015 · 0 comments · Fixed by #274

Comments

@matthid
Copy link
Member

matthid commented Mar 24, 2015

Some background: Antaris/RazorEngine#244

Basically RazorEngine was leaking temp files sine 3.5.0. Now I have a fix/workaround in place but it only works when RazorEngine is not used in the default AppDomain. So we need to take additional actions here in F# Formatting (because the primary usage is via FSI, which sadly uses the default AppDomain..).

Because spinning up AppDomains in F# scripts will be very difficult (to get a dynamic assembly from one into another AppDomain), I can only see two options:

  • Spin up the AppDomain internally in F# Formatting (if we are in the default AppDomain)
    But I guess this is only possible for high level APIs like MetadataFormat.Generate (I don't think we can support high order functions)
  • Handle this in FAKE: Spin up a new AppDomain within FAKE and run the build-script from there
    Of course we need to also change https://github.com/fsprojects/ProjectScaffold/blob/master/build.template#L199 and spin up another FAKE process instead of calling fsi.exe.
    At this point everything should work fine.

IMHO we should opt for 2 (it should be easier to implement as well) and leave F# Formatting unchanged, however I would like to hear what you think.
/cc @tpetricek @forki

matthid added a commit to matthid/FSharp.Formatting that referenced this issue Mar 27, 2015
…ownsides when F# Formatting is used as a library in a long running application, but it is an improvement for now.
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 a pull request may close this issue.

1 participant