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

Can't background a job after a previously-backgrounded job completes #7483

Closed
ridiculousfish opened this issue Nov 15, 2020 · 1 comment
Closed
Assignees
Milestone

Comments

@ridiculousfish
Copy link
Member

This tracks the following regression relative to 3.1.2, I'm opening this so it doesn't get dropped:

  1. Run sleep 5
  2. Hit control-Z to background it
  3. fg to foreground it
  4. Allow the sleep complete normally
  5. Run sleep 100 and try to hit control-Z to background it

it will no longer background. stty -a shows the tty mode goes from susp = ^Z (correct) to susp = <undef> (the bug).

This bisects to 3d2dc85 however that fix only revealed a latent bug; the root cause is the fix for #2114.

@ridiculousfish ridiculousfish added this to the fish 3.2.0 milestone Nov 15, 2020
@ridiculousfish ridiculousfish self-assigned this Nov 15, 2020
@ridiculousfish
Copy link
Member Author

ridiculousfish commented Nov 15, 2020

Notes to myself mostly but fun reading if you are inclined:

The issue is annoyingly convoluted. Interactive fish stores two tty modes: shell_modes for itself, and tty_modes_for_external_cmds for external procs. For example, ctrl-Z should send SIGTSTP for external procs, but not for fish, because maybe you want to bind ctrl-Z (per 3d2dc85).

Now maybe your external process is literally stty which changes the tty modes. It wouldn't be very useful if those changes were dropped, so when fish gets control back it reads the tty mode and updates tty_modes_for_external_cmds.

The fix for #2114 caused us to more eagerly restore the tty to shell-mode when a previously-backgrounded job completes. So we restore the tty to shell-mode, then later we save that as the mode for external procs. So now every proc is like a shell, which is why ctrl-Z fails.

The desire in #2114 was to support foreground and backgrounding jobs from key bindings (!), not from ordinary commands. The proper fix is to extend the tty-mode save/restore to jobs spawned from key bindings, not just those typed at the command line.

ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this issue Nov 19, 2020
Prior to this change, for bindings which have script commands, the
inputter would execute them directly. However an upcoming fix for fish-shell#7483
will require more integration with the reader. Switch to a new model where
the reader passes in a function to use for executing script commands.
ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this issue Nov 19, 2020
Prior to this change, when a process resumes because it is brought back
to the foreground, we would reset the terminal attributes to shell mode.
This fixed fish-shell#2114 but subtly introduced fish-shell#7483.

This backs out 9fd9f70, re-introducing fish-shell#2114 and re-fixing fish-shell#7483.
A followup fix will re-fix fish-shell#2114; these are broken out separately for
bisecting purposes.

Fixes fish-shell#7483.
ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this issue Nov 19, 2020
ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this issue Nov 20, 2020
Prior to this change, when a process resumes because it is brought back
to the foreground, we would reset the terminal attributes to shell mode.
This fixed fish-shell#2114 but subtly introduced fish-shell#7483.

This backs out 9fd9f70, re-introducing fish-shell#2114 and re-fixing fish-shell#7483.
A followup fix will re-fix fish-shell#2114; these are broken out separately for
bisecting purposes.

Fixes fish-shell#7483.
ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this issue Nov 20, 2020
ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this issue Nov 20, 2020
ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this issue Nov 21, 2020
Prior to this change, for bindings which have script commands, the
inputter would execute them directly. However an upcoming fix for fish-shell#7483
will require more integration with the reader. Switch to a new model where
the reader passes in a function to use for executing script commands.
ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this issue Nov 21, 2020
Prior to this change, when a process resumes because it is brought back
to the foreground, we would reset the terminal attributes to shell mode.
This fixed fish-shell#2114 but subtly introduced fish-shell#7483.

This backs out 9fd9f70, re-introducing fish-shell#2114 and re-fixing fish-shell#7483.
A followup fix will re-fix fish-shell#2114; these are broken out separately for
bisecting purposes.

Fixes fish-shell#7483.
ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this issue Nov 21, 2020
ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this issue Nov 24, 2020
ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this issue Nov 24, 2020
Prior to this change, for bindings which have script commands, the
inputter would execute them directly. However an upcoming fix for fish-shell#7483
will require more integration with the reader. Switch to a new model where
the reader passes in a function to use for executing script commands.
ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this issue Nov 24, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant