Skip to content

Commit 8a471bb

Browse files
Fix filepath identity in cradle dependencies (#2106)
When doing reactive change tracking we track the set of dirty keys as opposed to conservative change tracking where we check every key on every build. Filepath identity becomes even more important in this setting, opening a new type of bug where the filepath used in the dirty key is equivalent but not structurally equal to the filepath used in the dependency graph. This can happen e.g. with cradle dependencies where hie-bios gives us a set of relative paths and we create GetFileModification keys for them as dependencies of the GhcSession, but the LSP client raises FileWatched notifications using absolute paths that we use to create dirty keys. To prevent this class of bugs, we would need to strengthen the notion of NormalizedFilePath to make it always absolute/relative. In practice, since 99% of the paths are introduced by the LSP client, we just need to make sure that the other 1% (introduced by e.g. hie-bios) follow the same convention. This commit fixes the hie-bios paths to make them absolute before creating rule keys, following the LSP convention Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
1 parent 57ffd74 commit 8a471bb

File tree

2 files changed

+12
-9
lines changed

2 files changed

+12
-9
lines changed

ghcide/bench/lib/Experiments.hs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,10 @@ experiments =
156156
forM_ identifierP $ \p -> changeDoc doc [charEdit p]
157157
)
158158
( \docs -> do
159-
Just hieYaml <- uriToFilePath <$> getDocUri "hie.yaml"
160-
liftIO $ appendFile hieYaml "##\n"
159+
hieYamlUri <- getDocUri "hie.yaml"
160+
liftIO $ appendFile (fromJust $ uriToFilePath hieYamlUri) "##\n"
161161
sendNotification SWorkspaceDidChangeWatchedFiles $ DidChangeWatchedFilesParams $
162-
List [ FileEvent (filePathToUri "hie.yaml") FcChanged ]
162+
List [ FileEvent hieYamlUri FcChanged ]
163163
forM_ docs $ \DocumentPositions{..} ->
164164
changeDoc doc [charEdit stringLiteralP]
165165
waitForProgressDone
@@ -171,10 +171,10 @@ experiments =
171171
bench
172172
"hover after cradle edit"
173173
(\docs -> do
174-
Just hieYaml <- uriToFilePath <$> getDocUri "hie.yaml"
175-
liftIO $ appendFile hieYaml "##\n"
174+
hieYamlUri <- getDocUri "hie.yaml"
175+
liftIO $ appendFile (fromJust $ uriToFilePath hieYamlUri) "##\n"
176176
sendNotification SWorkspaceDidChangeWatchedFiles $ DidChangeWatchedFilesParams $
177-
List [ FileEvent (filePathToUri "hie.yaml") FcChanged ]
177+
List [ FileEvent hieYamlUri FcChanged ]
178178
flip allWithIdentifierPos docs $ \DocumentPositions{..} -> isJust <$> getHover doc (fromJust identifierP)
179179
),
180180
---------------------------------------------------------------------------------------

ghcide/src/Development/IDE/Core/Rules.hs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ import qualified Language.LSP.Server as LSP
138138
import Language.LSP.Types (SMethod (SCustomMethod))
139139
import Language.LSP.VFS
140140
import Module
141-
import System.Directory (canonicalizePath)
141+
import System.Directory (canonicalizePath, makeAbsolute)
142142
import TcRnMonad (tcg_dependent_files)
143143

144144
import Control.Applicative
@@ -674,9 +674,12 @@ loadGhcSession = do
674674

675675
-- add the deps to the Shake graph
676676
let addDependency fp = do
677-
let nfp = toNormalizedFilePath' fp
677+
-- VSCode uses absolute paths in its filewatch notifications
678+
afp <- liftIO $ makeAbsolute fp
679+
let nfp = toNormalizedFilePath' afp
678680
itExists <- getFileExists nfp
679-
when itExists $ void $ use_ GetModificationTime nfp
681+
when itExists $ void $ do
682+
use_ GetModificationTime nfp
680683
mapM_ addDependency deps
681684

682685
opts <- getIdeOptions

0 commit comments

Comments
 (0)