Reload required when adding new dependencies to package.yaml/.cabal file

[ratherforky]

When adding new dependencies to package.yaml you need to reload VSCode to keep ghcide up to date.

To reproduce:

• Add a new dependency to the package.yaml of a project
• Import a module from that new dependency
• You get an error for the import
• Reload and the error should go away

Adding a new dependency happens relatively infrequently so this isn't a big deal, but I thought it might be worth noting as a limitation if it isn't easy to fix.

[cocreature]

Thanks for the issue, this is a duplicate of haskell/ghcide#50 so I’m closing it.

[stevenfontanella]

Can we reopen this since the ghcide project has been archived? I've also seen this behavior when updating the .hlint.yaml; vscode needs to be reloaded for its changes to take effect.

[jneira]

hi, thanks to note it, however, all the issues were translated to this repo, ghcide#50 is #1113 which is also closed, so reopening this one

[stevenfontanella]

Thanks for reopening. I didn't know how to find the corresponding issue for ghcide#50. Now that you linked it I see that that issue was supposed to be solved. Do we know what's wrong? I'm still able to reproduce this using the steps from @ratherforky here. Of course it's possible that something is wrong with my setup too. I'm running haskell-language-server-1.3.0-linux-8.10.4 and the haskell vscode extension 1.6.0.

[ratherforky]

The other issue thread mentions a PR being worked on, but doesn't seem to have an actual PR linked to it, so it seems like it was just never implemented (certainly I still reload every time I add a dependency with HLS)

[ratherforky]

I found the commit talked about in that other issue (the commit message linked to the issue with a url rather than a # to the issue number, so that might be why it didn't get linked back to the issue): 0782a6a

Pull request: haskell/ghcide#408

The test for it is sessionDepsArePickedUp:

haskell-language-server/ghcide/test/exe/Main.hs

Line 5174 in 5d83b63

sessionDepsArePickedUp :: TestTree

It seems to only test the case where hie.yaml is changed directly rather than a package.yaml or *.cabal file being changed, and only for a new language extension being added to the cradle. I always use Stack, so I never have a hie.yaml file.

Upon further investigation, it turns out that creating a hie.yaml file for a stack project doesn't help. However, although making a change in package.yaml will not reload anything by itself, making any change at all to hie.yaml will trigger a reload and implement the package.yaml changes. In fact I don't even have to make a change to hie.yaml to trigger a reload, just pressing save with no changes at all will do it (my guess is this is just a vscode quirk). This explains how the test keeps passing while not actually picking up changes to the cradle dependencies like we want it to (adding an explicit package.yaml dependency to hie.yaml doesn't seem to help, presumably cradle: stack: already adds the dependency anyway).

Steps to reproduce

1. Make a new stack project

$ stack new depsReloading

2. Open the project directory in vscode with HLS installed, and add hie.yaml to the root directory, with the contents:

cradle:
  stack:

3. In src/Lib.hs, add:

import Data.Text

This should cause an error without text as a dependency
4. Add text to the package.yaml dependencies and save. We want Lib.hs to no longer show an error after doing this, but the error will still be there.
5. Make a meaningless change to hie.yaml, like adding a newline, and save.
6. Lib.hs should now show no error, without the window ever having to be reloaded

In case this helps at all, nothing is logged when I save package.yaml, but the full logging output from HLS when I save hie.yaml is:

2021-08-16 00:16:40.796900321 [ThreadId 1713] INFO hls:	Consulting the cradle for "src/Lib.hs"
Output from setting up the cradle Cradle {cradleRootDir = "~/Playground/depsReloading", cradleOptsProg = CradleAction: Stack}
> Using main module: 1. Package `depsReloading' component depsReloading:exe:depsReloading-exe with main-is file: ~/Playground/depsReloading/app/Main.hs
> Building all executables for `depsReloading' once. After a successful build of all of them, only specified executables will be rebuilt.
> depsReloading> initial-build-steps (lib + exe)
> The following GHC options are incompatible with GHCi and have not been passed to it: -threaded
> Configuring GHCi with the following packages: depsReloading
> 
> * * * * * * * *
> 
> Warning: Multiple files use the same module name:
>          * Paths_depsReloading found at the following paths
>            * ~/Playground/depsReloading/.stack-work/dist/x86_64-linux-tinfo6/Cabal-3.2.1.0/build/autogen/Paths_depsReloading.hs (depsReloading:lib)
>            * ~/Playground/depsReloading/.stack-work/dist/x86_64-linux-tinfo6/Cabal-3.2.1.0/build/depsReloading-exe/autogen/Paths_depsReloading.hs (depsReloading:exe:depsReloading-exe)
> * * * * * * * *
> 
> ~/Playground/depsReloading/.stack-work/install/x86_64-linux-tinfo6/469d4dcefd60318546cd2e54fa47919ff0e43a5b0013baf7c80aad22f70eec3e/8.10.4/pkgdb:~/.stack/snapshots/x86_64-linux-tinfo6/469d4dcefd60318546cd2e54fa47919ff0e43a5b0013baf7c80aad22f70eec3e/8.10.4/pkgdb:~/.stack/programs/x86_64-linux/ghc-tinfo6-8.10.4/lib/ghc-8.10.4/package.conf.d
2021-08-16 00:16:42.271252207 [ThreadId 1713] INFO hls:	Using interface files cache dir: ~/.cache/ghcide/main-64b0c33860f596bb285ce4bd3e897a916cd87001
2021-08-16 00:16:42.27142081 [ThreadId 1713] INFO hls:	Making new HscEnv[main]

[ratherforky]

Had to do a lot of git archeology to find what happened to this patch.

cradleToSession, which afaict was the main thing implementing the change, got moved to ghcide/exe/Rules.hs, and then subsequently got nuked by this commit: 9837305#diff-4769336a5e6417e801da109eeb61cde82219e25ca5833370b00fb525659fb8e2R267

It does seem to have replaced at least some of the logic (afaict) in a very long loadSession function that now lives here:

haskell-language-server/ghcide/session-loader/Development/IDE/Session.hs

Line 286 in 5d83b63

let deps = componentDependencies opts ++ maybeToList hieYaml

A key part of cradleToSession in the original patch I can't see an obvious equivalent to is the need on the cradle component dependencies (i.e. in our case adding package.yaml to the Shake dependency graph)

haskell-language-server/ghcide/exe/Main.hs

Line 246 in 0782a6a

need existingDeps

I don't currently understand what the new code does for the most part, but I suspect the need being dropped might be the culprit and since the original patch never tested this case fully, the regression could have easily flown under the radar. I could be way off though (I know a bit about Shake, but very little about HLS/ghcide), so input from people who know more about the internals would be greatly appreciated.

[jneira]

@ratherforky many thanks for tne investigation, i guess you are right. Not sure about the old code but now hie-bios (the lib defining and handling "cradles") can gives us the set of files to watch via

https://github.com/mpickering/hie-bios/blob/3703788b30b08435b4fd7d7c3ffceaa16a8b6b8f/src/HIE/Bios/Cradle.hs#L452-L457

and

https://github.com/mpickering/hie-bios/blob/3703788b30b08435b4fd7d7c3ffceaa16a8b6b8f/src/HIE/Bios/Cradle.hs#L644-L650

The new coode should use it (if it does not do it already)