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

Refactorings and jar definition lookups do not work on Windows #223

Closed
PEZ opened this issue Dec 28, 2020 · 41 comments · Fixed by #226
Closed

Refactorings and jar definition lookups do not work on Windows #223

PEZ opened this issue Dec 28, 2020 · 41 comments · Fixed by #226
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@PEZ
Copy link
Contributor

PEZ commented Dec 28, 2020

I am not sure this is not related to Calva, but it seems we do things right on the client end when I look at it.

Anyway, for this example, on Windows,, definitions for remove, apply, mapv etcetera do not work. Definitions for util/add7 and for v works.

(ns pez.lab.seven-segments
  (:require [clojure.string :as string]
            [pez.util :as util]))

(comment
  (util/add7 8))

(defn transpose [v]
  (->> v
       (remove nil?)
       (apply mapv vector)))
...

Also refactorings like trying to unwind the thread on v doesn't work. It gives a warning in the log:

WARN clojure-lsp.handlers: Could not find a form at this location. ...what looks like the right location to me...

This seem somewhat covered by #25

@kstehn
Copy link

kstehn commented Dec 28, 2020

Regarding the jar definition i found in the lsp log that it cant start lein.
I then looked into the code and basicly lein classpath gets executed by the java shell.

Started my own repl and tested:
(clojure.java.shell/sh "lein" "classpath") => not found file
(clojure.java.shell/sh "which" "lein") => correct path to bin
(clojure.java.shell/sh "lein.bat" "classpath") => works as expected

@fonghou
Copy link

fonghou commented Dec 28, 2020

@kstehn, that can be fixed in .lsp/config.edn

{:project-specs [{:project-path "project.clj"
                         :classpath-cmd ["lein.bat" "classpath"]}]}

However, It seems clojure-lsp in calva use clj-kondo to scan classpath dependent jars and cache metadata in .clj-kondo/.cache, but clj-kondo doesn't seem do that either on windows.

@ericdallo
Copy link
Member

ericdallo commented Dec 28, 2020

@PEZ could you confirm @fonghou fix the issue? I wonder what we could change on clojure-lsp for a permanent fix

@ericdallo ericdallo added help wanted Extra attention is needed bug Something isn't working labels Dec 28, 2020
@fonghou
Copy link

fonghou commented Dec 28, 2020

@PEZ's workaround restored most of calva features with connected repl (autocomplete still doesn't work in comment block though).

@ericdallo, I think the fundamental problem on windows is that clj-kondo on windows does not index dependent jars from classpath path (see below screenshot). If clojure-lsp in calva depends on it, most of clojure-lsp features that rely on these metadata won't work.

image

My setup on windows, lein classpath only has one jar in .cpcache because windows can't handle long classpath. The full classpath is in META-INF/MANIFEST.MF. I don't think clj-kondo scan jars from there. Hence only two .json files from project src showed up .clj-kondo/.cache.

One more observation, I used to use clojure-lsp in coc-nvim running in WLS Ubuntu on the same windows laptop, I remember everyting work fine including refactoring and autocomplete features that depends on full classpath metadata. But these feature seems not avaliable in calva integrated clojure-lsp even VSCode is started from remote WSL (I verified integrated clj-kondo indeed scan all jars successfully in this case).

@ericdallo
Copy link
Member

Sorry, but I still don't get it if it's a clj-kondo, clojure-lsp or Calva issue 🤷🏻‍♂️

@fonghou
Copy link

fonghou commented Dec 28, 2020

Things seem get complicated when all three interact with each other :-)

No worries, what's been added are still great. Thank you very much for your work!

@ericdallo
Copy link
Member

Thanks!

Still, if we have any evidence that may be a bug on clojure-lsp I'll glad to help, let's wait for more users report, also I have no Windows to test the changes

@bpringe
Copy link
Contributor

bpringe commented Dec 29, 2020

I tested this in a Windows VM and can reproduce. This is the code I'm using on both, in a windows.clj file. I put my cursor at hiccup.core in the ns require and run Go To Definition.

(ns windows
  (:require [hiccup.core :as h]))

(comment
  (h/html [:div "hello"]))

This is the request/response on Linux (working):

[Trace - 10:12:15 AM] Sending request 'textDocument/definition - (110)'.
Params: {
    "textDocument": {
        "uri": "file:///home/brandon/development/clojure-test/src/windows.clj"
    },
    "position": {
        "line": 1,
        "character": 21
    }
}


[Trace - 10:12:15 AM] Received response 'textDocument/definition - (110)' in 8ms.
Result: {
    "uri": "jar:file:///home/brandon/.m2/repository/hiccup/hiccup/1.0.5/hiccup-1.0.5.jar!/hiccup/core.clj",
    "range": {
        "start": {
            "line": 0,
            "character": 4
        },
        "end": {
            "line": 0,
            "character": 15
        }
    }
}

