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

Version 0.9.1 seems to have broken prompt handling in some cases #142

Open
thomasjm opened this issue Apr 16, 2024 · 11 comments
Open

Version 0.9.1 seems to have broken prompt handling in some cases #142

thomasjm opened this issue Apr 16, 2024 · 11 comments

Comments

@thomasjm
Copy link

thomasjm commented Apr 16, 2024

Starting at 0.9.1, I can no longer start a kernel. I'm using Papermill 2.5.0 and Bash 5.2, both provided by Nixpkgs. I get error output like the below, which seems to involve the prompt regex.

The likely commit seems to be 9049a84

CC @kdm9

When I launch the Bash binary normally with the indicated rcfile, it seems fine (the prompt is just a single $).

EDIT: actually, it seems this may be because of a lack of this patch: #69. This PR was previously incorporated into Nixpkgs, but was removed when updating to 0.9.1. Not sure why, perhaps it didn't merge cleanly with 9049a84.

Starting kernel: ['/nix/store/x35v2zqxli4bvnxksj487clhrpdwbh0m-python3-3.11.8-env/bin/python', '-m', 'bash_kernel', '-f', '/tmp/tmp5r68mz62.json']
Connecting to: tcp://127.0.0.1:39427
connecting iopub channel to tcp://127.0.0.1:53175
Connecting to: tcp://127.0.0.1:53175
connecting shell channel to tcp://127.0.0.1:59761
Connecting to: tcp://127.0.0.1:59761
connecting stdin channel to tcp://127.0.0.1:33515
Connecting to: tcp://127.0.0.1:33515
connecting heartbeat channel to tcp://127.0.0.1:56607
Using selector: EpollSelector
connecting control channel to tcp://127.0.0.1:39427
Connecting to: tcp://127.0.0.1:39427
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/nix/store/x35v2zqxli4bvnxksj487clhrpdwbh0m-python3-3.11.8-env/lib/python3.11/site-packages/bash_kernel/__main__.py", line 3, in <module>
    IPKernelApp.launch_instance(kernel_class=BashKernel)
  File "/nix/store/x35v2zqxli4bvnxksj487clhrpdwbh0m-python3-3.11.8-env/lib/python3.11/site-packages/traitlets/config/application.py", line 1045, in launch_instance
    app.initialize(argv)
  File "/nix/store/x35v2zqxli4bvnxksj487clhrpdwbh0m-python3-3.11.8-env/lib/python3.11/site-packages/traitlets/config/application.py", line 113, in inner
    return method(app, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/x35v2zqxli4bvnxksj487clhrpdwbh0m-python3-3.11.8-env/lib/python3.11/site-packages/ipykernel/kernelapp.py", line 707, in initialize
    self.init_kernel()
  File "/nix/store/x35v2zqxli4bvnxksj487clhrpdwbh0m-python3-3.11.8-env/lib/python3.11/site-packages/ipykernel/kernelapp.py", line 555, in init_kernel
    kernel = kernel_factory(
             ^^^^^^^^^^^^^^^
  File "/nix/store/x35v2zqxli4bvnxksj487clhrpdwbh0m-python3-3.11.8-env/lib/python3.11/site-packages/traitlets/config/configurable.py", line 555, in instance
    inst = cls(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/x35v2zqxli4bvnxksj487clhrpdwbh0m-python3-3.11.8-env/lib/python3.11/site-packages/bash_kernel/kernel.py", line 104, in __init__
    self._start_bash()
  File "/nix/store/x35v2zqxli4bvnxksj487clhrpdwbh0m-python3-3.11.8-env/lib/python3.11/site-packages/bash_kernel/kernel.py", line 130, in _start_bash
    self.bashwrapper = IREPLWrapper(child, u'\$', prompt_change, self.unique_prompt,
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/x35v2zqxli4bvnxksj487clhrpdwbh0m-python3-3.11.8-env/lib/python3.11/site-packages/bash_kernel/kernel.py", line 45, in __init__
    replwrap.REPLWrapper.__init__(self, cmd_or_spawn, orig_prompt,
  File "/nix/store/x35v2zqxli4bvnxksj487clhrpdwbh0m-python3-3.11.8-env/lib/python3.11/site-packages/pexpect/replwrap.py", line 55, in __init__
    self._expect_prompt()
  File "/nix/store/x35v2zqxli4bvnxksj487clhrpdwbh0m-python3-3.11.8-env/lib/python3.11/site-packages/bash_kernel/kernel.py", line 72, in _expect_prompt
    pos = self.child.expect_list([re.compile(x) for x in prompts], timeout=timeout)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/x35v2zqxli4bvnxksj487clhrpdwbh0m-python3-3.11.8-env/lib/python3.11/site-packages/pexpect/spawnbase.py", line 372, in expect_list
    return exp.expect_loop(timeout)
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/x35v2zqxli4bvnxksj487clhrpdwbh0m-python3-3.11.8-env/lib/python3.11/site-packages/pexpect/expect.py", line 181, in expect_loop
    return self.timeout(e)
           ^^^^^^^^^^^^^^^
  File "/nix/store/x35v2zqxli4bvnxksj487clhrpdwbh0m-python3-3.11.8-env/lib/python3.11/site-packages/pexpect/expect.py", line 144, in timeout
    raise exc
pexpect.exceptions.TIMEOUT: Timeout exceeded.
<pexpect.pty_spawn.spawn object at 0x7fda869c3a10>
command: /nix/store/r9h133c9m8f6jnlsqzwf89zg9w0w78s8-bash-5.2-p15/bin/bash
args: [b'/nix/store/r9h133c9m8f6jnlsqzwf89zg9w0w78s8-bash-5.2-p15/bin/bash', b'--rcfile', b'/nix/store/x35v2zqxli4bvnxksj487clhrpdwbh0m-python3-3.11.8-env/lib/python3.11/site-packages/pexpect/bashrc.sh']
buffer (last 100 chars): 'PROMPT_BXYHNBYPIEVG\\[\\]>'
before (last 100 chars): 'PROMPT_BXYHNBYPIEVG\\[\\]>'
after: <class 'pexpect.exceptions.TIMEOUT'>
match: None
match_index: None
exitstatus: None
flag_eof: False
pid: 46662
child_fd: 47
closed: False
timeout: 30
delimiter: <class 'pexpect.exceptions.EOF'>
logfile: None
logfile_read: None
logfile_send: None
maxread: 2000
ignorecase: False
searchwindowsize: None
delaybeforesend: 0.05
delayafterclose: 0.1
delayafterterminate: 0.1
searcher: searcher_re:
    0: re.compile('(\\(\\w+\\) )?PROMPT_BXYHNBYPIEVG>')
    1: re.compile('PROMPT_BXYHNBYPIEVG\\+')
@kdm9
Copy link
Collaborator

kdm9 commented Apr 17, 2024

@thomasjm thanks for this report. Could you please provide a minimal example within whichever system you're launching the bash kernel (nix + codedown?), so I can dig in and debug? Using stock jupyter-lab + bash_kernel >=0.9.1 works fine for me, as does execution via papermill, all with bash v5.2, so I can't reproduce this yet sorry.

@thomasjm
Copy link
Author

Here you go, this reproduces it using the bash_kernel/papermill/bash on Nixpkgs master:

https://github.com/thomasjm/bash-prompt-repro

@kdm9
Copy link
Collaborator

kdm9 commented Apr 18, 2024

@thomasjm on a deadline this week, will take a look on the weekend -- feel free to annoy me if you don't hear from me by monday.

best,
K

@thomasjm
Copy link
Author

Just checking in, would love to get a fix for this!

@kdm9
Copy link
Collaborator

kdm9 commented Apr 24, 2024

Sorry, on a reporting deadline till later this week -- started debugging on the weekend but a fix will have to wait till later this week or the coming weekend

@thomasjm
Copy link
Author

thomasjm commented May 5, 2024

Hello, just another ping on this!

@kdm9
Copy link
Collaborator

kdm9 commented May 6, 2024

OK, so I've worked out that the issue is with handling the "invisible" characters in the prompt (the '\[\]' introduced in 9049a84, which is adapted from the previous code that was itself taken from pexpect per the comment). I've no idea why bash as packaged in Nix should behave any differently to bash elsewhere in this regard though, and I don't have a nice solution. I could remove it, but then as the comment notes, we'd be bitten if anybody runs env or other similar tools that print PS1/PS2.

Thoughts, @takluyver?

I'll continue poking later, but for now @thomasjm you (or the nix pkg maintainer) could patch out the + '\[\]' from those lines just for the nix package and accept the fact that running env will confuse bash_kernel -- better that than the current situation where bash_kernel immediately crashes on nix.

I'm reticent to do that globally, as the code here seems to work everywhere except in nix. I'd prefer a solution that has the nice effect of avoiding breaking env, but also works on nix.

@takluyver
Copy link
Owner

the '\[\]' introduced in 9049a84 ...

Note that those 'invisible' \[\] characters weren't introduced then - the rest of the prompt was changed from a hardcoded [PEXPECT_PROMPT> to a randomly generated one like PROMPT_CPJHBOCHKCCZ>, but the invisible \[\] go back at least to PR #46 in 2016, and I'd guess before that they were inherited from Pexpect.

But I agree they seem relevant - for some reason, whether it's the changes in that PR or something else, they seem to be showing up in the displayed prompt, which stops it being found as a prompt.

@thomasjm
Copy link
Author

Thanks for looking into this @kdm9 and @takluyver !

I found a fix, which I've introduced in NixOS/nixpkgs#311507. The fix is to use Nixpkgs' bashInteractive rather than bash. The difference is that the former has a separate GNU readline included and the latter uses Bash's built-in version. Presumably this affects the way invisible characters are handled.

Here's some documentation on the flag that controls this when building Bash.

Hopefully this helps clarify the situations where the current prompt approach breaks down. Not sure if it's important to fix this case--for my part I'll be happy once the Nixpkgs PR is merged.

@kdm9
Copy link
Collaborator

kdm9 commented May 14, 2024

OK, many thanks for the update -- I suspected it would be something like that! I will add a note to the README the we need proper readline support, but we should really detect if we have readline and only add those characters if we do.

@takluyver
Copy link
Owner

That makes sense, nice work figuring it out. 👍

kdm9 added a commit that referenced this issue May 14, 2024
@kdm9 kdm9 added wontfix and removed bug labels May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants