emacs-lsp / lsp-python-ms

lsp-mode :heart: Microsoft's python language server
https://emacs-lsp.github.io/lsp-python-ms
BSD 3-Clause "New" or "Revised" License
190 stars 41 forks source link

add unzip to documentation #113

Closed addy419 closed 4 years ago

addy419 commented 4 years ago

I wanted to install mspyls but it kept failing everytime. After checking the source code for a while, I found out that the zip extraction depends on unzip which was not installed on my system. It worked right after I installed it. Just add unzip as a warning or an explicit instruction so that users are aware if they are missing unzip.

seagle0128 commented 4 years ago

Did you see the log like this?

Unable to extract 'xxx.zip' to 'xxx'! Please extract manually.
addy419 commented 4 years ago

Yeah. But it doesn't exactly direct to download unzip. Adding a hint will help new users.

harabat commented 2 years ago

I'm adding the actual error message to make it easier for people to find this issue:

LSP :: Server mspyls install process failed with the following error message: Unable to extract ’/tmp/mspylsXXXXXX.zip’ to ’~/.emacs.d/.local/etc/lsp/mspyls/’! Please check unzip, powershell or extract manually..

Additional information for those running into this error:

seagle0128 commented 2 years ago

@harabat It seems a connection issue. The zip file was not downloaded successfully.

harabat commented 2 years ago

Couldn't have been, the connection was fine every time I tried (both a week ago and yesterday), but the lsp-install-server command kept creating empty mspylsXXXXXX.zip files in /tmp/ and throwing the above error up until the point where I installed unzip.

At most, it might have been a Doom specific issue, but I haven't looked into the source to say for sure.

seagle0128 commented 2 years ago

What's the size of mspylsXXXXXX.zip? Can you unzip it manually?

harabat commented 2 years ago

@seagle0128

I have 4 of these in my /tmp/ folder because of yesterday's attempts (the ones from a week ago have been cleaned), they're all 0B, empty, and unzipping them manually does not work either ("unsupported file type" error), but opening any other zip file works.

===

If it helps you diagnose the issue:

The 5th mspylsXXXXXX.zip file in my /tmp/ is 32.3 MiB, has been created 1 minute after I installed unzip, and is the one that led to the installation of mspyls.

It seems like lsp-python-ms first creates the file, then checks whether unzip is installed, and throws the error without downloading the file if there's no unzip. A cursory reading of the source code (line 244) seems to support this hypothesis.

===

Should the whole (unzip script ...) part be moved to after line 265's (lsp--info "Downloading Microsoft Python Language Server...done")? Can do the PR if you think this can be the solution.

harabat commented 2 years ago

Based on my limited elisp, the PR could be :

    (let* ((temp-file (make-temp-file "mspyls" nil ".zip"))
           (install-dir (expand-file-name lsp-python-ms-dir)))
           ;; unzip part moved from here

      (lsp--info "Downloading Microsoft Python Language Server...")

      (url-retrieve
       (lsp-python-ms-latest-nupkg-url lsp-python-ms-nupkg-channel)
       (lambda (_data)
         ;; Skip http header
         (re-search-forward "\r?\n\r?\n")

         ;; Save to the temp file
         (let ((coding-system-for-write 'binary))
           (write-region (point) (point-max) temp-file))

         (lsp--info "Downloading Microsoft Python Language Server...done")

         ;; unzip part moved to here
         (let ((unzip-script (cond ((executable-find "unzip")
                                (format "mkdir -p %s && unzip -qq %s -d %s"
                                        install-dir temp-file install-dir))
                               ((executable-find "powershell")
                                (format "powershell -noprofile -noninteractive \
  -nologo -ex bypass Expand-Archive -path '%s' -dest '%s'" temp-file install-dir))
                               (t (user-error "Unable to extract '%s' to '%s'! \
  Please check unzip, powershell or extract manually." temp-file install-dir))))))

         ;; Extract the archive
         (lsp--info "Extracting Microsoft Python Language Server...")
seagle0128 commented 2 years ago

Did you get the message "Unable to extract '%s' to '%s'! Please check unzip, powershell or extract manually."? Is it enough for your case? unzip is necessary to extract.

harabat commented 2 years ago

@seagle0128

I think you might have misread my comments: the lsp-install-server command in lsp-python-ms.el works the following way:

If unzip/powershell is installed:

  1. Create an empty file mspylsXXXXXX.zip
  2. Check if there's unzip or powershell installed
  3. Download the mspyls file

If unzip/powershell is not installed:

  1. Create an empty file mspylsXXXXXX.zip
  2. Check if there's unzip or powershell installed
  3. Throw an error that doesn't explicitly say to install unzip and mistakenly asks the user to extract manually from an empty file

Either the unzip checking process needs to be moved to after the file's download (which my suggested PR does), or the message needs to be modified to say "Please install unzip or powershell." and not mention anything about extracting manually.

seagle0128 commented 2 years ago

I see you might be confused. I update the messages.

BTW, it's deprecated and the successor is lsp-pyright.

harabat commented 2 years ago

Thanks for converting user-error to lsp-info, that solves the issue.