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

feat: use lake serve #97

Closed
wants to merge 39 commits into from
Closed

feat: use lake serve #97

wants to merge 39 commits into from

Conversation

lovettchris
Copy link
Contributor

@lovettchris lovettchris commented Jan 8, 2022

See Zulip Discussion

Proposal, replace the following setting:
image

With:
image

Then the rules for using lake serve would be:

  1. is 'lake' or 'lake.exe' found in the toolchainPath, If not don't use lake
  2. is there a lakefile.lean in the current workspace root? If not don't use lake
  3. If the above are both good, use lake serve with optional +version if there's a workspace override in effect.
  4. If lake serve -- args fails fall back on lean --server.

@lovettchris lovettchris self-assigned this Jan 10, 2022
@lovettchris lovettchris linked an issue Jan 13, 2022 that may be closed by this pull request
Copy link
Member

@gebner gebner left a comment

Choose a reason for hiding this comment

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

Otherwise lgtm. Let's wait for the lake version bump to land in lean and check that the integration works as expected, then merge.

vscode-lean4/src/leanclient.ts Outdated Show resolved Hide resolved
vscode-lean4/src/leanclient.ts Outdated Show resolved Hide resolved
Co-authored-by: Gabriel Ebner <gebner@gebner.org>
@lovettchris
Copy link
Contributor Author

Otherwise lgtm. Let's wait for the lake version bump to land in lean and check that the integration works as expected, then merge.

When will that be, I don't see any Lake PR that is removing the "pre" specialDesc label...

@tydeu
Copy link
Member

tydeu commented Jan 28, 2022

@lovettchris my hope is to get it done this weekend, now that I have finished with other obligations that had been occupying my time.

@lovettchris
Copy link
Contributor Author

@gebner I merged this with master, but it was a nasty merge, so will need thorough testing.

@lovettchris
Copy link
Contributor Author

@gebner - another tricky merge with master. We both changed ensureClient to take a Uri, but the new version also returns a boolean telling caller if this was a new client or a cached client, needed in the implementation of installChanged. So anyway, I tested it on a multi-workspace situation and it seems to work, the latest build of Lean now has "Lake 3.0.0" and so lake is now showing up in the ps trace, and you can mix and match, here's a multi-workspace with one using older version of lean so it's running "lean --server" directly, while the other is using Lake. Nice.

Code.exe: "Code.exe" .
  Code.exe: "Code.exe" --ms-enable-electron-run-as-node --nolazy --inspect-brk=53727 "resources\app\out\bootstrap-fork" --type=extensionHost --skipWorkspaceStorageLock
    cmd.exe: C:\WINDOWS\system32\cmd.exe /d /s /c "lean --server d:\Temp\lean_examples\test"
      lean.exe: lean  --server d:\Temp\lean_examples\test
        lean.exe: "leanprover--lean4---nightly-2022-01-20\bin\lean.exe" --server d:\Temp\lean_examples\test
          lean.exe: leanprover--lean4---nightly-2022-01-20\bin\lean.exe "--worker" "file:///d%3A/Temp/lean_examples/test/Test.lean"
    cmd.exe: C:\WINDOWS\system32\cmd.exe /d /s /c "lake serve -- d:\Temp\lean_examples\Foo"
      lake.exe: lake  serve -- d:\Temp\lean_examples\Foo
        lake.exe: "leanprover--lean4---nightly\bin\lake.exe" serve -- d:\Temp\lean_examples\Foo
          lean.exe: leanprover--lean4---nightly\bin\lean.exe "--server" "d:\Temp\lean_examples\Foo"
            lean.exe: leanprover--lean4---nightly\bin\lean.exe "--worker" "file:///d%3A/Temp/lean_examples/Foo/Main.lean"

@lovettchris
Copy link
Contributor Author

I am seeing one interesting warning when I remove a folder from my multi-folder workspace:

Watchdog error: Got `shutdown` request, expected an `exit` notification

Do you know what LSP is talking about?

@Vtec234
Copy link
Member

Vtec234 commented Feb 3, 2022

@lovettchris : according to the LSP spec, after the client sends a shutdown request it should follow that up with an exit notification but not anything else inbetween. So I'm guessing the client actually does send something else, which a spec violation. It would be better to print out the actual message received here. We could also make the server ignore this, but I rather keep it spec-compliant and fix the client.

vscode-lean4/src/leanclient.ts Outdated Show resolved Hide resolved
vscode-lean4/src/utils/leanpkg.ts Outdated Show resolved Hide resolved
thanks, didn't know about that handy constructor.

Co-authored-by: Gabriel Ebner <gebner@gebner.org>
@tydeu
Copy link
Member

tydeu commented Feb 4, 2022

@lovettchris A general note: In elan paralance, the toolchain path is to the root directory (sysroot) of the toolchain (e.g., leanprover--lean4---nightly), not the bin directory of the toolchain. So VSCode should ideally be using the executables at <toolchainPath>/bin.

Add validation code to selectToolchain to ensure the path specified looks right, with graceful handling if user provides the 'bin' folder instead of the parent folder.
@lovettchris
Copy link
Contributor Author

lovettchris commented Feb 8, 2022

@lovettchris A general note: In elan paralance, the toolchain path is to the root directory (sysroot) of the toolchain (e.g., leanprover--lean4---nightly), not the bin directory of the toolchain. So VSCode should ideally be using the executables at <toolchainPath>/bin.

Done. But in testing this things are going a bit wonky, I tried debugging it but got stuck, so I would love your help. It seems now that when I use SelectToolchain to select a different version, or a custom path, the lean server restarts no problem, I get "running" state and the pstree shows the right version of lean.exe is running, but any change to a .lean source file returns an error saying:

Watchdog error: cannot find open document 'file:///d%3A/Temp/lean_examples/Foo/Main.lean'
[Info  - 8:57:59 PM] Connection to server got closed. Server will restart.
Watchdog error: cannot find open document 'file:///d%3A/Temp/lean_examples/Foo/Main.lean'
[Info  - 8:58:00 PM] Connection to server got closed. Server will restart.

Here are the log files for the newly restarted lean server (started with lake serve):

logs.zip

I have no idea why a newly created lean client things the file is closed. It is not closed... Interestingly, if I do a Developer: Reload Window command then everything works fine on the newly selected version... but a Lean4: Restart Server command does not fix it. So some sort of weird residual thing going on...

@lovettchris
Copy link
Contributor Author

Update: there's huge merge conflicts between this PR and fix: #115 : Infoview not working for external file so I'm going to pre-empt that mess, and re-apply the changes in this PR on top of that PR carefully checking every line and perhaps that will solve this weird problem I'm running into here.

@lovettchris
Copy link
Contributor Author

Indeed, re-applying the changes on top of #117 solved the problem.

@lovettchris lovettchris deleted the clovett/lake_serve branch February 10, 2022 21:26
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.

Start lean server using lake serve
5 participants