wyuenho / emacs-pet

Tracks down the correct Python tooling executables from your virtualenvs so you can glue the binaries to Emacs and delete code in init.el
GNU General Public License v3.0
120 stars 14 forks source link

`pet-exectuable-find` calls of `process-lines` using `pyenv which` can fail, setting vars to `nil` #15

Closed Walheimat closed 1 year ago

Walheimat commented 1 year ago

Description

The process-lines call in penultimate branch of pet-executable-find passes which and the command as args to pyenv, which fails.

Reproduction steps

Have pyenv installed. Install a package with a var that pet will set, for example, python-black-command when using python-black Emacs package.

Now, outside of a virtual env/poetry project when opening a Python file (this is the (executable-find "pyenv") branch), the call (process-lines "pyenv" "which" "black") during pet-executable-find errors out and sets python-black-command to nil. This doesn't seem to be a path issue, since alternatively (shell-command-to-string "pyenv which black") succeeds.

Expected behavior

I'm not sure why process-lines fails here ((process-lines "pyenv" "help") for example, succeeds), however, I'd expect pet not to set a variable that could otherwise have a working default value ("black" in this case if it is in path).

PET version

1.1.0

Emacs version

29

Desktop (please complete the following information):

System tools versions

Additional context

Debugger entered--Lisp error: (error "pyenv exited with status 1")
  error("%s exited with status %s" "pyenv" 1)
  (if (eq status 0) nil (error "%s exited with status %s" program status))
  (if status-handler (funcall status-handler status) (if (eq status 0) nil (error "%s exited with status %s" program status)))
  (let ((status (apply #'call-process program nil (current-buffer) nil args))) (if status-handler (funcall status-handler status) (if (eq status 0) nil (error "%s exited with status %s" program status))) (goto-char (point-min)) (let (lines) (while (not (eobp)) (setq lines (cons (buffer-substring-no-properties (line-beginning-position) (line-end-position)) lines)) (forward-line 1)) (nreverse lines)))
  (progn (let ((status (apply #'call-process program nil (current-buffer) nil args))) (if status-handler (funcall status-handler status) (if (eq status 0) nil (error "%s exited with status %s" program status))) (goto-char (point-min)) (let (lines) (while (not (eobp)) (setq lines (cons (buffer-substring-no-properties (line-beginning-position) (line-end-position)) lines)) (forward-line 1)) (nreverse lines))))
  (unwind-protect (progn (let ((status (apply #'call-process program nil (current-buffer) nil args))) (if status-handler (funcall status-handler status) (if (eq status 0) nil (error "%s exited with status %s" program status))) (goto-char (point-min)) (let (lines) (while (not (eobp)) (setq lines (cons (buffer-substring-no-properties ... ...) lines)) (forward-line 1)) (nreverse lines)))) (and (buffer-name temp-buffer) (kill-buffer temp-buffer)))
  (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn (let ((status (apply #'call-process program nil (current-buffer) nil args))) (if status-handler (funcall status-handler status) (if (eq status 0) nil (error "%s exited with status %s" program status))) (goto-char (point-min)) (let (lines) (while (not (eobp)) (setq lines (cons ... lines)) (forward-line 1)) (nreverse lines)))) (and (buffer-name temp-buffer) (kill-buffer temp-buffer))))
  (let ((temp-buffer (generate-new-buffer " *temp*" t))) (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn (let ((status (apply ... program nil ... nil args))) (if status-handler (funcall status-handler status) (if (eq status 0) nil (error "%s exited with status %s" program status))) (goto-char (point-min)) (let (lines) (while (not ...) (setq lines ...) (forward-line 1)) (nreverse lines)))) (and (buffer-name temp-buffer) (kill-buffer temp-buffer)))))
  process-lines-handling-status("pyenv" nil "which" "black")
  process-lines("pyenv" "which" "black")
  elisp--eval-last-sexp(nil)
  eval-last-sexp(nil)
  funcall-interactively(eval-last-sexp nil)
  command-execute(eval-last-sexp)
Walheimat commented 1 year ago

In case anyone else has this problem, you can use the following advice to avoid it:

(advice-add 'pet-executable-find :after-until (lambda (it) it))

Which means that if pet-executable-find should return nil, instead return the argument it was called with.

wyuenho commented 1 year ago

From your out-of-project python buffer, what the result of eval-expression RET (executable-find "pyenv") ?

Also, how do you setup pyenv in your shell?

Walheimat commented 1 year ago

That works of course, we enter the branch because the executable can be found.

The output on this machine is "/home/karusster/.pyenv/bin/pyenv".

(process-lines "/home/karusster/.pyenv/bin/pyenv" "which" "black") fails the same way (process-lines "pyenv" "which" "black") does. The problem is using a subcommand here, as I described in the original post. When I do what process-lines does, calling call-process, I also get the reason for the exit:

(apply #'call-process "pyenv" nil (current-buffer) nil (list "which black")) ; or
(apply #'call-process "/home/karusster/.pyenv/bin/pyenv" nil (current-buffer) nil (list "which black"))
;; results in pyenv: no such command `which black'

pyenv which black yields /home/karusster/.local/share/pyenv/versions/3.9-dev/bin/black in sh, bash, fish etc.; (shell-command-to-string "pyenv which black") yields "/home/karusster/.local/share/pyenv/versions/3.9-dev/bin/black".

If this is a call-process vs pyenv quirk, I don't mind since I don't have this problem usually (since I'm using poetry in all projects and pet does what it promises). But it would be nice if buffers didn't just set the vars to nil on failure.

Walheimat commented 1 year ago

I used this for the shell setups: https://github.com/pyenv/pyenv#set-up-your-shell-environment-for-pyenv

wyuenho commented 1 year ago

(apply #'call-process "pyenv" nil (current-buffer) nil (list "which black")) ; or (apply #'call-process "/home/karusster/.pyenv/bin/pyenv" nil (current-buffer) nil (list "which black")) ;; results in pyenv: no such command `which black'

You are creating a list of one element here with a command called "which black", which of course doesn't exist. Try (list "which" "black")

Walheimat commented 1 year ago

That's true, sorry for the mess up. It still fails but with a more interesting message, namely pyenv: no local version configured for this directory, which I can remedy. Digging a bit deeper, it looks like PYENV_ROOT is not visible to my Emacs daemon for whatever reason and it's the root problem.

That's good, however, the issue was less about this command failing than about pet setting various custom variables to nil on error. That's not what I'd expect it to do.

Updated the title to better reflect this.

wyuenho commented 1 year ago

In your case, even if I let the pyenv branch fall through, it's still not going to work because the binary isn't even in path. Setting the executable variable to nil means pet has exhausted all possibilities for looking up the executable, that means even if pet leaves it alone, whatever package you are using is still only going to use executable-find and friends to find that binary, and it's going to fail again because by this time pet has already tried that.

To fix your problem, you need to use exec-path-from-shell, direnv, or move your pyenv init snippet into .bash_profile or equivalent configuration file for your chosen shell such that it will be picked up by emacs as a process environment as opposed to an interactive shell environment.

Walheimat commented 1 year ago

It is found by executable-find, but it's fine, my workaround works for me, and this is an edge case anyway, if I remove the pyenv branch from the function, and it falls through, the variable would be correctly set by executable-find.

wyuenho commented 1 year ago

Sorry I just reread the comments. I'm not entirely sure how to deal with this situation. Is black installed with pyenv or not? Related question, if it's installed with pyenv, do you want pyenv to pick up the global version? Is this just a case of forgetting to set a default global version in pyenv?

The problem with falling back to executable-find is equivalent to going back to the exact problem this whole package is created to solve. It's an easy change, but I want to understand what exactly went wrong and what can be done about it.

wyuenho commented 1 year ago

No response after 9 months. I'm closing this. Let me know if there's something reasonable to be done.