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

Shell prompt stops responding to terminal resize event after poetry shell #6351

Closed
3 tasks done
PythonTryHard opened this issue Sep 2, 2022 · 18 comments
Closed
3 tasks done
Labels
area/shell Related to `poetry shell` kind/bug Something isn't working as expected status/confirmed Issue is reproduced and confirmed version/1.2.0

Comments

@PythonTryHard
Copy link

  • I am on the latest Poetry version.
  • I have searched the issues of this repo and believe that this is not a duplicate.
  • If an exception occurs when executing a command, I executed it again in debug mode (-vvv option).
  • OS version and name: KDE neon 5.25 (based on Ubuntu 20.04.1 LTS) fully up-to-date.
  • Poetry version: 1.2.0
  • Terminal used: Konsole 22.08.0

Issue

Steps to reproduce:

  1. Set up a zsh shell with oh-my-zsh installed. Any theme would do, but for ease of viewing, set ZSH_THEME in .zshrc to "dst".
  2. Resize the terminal without being inside poetry shell.
  3. Enter poetry shell. Resize the terminal.

Expected behaviour:
The prompt responds normally to terminal resize events.

Actual behaviour:
The prompt does not respond to terminal resize events. What

Notes:

  • Only zsh with oh-my-zsh has been tested
  • Only tested on Konsole terminal, other terminals have not been tested
  • This issue does not occur on v1.1.x releases of Poetry.
  • This affects only the prompt, writing to stdout is not affected
@PythonTryHard PythonTryHard added kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Sep 2, 2022
@neersighted
Copy link
Member

Hi @PythonTryHard -- a minimal reproduction without oh-my-zsh would be wonderful. Certainly I would consider the stock ZSH prompt not working in the subshell to be a Poetry issue, but I don't think we can reasonably work around bugs in every third party prompt (even one as widely used as oh-my-zsh). If it can be demonstrated that Poetry does this even without oh-my-zsh, we can certainly consider it a flaw in Poetry.

@neersighted neersighted added status/needs-reproduction Issue needs a minimal reproduction to be confirmed and removed status/triage This issue needs to be triaged labels Sep 2, 2022
@PythonTryHard
Copy link
Author

It can be reproduced with .zshrc being

PROMPT="%n@%M> "
RPROMPT="%T%D"

Though frankly, you should only need the RPROMPT option since it's more obvious.

@neersighted neersighted added status/accepted Feature request accepted for the roadmap area/shell Related to `poetry shell` status/confirmed Issue is reproduced and confirmed version/1.2.0 and removed status/needs-reproduction Issue needs a minimal reproduction to be confirmed status/accepted Feature request accepted for the roadmap labels Sep 2, 2022
@l3s2d
Copy link

l3s2d commented Sep 5, 2022

This issue is present in the bash and fish default prompts as well.

@PythonTryHard
Copy link
Author

Given that bash and fish are also affected, I'm renaming the issue to "Shell prompt stops responding to terminal resize event after poetry after" since it's not an isolated case anymore

@PythonTryHard PythonTryHard changed the title oh-my-zsh prompt stops responding to terminal resize event after poetry shell Shell prompt stops responding to terminal resize event after poetry shell Sep 5, 2022
@Secrus
Copy link
Member

Secrus commented Sep 6, 2022

Could someone affected provide a recording of what exactly is happening? From my initial investigation, it works fine.

@PythonTryHard
Copy link
Author

PythonTryHard commented Sep 7, 2022

@Secrus Here you go. This is done with the .zshrc above. I went with

sudo useradd -m zsh_repro
sudo su zsh_repro
cd ~

to ensure it's just bare zsh and that I don't accidentally invoke something from my main user:

  • Outside of poetry shell:
outside-poetry-shell.mp4
  • Inside of poetry shell:
inside-poetry-shell.mp4

@Secrus
Copy link
Member

Secrus commented Sep 7, 2022

@PythonTryHard could you check if the same happens with pipenv and pipenv shell command?

@PythonTryHard
Copy link
Author