And this is the request/response on Windows (not working - also it's an image because not easy to copy/paste from my VM without some added effort:

image

@bpringe
Copy link
Contributor

bpringe commented Dec 29, 2020

Both of the above are in a deps.edn project

@ericdallo
Copy link
Member

Oh, so it looks separator issue on C:// begging parsed as C%3A/ indeed

@ericdallo
Copy link
Member

@bpringe you may have noticed that, but Calva probably is sending the uri wrong in the request, right?

@bpringe
Copy link
Contributor

bpringe commented Dec 29, 2020

I'm not sure how clojure-lsp is saving the uri. What does it want? Maybe file:///c:/...? This is just how our client, vscode-languageclient, seems to be sending it, but I think we can intercept and modify it.

@ericdallo
Copy link
Member

Not sure how is the correct to be sent, could you check other LSP requests/responses and how it's sending the uri?
When LSP initialize it may log uri too

@bpringe
Copy link
Contributor

bpringe commented Dec 29, 2020

Well, other requests that work are also sending the uri like that:

image

So I think this is probably related to jar files + windows, specifically. 🤔

Some snippets from the initialize request:

image
image

@ericdallo
Copy link
Member

Yeah, probably... we'd need to know how we should send the jar files path for windows :/
I wonder how LSP for java/scala do that

@kstehn
Copy link

kstehn commented Dec 29, 2020

So the Uri isnt the Problem here.
But you are right That its jar + Windows.

I dont know if you can find it in your lsp logs but on my machine it tries to execute lein classpath to Index all jar files and the containing namespaces but it failes because the shell cant find lein

@bpringe
Copy link
Contributor

bpringe commented Dec 29, 2020

Got it, I'll look into that. I'm using clj but could be the same thing.

@bpringe
Copy link
Contributor

bpringe commented Dec 29, 2020

Aha! From /tmp/lsp.out (pasting from my Windows VM):

WARN  clojure-lsp.crawler: Error while looking up classpath info in c:\Users\brand\development\clojure-test Cannot run program "clj" (in directory "c:\Users\brand\development\clojure-test"): CreateProcess error=2, The system cannot find the file specified
java.io.IOException: Cannot run program "clj" (in directory "c:\Users\brand\development\clojure-test"): CreateProcess error=2, The system cannot find the file specified
	at java.lang.ProcessBuilder.start(Unknown Source)
	at java.lang.Runtime.exec(Unknown Source)
	at clojure.java.shell$sh.invokeStatic(shell.clj:113)
	at clojure.java.shell$sh.doInvoke(shell.clj:79)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invokeStatic(core.clj:665)
	at clojure.core$apply.invoke(core.clj:660)
	at clojure_lsp.crawler$lookup_classpath.invokeStatic(crawler.clj:228)
	at clojure_lsp.crawler$lookup_classpath.invoke(crawler.clj:225)

@bpringe
Copy link
Contributor

bpringe commented Dec 29, 2020

We may need to play with configuration to find what lsp needs, as @fonghou was saying.

@bpringe
Copy link
Contributor

bpringe commented Dec 29, 2020

Thanks for the info with the sh function @kstehn. I'm investigating that more now.

@bpringe
Copy link
Contributor

bpringe commented Dec 29, 2020

I've added this config and I don't get the above exception any more, but LSP still does not find the reference in the jar dep. Btw (clojure.java.shell/sh "powershell.exe" "clj" "-Spath") does work.

{:project-specs [{:project-path "deps.edn"
                  :classpath-cmd ["powershell.exe" "clj" "-Spath"]}]}
WARN  clojure-lsp.handlers: Finding definition file:///c:/Users/brand/development/clojure-test/src/windows.clj row 2 col 22 cursor hiccup.core
WARN  clojure-lsp.handlers: Could not find definition for element under cursor, I think your cursor is: "hiccup.core" qualified as: hiccup.core

@bpringe
Copy link
Contributor

bpringe commented Dec 29, 2020

On windows, with the above classpath-cmd, the classpath is returned like "src;C:\\Users\\brand\\.m2\\repository\\hiccup\\hiccup\\1.0.5\\hiccup-1.0.5.jar;C:\\Users\\brand\\.m2\\repository\\org\\clojure\\clojure\\1.10.1\\clojure-1.10.1.jar;C:\\Users\\brand\\.m2\\repository\\org\\clojure\\core.specs.alpha\\0.2.44\\core.specs.alpha-0.2.44.jar;C:\\Users\\brand\\.m2\\repository\\org\\clojure\\spec.alpha\\0.2.176\\spec.alpha-0.2.176.jar\r\n"

In the lsp messages I see this after trying to go to definition:

[Trace - 7:48:55 PM] Sending request 'textDocument/definition - (11)'.
Params: {
    "textDocument": {
        "uri": "file:///c%3A/Users/brand/development/clojure-test/src/windows.clj"
    },
    "position": {
        "line": 1,
        "character": 23
    }
}


[Trace - 7:48:55 PM] Received response 'textDocument/definition - (11)' in 7ms.
Result: {
    "uri": "jar:file://C:\\Users\\brand\\.m2\\repository\\hiccup\\hiccup\\1.0.5\\hiccup-1.0.5.jar!/hiccup/core.clj",
    "range": {
        "start": {
            "line": 0,
            "character": 4
        },
        "end": {
            "line": 0,
            "character": 15
        }
    }
}


[Trace - 7:48:55 PM] Sending request 'clojure/dependencyContents - (12)'.
Params: {
    "uri": "jar:file://C:\\Users\\brand\\.m2\\repository\\hiccup\\hiccup\\1.0.5\\hiccup-1.0.5.jar!/hiccup/core.clj"
}


[Trace - 7:48:58 PM] Received response 'clojure/dependencyContents - (12)' in 2302ms.
No result returned.

So we can see lsp is sending back the document uri like "jar:file://C:\\Users\\brand\\.m2\\repository\\hiccup\\hiccup\\1.0.5\\hiccup-1.0.5.jar!/hiccup/core.clj" but the dependencyContents request with the same uri is getting nothing in the response.

@bpringe
Copy link
Contributor

bpringe commented Dec 29, 2020

Also regarding my message about lsp still not finding the ref after config change, I had to delete the sqllite db in .lsp the restart the server, that's when I got the results mentioned above.

@ericdallo
Copy link
Member

Hum, quite interesting, this is the code from the dependencyContents:
https://github.com/clojure-lsp/clojure-lsp/blob/master/src/clojure_lsp/handlers.clj#L340-L348

@ericdallo
Copy link
Member

ericdallo commented Dec 29, 2020

the issue is: it tries to find in db the uri: "jar:file://C:\\Users\\brand\\.m2\\repository\\hiccup\\hiccup\\1.0.5\\hiccup-1.0.5.jar!/hiccup/core.clj
But it will only work for file:///c%3A/Users/brand/development/clojure-test/src/windows.clj like your first request

@bpringe
Copy link
Contributor

bpringe commented Dec 29, 2020

When I look in the sqlite db, though, the uri in the jar_envs column for hiccup is this, which seems to match the request: "jar:file://C:\\Users\\brand\\.m2\\repository\\hiccup\\hiccup\\1.0.5\\hiccup-1.0.5.jar!/hiccup/core.clj" 🤔

@ericdallo
Copy link
Member

@bpringe could you add a (log/info) here logging doc and/or doc-id and compile it with lein bin and check what shows in /tmp/clojure-lsp.out?

@ericdallo
Copy link
Member

FYI, we now include clojure-lsp.bar binary in releases: https://github.com/clojure-lsp/clojure-lsp/releases/edit/2020.12.29-20.42.24

@bpringe
Copy link
Contributor

bpringe commented Dec 29, 2020

Took me a while to get dependencies set up in my fresh Windows VM, but finally here are the logs:

INFO  clojure-lsp.handlers: Finding definition file:///c:/Users/brand/development/clojure-test/src/core.clj row 2 col 23 cursor hiccup.core
DEBUG clojure-lsp.main: :definition 4 ([:codeAction #{8625616749999}])
INFO  clojure-lsp.handlers: doc>>> #object[org.eclipse.lsp4j.TextDocumentIdentifier 0x6b86c00d TextDocumentIdentifier [
  uri = "jar:file://C:\Users\brand\.m2\repository\hiccup\hiccup\1.0.5\hiccup-1.0.5.jar!/hiccup/core.clj"
]]
INFO  clojure-lsp.handlers: doc-id>>> jar:file://C:\Users\brand\.m2\repository\hiccup\hiccup\1.0.5\hiccup-1.0.5.jar!/hiccup/core.clj
ERROR clojure-lsp.main: #error {
 :cause \\C\Users\brand\.m2\repository\hiccup\hiccup\1.0.5\hiccup-1.0.5.jar: The network path was not found
 :via
 [{:type java.nio.file.FileSystemException
   :message \\C\Users\brand\.m2\repository\hiccup\hiccup\1.0.5\hiccup-1.0.5.jar: The network path was not found
   :at [sun.nio.fs.WindowsException translateToIOException WindowsException.java 92]}]
 :trace
 [[sun.nio.fs.WindowsException translateToIOException WindowsException.java 92]
  [sun.nio.fs.WindowsException rethrowAsIOException WindowsException.java 103]
  [sun.nio.fs.WindowsException rethrowAsIOException WindowsException.java 108]
  [sun.nio.fs.WindowsFileAttributeViews$Basic readAttributes WindowsFileAttributeViews.java 53]
  [sun.nio.fs.WindowsFileAttributeViews$Basic readAttributes WindowsFileAttributeViews.java 38]
  [sun.nio.fs.WindowsFileSystemProvider readAttributes WindowsFileSystemProvider.java 198]
  [java.nio.file.Files readAttributes Files.java 1843]
  [java.util.zip.ZipFile$Source get ZipFile.java 1198]
  [java.util.zip.ZipFile$CleanableResource <init> ZipFile.java 701]
  [java.util.zip.ZipFile <init> ZipFile.java 240]
  [java.util.zip.ZipFile <init> ZipFile.java 171]
  [java.util.jar.JarFile <init> JarFile.java 347]
  [sun.net.www.protocol.jar.URLJarFile <init> URLJarFile.java 103]
  [sun.net.www.protocol.jar.URLJarFile getJarFile URLJarFile.java 72]
  [sun.net.www.protocol.jar.JarFileFactory get JarFileFactory.java 94]
  [sun.net.www.protocol.jar.JarURLConnection connect JarURLConnection.java 125]
  [sun.net.www.protocol.jar.JarURLConnection getJarFile JarURLConnection.java 92]
  [clojure_lsp.handlers$fn__25231 invokeStatic handlers.clj 347]
  [clojure_lsp.handlers$fn__25231 invoke handlers.clj 340]
  ... more (omitted)

Note the doc>>> and doc-id>>> logs I added.

I also got that error when I called extension directly in the repl. I see in that error: \\C\Users... is the network path not found. I wonder if this is supposed to be \\C:\Users... (with a :), or something else...

@ericdallo
Copy link
Member

Thank you @bpringe, not sure yet what is causing it...
I can see this is a known issue for a while: #25 (comment)

@ericdallo
Copy link
Member

@bpringe I think the worst here is don't know what is the correct uri path to respond to client for windows users...
I'd suggest changing the clojure/dependencyContents request sending different paths and check which one will work... I can't think now in anything better than that though

@bpringe
Copy link
Contributor

bpringe commented Dec 30, 2020

Thanks @ericdallo. I'm currently playing in the repl in clojure-lsp on Windows with the extension function and trying to see if I can find what kind of path it likes.

@bpringe
Copy link
Contributor

bpringe commented Dec 30, 2020

I found that the code works if I change the // (double slash) after file: to a / (single slash). You can run the following code in the clojure-lsp.handlers namespace on Windows and it works:

(import (org.eclipse.lsp4j TextDocumentIdentifier))
(->> "jar:file:/C:\\Users\\brand\\.m2\\repository\\hiccup\\hiccup\\1.0.5\\hiccup-1.0.5.jar!/hiccup/core.clj"
     TextDocumentIdentifier.
     (extension "dependencyContents"))

clojure-lsp stores them with file:// so it fails. I know we could modify the uri when the request is sent (not yet positive this would work though), but I'm guessing this should be fixed in the clojure-lsp db because maybe it will affect other things?

@ericdallo
Copy link
Member

Well done!
Right, it's one single place that store the path for jars: https://github.com/clojure-lsp/clojure-lsp/blob/master/src/clojure_lsp/crawler.clj#L65
But we need to take care what we will change here, I wonder if file:/ without the last slash also works for unix OS

@bpringe
Copy link
Contributor

bpringe commented Dec 30, 2020

Though it's not very explicit, I think these docs point to using a single / (at least in the one file system example): https://docs.oracle.com/javase/8/docs/api/java/net/JarURLConnection.html. We'll find out if it's okay with some testing. 😄

@ericdallo
Copy link
Member

Indeed!
I can try this tomorrow and say if it works for unix

@bpringe
Copy link
Contributor

bpringe commented Dec 31, 2020

This is actually two separate issues, one of them to be fixed in lsp (jar files navigation), and one to be fixed in Calva (refactorings). The refactorings issue can be tracked here.

@ericdallo
Copy link
Member

@bpringe LMK if you need any help, I saw your PR, so the file:/// didn't work for both OS, right?

@bpringe
Copy link
Contributor

bpringe commented Dec 31, 2020

file:/// (or anything more than 2 slashes) does work for both. I'll finish the PR soon

@ericdallo
Copy link
Member

With the fix from @bpringe, the issue should be fixed now.
@fonghou @kstehn could you confirm that it works on windows now?

Thanks for all the help!

@bpringe
Copy link
Contributor

bpringe commented Dec 31, 2020

If you're trying through a Calva release, I still need to include the new version, but I'll update this issue when I do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants