From 3e21e0493f4738c9b247499721364624c3a88421 Mon Sep 17 00:00:00 2001 From: Mihai Maruseac Date: Sun, 22 Apr 2018 20:19:40 -0700 Subject: [PATCH 1/9] Export utility functions for internal libraries These should have been part of #3955 to be honest but were not needed at the time. Now they are needed, so I'm adding them as a separate commit, to be easy to cherry-pick them if the rest of the PR is not getting merged. --- src/Stack/Types/NamedComponent.hs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/Stack/Types/NamedComponent.hs b/src/Stack/Types/NamedComponent.hs index 68b0a52dda..09bdc3fbf3 100644 --- a/src/Stack/Types/NamedComponent.hs +++ b/src/Stack/Types/NamedComponent.hs @@ -8,7 +8,9 @@ module Stack.Types.NamedComponent , exeComponents , testComponents , benchComponents + , internalLibComponents , isCLib + , isCInternalLib , isCExe , isCTest , isCBench @@ -59,10 +61,20 @@ benchComponents = Set.fromList . mapMaybe mBenchName . Set.toList mBenchName (CBench name) = Just name mBenchName _ = Nothing +internalLibComponents :: Set NamedComponent -> Set Text +internalLibComponents = Set.fromList . mapMaybe mInternalName . Set.toList + where + mInternalName (CInternalLib name) = Just name + mInternalName _ = Nothing + isCLib :: NamedComponent -> Bool isCLib CLib{} = True isCLib _ = False +isCInternalLib :: NamedComponent -> Bool +isCInternalLib CInternalLib{} = True +isCInternalLib _ = False + isCExe :: NamedComponent -> Bool isCExe CExe{} = True isCExe _ = False From 15137cda073846c94fe308932f3c973f795d4683 Mon Sep 17 00:00:00 2001 From: Mihai Maruseac Date: Sun, 22 Apr 2018 20:20:45 -0700 Subject: [PATCH 2/9] Remove obsolete comment. In #3955, as support for compiling with sublibraries was added, a stale comment was left out in the code. Removed here as a separate commit to be easy to cherry-pick if needed. --- src/Stack/Package.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Stack/Package.hs b/src/Stack/Package.hs index 47e43bd3ea..ec0d3437a7 100644 --- a/src/Stack/Package.hs +++ b/src/Stack/Package.hs @@ -698,7 +698,7 @@ packageDescModulesAndFiles :: PackageDescription -> RIO Ctx (Map NamedComponent (Map ModuleName (Path Abs File)), Map NamedComponent (Set DotCabalPath), Set (Path Abs File), [PackageWarning]) packageDescModulesAndFiles pkg = do - (libraryMods,libDotCabalFiles,libWarnings) <- -- FIXME add in sub libraries + (libraryMods,libDotCabalFiles,libWarnings) <- maybe (return (M.empty, M.empty, [])) (asModuleAndFileMap libComponent libraryFiles) From bc445516c0e5e1906ef63fa781f687c1638c1bfa Mon Sep 17 00:00:00 2001 From: Mihai Maruseac Date: Sun, 22 Apr 2018 20:22:23 -0700 Subject: [PATCH 3/9] Don't ignore sublibs and foreign libraries in Ghci.hs --- src/Stack/Ghci.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Stack/Ghci.hs b/src/Stack/Ghci.hs index c2a624dedc..f86717f547 100644 --- a/src/Stack/Ghci.hs +++ b/src/Stack/Ghci.hs @@ -661,7 +661,7 @@ wantedPackageComponents _ (TargetComps cs) _ = cs wantedPackageComponents bopts (TargetAll ProjectPackage) pkg = S.fromList $ (case packageLibraries pkg of NoLibraries -> [] - HasLibraries _names -> [CLib]) ++ -- FIXME. This ignores sub libraries and foreign libraries. Is that OK? + HasLibraries names -> [CLib] ++ map CInternalLib (S.toList names)) ++ map CExe (S.toList (packageExes pkg)) <> (if boptsTests bopts then map CTest (M.keys (packageTests pkg)) else []) <> (if boptsBenchmarks bopts then map CBench (S.toList (packageBenchmarks pkg)) else []) From 89f5485adfba4a14f17f1420cb3bc6f2233b0328 Mon Sep 17 00:00:00 2001 From: Mihai Maruseac Date: Sun, 22 Apr 2018 20:25:37 -0700 Subject: [PATCH 4/9] Generate package description containing sublib info too --- src/Stack/Package.hs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Stack/Package.hs b/src/Stack/Package.hs index ec0d3437a7..645a9d6910 100644 --- a/src/Stack/Package.hs +++ b/src/Stack/Package.hs @@ -411,6 +411,12 @@ generatePkgDescOpts sourceMap installedMap omitPkgs addPkgs cabalfp pkg componen [] (return . generate CLib . libBuildInfo) (library pkg) + , mapMaybe + (\sublib -> do + let maybeLib = CInternalLib . T.pack . Cabal.unUnqualComponentName <$> libName sublib + flip generate (libBuildInfo sublib) <$> maybeLib + ) + (subLibraries pkg) , fmap (\exe -> generate From 3f63b5ce483a27da2242a00552228b639b8a2dbc Mon Sep 17 00:00:00 2001 From: Mihai Maruseac Date: Sun, 22 Apr 2018 20:24:08 -0700 Subject: [PATCH 5/9] Handle internal libraries when building package description for GHCi. When GHCi starts it looks for a package in the format given by Cabal but stack used to pass the package name as the name of the sublibrary. So we remove the sublibrary from the list of packages GHCi should search for and add the munged name to the same list by using the `omitPkgs` and `addPkgs` lists. --- src/Stack/Package.hs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Stack/Package.hs b/src/Stack/Package.hs index 645a9d6910..40cd67c4e2 100644 --- a/src/Stack/Package.hs +++ b/src/Stack/Package.hs @@ -64,6 +64,7 @@ import qualified Distribution.Types.CondTree as Cabal import qualified Distribution.Types.ExeDependency as Cabal import Distribution.Types.ForeignLib import qualified Distribution.Types.LegacyExeDependency as Cabal +import Distribution.Types.MungedPackageName import qualified Distribution.Types.UnqualComponentName as Cabal import qualified Distribution.Verbosity as D import Lens.Micro (lens) @@ -299,8 +300,13 @@ packageFromPackageDescription packageConfig pkgFlags (PackageDescriptionPair pkg , packageOpts = GetPackageOpts $ \sourceMap installedMap omitPkgs addPkgs cabalfp -> do (componentsModules,componentFiles,_,_) <- getPackageFiles pkgFiles cabalfp + let internals = S.toList $ internalLibComponents $ M.keysSet componentsModules + excludedInternals <- mapM parsePackageName internals + mungedInternals <- mapM (parsePackageName . toInternalPackageMungedName) internals componentsOpts <- - generatePkgDescOpts sourceMap installedMap omitPkgs addPkgs cabalfp pkg componentFiles + generatePkgDescOpts sourceMap installedMap + (excludedInternals ++ omitPkgs) (mungedInternals ++ addPkgs) + cabalfp pkg componentFiles return (componentsModules,componentFiles,componentsOpts) , packageHasExposedModules = maybe False @@ -325,6 +331,10 @@ packageFromPackageDescription packageConfig pkgFlags (PackageDescriptionPair pkg $ filter (buildable . foreignLibBuildInfo) $ foreignLibs pkg + toInternalPackageMungedName + = T.pack . unMungedPackageName . computeCompatPackageName (pkgName pkgId) + . Just . Cabal.mkUnqualComponentName . T.unpack + -- Gets all of the modules, files, build files, and data files that -- constitute the package. This is primarily used for dirtiness -- checking during build, as well as use by "stack ghci" From c972c8951c713de4590a6f14d03fab511486183b Mon Sep 17 00:00:00 2001 From: Mihai Maruseac Date: Sun, 22 Apr 2018 21:58:13 -0700 Subject: [PATCH 6/9] Add field in pkg description for internal libs and use in GHCi. We need to make a distinction between internal libraries and foreign libraries since we want GHCi to reload when an internal library file changes but we don't want it to confuse an internal library with a foreign one. --- src/Stack/Ghci.hs | 1 + src/Stack/Package.hs | 1 + src/Stack/Types/Package.hs | 1 + 3 files changed, 3 insertions(+) diff --git a/src/Stack/Ghci.hs b/src/Stack/Ghci.hs index f86717f547..07984867ee 100644 --- a/src/Stack/Ghci.hs +++ b/src/Stack/Ghci.hs @@ -663,6 +663,7 @@ wantedPackageComponents bopts (TargetAll ProjectPackage) pkg = S.fromList $ NoLibraries -> [] HasLibraries names -> [CLib] ++ map CInternalLib (S.toList names)) ++ map CExe (S.toList (packageExes pkg)) <> + (map CInternalLib $ S.toList $ packageInternalLibraries pkg) <> (if boptsTests bopts then map CTest (M.keys (packageTests pkg)) else []) <> (if boptsBenchmarks bopts then map CBench (S.toList (packageBenchmarks pkg)) else []) wantedPackageComponents _ _ _ = S.empty diff --git a/src/Stack/Package.hs b/src/Stack/Package.hs index 40cd67c4e2..0a4edeaffd 100644 --- a/src/Stack/Package.hs +++ b/src/Stack/Package.hs @@ -280,6 +280,7 @@ packageFromPackageDescription packageConfig pkgFlags (PackageDescriptionPair pkg | null extraLibNames -> NoLibraries | otherwise -> error "Package has buildable sublibraries but no buildable libraries, I'm giving up" Just _ -> HasLibraries foreignLibNames + , packageInternalLibraries = subLibNames , packageTests = M.fromList [(T.pack (Cabal.unUnqualComponentName $ testName t), testInterface t) | t <- testSuites pkgNoMod diff --git a/src/Stack/Types/Package.hs b/src/Stack/Types/Package.hs index d0a6b55458..6eb77567ae 100644 --- a/src/Stack/Types/Package.hs +++ b/src/Stack/Types/Package.hs @@ -129,6 +129,7 @@ data Package = ,packageFlags :: !(Map FlagName Bool) -- ^ Flags used on package. ,packageDefaultFlags :: !(Map FlagName Bool) -- ^ Defaults for unspecified flags. ,packageLibraries :: !PackageLibraries -- ^ does the package have a buildable library stanza? + ,packageInternalLibraries :: !(Set Text) -- ^ names of internal libraries ,packageTests :: !(Map Text TestSuiteInterface) -- ^ names and interfaces of test suites ,packageBenchmarks :: !(Set Text) -- ^ names of benchmarks ,packageExes :: !(Set Text) -- ^ names of executables From 7e0eba70e6841be3e967748adeb31bb5b4ddcf8d Mon Sep 17 00:00:00 2001 From: Mihai Maruseac Date: Sun, 22 Apr 2018 20:31:59 -0700 Subject: [PATCH 7/9] Update ChangeLog --- ChangeLog.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ChangeLog.md b/ChangeLog.md index 45001c931c..cb46d53451 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -14,6 +14,11 @@ Other enhancements: Bug fixes: * `~/.stack/config.yaml` and `stack.yaml` terminating by newline +* `stack ghci` on a package with internal libraries was erroneously looking + for a wrong package corresponding to the internal library and failing to + load any module. This has been fixed now and changes to the code in the + library and the sublibrary are properly tracked. See + [#3926](https://github.com/commercialhaskell/stack/issues/3926). ## v1.7.1 From e8d40c23fbfcd38dd9b615c7265ce1ecf542d0b3 Mon Sep 17 00:00:00 2001 From: Mihai Maruseac Date: Mon, 23 Apr 2018 06:04:50 -0700 Subject: [PATCH 8/9] Hlint style fixes --- src/Stack/Ghci.hs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Stack/Ghci.hs b/src/Stack/Ghci.hs index 07984867ee..bf98d7782b 100644 --- a/src/Stack/Ghci.hs +++ b/src/Stack/Ghci.hs @@ -661,9 +661,9 @@ wantedPackageComponents _ (TargetComps cs) _ = cs wantedPackageComponents bopts (TargetAll ProjectPackage) pkg = S.fromList $ (case packageLibraries pkg of NoLibraries -> [] - HasLibraries names -> [CLib] ++ map CInternalLib (S.toList names)) ++ + HasLibraries names -> CLib : map CInternalLib (S.toList names)) ++ map CExe (S.toList (packageExes pkg)) <> - (map CInternalLib $ S.toList $ packageInternalLibraries pkg) <> + map CInternalLib (S.toList $ packageInternalLibraries pkg) <> (if boptsTests bopts then map CTest (M.keys (packageTests pkg)) else []) <> (if boptsBenchmarks bopts then map CBench (S.toList (packageBenchmarks pkg)) else []) wantedPackageComponents _ _ _ = S.empty From 2936af5ffcfcb81e30e1b80456691b5207dda7af Mon Sep 17 00:00:00 2001 From: Mihai Maruseac Date: Mon, 23 Apr 2018 06:06:26 -0700 Subject: [PATCH 9/9] Add integration test including reloading and starting from clean --- .../tests/3926-ghci-with-sublibraries/Main.hs | 44 +++++++++++++++++++ .../files/Setup.hs | 2 + .../files/files.cabal | 22 ++++++++++ .../files/src-exe/Main.hs | 7 +++ .../files/src-internal/Internal.v1 | 1 + .../files/src-internal/Internal.v2 | 4 ++ .../files/src/Lib.v1 | 3 ++ .../files/src/Lib.v2 | 6 +++ .../files/stack.yaml | 4 ++ 9 files changed, 93 insertions(+) create mode 100644 test/integration/tests/3926-ghci-with-sublibraries/Main.hs create mode 100644 test/integration/tests/3926-ghci-with-sublibraries/files/Setup.hs create mode 100644 test/integration/tests/3926-ghci-with-sublibraries/files/files.cabal create mode 100644 test/integration/tests/3926-ghci-with-sublibraries/files/src-exe/Main.hs create mode 100644 test/integration/tests/3926-ghci-with-sublibraries/files/src-internal/Internal.v1 create mode 100644 test/integration/tests/3926-ghci-with-sublibraries/files/src-internal/Internal.v2 create mode 100644 test/integration/tests/3926-ghci-with-sublibraries/files/src/Lib.v1 create mode 100644 test/integration/tests/3926-ghci-with-sublibraries/files/src/Lib.v2 create mode 100644 test/integration/tests/3926-ghci-with-sublibraries/files/stack.yaml diff --git a/test/integration/tests/3926-ghci-with-sublibraries/Main.hs b/test/integration/tests/3926-ghci-with-sublibraries/Main.hs new file mode 100644 index 0000000000..6dc68de1d7 --- /dev/null +++ b/test/integration/tests/3926-ghci-with-sublibraries/Main.hs @@ -0,0 +1,44 @@ +import Control.Concurrent +import Control.Monad.IO.Class +import Control.Monad +import Data.List +import StackTest + +main :: IO () +main = do + stack ["clean"] -- to make sure we can load the code even after a clean + copy "src/Lib.v1" "src/Lib.hs" + copy "src-internal/Internal.v1" "src-internal/Internal.hs" + forkIO fileEditingThread + replThread + +replThread :: IO () +replThread = repl [] $ do + replCommand ":main" + line <- replGetLine + when (line /= "hello world") $ error "Main module didn't load correctly." + liftIO $ threadDelay 1000000 -- wait for an edit of the internal library + reloadAndTest "testInt" "42" "Internal library didn't reload." + liftIO $ threadDelay 1000000 -- wait for an edit of the internal library + reloadAndTest "testStr" "\"OK\"" "Main library didn't reload." + +fileEditingThread :: IO () +fileEditingThread = do + threadDelay 1000000 + -- edit the internal library and return to ghci + copy "src-internal/Internal.v2" "src-internal/Internal.hs" + threadDelay 1000000 + -- edit the internal library and end thread, returning to ghci + copy "src/Lib.v2" "src/Lib.hs" + +reloadAndTest :: String -> String -> String -> Repl () +reloadAndTest cmd exp err = do + reload + replCommand cmd + line <- replGetLine + unless (exp `isSuffixOf` line) $ error err + +reload :: Repl () +reload = replCommand ":reload" >> loop + where + loop = replGetLine >>= \line -> unless ("Ok" `isInfixOf` line) loop diff --git a/test/integration/tests/3926-ghci-with-sublibraries/files/Setup.hs b/test/integration/tests/3926-ghci-with-sublibraries/files/Setup.hs new file mode 100644 index 0000000000..9a994af677 --- /dev/null +++ b/test/integration/tests/3926-ghci-with-sublibraries/files/Setup.hs @@ -0,0 +1,2 @@ +import Distribution.Simple +main = defaultMain diff --git a/test/integration/tests/3926-ghci-with-sublibraries/files/files.cabal b/test/integration/tests/3926-ghci-with-sublibraries/files/files.cabal new file mode 100644 index 0000000000..867797cb7b --- /dev/null +++ b/test/integration/tests/3926-ghci-with-sublibraries/files/files.cabal @@ -0,0 +1,22 @@ +name: files +version: 0.1.0.0 +build-type: Simple +cabal-version: >= 2.0 + +library + hs-source-dirs: src + exposed-modules: Lib + build-depends: base, lib + default-language: Haskell2010 + +library lib + hs-source-dirs: src-internal + exposed-modules: Internal + build-depends: base + default-language: Haskell2010 + +executable exe + hs-source-dirs: src-exe + main-is: Main.hs + build-depends: base, files + default-language: Haskell2010 diff --git a/test/integration/tests/3926-ghci-with-sublibraries/files/src-exe/Main.hs b/test/integration/tests/3926-ghci-with-sublibraries/files/src-exe/Main.hs new file mode 100644 index 0000000000..cafae24793 --- /dev/null +++ b/test/integration/tests/3926-ghci-with-sublibraries/files/src-exe/Main.hs @@ -0,0 +1,7 @@ +module Main where + +import Lib + +main :: IO () +main = do + putStrLn "hello world" diff --git a/test/integration/tests/3926-ghci-with-sublibraries/files/src-internal/Internal.v1 b/test/integration/tests/3926-ghci-with-sublibraries/files/src-internal/Internal.v1 new file mode 100644 index 0000000000..d066bb085e --- /dev/null +++ b/test/integration/tests/3926-ghci-with-sublibraries/files/src-internal/Internal.v1 @@ -0,0 +1 @@ +module Internal where diff --git a/test/integration/tests/3926-ghci-with-sublibraries/files/src-internal/Internal.v2 b/test/integration/tests/3926-ghci-with-sublibraries/files/src-internal/Internal.v2 new file mode 100644 index 0000000000..da8a642c7b --- /dev/null +++ b/test/integration/tests/3926-ghci-with-sublibraries/files/src-internal/Internal.v2 @@ -0,0 +1,4 @@ +module Internal where + +testInt :: Int +testInt = 42 diff --git a/test/integration/tests/3926-ghci-with-sublibraries/files/src/Lib.v1 b/test/integration/tests/3926-ghci-with-sublibraries/files/src/Lib.v1 new file mode 100644 index 0000000000..1369151610 --- /dev/null +++ b/test/integration/tests/3926-ghci-with-sublibraries/files/src/Lib.v1 @@ -0,0 +1,3 @@ +module Lib where + +import Internal diff --git a/test/integration/tests/3926-ghci-with-sublibraries/files/src/Lib.v2 b/test/integration/tests/3926-ghci-with-sublibraries/files/src/Lib.v2 new file mode 100644 index 0000000000..d9892d6826 --- /dev/null +++ b/test/integration/tests/3926-ghci-with-sublibraries/files/src/Lib.v2 @@ -0,0 +1,6 @@ +module Lib where + +import Internal + +testStr :: String +testStr = "OK" diff --git a/test/integration/tests/3926-ghci-with-sublibraries/files/stack.yaml b/test/integration/tests/3926-ghci-with-sublibraries/files/stack.yaml new file mode 100644 index 0000000000..df13716817 --- /dev/null +++ b/test/integration/tests/3926-ghci-with-sublibraries/files/stack.yaml @@ -0,0 +1,4 @@ +resolver: ghc-8.2.2 +extra-deps: +- stm-2.4.4.1 +- mtl-2.2.1