PythonTryHard commented Sep 7, 2022

@Secrus In the same bare user at ~ I ran

/opt/python-3.10.6/bin/pip install --user pipenv  # My Python is hand-compiled and installed there
.local/bin/pipenv shell                                                                               

Output (pipenv defaulted to system Python):

Creating a virtualenv for this project...
Pipfile: /home/zsh_repro/Pipfile
Using /usr/bin/python (3.8.10) to create virtualenv...
⠏ Creating virtual environment...created virtual environment CPython3.8.10.final.0-64 in 1022ms
  creator CPython3Posix(dest=/home/zsh_repro/.local/share/virtualenvs/zsh_repro-pfePJbLW, clear=False, no_vcs_ignore=False, global=False)
  seeder FromAppData(download=False, pip=bundle, setuptools=bundle, wheel=bundle, via=copy, app_data_dir=/home/zsh_repro/.local/share/virtualenv)
    added seed packages: pip==22.2.2, setuptools==65.3.0, wheel==0.37.1
  activators BashActivator,CShellActivator,FishActivator,NushellActivator,PowerShellActivator,PythonActivator

✔ Successfully created virtual environment!
Virtualenv location: /home/zsh_repro/.local/share/virtualenvs/zsh_repro-pfePJbLW
Creating a Pipfile for this project...
Launching subshell in virtual environment...

and nope, pipenv shell doesn't exhibit the same behaviour as poetry. I also tried forcing pipenv to use my 3.10, and pipenv shell is still behaving normally.

@nalabelle
Copy link

nalabelle commented Sep 10, 2022

Poetry at 1.1.15 uses clikit's Terminal implementation.
Poetry at 1.2.0 updated to cleo's Terminal

cleo's terminal at 1.0.0a5 has a bug. clikit's implmentation initializes the width and height as instance variables. cleo's implementation at this version initializes the width and height as variables in the __class__ attribute. When terminal is reinstantiated, clikit's version would reinitialize the width and height, but since cleo's version is storing the variable in the __class__ attribute, which belongs to the class and not the object, it retains the settings from the original initialization.

This is fixed as of now in master, as per PR #6224, as cleo and poetry both replaced their implementation with calls using shutil, but remains an issue in the 1.2.0 release.

I believe this may be related to issue #4165.

@Secrus
Copy link
Member

Secrus commented Sep 10, 2022

Poetry at 1.1.15 uses clikit's Terminal implementation. Poetry at 1.2.0 updated to cleo's Terminal

cleo's terminal at 1.0.0a5 has a bug. clikit's implmentation initializes the width and height as instance variables. cleo's implementation at this version initializes the width and height as variables in the __class__ attribute. When terminal is reinstantiated, clikit's version would reinitialize the width and height, but since cleo's version is storing the variable in the __class__ attribute, which belongs to the class and not the object, it retains the settings from the original initialization.

This is fixed as of now in master, as per PR #6224, as cleo and poetry both replaced their implementation with calls using shutil, but remains an issue in the 1.2.0 release.

I believe this may be related to issue #4165.

Well, that last part is not true. Change to shutil made it into the 1.2 release: shell.py. I have been looking into what is happening here and my current guess is that somehow old Cleo approach with custom Terminal class, and the change in Poetry 1.2 to using shutil solution somehow conflict. I have yet to test it though. Also, with Cleo getting a 1.0 stable release pretty soon, it might resolve on its own with that change.

@dimbleby
Copy link
Contributor

Change to shutil made it into the 1.2 release

This is not correct, you've linked to the branch (where the fix has been backported) but it did not make it to the 1.2.0 release - https://github.com/python-poetry/poetry/blob/1.2.0/src/poetry/utils/shell.py#L102

@Secrus
Copy link
Member

Secrus commented Sep 11, 2022

Change to shutil made it into the 1.2 release

This is not correct, you've linked to the branch (where the fix has been backported) but it did not make it to the 1.2.0 release - https://github.com/python-poetry/poetry/blob/1.2.0/src/poetry/utils/shell.py#L102

