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

fish lacks manual disown capability/command, in case we want running, owned jobs to stop upon terminal exit #2810

Closed
Pysis868 opened this issue Mar 11, 2016 · 36 comments

Comments

@Pysis868
Copy link

@Pysis868 Pysis868 commented Mar 11, 2016

I wanted to effectively 'disown' jobs/processes like I do in bash, manually.

I have found out that this happens automatically (#773) (Fish User Discussions - "disown" command in fish), but feel there may be some that would like it to be a manual process. As the following steps highlight, some may want jobs to be started with the intent of stopping when the parent terminal session exits.

Also, I have not found this explanation of the functionality in the official documentation (fish shell - Commands - jobs). Could this be added?

Reproduction Steps:

~> jobs
jobs: There are no jobs
~> sleep 12m &
~> jobs
Job Group   State   Command
1   68591   running sleep 12m &
~> ps aux|grep sleep
user          68602   0.0  0.0  2424084    336 s001  R+    4:13PM   0:00.00 grep --color=auto sleep
user          68591   0.0  0.0  2434840    752 s001  S     4:13PM   0:00.00 sleep 12m
~> exit

Open a new terminal

~> ps aux|grep sleep
user          68876   0.0  0.0  2423980    220 s001  R+    4:14PM   0:00.00 grep --color=auto sleep
user          68591   0.0  0.0  2434840    752   ??  S     4:13PM   0:00.00 sleep 12m

Expected behavior:

sleep 12m would not still be running.

Observed behavior:

sleep 12m is still running.

Additional information:

  • Slightly (Un)related: #1247

Fish version: fish, version 2.2.0

Operating system: Mac OS X El Capitan 10.11.3.
I seem to have forgotten how I have installed fish, my terminal histories don't have it. I would say it's (Home)brew, and it does state fish is Poured from bottle.

Terminal or terminal emulator: iTerm2 ("xterm-256color")

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Mar 11, 2016

I'm opposed to implementing a bash like "disown" command since, among other things, it would require changing the current behavior in a non-backward compatible manner. I do agree the current behavior should be documented including rationale for why fish does this by default rather than requiring the user use nohup or disown.

@krader1961 krader1961 added this to the fish-tank milestone Jun 9, 2016
@krader1961 krader1961 removed this from the fish-tank milestone Nov 17, 2016
@krader1961 krader1961 removed this from the fish-tank milestone Nov 17, 2016
@krader1961 krader1961 added this to the fish-future milestone Feb 3, 2017
@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Feb 3, 2017

Note that as of fish 2.5.0 we no longer automatically disown backgrounded jobs. Fish now behaves like bash, zsh, etc. and sends SIGHUP to them when the shell exits. This increases the need to implement a disown command.

@fgrsnau
Copy link

@fgrsnau fgrsnau commented Feb 4, 2017

I would argue that this is no longer a feature request but a bug/regression as all custom fish function relying on this behaviour now fail.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Feb 4, 2017

You can argue that point all you want, @fgrsnau, but you're wrong. The behavior you prefer was introduced by commit bff68f3 in March 2015 to address the misguided request in issue #1771. Note that what was originally asked for was support for bash syntax like (kate &). That "works" in bash because the backgrounded job is not launched by the shell in which you type that command -- it's launched by a subshell. So the kate command is not in the job table of your interactive shell and thus won't be sent a SIGHUP. The person who opened that issue was not asking that all background jobs of the current interactive shell be immune to SIGHUP. Then 1player chimed in with a comment that is clearly wrong. Unfortunately that convinced @zanchey implement the behavior I recently reverted because people were surprised by it. See issue #3497. It is unfortunate that no one at the time recognized that the proper fix for the request in issue #1771 was to add a disown command rather than make that the default behavior.

The nohup command is the traditional solution to keep a background job from being killed. A disown command, ala bash, simply makes it possible to retroactively achieve the same result if you forget to use nohup. We would be happy to expeditiously review and merge a patch from yourself or anyone else willing to do the work to implement a disown command.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Feb 8, 2017

Until someone implements disown you can either use nohup or the double-fork trick: fish -c 'emacs &'. Either one can be implemented as a function. For example:

function disown
    nohup $argv </dev/null >/dev/null 2>&1 &
end

or

function disown
    set -l escaped_argv (string escape --no-quoted $argv)
    fish -c "$escaped_argv &"
end

You can get completions for the actual command by using the same trick as the sudo completion. Assuming you name the function disown:

complete -c disown -d "Command to run" -x -a "(__fish_complete_subcommand)"

@jonhoo
Copy link
Contributor

@jonhoo jonhoo commented Feb 8, 2017

Unfortunately that implementation of disown does not allow you to retroactively disown a a process.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Feb 8, 2017

That is true, @jonhoo. The workaround I posted does not let you retroactively disown an already running job. It does, however, make it easy to do so if you know up front that is what you want which is the usual case. I'm hoping someone like yourself who feels passionate about this will take the time to create a patch that we can review and merge which implements a full-featured implementation.

@jonhoo
Copy link
Contributor

@jonhoo jonhoo commented Feb 8, 2017

@krader1961 it wasn't meant as criticism, just as an observation :)

@joehillen
Copy link

@joehillen joehillen commented Mar 10, 2017

I didn't know about the change of sending SIGHUP. fish just killed my background jobs. I am disappoint.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Mar 10, 2017

@joehillen: The change in fish behavior that disappointed you is the correct behavior and is consistent with other shells like bash. If you do not want backgrounded jobs killed by a SIGHUP signal you should use nohup to launch them. Having said that it is possible you have found a bug in how fish handles background jobs. If you believe that is the case please open a new issue with sufficient detail that someone else can reproduce the problem.

@joehillen
Copy link

@joehillen joehillen commented Mar 10, 2017

You shouldn't have "corrected" the behavior without first implementing disown. I might have to stop using fish because of this

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Mar 11, 2017

@joehillen: As I said earlier I would love to see someone like yourself who is passionate about this topic, and feels that nohup is insufficient, create a change to implement a disown builtin to fish.

It's not clear to me why needing to use nohup to launch jobs you don't want killed when the shell exits is something that would cause you to switch shells.

Do you understand that bash or zsh will also kill your background jobs if you did not launch them via nohup or an equivalent mechanism? Yes, it is true that they provide a disown command so that you can, after the fact, register your desire the background job not be killed when the shell exits. I, and everyone else using fish, would love to see fish have a disown command. The lack of that command, however, does not justify the previous, broken, behavior. Behavior that was introduced just two years ago in response to someone who was clearly confused about how a shell should behave. Prior to that ill-advised change fish behaved as it does now.

@joehillen
Copy link

@joehillen joehillen commented Mar 11, 2017

No thanks

@defaultxr
Copy link
Contributor

@defaultxr defaultxr commented Mar 11, 2017

I don't get how fish behaving differently than other shells means that fish was "broken". Fish is different from bash and zsh in a lot of ways, but I would say that most of those differences make fish better, not broken.

I also find the change to background jobs behavior very annoying and I also would consider fish more broken now that it's not possible to save those jobs if you didn't remember to nohup them. Effectively punishing the user just because they forgot seems a bit antithetical to the philosophy of fish to me, but what do I know.

This isn't a real solution, but I don't feel like learning C and researching the innards of fish just to implement disown. This is a horrible hack to get back something approaching the old behavior:

function preprocess
	set cmd (commandline)
    if string match '*&' $cmd
        commandline -r (echo -n 'nohup ')(string trim -r -c '&' $cmd)(echo -n ' ^/dev/null >/dev/null &')
    end
    commandline -f execute
end

Then just do: bind \r preprocess and any command lines ending in & will automatically be nohup'd. This won't work for functions written in fish itself, and probably isn't a good idea in general since there are surely many kinds of commands that will cause it to fail, or will fail because of it. If anyone has any suggestions for how to improve it, feel free to share.

@Ericson2314
Copy link

@Ericson2314 Ericson2314 commented Mar 14, 2017

@defaultxr signals are gross and the norms rather implicit. Because of the mess we should stick with status quo unless there is good reason to be different. This was a gratuitous different.

I didn't know about nohup, that will suffice for now thanks. disown having the better name does mean I am not alone, and this probably causes more demand for it.

@actionless
Copy link
Contributor

@actionless actionless commented Mar 15, 2017

but what was the motivation to break the current bg behavior (as retroactive disown) in a minor release?

it's not the way how things are being done -- you need either to roll out a major release or do required fix without breaking stuff

i've just updated and so much pissed off

P.S. also it sounds completely hypocritical to me to reference bash as an excuse to breaking this behavior but be happy with not supporting && as in bash or everywhere else

@braham-snyder
Copy link

@braham-snyder braham-snyder commented Mar 15, 2017

On the one hand, I understand why some people are upset (and I don't think their objections are entirely invalid). On the other hand, the authors of fish owe you nothing (in fact, we owe them--thank you guys, by the way), and rudely criticizing their design choices is counterproductive.

@actionless
Copy link
Contributor

@actionless actionless commented Mar 15, 2017

Please note what i am criticizing not the design choice itself but the way how versioning was handled in that regard. Without even any deprecation notice in few minor releases before.

And talking of rudeness -- such behavior breakage is a good example of it.
Just reverting some commit because of changing mind without any objective reasons not seems acceptable in this case.

However regarding the design choice, i did some test right now and it confused me even more:
while it was stated what the change was motivated by desire to behave closer to more common shells like bash and zsh, in bash it actually works the same as before this accident, only zsh is killing processeses on exit.
So just blindly following zsh example without providing any possible workaround (like special option for bg command, disown or something else in a similar way) doesn't seems even logical.

@braham-snyder
Copy link

@braham-snyder braham-snyder commented Mar 15, 2017

While I'm not sure whether I agree with each of these, I don't think it's unreasonable to at least suggest that: such a change might be better placed in a major release (so as to surprise fewer people), and/or after disown (or similar) is implemented, and/or after more deprecation notices.

That said, my point was that constructively criticizing more respectfully is likely in everyone's best interest--e.g., angrily demanding things from FOSS authors has a fair chance of making them less likely to help you out (even when you have valid suggestions).

Regarding "without any objective reasons", etc.: #3497

@actionless
Copy link
Contributor

@actionless actionless commented Mar 15, 2017

3497

two much blah-blah-blah about foss-something but you haven't even read the ticket to which you are referencing -- it is stated there what the reason why they want to do this change is to achieve behavior like in bash, while in bash it's actually not works like that.

#3497 (comment)

@braham-snyder
Copy link

@braham-snyder braham-snyder commented Mar 15, 2017

...and then, if you had kept reading, you would have seen #3497 (comment):

The problem is that if you kill the shell via SIGHUP then those processes are not kept running. Even in bash. The inconsistency causes more problems than it solves. Fish used to be consistent in how it handled the two cases, then wasn't consistent for 1.5 years, and is once again consistent.

@actionless
Copy link
Contributor

@actionless actionless commented Mar 15, 2017

that's not the reason to break the current behavior which was introduced purposely before

breaking the behavior is not the way of making things consistent when it's achievable with making things explicit (like having an argument for bg or other options described above)

and the main point, which was actually made me mad, was what things can't be purposely broken in minor releases

P.S. @braham-snyder funny what you decided to mark all my previous comments with thumb downs only after my last comment, not after reading each one

please don't take it personally if I could ignore your replies

@zanchey
Copy link
Member

@zanchey zanchey commented Mar 15, 2017

Don't make me turn this issue around. I'll stop the car. I WILL STOP. THE. CAR.

There's a lot of heat and light being generated here. I, at least, regret causing so much inconvenience to so many (including myself); I think part of the problem was that the full implications of the change were not realised until after the release. We didn't sit down and say "let's break something which works".

@krader1961 has appealed several times for help with a disown implementation, which on brief inspection appears to be a delicate and potentially complex undertaking. I'd like to say it will be ready for 2.6.0 but I honestly have no way of offering that. Sorry.

@actionless
Copy link
Contributor

@actionless actionless commented Mar 16, 2017

instead of using previously included functionality (which is still available in bash) you are recommending me to travel back in time and re-run the command with nohup? perfect logic, i have nothing to add, i think i'll better avoid myself from this discussion since you are just trying to justify your mistaken actions instead of trying to find the real solution for the problem

@ypnos
Copy link

@ypnos ypnos commented Mar 20, 2017

I would like to add that the changed behavior of version 2.5 from my point of view as a user is nothing short of a regression. Fish 2.5 takes away a functionality that I constantly use, which is, to decide at the point of ending the session whether I want background jobs to continue.

for more than ten years, fish behaved like every other UNIX shell (and does once again)

This is simply not true from a user's perspective. In bash, I can, now and then, kill background jobs by closing the terminal (SIGHUP is propagated), or keep them running by exiting the shell with Ctrl+D.

I understand the technicalities behind the claim, but for me as a user what matters is how I can or cannot use the software. For me, "it was theoretically wrong all the time" is not an appropriate answer.

I deeply respect your voluntary commitment and hard work on this project and am unfortunately not able to provide a pull request myself at this time. I would suggest, however, that you better understand the concerns of users (like myself) expressed in this thread and that they are not simply "wrong".

@zanchey
Copy link
Member

@zanchey zanchey commented Mar 21, 2017

Let's spec out the disown builtin.

zsh uses disown [jobspec ...], with the "current" job being disowned if no specification is provided. An environment variable is used to control the behaviour if the job being disowned is stopped. zsh also has special syntax (&!) for starting a job and immediately disowning it. The zsh manual does not define what the current job is.

bash uses disown [-ar] [-h] [jobspec ...]:

  • -h does not remove the job from the list of jobs, but marks it so that SIGHUP is not sent (I don't think we need this)
  • -a disowns all jobs
  • -ar disowns all running jobs, not stopped jobs

If run on a stopped job, a warning is printed but the job is still disowned.

The current job is "the last job stopped while it was in the foreground or started in the background" (bash manual page).

My suggestion for a fish builtin is to follow the zsh syntax (disown [jobspec ...], use the same logic as fg to choose the current job if no spec is given (last constructed job), and to print a similar warning to bash and zsh (with AUTO_CONTINUE set to 0) on how to restart stopped disowned jobs. I don't think always restarting stopped background jobs is a good idea. We could add the -a and -ar flags if there's a request for them at a future date.

I do like the syntax for starting a disowned job in zsh, but I'm not sure whether it is the right thing to add to fish. Perhaps disown could also take a command line? disown -c?

zanchey added a commit to zanchey/fish-shell that referenced this issue Mar 23, 2017
Closes fish-shell#2810.

The syntax mirrors that of zsh.
zanchey added a commit to zanchey/fish-shell that referenced this issue Mar 23, 2017
Closes fish-shell#2810.

The syntax mirrors that of zsh.
zanchey added a commit to zanchey/fish-shell that referenced this issue Mar 23, 2017
Closes fish-shell#2810.

The syntax mirrors that of zsh.
@zanchey zanchey mentioned this issue Mar 23, 2017
3 tasks
zanchey added a commit to zanchey/fish-shell that referenced this issue Mar 23, 2017
Closes fish-shell#2810.

The syntax mirrors that of zsh.
zanchey added a commit to zanchey/fish-shell that referenced this issue Apr 19, 2017
Closes fish-shell#2810.

The syntax mirrors that of zsh.
zanchey added a commit to zanchey/fish-shell that referenced this issue Apr 21, 2017
Closes fish-shell#2810.

The syntax mirrors that of zsh.
zanchey added a commit to zanchey/fish-shell that referenced this issue Apr 21, 2017
Closes fish-shell#2810.

The syntax mirrors that of zsh.
zanchey added a commit to zanchey/fish-shell that referenced this issue Apr 21, 2017
Closes fish-shell#2810.

The syntax mirrors that of zsh.
zanchey added a commit to zanchey/fish-shell that referenced this issue Apr 23, 2017
Closes fish-shell#2810.

The syntax mirrors that of zsh.
zanchey added a commit to zanchey/fish-shell that referenced this issue Apr 23, 2017
Closes fish-shell#2810.

The syntax mirrors that of zsh.
zanchey added a commit to zanchey/fish-shell that referenced this issue Apr 24, 2017
Closes fish-shell#2810.

The syntax mirrors that of zsh.
@krader1961 krader1961 added this to the 2.6.0 milestone Apr 28, 2017
@krader1961 krader1961 removed this from the fish-future milestone Apr 28, 2017
@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Apr 28, 2017

To everyone who has an opinion on this issue should a fish version of disown behave like zsh and tell you that you need to kill -SIGCONT -- -$pgid or be friendly and automatically send the SIGCONT to the process group? Note that the former behavior means the job will not be sent a SIGHUP signal when the parent fish shell exits but neither will it run after being disowned. There is a lengthy discussion about this point in PR #3906. Some people feel fish should behave like bash/zsh and not automatically resume running the job and print a warning about how to resume running the job. Others, like myself, believe that it is friendlier to automatically resume running the job.

Consider if you type sleep 333 then press [ctrl-Z] followed by disown. Should that sleep command automatically resume running or should you have to type kill -SIGCONT %1?

@jonas-schievink
Copy link

@jonas-schievink jonas-schievink commented Apr 28, 2017

Your example makes me feel like the "friendly" behaviour is much more convenient in day-to-day use. Printing a note saying that the job was resumed automatically seems like a good idea, as well as having an option to disable this behaviour (if I want the bash/zsh behaviour I could always alias disown="disown --no-cont" or whatever that option will be called).

@actionless
Copy link
Contributor

@actionless actionless commented Apr 28, 2017

i think the most important to have both choices using command-line arguments, it's often the best solution if there are too much opinions

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Apr 29, 2017

@jonas-schievink and @actionless: What is the use case for a --no-continue flag? Without a compelling use case we should not implement such a flag.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Apr 29, 2017

To follow up my previous question: Why should bg continue the job but disown not do so? That seems like a pointless, confusing, inconsistency. I did some googling but could not find any explanation for the bash behavior and the bash man page does not explain why it behaves that way. The zshbuiltins man page points out that zsh has a AUTO_CONTINUE option which can be set to get the behavior I'm arguing for. Apparently when zsh first implemented disown they maintained compatibility with bash. Someone then realized that behavior didn't make sense and did what zsh developers love to do: They added a new configurable option.

I'm not going to allow a --no-continue flag to be introduced unless someone can explain why it is needed other than to behave like bash.

@jonas-schievink
Copy link

@jonas-schievink jonas-schievink commented Apr 29, 2017

I can't think of one. I wouldn't use it anyways because the default does exactly what I want, it's just an option to restore compatibility.

@zanchey zanchey closed this in 4fde67f Apr 29, 2017
@zanchey
Copy link
Member

@zanchey zanchey commented May 1, 2017

For now, I have not implemented the flags. disown will be in the next release, which I'm hoping to start soon.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented May 1, 2017

For now, I have not implemented the flags. ...

I think that is reasonable. While I have never used disown I strongly suspect that most users are disowning the most recent job they launched and less often an explicitly named job. As with my previous questions regarding why bash does not automatically continue the jobs being disowned I am perplexed why there is as disown -r flag. It seems to me that the people who implemented the disown command in bash did not really think about how it would be used in practice and instead worried about esoteric use cases that are extremely unlikely to be an issue in daily use. And could probably be handled better via other mechanisms (e.g., the legacy nohup command).

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet