Skip to content

Pass environment variables to LSP #494

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Nov 6, 2021

Conversation

jacobprudhomme
Copy link
Contributor

Summary

Made it so that the user can define environment variables that will then be passed to the LSP upon execution. Resolves #214

Changes

  • Added extension settings to define environment variables
  • Passed environment variables to LSP

Notes

I had a bit of trouble testing this, I'm not sure if the environment variables actually get passed. Is there a test scenario someone can help me come up with to test this?

@jneira jneira self-requested a review October 30, 2021 12:15
@jneira
Copy link
Member

jneira commented Oct 30, 2021

Hi many thanks for the pr, loosk great overall (will commentabout some details)

I had a bit of trouble testing this, I'm not sure if the environment variables actually get passed. Is there a test scenario someone can help me come up with to test this?

Well, to test this manually we could change a environment variable hls specific, like HIE_BIOS_CACHE, and check hls start to write to that path instead to ~/.cache/hie-bios.

It could be done in automatic tests, reproducing those steps and wait a reasonable amount of time to check the directory is created.

@jneira
Copy link
Member

jneira commented Nov 3, 2021

Another improvement could be add a log output showing the environment vars added. You already have the logger at hand to acomplish it.

@jacobprudhomme
Copy link
Contributor Author

jacobprudhomme commented Nov 3, 2021

Well, to test this manually we could change a environment variable hls specific, like HIE_BIOS_CACHE, and check hls start to write to that path instead to ~/.cache/hie-bios.

Good news @jneira! It looks like everything works when testing manually!

It could be done in automatic tests, reproducing those steps and wait a reasonable amount of time to check the directory is created.

Unfortunately, while trying to write an automatic test for this, the directory doesn't show up, even after setting the timeout to a minute. I can't yet debug the tests to find out why, because I get Error: Cannot find module '(...)/vscode-haskell/out/test' when I try to launch debugging for the Extension Tests profile. Is this something you've encountered before?

Another improvement could be add a log output showing the environment vars added. You already have the logger at hand to acomplish it.

For this, do you want me to log simply that the environment variables have been added, or do you want me to log the list of all environment variables added?

@jneira
Copy link
Member

jneira commented Nov 3, 2021

Good news @jneira! It looks like everything works when testing manually!

Great!

Unfortunately, while trying to write an automatic test for this, the directory doesn't show up, even after setting the timeout to a minute. I can't yet debug the tests to find out why, because I get Error: Cannot find module '(...)/vscode-haskell/out/test' when I try to launch debugging for the Extension Tests profile. Is this something you've encountered before?

debug normal code works for me but i did not try test code, will take a look

For this, do you want me to log simply that the environment variables have been added, or do you want me to log the list of all environment variables added?

I think show the actual env vars would be useful, to double check the final value, as it could be set at global scope or per project and to be included in the logs users send us when there are problems.

I just opened a pr that logs all the env vars in case could be useful for doing that: https://github.com/haskell/vscode-haskell/pull/495/files

@jacobprudhomme
Copy link
Contributor Author

jacobprudhomme commented Nov 3, 2021

@jneira where can I view these logs, to make sure that my changes are working? I have looked around at the Extension Host logs and the logfile that the extension writes to, but cannot find any occurrence of the logger call I added, nor of the ones surrounding it (for example, the ones on lines 236-237). I have also set the log level to debug, but that doesn't seem to have changed anything.

@jneira
Copy link
Member

jneira commented Nov 3, 2021

The log is in a specific buffer in the output panel, called haskell (dirName) I can see thte expected output:

imagen

@jacobprudhomme
Copy link
Contributor Author

Got it! For some reason it wasn't showing it before, but it is now.

@jacobprudhomme
Copy link
Contributor Author

jacobprudhomme commented Nov 4, 2021

@jneira it seems that the cache directory only gets created for stack and cabal projects. I tried writing a basic stack.yaml/<project>.cabal file to the root of the test workspace, hoping that was all that was needed to generate the cache directory. Unfortunately, I've learned HLS will actually try to run stack/cabal, upon detecting the type of project the workspace is.

If the necessary build tool is not available, the tests fail. I don't think it's ideal to introduce the necessity of having stack or cabal just to run this one test, though.

Is there maybe an environment variable that would be simpler to test automatically? I couldn't find any documentation on what other configuration HLS accepts via environment variables.

@jneira
Copy link
Member

jneira commented Nov 4, 2021

Is there maybe an environment variable that would be simpler to test automatically? I couldn't find any documentation on what other configuration HLS accepts via environment variables.

hmm ghcide honours the xdg standard for its cache so if we change XDG_CACHE_HOME to a known location i think it will write something in it even only using ghc

@jacobprudhomme
Copy link
Contributor Author

@jneira sorry to bother you so much! You were right, setting XDG_CACHE_DIR to something else worked. Unfortunately, now I'm really stumped.

When I run the tests, even though I see the directory being created when the test window opens up, my test case still waits 30 seconds and then declares it has not found the directory. I have no clue why this would be happening.

@jneira
Copy link
Member

jneira commented Nov 5, 2021

When I run the tests, even though I see the directory being created when the test window opens up, my test case still waits 30 seconds and then declares it has not found the directory. I have no clue why this would be happening.

Hi, dont worry, the pr is looking great. I think the directory is already created due to previous timeouts when the test case start, so the watcher is not notificating its creation. Maybe a simple existence test reading directly from the file system will work (with a comment noting cause we are not using the watcher).

@jacobprudhomme
Copy link
Contributor Author

Alright, that seems to have settled everything!

@jneira
Copy link
Member

jneira commented Nov 6, 2021

The linux fail is not related with this pr and i hit it in #495. So i am gonna merge as is, many many thans for work on this

@jneira jneira merged commit 2559cae into haskell:master Nov 6, 2021
@jacobprudhomme jacobprudhomme deleted the pass-env-vars-to-server branch November 6, 2021 23:07
@jacobprudhomme
Copy link
Contributor Author

Thank you for all the help in making my first changes to the project!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set project specific environment variables for lsp server
2 participants