🤣 seems that I am blind. Good to know, I was going crazy trying to figure out why it breaks... So, basically, we are waiting for 1.2.1 release.

@esiegerman
Copy link

esiegerman commented Sep 12, 2022

I've encountered what appears to be the same problem, with vim -- not a shell -- exhibiting the symptoms, and in a way that can be directly queried.

System:

  • Ubuntu 20.04 LTS
  • GNOME desktop
  • GNOME Terminal 3.36.2
  • python 3.8.10 (as installed by Ubuntu)
  • vim 8.1 (as installed by Ubuntu)
  • bash 5.0.17(1)-release (as installed by Ubuntu)

First, here's a control, showing what should happen:

  1. Maximize the terminal window
  2. Launch vim
  3. Type :set columns?<Enter> # This asks for the current value of the columns setting, which vim sets from the window size
  4. Note the value it reports (183 in my case)
  5. Quit vim: :qa!<Enter> # WARNING: throws away any unsaved work -- but it's a useful "get me outta here!" recipe for non-vim-users :-)
  6. Change the window's width
  7. Relaunch vim
  8. Repeat steps 3 and 4; the size reported by vim should be different (it became 117 in my case)

N.B.: Quitting and relaunching vim (steps 5 and 7) shouldn't be necessary; vim notices when the terminal window has resized, and adapts accordingly. I'll explain below why I added those two steps.

Now, to reproduce the issue:

Follow the above steps, but after step (1), type poetry shell, so that the rest of the steps happen in the subshell.
Notice that at step (6), the value reported has not changed -- it's still the width of the maximized window, even though the window is no longer maximized.

N.B. Restarting vim (steps 5 and 7) demonstrates that the problem is not a single vim process's failure to notice the size change. Even a vim that's newly launched after the window is resized reports the width from before the resize occurred.

To try to isolate the problem, I did manually what I believe poetry shell is doing, i.e.:

  1. bash
  2. . ~/.cache/pypoetry/virtualenvs/${env_name}/bin/activate

This does work properly. So it's not the activation of the venv per se that's causing the problem; it's something else that poetry shell is doing.

Nor is it a change to the UNIX environment that's doing it. env(1) produces almost the same output from the poetry shell subshell that demonstrates the problem as from the manually-activated-venv shell that doesn't. The two exceptions are that the former has these additional variables:
PLAT linux-x86_64
POETRY_ACTIVE 1

From the poetry subshell, I unset those two environment variables, which exactly mimicked the environment from the manual-activation shell -- but that did not fix the problem. So whatever poetry shell is doing, it's not a simple environment-variable change.

@neersighted
Copy link
Member

The issue is known/solved on the 1.2 branch -- the signal handler for SIGWINCH used a bogus/incorrect method to query the new window size (and thus passed a bad size to the child process). Please try the 1.2 branch and request a re-open if this can be reproduced there:

pipx install --suffix @b1.2 git+https://github.com/python-poetry/poetry@1.2

@esiegerman
Copy link

pipx install --suffix @b1.2 git+https://github.com/python-poetry/poetry@1.2

Yes, that version works. Thanks.

@jclerman
Copy link

jclerman commented Oct 7, 2022

This was effectively fixed in the 1.2.1 release (right?) but isn't listed in the release-notes for that release. Could it be added there?

@neersighted
Copy link
Member

Good catch -- looks like it was overlooked as #6224 was labeled as a refactor and not a bug fix (and thus dropped as a non-changelog change) since it wasn't realized this would also fix a bug (instead of just being more correct).

Feel free to issue a PR with the changelog entry -- I can update the release notes and release a post-release on PyPI (though this latter step is a chore and we'll likely skip it as we intend to cut 1.2.2 imminently).

Copy link

github-actions bot commented Mar 1, 2024

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/shell Related to `poetry shell` kind/bug Something isn't working as expected status/confirmed Issue is reproduced and confirmed version/1.2.0
Projects
None yet
Development

No branches or pull requests

8 participants