Skip to content
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

Exit with non-zero exitcode if wrapper fails to launch #4110

Merged
merged 3 commits into from Mar 3, 2024

Conversation

fendor
Copy link
Collaborator

@fendor fendor commented Mar 1, 2024

As reported by #3748 (comment), the tests are not run correctly, since there is no GHC installed. We install the recommended GHC version to make sure one (hopefully) supported GHC version is installed for running the wrapper test.

Further, if setup fails, HLS-wrapper launches an LSP server that allows users to restart the server once they have fixed potential setup issues. That's only desired if we are indeed launching as an LSP server. Thus, we exit with a non-zero exit code if setup fails and we are not launching in LSP mode.

@fendor fendor requested a review from michaelpj as a code owner March 1, 2024 12:45
@fendor fendor requested review from hasufell and michaelpj and removed request for michaelpj March 1, 2024 12:45
@hasufell
Copy link
Member

hasufell commented Mar 1, 2024

I can't really judge whether this is correct. Have you tested it?

@fendor
Copy link
Collaborator Author

fendor commented Mar 1, 2024

I have tested the Wrapper change locally. On 2.6.0.0, when the wrapper runs typecheck and fails project setup (e.g. unknown ghc version), then it launches the lsp server any way. Now, it doesn't, which is the correct behaviour.

Only when in LSP mode, we want to launch the LSP server that offers the
restart capability.
@fendor fendor added the status: needs review This PR is ready for review label Mar 2, 2024
@michaelpj michaelpj enabled auto-merge (squash) March 3, 2024 21:27
@michaelpj michaelpj merged commit 2ec645d into haskell:master Mar 3, 2024
38 checks passed
@fendor fendor mentioned this pull request Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs review This PR is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants