Temporary files with cabal project

[lierdakil]

Hello. Thank you for ghc-mod, this is invaluable tool for me.

However, recent changes in master make my life a little bit harder.

I'm using ghc-mod as a backend for https://github.com/atom-haskell/ide-haskell package for Atom editor. Until recently, it was possible to run ghc-mod commands on a file currently being edited (i.e. one with unsaved changes), by writing whole buffer to temporary file and running ghc-mod/ghc-modi command on that.

That does not work with master version though, which breaks the workflow I have now.

Just to be clear, I'm talking about cabal projects.

As far as I understand, ghc-mod now follows cabal project definition more closely, so it ignores cabal project settings for files not included in said project (with other-modules, I reckon). This is a reasonable convention, but I would prefer there was some way to override that, for example by supplying filename and contents separately, f.ex. ghc-mod check <filename> <contents-file>. Any syntax will do.

Another option would be to supply file contents on stdin, but honestly, I doubt it's possible with ghc-modi, so it's not really a good option.

I can spend some time on implementing this, I just want to make sure it won't be rejected outright and understand what to implement in the first place.

Thank you again for your work.

[DanielG]

I'd be happy to merge this. But I think doing this right might be a bit tricky. AFAIK the only places where we read the contents of the source files are in L/H/Target.hs and L/H/HomeModuleGraph.hs. You need to be really careful with the path names in HomeModuleGraph they always have to be cannonicalized and you don't want your temporary files to show up in the module graph.

As far as the actual implementation goes I think a reasonable approach might be a table mapping the real (cannonicalized) path to a datatype, something along the lines of data File = RegularFile FilePath | RedirectedFile { realFile :: FilePath, redirectedFile :: FilePath } | MemoryFile FilePath ByteString. You obviously don't have to implement MemoryFile bit if you're not going to use it. I'd like to have support uploading files into ghc-mod directly at some later point though so while you're at it might as well put the proper holes in for that support :)

[lierdakil]

Thank you for timely response.

Actually, MemoryFile would be a great option to avoid temporary files altogether. But I'm not entirely sure I know how to implement it. We'll see though, might be easier than I think, since I haven't really dived in ghc-mod code yet.

[DanielG]

I'm not sure ghc's API allows for something like MemoryFile so before passing it to ghc we'd still have to write it out into a temporary file anyways :/

One other nasty little detail. GHC's log messages will include the path of your temporary file and you'd have to somehow replace that with the original file name unless you handle that in the frontend?

[lierdakil]

At the moment I handle that in the frontend, yes. But might as well run string replacement on ghc log in ghc-mod, I think. At least, all required information is there.

[lierdakil]

I suppose GHC API actually allows in-memory files:

A target may be supplied with the actual text of the module. If so, use this instead of the file contents (this is for use in an IDE where the file hasn't been saved by the user yet).

from https://www.haskell.org/platform/doc/2013.2.0.0/ghc-api/GHC.html#t:Target

That would require some changes to ghc-mod internals, though, since I think it passes file/module names as Strings, not as Targets.

[DanielG]

@kazu-yamamoto it might be a good idea to also implement this in the elisp frontend, we don't want atom to have more powerful ghc-mod integration than it's native frontend, now do we ;)

[hoovercj]

Apologies for commenting on a closed issue @DanielG, @lierdakil, but this seemed like a relevant place to ask.

I've started putting something together that leverages ghc-mod. I'm currently testing with v5.4. As this issue addresses, I'd like to get info for files that have changed and haven't been saved yet. I'm using ghc-mod legacy-interactive. If I don't map files things seem to be working great. However, if I surround calls to info with calls to map-file/unmap-file ghc-mod begins to claim that the symbol is defined in a temp file rather than the real file.

Do you have any idea what might be causing this or what I can do to avoid it?

[hoovercj]

A minimal example that can be run from the command line:

I have file A.hs that exports symbol B and is already saved in the filesystem. If I enter ghc-interactive mode, map the file, and then ask info A.hs B, it says the symbol is defined at A.hs. If I then unmap A.hs and again ask info A.hs B it says B is defined at path\to\tempfile, even though A.hs exists in the file system and contains a definition for B.

I don't see this issue manifesting in lierdakil's atom extension though which appears to do something similar. It seems like I might be able to avoid the path issue by just not unmapping the files. But I'm curious why unmapping them works for lierdakil.

[lierdakil]

@hoovercj, probably a cache issue. My package is affected as well, btw. I don't map saved files though, so it's not as apparent. Thanks for bringing this to attention.

[hoovercj]

Thanks for the response, @lierdakil. I did just repro it in Atom as you said; for some reason I wasn't able to before.

Is this an issue you think is fixable in ghc-mod? Or just something we have to work around?

[lierdakil]

If my understanding of the issue is correct, we just need to add mapped files to World (defined in https://github.com/DanielG/ghc-mod/blob/master/Language/Haskell/GhcMod/World.hs). Should be simple enough. I'm not exactly sure if it's correct though, but it's easy enough to check.

[lierdakil]

diff --git a/Language/Haskell/GhcMod/Types.hs b/Language/Haskell/GhcMod/Types.hs
index 779c5c9..f5c8539 100644
--- a/Language/Haskell/GhcMod/Types.hs
+++ b/Language/Haskell/GhcMod/Types.hs
@@ -71,7 +71,7 @@ data OutputStyle = LispStyle  -- ^ S expression style.
 newtype LineSeparator = LineSeparator String deriving (Show)

 data FileMapping =  FileMapping {fmPath :: FilePath, fmTemp :: Bool}
-                  deriving Show
+                  deriving (Show, Eq)

 type FileMappingMap = Map FilePath FileMapping

diff --git a/Language/Haskell/GhcMod/World.hs b/Language/Haskell/GhcMod/World.hs
index 89596b8..b85de51 100644
--- a/Language/Haskell/GhcMod/World.hs
+++ b/Language/Haskell/GhcMod/World.hs
@@ -20,6 +20,7 @@ data World = World {
   , worldCabalConfig   :: Maybe TimedFile
   , worldCabalSandboxConfig :: Maybe TimedFile
   , worldSymbolCache   :: Maybe TimedFile
+  , worldMappedFiles   :: FileMappingMap
   } deriving (Eq)

 timedPackageCaches :: IOish m => GhcModT m [TimedFile]
@@ -36,6 +37,7 @@ getCurrentWorld = do
     mCabalConfig <- liftIO $ timeMaybe (setupConfigFile crdl)
     mCabalSandboxConfig <- liftIO $ timeMaybe (sandboxConfigFile crdl)
     mSymbolCache <- liftIO $ timeMaybe (symbolCache crdl)
+    mappedFiles <- getMMappedFiles

     return World {
         worldPackageCaches = pkgCaches
@@ -43,6 +45,7 @@ getCurrentWorld = do
       , worldCabalConfig   = mCabalConfig
       , worldCabalSandboxConfig = mCabalSandboxConfig
       , worldSymbolCache   = mSymbolCache
+      , worldMappedFiles   = mappedFiles
       }

 didWorldChange :: IOish m => World -> GhcModT m Bool

Ok, so here's diff that fixes the issue on my simple test. I'll make a PR later, but I doubt it will make it to Hackage soon, since @DanielG tries to avoid minor releases right now, and I don't think we're ready to release 5.6.0.0 just yet.

[hoovercj]

Wow, great. I can easily work around this for now knowing that it it's a known bug, but just so I can plan ahead, does @DanielG have any idea when the next release might be? Are we talking a month, multiple months, etc.?

[DanielG]

Oh we can do a minor release if we need to I just want to encourage everyone not to worry about API compatibility whenever convenient, otherwise we'll never get that messy API we have into shape :)

Anyways release time line. If we have a PR for this we can release a minor version right away if you guys test it first.

[hoovercj]

I'm incredibly new to Haskell (part of the reason I'm interacting with this is so I can have better haskell support for the tools I use) so if there are easy steps to test this myself then I'm more than happy to.

I can look at the docs tomorrow to see if it's explained there, otherwise some quick steps would help.

[lierdakil]

@hoovercj, something along the lines of

git clone https://github.com/atom-haskell/ghc-mod -b fix-map-cache
cd ghc-mod
cabal sandbox init
cabal install

will get you patched ghc-mod installed in .cabal-sandbox/bin/. You can then point anything you need at it, or run cabal exec <something> , which will add .cabal-sandbox/bin/ to PATH for <something>.

I also used this to run my (simplistic) tests

#/usr/bin/bash

ghc-mod legacy-interactive << EOT
check A.hs
info A.hs b
map-file A.hs
$(cat A.hs)
$(echo -e "\x04")
info A.hs b
unmap-file A.hs
info A.hs b
map-file A.hs
$(cat A.hs)
$(echo -e "\x04")
info A.hs b
quit
EOT

A.hs:

module A where

b :: IO ()
b = undefined

[lierdakil]

P.S.

Output of my test script without patch applied:

OK
b :: IO ()  -- Defined at A.hs:4:1
OK
OK
b :: IO ()  -- Defined at A.hs:4:1
OK
OK
b :: IO ()  -- Defined at A.hs:4:1
OK
OK
b :: IO ()  -- Defined at A.hs:4:1
OK

And with patch applied:

OK
b :: IO ()  -- Defined at A.hs:4:1
OK
OK
b :: IO ()  -- Defined at A.hs:4:1
OK
OK
b :: IO ()  -- Defined at ghc-mod14254/A1804289383846930886.hs:4:1
OK
OK
b :: IO ()  -- Defined at A.hs:4:1
OK

_Note_ that dropping session makes subsequent info considerably slower. But I have no idea how to selectively tell GHC that some symbols should be purged from session cache, not at the moment.

[lierdakil]

Okay, so for the time being, a workaround would be to touch a mapped file after it was unmapped (i.e. update its modification time), so in general, this issue doesn't manifest (since users generally save files after changing them)

I need more time to implement this with cache invalidation though, cache code is... complicated. solved.

[hoovercj]

I'm confused by the above comments and strikeouts: are you suggesting that the current patch should solve everything? Or that the touch is in fact still necessary?

And as a follow up:

My "test" project (since I've never written anything complex enough in Haskell) is this project that my friend made:

https://github.com/danstiner/hfmt

I usually call info on Reformatter or HaskellSource in src/Language/Haskell/Format/Definitions.hs because they are defined such that often the first definition is returned with a proper path and the second is shown as defined in a temp file in the currently released ghc-mod.

sample code:

data Formatter = Formatter { unFormatter :: HaskellSource -> Either ErrorString Reformatted }

instance Monoid Formatter where
  mempty = Formatter (\source -> Right (Reformatted source []))
  (Formatter f) `mappend` (Formatter g) = Formatter (asReformatter g <=< f)

My initial exploration today shows that to be the case still in the patch in question but I want to make sure that I'm testing it properly. I'll check again tomorrow and try to come up with a more minimal reproduction scenario if I think it is still a problem.

[lierdakil]

Sorry about being confusing. So, long story short, fix-map-cache branch on https://github.com/atom-haskell/ghc-mod should work without touching. More specifically, commit https://github.com/atom-haskell/ghc-mod/commit/b7a1cf843771aff6e89bcbab88506496774e24b9 should fix this issue, at least according to my (limited) testing. If it doesn't work for you, we could try other things.

[hoovercj]

Replace your A.hs with this (and replace b with B) and check your output.

module Main (main) where

import Data.Monoid

import Distribution.Simple
main = defaultMain

data B = B { }

instance Monoid B where
  mempty = undefined []
  mappend = undefined

I just mapped the file and did info on B and got the following (even without unmapping it).

data B = B -- Defined at A.hs:8:1
instance Monoid B -- Defined at ghc-mod####\A###.hs:10:10

@lierdakil , thoughts?

[lierdakil]

Huh. Interesting. So while my solution works for variables, types are still broken. Okay. I did not expect that.

P.S. On further investigation, it seems like this is even more complicated. Okay, I give up. Dropping session has to work.

P.P.S. Instance is probably a different bug.