support old hls versions compatible with the requested ghc version #506

mduerig · 2021-11-22T21:36:24Z

Here's a simplistic PR for addressing #454. Probably too simplistic but maybe something to start with (I've never done any TypeScript before, and JavaScript last time in 1998 lol).
This PR cannot cope with prior versions of latestApprovedRelease.cache.json, so make sure to start with a clean local cache (on Linux at :~/.config/Code/User/globalStorage/haskell.haskell.

src/hlsBinaries.ts

jneira · 2021-11-22T21:52:03Z

Hi, many thanks for working in that important feature to handle gracefully ghc deprecation. Dont worry my first experience with typescript also was working with the extension. Better than javascript 😉

I have read the code and i am getting it downloads the info for all releases. However i dont see how it search through them to find the newer hls version with support for the ghc at hand.

To test this the more starightforwrd way would be add in the workflow script a ghc version which is not supported (like 8.10.5) and check all tests are green

vscode-haskell/.github/workflows/test.yml

Line 13 in 627b1da

os: [macos-11, ubuntu-latest, windows-latest]

src/hlsBinaries.ts

mduerig · 2021-11-22T22:04:34Z

However i dont see how it search through them to find the newer hls version with support for the ghc at hand.

This is here:
9c5b6e6#diff-6300b29e042762267f15748c5c6136cbedf8135e07a1881609a478404e1661baR358-R359

There must be better ways to do this but my TypeScript foo is lacking.

jneira · 2021-11-22T22:07:16Z

However i dont see how it search through them to find the newer hls version with support for the ghc at hand.

This is here: 9c5b6e6#diff-6300b29e042762267f15748c5c6136cbedf8135e07a1881609a478404e1661baR358-R359

There must be better ways to do this but my TypeScript foo is lacking.

oh yeah, it was in front of my eyes, thanks. It looks fine to me if it is searching from newer to older, have you checked the ordering?

jneira · 2021-11-22T22:09:06Z

Also i think this is important:

the extension should show an alert message (maybe a confirmation message?) when downloading and old version (as it will not have the same feature set and could suffer some bugs)

maybe users will prefer upgrade their ghc to use a newer version of hls

src/hlsBinaries.ts

jneira · 2021-11-23T09:41:03Z

have you checked the ordering?

the ordering is the correct one, examining https://api.github.com/repos/haskell/haskell-language-server/releases

jneira · 2021-11-23T11:12:22Z

To test this the more starightforwrd way would be add in the workflow script a ghc version which is not supported (like 8.10.5) and check all tests are green

Done with 5178ed2

jneira · 2021-11-23T11:25:39Z

Tests has been succesfull!

https://github.com/haskell/vscode-haskell/runs/4298441947?check_suite_focus=true

 2021-11-23 11:22:33.7270000 [client] INFO Downloading haskell-language-server
2021-11-23 11:22:33.7280000 [client] INFO Using /home/runner/work/vscode-haskell/vscode-haskell/test-workspace/bin to store downloaded binaries
2021-11-23 11:22:33.7280000 [client] INFO Fetching the latest release from GitHub or from cache
2021-11-23 11:22:34.2070000 [client] INFO The latest release is 1.5.0
2021-11-23 11:22:34.2070000 [client] INFO Figure out the ghc version to use or advertise an installation link for missing components
2021-11-23 11:22:34.2070000 [client] INFO Working out the project GHC version. This might take a while...
2021-11-23 11:22:35.9080000 [client] INFO Executing '/home/runner/work/vscode-haskell/vscode-haskell/test-workspace/bin/haskell-language-server-wrapper-1.5.0-linux --project-ghc-version' in cwd '/home/runner/work/vscode-haskell/vscode-haskell/test-workspace' to get the project or file ghc version
2021-11-23 11:22:36.0740000 [client] INFO Execution of '/home/runner/work/vscode-haskell/vscode-haskell/test-workspace/bin/haskell-language-server-wrapper-1.5.0-linux --project-ghc-version' terminated with code 0
2021-11-23 11:22:36.0750000 [client] INFO The GHC version for the project or file: 8.10.4
2021-11-23 11:22:36.0750000 [client] INFO Search for binary haskell-language-server-Linux-8.10.4 in release assets
2021-11-23 11:22:36.0760000 [client] INFO Downloading haskell-language-server 1.4.0 for GHC 8.10.4
2021-11-23 11:22:39.5240000 [client] INFO Activating the language server in working dir: /home/runner/work/vscode-haskell/vscode-haskell/test-workspace (the workspace folder)
2021-11-23 11:22:39.5260000 [client] INFO run command: /home/runner/work/vscode-haskell/vscode-haskell/test-workspace/bin/haskell-language-server-1.4.0-linux-8.10.4 --lsp -d -l hls.log
2021-11-23 11:22:39.5260000 [client] INFO debug command: /home/runner/work/vscode-haskell/vscode-haskell/test-workspace/bin/haskell-language-server-1.4.0-linux-8.10.4 --lsp -d -l hls.log
2021-11-23 11:22:39.5270000 [client] INFO server environment variables:
2021-11-23 11:22:39.5270000 [client] INFO   XDG_CACHE_HOME=/home/runner/work/vscode-haskell/vscode-haskell/test-workspace/cache-test
2021-11-23 11:22:39.5400000 [client] INFO Starting language server

So the code works as intended 😃

jneira

@mduerig looks really good, thanks.
I would change the naming scheme of functions and vars to make them consistent with the actual behaviour and add a window alert the first time the hls obsolete version is downloaded and a warning in the log each time it is used

mduerig · 2021-11-23T11:37:22Z

I would change the naming scheme of functions

Will do as soon as I get around to it. Thanks for reviewing.

* Align identifier names with actual behaviour * rename latestApprovedRelease.cache.json to approvedRelease.cache.json * check for empty array of releases * alert and log when falling back to and older hls version

src/hlsBinaries.ts

mduerig · 2021-11-24T08:34:35Z

Another issue appears when upgrading the plugin. In this case approvedReleases.cache.json is not yet present. Users with update behaviour never-check receive the message "Couldn't find any pre-built haskell-language-server binaries (and checking for newer versions is disabled)".

jneira · 2021-11-24T09:11:15Z

Another issue appears when upgrading the plugin. In this case approvedReleases.cache.json is not yet present. Users with update behaviour never-check receive the message "Couldn't find any pre-built haskell-language-server binaries (and checking for newer versions is disabled)".

oh, that's unfortunate, i guess if users had at some point the download active and then change to never-check the extension should be able to reuse the existing executables, am i understanding the issue correctly?

not sure what can we do if that is the case, search for the executable in the storage path anyways?

mduerig · 2021-11-24T09:36:01Z

if users had at some point the download active and then change to never-check the extension should be able to reuse the
existing executables, am i understanding the issue correctly?

Correct.

Maybe implement a fallback to the old file for readCachedReleaseData? Trying...

jneira · 2021-11-24T10:18:04Z

Maybe implement a fallback to the old file for readCachedReleaseData? Trying...

i would try to not use a file which should be deprecated and forgotten in the code. It even could be deleted by the user to workaround some issue

Other possible workarounds

the mentioned search for the executable haskell-language-server-${newerVersion}-${ghcVersion} in the storage dir. I mean find all files matching the glob haskell-language-server-*-${ghcVersion} and then choose the newer hls version amongs them. It would be independent of any cache file and more reliable but the code would be a little bit more complex.
download the cache metadata even if never-check is enabled but not using it for download anything else, we could use it for warnings about using a hls old version. It could be a little bit surprising see the extension do a http call when you have set never-check and it would fail with no connection available 🤔

…wnloading the releases fails

mduerig · 2021-11-24T11:06:53Z

Here's how this could look like considering the old cache file (latestApprovedRelease.cache.json): 15c8951

i would try to not use a file which should be deprecated and forgotten in the code. It even could be deleted by the user

I wouldn't worry too much about "deleted by the user" because in this case the behaviour is the same as before the changes here. In fact, deleting that file is probably the smoothest upgrade path. However, it bumps all GHC versions even for those who chose "never-check". The part about "deprecated and forgotten" is more worrisome...

the mentioned search for the executable haskell-language-server-${newerVersion}-${ghcVersion} in the storage dir.

The problem is that you don't know which version to search for. This information is in latestApprovedRelease.cache.json.

mduerig · 2021-11-24T11:09:03Z

I think the cleanest upgrade path is to check for existence of latestApprovedRelease.cache.json and covert it to a approvedReleases.cache.json if the latter does not exist already.

jneira · 2021-11-24T11:19:31Z

The problem is that you don't know which version to search for. This information is in latestApprovedRelease.cache.json.

You dont know but you can compute what is the newest one of all of them as you can extract the hls version from hls-1.3.0-ghc-8.10.4 and hls-1.4.0-ghc-8.10.4 -> 1.3.0 and 1.4.0 and compare them with the semver lib already being used. The file name schema becomes the release metadata

I think the cleanest upgrade path is to check for existence of latestApprovedRelease.cache.json and covert it to a approvedReleases.cache.json if the latter does not exist already.

Fair enough, it is working and we could remove it at some point i guess (or implement the file search in some other pr)
I would only ask you to add a comment in readOldCachedReleaseData with the rationale of continue using the old file format, noting it is deprecated

…te or downloading the releases fails" This reverts commit 15c8951.

mduerig · 2021-11-24T16:50:19Z

Here's the approach outlined before.

I prefer this to my other approach because it implements a clean upgrade path and leaves behind less technical debt.

mduerig · 2021-11-24T17:00:19Z

The file name schema becomes the release metadata

Agree this could be the better solution, not caching the meta data at all. I guess it would simplify the whole logic overall. But IMO this is out of scope for this PR. I'm fine either way: merge any of the solutions I proposed here and refactor later or discard this PR and wait for the bigger refactoring.

jneira

Agree this could be the better solution, not caching the meta data at all. I guess it would simplify the whole logic overall. But IMO this is out of scope for this PR. I'm fine either way: merge any of the solutions I proposed here and refactor later or discard this PR and wait for the bigger refactoring.

Dont worry the pr looks great as is, the final solution is good enough for now, we could refactor afterwards
Thanks a lot for tackle this!

support old hls versions compatible with the requested ghc version

9c5b6e6

mduerig mentioned this pull request Nov 22, 2021

Looking for hls old versions compatibles with the requested ghc version #454

Closed

mduerig commented Nov 22, 2021

View reviewed changes

src/hlsBinaries.ts Show resolved Hide resolved

mduerig commented Nov 22, 2021

View reviewed changes

src/hlsBinaries.ts Show resolved Hide resolved

mduerig commented Nov 22, 2021

View reviewed changes

src/hlsBinaries.ts Show resolved Hide resolved

jneira reviewed Nov 22, 2021

View reviewed changes

src/hlsBinaries.ts Outdated Show resolved Hide resolved

jneira mentioned this pull request Nov 23, 2021

HLS not working within GHC (meta improve troubleshooting) haskell/haskell-language-server#2387

Closed

Test ghc deprecated version

5178ed2

Use a real deprecated version

98e43da

jneira requested changes Nov 23, 2021

View reviewed changes

Address review comments:

73d7e0e

* Align identifier names with actual behaviour * rename latestApprovedRelease.cache.json to approvedRelease.cache.json * check for empty array of releases * alert and log when falling back to and older hls version

jneira reviewed Nov 24, 2021

View reviewed changes

src/hlsBinaries.ts Outdated Show resolved Hide resolved

mduerig added 2 commits November 24, 2021 08:50

Inform on fallback to outdated hls only on first download

db4a691

Silence warnings

a350425

Fall back to old cached releases if user doesn't want to update or do…

15c8951

…wnloading the releases fails

mduerig added 2 commits November 24, 2021 16:59

Revert "Fall back to old cached releases if user doesn't want to upda…

7753c4d

…te or downloading the releases fails" This reverts commit 15c8951.

Migrate existing old cache file to new file format

560b8e4

jneira approved these changes Nov 24, 2021

View reviewed changes

This was linked to issues Nov 24, 2021

Looking for hls old versions compatibles with the requested ghc version #454

Closed

Extension Stopped working due to HLS ghc deprecation #504

Closed

jneira merged commit 2fa7a9e into haskell:master Nov 24, 2021

myShoggoth mentioned this pull request Nov 29, 2021

Working with old versions of GHC in VSCode haskellfoundation/tech-proposals#16

Open

jneira mentioned this pull request Dec 4, 2021

[client][ERROR] No binary haskell-language-server-Windows-8.10.4.exe found #513

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support old hls versions compatible with the requested ghc version #506

support old hls versions compatible with the requested ghc version #506

mduerig commented Nov 22, 2021

jneira commented Nov 22, 2021 •

edited

Loading

mduerig commented Nov 22, 2021

jneira commented Nov 22, 2021

jneira commented Nov 22, 2021

jneira commented Nov 23, 2021

jneira commented Nov 23, 2021

jneira commented Nov 23, 2021

jneira left a comment

mduerig commented Nov 23, 2021 •

edited

Loading

mduerig commented Nov 24, 2021

jneira commented Nov 24, 2021

mduerig commented Nov 24, 2021 •

edited

Loading

jneira commented Nov 24, 2021 •

edited

Loading

mduerig commented Nov 24, 2021

mduerig commented Nov 24, 2021

jneira commented Nov 24, 2021 •

edited

Loading

mduerig commented Nov 24, 2021

mduerig commented Nov 24, 2021

jneira left a comment •

edited

Loading

support old hls versions compatible with the requested ghc version #506

support old hls versions compatible with the requested ghc version #506

Conversation

mduerig commented Nov 22, 2021

jneira commented Nov 22, 2021 • edited Loading

mduerig commented Nov 22, 2021

jneira commented Nov 22, 2021

jneira commented Nov 22, 2021

jneira commented Nov 23, 2021

jneira commented Nov 23, 2021

jneira commented Nov 23, 2021

jneira left a comment

Choose a reason for hiding this comment

mduerig commented Nov 23, 2021 • edited Loading

mduerig commented Nov 24, 2021

jneira commented Nov 24, 2021

mduerig commented Nov 24, 2021 • edited Loading

jneira commented Nov 24, 2021 • edited Loading

mduerig commented Nov 24, 2021

mduerig commented Nov 24, 2021

jneira commented Nov 24, 2021 • edited Loading

mduerig commented Nov 24, 2021

mduerig commented Nov 24, 2021

jneira left a comment • edited Loading

Choose a reason for hiding this comment

jneira commented Nov 22, 2021 •

edited

Loading

mduerig commented Nov 23, 2021 •

edited

Loading

mduerig commented Nov 24, 2021 •

edited

Loading

jneira commented Nov 24, 2021 •

edited

Loading

jneira commented Nov 24, 2021 •

edited

Loading

jneira left a comment •

edited

Loading