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

About "A second attempt to exit will terminate them" #6269

Closed
yetangye opened this issue Oct 31, 2019 · 8 comments
Closed

About "A second attempt to exit will terminate them" #6269

yetangye opened this issue Oct 31, 2019 · 8 comments

Comments

@yetangye
Copy link

@yetangye yetangye commented Oct 31, 2019

Someday, when I launched fish from a bash, after a very short time I exited fish (3-4 seconds),
After than I noticed a warning "A second attempt to exit will terminate them", I don't know what is the affect to my fish.. And I think this warning behavior should be avoid, it make user be worrying.

Last login: Sat Oct 26 12:26:09 on ttys001
⋊> fish
Welcome to fish, the friendly interactive shell

⋊> ls
    Desktop         Downloads Movies             Parallels Public
    Documents Library         Music                 Pictures

⋊> exit
    There are still jobs active:

    PID     Command
    23655     $py $update_args >/dev/null 2>&1 &

A second attempt to exit will terminate them.
Use 'disown PID' to remove jobs from the list without terminating them.

⋊> exit

Below is env info:

⋊> ~ fish --version                                                                        19:17:10
fish, version 3.0.2

⋊> ~ echo $version                                                                         16:06:49
3.0.2

⋊> ~ uname -a                                                                              16:06:50
Darwin tamaca.local 19.0.0 Darwin Kernel Version 19.0.0: Thu Oct 17 16:17:15 PDT 2019; root:xnu-6153.41.3~29/RELEASE_X86_64 x86_64

⋊> ~ echo $TERM                                                                            16:07:03
xterm-256color

⋊> ~ sh -c 'env HOME=$(mktemp -d) fish'                                                    16:08:36
Welcome to fish, the friendly interactive shell

@krobelus
Copy link
Member

@krobelus krobelus commented Oct 31, 2019

This job is just generating completions based on your system's man pages, fish does that when you start it for the first time.
You can also re-generate the completions manually using the function fish_update_completions.
They will be placed in ~/.local/share/fish/generated_completions.

I agree that this warning can be confusing for new users, but it's probably better than keeping the job running after exiting fish. Maybe we can make the job name more descriptive.

@zanchey zanchey added this to the fish-future milestone Oct 31, 2019
@faho
Copy link
Member

@faho faho commented Oct 31, 2019

To be clear, this warning is being thrown because there's a job still running in the background. In this case that job is fish generating man page completions. Usually this happens once, because it only does this if the directory doesn't exist.

The options I can think of are:

  • Don't show the warning in general
  • disown this job so it doesn't get the warning
  • Don't background this

@krobelus
Copy link
Member

@krobelus krobelus commented Oct 31, 2019

The warning is very useful in general (looks like exit in bash silently terminates jobs that were resumed with bg).
I'd run the command like this so the user knows what's going on:

$cmd (: fish_update_completions: generating completions from man pages) >/dev/null 2>&1 &

Or just use disown/setsid (but then I'd still be inclined to kill it when fish exits).
It's not terribly important since that job is only run once.

@yetangye
Copy link
Author

@yetangye yetangye commented Oct 31, 2019

The general users don't know what the command "$py $update_args >/dev/null 2>&1 &" is doing, it's really confusing(to me, the command looks like fish is updating itself, but interprupted by me, so I began to worring Is fish installation still being integrity..), and the users also don't know what should do next..

I think the warning message should be improved, tell the users can execute fish_update_completions command again by themself.

Or even if "fish_update_completions" interpruped by user's exit and next time fish launches, fish can detect is generating of ~/.local/share/fish/generated_completions finished normally, if not, execute fish_update_completions command in background again, and let users don't know this background step.

Of course, as @faho suggestion "Don't background this" is also a good option, just like zsh doing.

@zanchey
Copy link
Member

@zanchey zanchey commented Nov 1, 2019

I don't think blocking on first start is a good idea.

@mqudsi
Copy link
Contributor

@mqudsi mqudsi commented Dec 9, 2019

I've ran into this many times myself when I just run fish to make sure it's correctly in the path and running fine after a clean install but exit before the background python job is done. I would be fine with just special-casing this task as a disposable that can be ignored (and re-run at next launch instead, ad infinitum).

@ridiculousfish
Copy link
Member

@ridiculousfish ridiculousfish commented Dec 13, 2019

Any reason it should not just be disowned?

@mqudsi
Copy link
Contributor

@mqudsi mqudsi commented Dec 18, 2019

My reason was because it's a function and all hell breaks loose but that's already handled by invoking python directly and not executing any follow up code... mainly because we don't have a fish --no-config so config.fish would be sourced all over again, but that would have been easily worked around by setting a temporary universal variable as a poor man's mutex or .pid run file.

As it is, the approach used to determine if the completions were previously generated relied merely on the existence of the completions directory, which is created as a precursor to actually generating the completions, so in the event of an early exit (like reported in this issue), the completions were never actually run.

364929e disowns the process per @ridiculousfish's recommendation; we may want to think about setting a flag when the completions have actually been generated and checking that instead in the future, but as it's not critical for them to be generated, this'll do for now.

@zanchey zanchey removed this from the fish-future milestone Dec 19, 2019
@zanchey zanchey added this to the fish 3.1.0 milestone Dec 19, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants