Skip to content
This repository was archived by the owner on Oct 7, 2020. It is now read-only.

Conversation

alanz
Copy link
Collaborator

@alanz alanz commented Nov 6, 2018

This is a merge of #905, bringing in the work they have done on cabal-helper and ghc-mod to support cabal new-build in addition to the existing cabal build and stack support.

I will be running it for a few days to get a feel for it, before merging.

And I think prior to the merge we should bump a version and tag it, so people have a clear point to return to if we hit problems with this.

Thanks to @dsturnbull for creating the original patch.

dsturnbull and others added 13 commits October 26, 2018 13:44
use new ghc-mod branch

New version of cabal-helper

fix shadowing, point to cabal-helper submodule

new cabal-helper

new ghc-mod

new ghc-mod

fix merge

cabal-helper-rebased

pretty-show & ghc-mod using the right cabal-helper version range

bump ghc-mode-core

bump cabal-helper

new cabal-helper

new cabal-helper

new cabal-helper
adding nix shell for building and using HIE
@alanz
Copy link
Collaborator Author

alanz commented Nov 7, 2018

@dsturnbull I see that the identification of a preinstalled Cabal library has changed,

-listCabalVersions :: Options -> IO [Version]
+listCabalVersions :: Options -> MaybeT IO [Version]
 listCabalVersions opts = listCabalVersions' opts Nothing
 
--- TODO: Include sandbox? Probably only relevant for build-type:custom projects.
-listCabalVersions' :: Options -> Maybe PackageDbDir -> IO [Version]
-listCabalVersions' Options {..} mdb = do
-  let mdbopt = ("--package-conf="++) <$> unPackageDbDir <$> mdb
-      opts = ["list", "--simple-output", "Cabal"] ++ maybeToList mdbopt
-
-  catMaybes . map (fmap snd . parsePkgId . fromString) . words
-          <$> readProcess oGhcPkgProgram opts ""
+listCabalVersions' :: Options -> Maybe PackageDbDir -> MaybeT IO [Version]
+listCabalVersions' opts@Options {..} mdb = do
+  case mdb of
+    Nothing -> mzero
+    Just (PackageDbDir db_path) -> do
+      exists <- liftIO $ doesDirectoryExist db_path
+      case exists of
+        False -> mzero
+        True  -> MaybeT $ logIOError opts "listCabalVersions'" $ Just <$> do
+          let mdbopt = ("--package-conf="++) <$> unPackageDbDir <$> mdb
+              args = ["list", "--simple-output", "Cabal"] ++ maybeToList mdbopt
+
+          catMaybes . map (fmap snd . parsePkgId . fromString) . words
+                   <$> readProcess' opts oGhcPkgProgram args ""

the critical feature being that if Nothing is passed in as the possible package db location it does not even try to look for anything. And it is pretty much always used with Nothing, via the listCabalVersions call.

Is this deliberate? Is it because of the .ghc.env* files?

This results in the Cabal library being recompiled pretty much every time the wrapper is rebuilt, which is a huge delay and waste.

@alanz alanz changed the title [WIP] cabal new-build merged from arbor repo cabal new-build merged from arbor repo Nov 8, 2018
@alanz alanz requested a review from lukel97 November 8, 2018 21:00
@alanz
Copy link
Collaborator Author

alanz commented Nov 8, 2018

I think this is ready to land.

I have tested it locally on a cabal old build, cabal new one, and a stack one, and it seems to behave.

Any reasons not to merge once CI is complete?

cc @dsturnbull

@newhoggy
Copy link

newhoggy commented Nov 8, 2018

I'm @dsturnbull's colleague. @dsturnbull might be disappearing for a few days.

@AlexeyRaga and I have looked at the snippet you posted and it looks like it wasn't due to any of our commits.

Ping @DanielG

9142d8a9 src/CabalHelper/Compiletime/Compile.hs (Daniel Gröber 2018-08-12 04:45:34 +0200 663) listCabalVersions :: Options -> MaybeT IO [Version]
c7aba0db CabalHelper/Compile.hs                 (Daniel Gröber 2015-08-21 06:34:09 +0200 664) listCabalVersions opts = listCabalVersions' opts Nothing
c7aba0db CabalHelper/Compile.hs                 (Daniel Gröber 2015-08-21 06:34:09 +0200 665)
9142d8a9 src/CabalHelper/Compiletime/Compile.hs (Daniel Gröber 2018-08-12 04:45:34 +0200 666) listCabalVersions' :: Options -> Maybe PackageDbDir -> MaybeT IO [Version]
9142d8a9 src/CabalHelper/Compiletime/Compile.hs (Daniel Gröber 2018-08-12 04:45:34 +0200 667) listCabalVersions' opts@Options {..} mdb = do
9142d8a9 src/CabalHelper/Compiletime/Compile.hs (Daniel Gröber 2018-08-12 04:45:34 +0200 668)   case mdb of
9142d8a9 src/CabalHelper/Compiletime/Compile.hs (Daniel Gröber 2018-08-12 04:45:34 +0200 669)     Nothing -> mzero
9142d8a9 src/CabalHelper/Compiletime/Compile.hs (Daniel Gröber 2018-08-12 04:45:34 +0200 670)     Just (PackageDbDir db_path) -> do
9142d8a9 src/CabalHelper/Compiletime/Compile.hs (Daniel Gröber 2018-08-12 04:45:34 +0200 671)       exists <- liftIO $ doesDirectoryExist db_path
9142d8a9 src/CabalHelper/Compiletime/Compile.hs (Daniel Gröber 2018-08-12 04:45:34 +0200 672)       case exists of
9142d8a9 src/CabalHelper/Compiletime/Compile.hs (Daniel Gröber 2018-08-12 04:45:34 +0200 673)         False -> mzero
9142d8a9 src/CabalHelper/Compiletime/Compile.hs (Daniel Gröber 2018-08-12 04:45:34 +0200 674)         True  -> MaybeT $ logIOError opts "listCabalVersions'" $ Just <$> do
9142d8a9 src/CabalHelper/Compiletime/Compile.hs (Daniel Gröber 2018-08-12 04:45:34 +0200 675)           let mdbopt = ("--package-conf="++) <$> unPackageDbDir <$> mdb
9142d8a9 src/CabalHelper/Compiletime/Compile.hs (Daniel Gröber 2018-08-12 04:45:34 +0200 676)               args = ["list", "--simple-output", "Cabal"] ++ maybeToList mdbopt
9142d8a9 src/CabalHelper/Compiletime/Compile.hs (Daniel Gröber 2018-08-12 04:45:34 +0200 677)
9142d8a9 src/CabalHelper/Compiletime/Compile.hs (Daniel Gröber 2018-08-12 04:45:34 +0200 678)           catMaybes . map (fmap snd . parsePkgId . fromString) . words
9142d8a9 src/CabalHelper/Compiletime/Compile.hs (Daniel Gröber 2018-08-12 04:45:34 +0200 679)                    <$> readProcess' opts oGhcPkgProgram args ""

@alanz
Copy link
Collaborator Author

alanz commented Nov 9, 2018

@newhoggy thanks for the info. Based on behaviour I think reverting it was the right thing to do though.

@alanz alanz merged commit d110059 into haskell:master Nov 9, 2018
@alanz alanz deleted the cabal-new-merged branch November 9, 2018 07:58
@alanz alanz added this to the prehistory milestone Feb 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants