Version 0.9.1 seems to have broken prompt handling in some cases
thomasjm opened this issue · 11 comments
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\\+')
@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.
Here you go, this reproduces it using the bash_kernel/papermill/bash on Nixpkgs master:
@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
Just checking in, would love to get a fix for this!
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
Hello, just another ping on this!
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.
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.
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.
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.
That makes sense, nice work figuring it out. 👍