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

Download and use clojure lsp native binaries #1043

Closed
wants to merge 28 commits into from

Conversation

bpringe
Copy link
Member

@bpringe bpringe commented Feb 24, 2021

What has Changed?

  • Calva now downloads the graalvm-compiled native binary for clojure-lsp for the user's OS and uses that instead of using a jar file
  • The jar file is no longer packaged with Calva, reducing the extension size
  • We no longer have to manually update clojure-lsp, or at least, all that is involved is updating the clojure-lsp version in config.ts
  • Clojure-lsp now starts faster and uses less CPU and memory while running
  • Adds status indication that clojure-lsp is downloading, then that it's activating
  • Download only occurs if the current version of clojure-lsp does not match the configured version, so a download occurs once per machine per clojure-lsp update made in Calva
    • The version is saved to a file after a successful download + unzip
    • Reading the version from a file is way faster than calling the binary to get the version using node. ~0.18 msecs vs ~430 msecs
  • Before downloading a new version, the current version of clojure-lsp is backed up. If the download fails, the backup is used

Todo list

  • Test on Mac
  • Test on Linux
  • Test on Windows
  • Add unit tests

Closes #1017
Addresses #990

My Calva PR Checklist

I have:

  • Read How to Contribute.
  • Directed this pull request at the dev branch. (Or have specific reasons to target some other branch.)
  • Made sure I have changed the PR base branch, so that it is not published. (Sorry for the nagging.)
  • Updated the [Unreleased] entry in CHANGELOG.md, linking the issue(s) that the PR is addressing.
  • Figured if anything about the fix warrants tests on Mac/Linux/Windows/Remote/Whatever, and either tested it there if so, or mentioned it in the PR.
  • Tested the VSIX built from the PR (so, after you've submitted the PR). You'll find the artifacts by clicking Show all checks in the CI section of the PR page, and then Details on the ci/circleci: build test. NB: There is a CircleCI bug that makes the Artifacts hard to find. Please see this issue for workarounds.
    • Tested the particular change
    • Figured if the change might have some side effects and tested those as well.
    • Smoke tested the extension as such.
  • Referenced the issue I am fixing/addressing in a commit message for the pull request.
    • If I am fixing the issue, I have used GitHub's fixes/closes syntax
    • If I am fixing just part of the issue, I have just referenced it w/o any of the "fixes” keywords.
  • Created the issue I am fixing/addressing, if it was not present.
  • Added to or updated docs in this branch, if appropriate

The Calva Team PR Checklist:

Before merging we (at least one of us) have:

  • Made sure the PR is directed at the dev branch (unless reasons).
  • Figured if anything about the fix warrants tests on Mac/Linux/Windows/Remote/Whatever, and tested it there if so.
  • Read the source changes.
  • Given feedback and guidance on source changes, if needed. (Please consider noting extra nice stuff as well.)
  • Tested the VSIX built from the PR (well, if this is a PR that changes the source code.)
    • Tested the particular change
    • Figured if the change might have some side effects and tested those as well.
    • Smoke tested the extension as such.
  • If need be, had a chat within the team about particular changes.

Ping @PEZ, @kstehn, @cfehse, @bpringe

@bpringe bpringe changed the title WIP - Download and use clojure lsp native binaries Download and use clojure lsp native binaries Mar 12, 2021
@bpringe bpringe requested a review from PEZ March 13, 2021 03:23
docs/site/clojure-lsp.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@PEZ PEZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bpringe
Copy link
Member Author

bpringe commented Mar 13, 2021

Just to track here, @PEZ reported an issue with navigating to project source definitions taking a while. He reports the lag does not exist if setting the clojure-lsp version to 2021.02.10-03.01.19 in config.ts, indicating this is likely related to a change in clojure-lsp.

@bpringe
Copy link
Member Author

bpringe commented Mar 22, 2021

Closing this and switching implementation to TypeScript due to #1074.

@bpringe bpringe closed this Mar 22, 2021
@bpringe bpringe deleted the use_clojure-lsp_native_binaries branch March 22, 2021 17:31
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.

2 participants