on master help sign in
Fix two regressions since 0.1.0 (#471)

* Fix isWorkspaceFile for relative paths This fixes a performance regression on GetFileExists * Avoid interrupting hie-bios when it's doing its thing I noticed that the GHC hie-bios direct cradle, which uses Hadrian, a Shake build system, was failing to start due to the following problem: 1. ghcide starts evaluating the LoadCradle node 2. The evaluation gets cancelled 3. Immediately after, ghcide starts evaluating LoadCradle again 4. Hadrian fails, since there is still another Hadrian process alive taking its Shake lock * Improve watched files test suite

commitc74e9b51f131df446c8e011ae79f3370c683f12b authorPepe Iborra <pepeiborra@gmail.com> dateTue 10 Mar 2020, 17:06 (7 years ago) parent0bf4e91
5 files changed+64−30
exe/Rules.hs+10−4 src/Development/IDE/Core/FileExists.hs+10−4 src/Development/IDE/Core/IdeConfiguration.hs+11−6 src/Development/IDE/Core/Shake.hs+1−1 test/exe/Main.hs+32−15
exe/Rules.hs+10−4
@@ -14,10 +14,10 @@ import Data.ByteString.Base16 (encode)
1414import qualified Data.ByteString.Char8 as B
1515import Data.Functor ((<&>))
1616import Data.Maybe (fromMaybe)
17−import Data.Text (Text)
17+import Data.Text (pack, Text)
1818import Development.IDE.Core.Rules (defineNoFile)
1919import Development.IDE.Core.Service (getIdeOptions)
20−import Development.IDE.Core.Shake (sendEvent, define, useNoFile_)
20+import Development.IDE.Core.Shake (actionLogger, sendEvent, define, useNoFile_)
2121import Development.IDE.GHC.Util
2222import Development.IDE.Types.Location (fromNormalizedFilePath)
2323import Development.IDE.Types.Options (IdeOptions(IdeOptions, optTesting))
@@ -39,6 +39,7 @@ import System.FilePath.Posix (addTrailingPathSeparator,
3939import Language.Haskell.LSP.Messages as LSP
4040import Language.Haskell.LSP.Types as LSP
4141import Data.Aeson (ToJSON(toJSON))
42+import Development.IDE.Types.Logger (logDebug)
4243
4344-- Prefix for the cache path
4445cacheDir :: String
@@ -60,18 +61,23 @@ loadGhcSession =
6061
6162cradleToSession :: Rules ()
6263cradleToSession = define $ \LoadCradle nfp -> do
64+
6365 let f = fromNormalizedFilePath nfp
6466
6567 IdeOptions{optTesting} <- getIdeOptions
6668
69+ logger <- actionLogger
70+ liftIO $ logDebug logger $ "Running cradle " <> pack (fromNormalizedFilePath nfp)
71+
6772 -- If the path points to a directory, load the implicit cradle
6873 mbYaml <- doesDirectoryExist f <&> \isDir -> if isDir then Nothing else Just f
69− cradle <- liftIO $ maybe (loadImplicitCradle $ addTrailingPathSeparator f) loadCradle mbYaml
74+ cradle <- liftIO $ maybe (loadImplicitCradle $ addTrailingPathSeparator f) loadCradle mbYaml
7075
7176 when optTesting $
7277 sendEvent $ notifyCradleLoaded f
7378
74− cmpOpts <- liftIO $ getComponentOptions cradle
79+ -- Avoid interrupting `getComponentOptions` since it calls external processes
80+ cmpOpts <- liftIO $ mask $ \_ -> getComponentOptions cradle
7581 let opts = componentOptions cmpOpts
7682 deps = componentDependencies cmpOpts
7783 deps' = case mbYaml of
src/Development/IDE/Core/FileExists.hs+10−4
@@ -22,6 +22,7 @@ import Development.IDE.Core.FileStore
2222import Development.IDE.Core.IdeConfiguration
2323import Development.IDE.Core.Shake
2424import Development.IDE.Types.Location
25+import Development.IDE.Types.Logger
2526import Development.Shake
2627import Development.Shake.Classes
2728import GHC.Generics
@@ -90,12 +91,15 @@ getFileExists fp = use_ GetFileExists fp
9091-- Provides a fast implementation if client supports dynamic watched files.
9192-- Creates a global state as a side effect in that case.
9293fileExistsRules :: IO LspId -> ClientCapabilities -> VFSHandle -> Rules ()
93−fileExistsRules getLspId ClientCapabilities{_workspace}
94+fileExistsRules getLspId ClientCapabilities{_workspace} vfs
9495 | Just WorkspaceClientCapabilities{_didChangeWatchedFiles} <- _workspace
9596 , Just DidChangeWatchedFilesClientCapabilities{_dynamicRegistration} <- _didChangeWatchedFiles
9697 , Just True <- _dynamicRegistration
97− = fileExistsRulesFast getLspId
98− | otherwise = fileExistsRulesSlow
98+ = fileExistsRulesFast getLspId vfs
99+ | otherwise = do
100+ logger <- logger <$> getShakeExtrasRules
101+ liftIO $ logDebug logger "Warning: Client does not support watched files. Falling back to OS polling"
102+ fileExistsRulesSlow vfs
99103
100104-- Requires an lsp client that provides WatchedFiles notifications.
101105fileExistsRulesFast :: IO LspId -> VFSHandle -> Rules ()
@@ -103,7 +107,9 @@ fileExistsRulesFast getLspId vfs = do
103107 addIdeGlobal . FileExistsMapVar =<< liftIO (newVar [])
104108 defineEarlyCutoff $ \GetFileExists file -> do
105109 isWf <- isWorkspaceFile file
106− if isWf then fileExistsFast getLspId vfs file else fileExistsSlow vfs file
110+ if isWf
111+ then fileExistsFast getLspId vfs file
112+ else fileExistsSlow vfs file
107113
108114fileExistsFast :: IO LspId -> VFSHandle -> NormalizedFilePath -> Action (Maybe BS.ByteString, ([a], Maybe Bool))
109115fileExistsFast getLspId vfs file = do
src/Development/IDE/Core/IdeConfiguration.hs+11−6
@@ -17,6 +17,7 @@ import Development.IDE.Core.Shake
1717import Development.IDE.Types.Location
1818import Development.Shake
1919import Language.Haskell.LSP.Types
20+import System.FilePath (isRelative)
2021
2122-- | Lsp client relevant configuration details
2223data IdeConfiguration = IdeConfiguration
@@ -58,9 +59,13 @@ modifyWorkspaceFolders ide f = do
5859 writeVar var (IdeConfiguration (f ws))
5960
6061isWorkspaceFile :: NormalizedFilePath -> Action Bool
61−isWorkspaceFile file = do
62− IdeConfiguration {..} <- getIdeConfiguration
63− let toText = getUri . fromNormalizedUri
64− return $ any
65− (\root -> toText root `isPrefixOf` toText (filePathToUri' file))
66− workspaceFolders
62+isWorkspaceFile file =
63+ if isRelative (fromNormalizedFilePath file)
64+ then return True
65+ else do
66+ IdeConfiguration {..} <- getIdeConfiguration
67+ let toText = getUri . fromNormalizedUri
68+ return $
69+ any
70+ (\root -> toText root `isPrefixOf` toText (filePathToUri' file))
71+ workspaceFolders
src/Development/IDE/Core/Shake.hs+1−1
@@ -20,7 +20,7 @@
2020-- between runs. To deserialise a Shake value, we just consult Values.
2121module Development.IDE.Core.Shake(
2222 IdeState, shakeExtras,
23− ShakeExtras(..), getShakeExtras,
23+ ShakeExtras(..), getShakeExtras, getShakeExtrasRules,
2424 IdeRule, IdeResult, GetModificationTime(..),
2525 shakeOpen, shakeShut,
2626 shakeRun,
test/exe/Main.hs+32−15
@@ -12,6 +12,7 @@ import Control.Applicative.Combinators
1212import Control.Exception (catch)
1313import Control.Monad
1414import Control.Monad.IO.Class (liftIO)
15+import Data.Aeson (Value)
1516import Data.Char (toLower)
1617import Data.Foldable
1718import Data.List
@@ -429,21 +430,28 @@ codeLensesTests = testGroup "code lenses"
429430
430431watchedFilesTests :: TestTree
431432watchedFilesTests = testGroup "watched files"
432− [ testSession "workspace file" $ do
433− _ <- openDoc' "A.hs" "haskell" "module A where"
434− RequestMessage{_params = RegistrationParams (List regs)} <- skipManyTill anyMessage (message @RegisterCapabilityRequest)
435− let watchedFileRegs =
436− [ args | Registration _id WorkspaceDidChangeWatchedFiles args <- regs ]
437− liftIO $ assertBool "watches workspace files" $ not $ null watchedFileRegs
438− , testSession "non workspace file" $ do
439− _ <- openDoc' "/tmp/A.hs" "haskell" "module A where"
440− msgs <- manyTill (Just <$> message @RegisterCapabilityRequest <|> Nothing <$ anyMessage) (message @WorkDoneProgressEndNotification)
441− let watchedFileRegs =
442− [ args
443− | Just (RequestMessage{_params = RegistrationParams (List regs)}) <- msgs
444− , Registration _id WorkspaceDidChangeWatchedFiles args <- regs
445− ]
446− liftIO $ watchedFileRegs @?= []
433+ [ testSession' "workspace files" $ \sessionDir -> do
434+ liftIO $ writeFile (sessionDir </> "hie.yaml") $ "cradle: {direct: {arguments: [\"-isrc\"]}}"
435+ _ <- openDoc' "A.hs" "haskell" "{-#LANGUAGE NoImplicitPrelude #-}\nmodule A where\nimport B"
436+ watchedFileRegs <- getWatchedFilesSubscriptionsUntilProgressEnd
437+
438+ -- Expect 6 subscriptions (A does not get any because it's VFS):
439+ -- - /path-to-workspace/B.hs
440+ -- - /path-to-workspace/B.lhs
441+ -- - B.hs
442+ -- - B.lhs
443+ -- - src/B.hs
444+ -- - src/B.lhs
445+ liftIO $ length watchedFileRegs @?= 6
446+
447+ , testSession' "non workspace file" $ \sessionDir -> do
448+ liftIO $ writeFile (sessionDir </> "hie.yaml") $ "cradle: {direct: {arguments: [\"-i/tmp\"]}}"
449+ _ <- openDoc' "A.hs" "haskell" "{-# LANGUAGE NoImplicitPrelude#-}\nmodule A where\nimport B"
450+ watchedFileRegs <- getWatchedFilesSubscriptionsUntilProgressEnd
451+
452+ -- Expect 4 subscriptions:
453+ liftIO $ length watchedFileRegs @?= 4
454+
447455 -- TODO add a test for didChangeWorkspaceFolder
448456 ]
449457
@@ -2229,3 +2237,12 @@ nthLine i r
22292237 | i == 0 && Rope.rows r == 0 = r
22302238 | i >= Rope.rows r = error $ "Row number out of bounds: " <> show i <> "/" <> show (Rope.rows r)
22312239 | otherwise = Rope.takeWhile (/= '\n') $ fst $ Rope.splitAtLine 1 $ snd $ Rope.splitAtLine (i - 1) r
2240+
2241+getWatchedFilesSubscriptionsUntilProgressEnd :: Session [Maybe Value]
2242+getWatchedFilesSubscriptionsUntilProgressEnd = do
2243+ msgs <- manyTill (Just <$> message @RegisterCapabilityRequest <|> Nothing <$ anyMessage) (message @WorkDoneProgressEndNotification)
2244+ return
2245+ [ args
2246+ | Just (RequestMessage{_params = RegistrationParams (List regs)}) <- msgs
2247+ , Registration _id WorkspaceDidChangeWatchedFiles args <- regs
2248+ ]