Need clarification on error message: modToPkg

[snoyberg]

I have a project which imports Control.Monad.CatchIO. Loading it up on our production server, I get the following error message:

External exception: modToPkg: Control.Monad.CatchIO: [("MonadCatchIO-mtl-0.3.0.5",True),("MonadCatchIO-transformers-0.3.0.0",True)]. Local exception: user error (too few bytes. Failed reading at byte position 1)

Any indication what this means?

Pinging @edsko @dcoutts @Mikolaj

[Mikolaj]

I'm looking at this. The code in question seems to be wrong, because module Control.Monad.CatchIO is visible in the two (not hidden) packages (and the module really differs in the two packages):

http://hackage.haskell.org/packages/archive/MonadCatchIO-mtl/0.3.0.5/doc/html/Control-Monad-CatchIO.html

http://hackage.haskell.org/packages/archive/MonadCatchIO-transformers/0.3.0.0/doc/html/Control-Monad-CatchIO.html

However, we are probably catching the problem too early. I'm just trying to reproduce, to see if GHC would catch it later that we do or perhaps not at all (e.g., if the module is not used).

[Mikolaj]

@snoyberg: I can't reproduce it neither on master nor on temporary-pkgdir-hack:

39cbdf2

Instead of the failure, I'm getting an error from GHC:

Unexpected errors: SourceError {errorKind = KindError, errorSpan = A.hs@2:8-2:29, errorMsg = "Ambiguous module name `Control.Monad.CatchIO':\n it was found in multiple packages:\n MonadCatchIO-transformers-0.3.0.0 MonadCatchIO-mtl-0.3.0.5\n"}

Could you give me a hint or a pointer to the code?

If you'd need a quickfix, I can easily disable the error message and just pick the first of the two packages for the functions that need it (e.g., autocompletion) for now. We think, if the module in question is really used, GHC will signal an error sooner or later, so it does not matter which package that contains the module is picked.

If I could reproduce your case, however, I could verify the quickfix and my assumptions.

[snoyberg]

I temporarily tried adding a package import statement (e.g., import "MonadCatchIO-transformers" Control.Monad.CatchIO), perhaps that's why I got the different message. I tried removing the package import statement, but the error message remained. Is it possible that it just sent ide-backend into a bad state it couldn't recover from?

[snoyberg]

I just went through a number of steps very carefully, and am pretty certain my previous guess is correct: once package imports are used to disambiguate, the modPkg error message is generated and cannot be recovered from without killing the IdeSession.

[Mikolaj]

Thank you very much. I'm able to reproduce now. Yes, you cannot recover without restarting the session, because the error is a kind of an assertion violation and (the server of) ide-backend just exits. I thought this particular assertion violation cannot happen before GHC gets a chance to complain, but you proved it can, so this is my wrong assumption and I will fix it.

As to the specifics of the bad state, I'm not the best person to speculate, I will ask others. I guess it may just be that the ide-backend server (and it's instance of GHC) is killed and then when you catch the exception from the ide-backend client and then ask ide-backend client to update session it doesn't update anything and when you ask for the last error message, it produces the same, stale message.

[edsko]

There are a few places in the ide-backend server where we have to guess a package from a module name:

1. When we compute package dependencies (pkgDepsFromModSummary in Haddock.hs) and when we construct Imports (importList in Run.hs). In both of these cases we work directly with ghc's ImportDecl, which contains the optional package qualifier (the "pkg" part in import "pkg" Stuff). If the imported module is available from multiple packages, we therefore have access to the package qualifier to disambiguate; I have pushed a commit to do this. @snoyberg : Let me know if you'd like me to cherry-pick this commit on the temporary-pkgdir-hack branch.
2. When we construct IdScope (scopeFromProv in HsWalk.hs) for an identifier. Unfortunately, in this case we are provided with an ImportSpec rather than an ImportDecl, and the ImportSpec does not include the package qualifier. The ghc source contains a TODO that the ModuleName in the ImpDeclSpec should contain be a ModuleId instead (i.e, should include a package identifier).

Until that ghc TODO is done there is no good solution to this problem -- we basically lose information along the way. We might just be able to reconstruct this info based on the location of the import in the source -- although that's a rather terrible hack -- if we consider this important enough. (Though a much better solution would be to implement ghc's TODO instead.) Either way a solution to this problem will require some time. The only consequence, however, is that the ImportedFrom field of an IdInfo lists the wrong package.

[snoyberg]

I want to make sure that the ide-backend doesn't lock up because someone put in an ambiguous import statement. Does (1) do that? I'm not sure what end-user issues result from (2), can you clarify?

[edsko]

ide-backend was locking up because am ambiguous package should never happen (not past ghc's error checking anyway), and we had an assertion that verified that. The reason it was happening is that the package import wasn't in fact ambiguous (due to the use of the PackageImports extension), but this additional information was not available in moduleToPackageId.

As long as we cannot solve (2), we can still have "ambiguous" imports, so moduleToPackageId now just picks the first potential candidate in that case, rather than throwing an exception. From an end user's perspective, this means that if, say, both the MonadCatchIO-mtl and MonadCatchIO-transformers package are exposed, and the user does

{-# LANGUAGE PackageImports #-}
module A where

import "MonadCatchIO-mtl" Control.Monad.CatchIO

f x = catches (print 1) [x]

the dependencies of the module, as well as the imports, will correctly refer to MonadCatchIO-mtl, but the IdInfo might be

catches (VarName) :: MonadCatchIO m => m a -> [Handler m a] -> m a 
defined in MonadCatchIO-mtl-X.Y.Z:Control.Monad.CatchIO at <no location info> 
(home MonadCatchIO-mtl-X.Y.Z:Control.Monad.CatchIO) 
(imported from MonadCatchIO-transformers-X.Y.Z:Control.Monad.CatchIO at A.hs@3:1-3:48)

instead of

catches (VarName) :: MonadCatchIO m => m a -> [Handler m a] -> m a 
defined in MonadCatchIO-mtl-X.Y.Z:Control.Monad.CatchIO at <no location info>
(home MonadCatchIO-mtl-X.Y.Z:Control.Monad.CatchIO) 
(imported from MonadCatchIO-mtl-X.Y.Z:Control.Monad.CatchIO at A.hs@3:1-3:48)

depending on which package we happen to find first that exposes the module (note the incorrect imported from).

[snoyberg]

This is an acceptable trade-off. There's no need to cherry-pick the commit, we'll be updating to master soon. I've asked @jwiegley and @mgsloan to add automated tests to check for this condition.

[snoyberg]

@edsko Sorry, but I need to revise my previous comment. We had to delay our update to master, so I'd like to ask you to cherry pick this fix to onto the commit we're currently using (f45e430). If this is a problem, please let me know. If this can be available by the end of the week, I'd appreciate it. And if this will take more than ~3 hours to implement, please touch base with me before starting.

Once this cherry pick is done, please ping myself and @jwiegley. John: This fix can be included in master or staging any time this week. I've looked over the code, and I'm preemptively saying it can go in during the code freeze.

Pinging @mgsloan as well, who reminded me about the issue.

[edsko]

@snoyberg @jwiegley done.

[snoyberg]

@edsko Thanks, I've confirmed that this all works correctly on our staging server.