pezra / rspec-mode

An RSpec minor mode for Emacs
257 stars 112 forks source link

Newly started spring server gets stopped by SIGHUP at end of run #216

Closed til closed 10 months ago

til commented 10 months ago

Assuming a rails project and rspec-mode are configured to use spring, e.g. with bin/spring: when no spring server was running yet, starting an rspec-mode compilation starts the spring server process as expected, but stops it immediately after the compilation run finishes instead of keeping it running.

When the spring server was started outside of Emacs before, e.g. from a terminal, this is not an issue – the running server is used by the compilation and keeps running after the compilation is finished.

The problem is that Emacs' compilation sends a SIGHUP on completion, but spring traps only SIGINT and SIGQUIT, not SIGHUP – stopping the server when the user closes the terminal is the desired behavior (I read somewhere on spring's GitHub issues).

What would be a good way to prevent the server getting stopped? Is there a way to configure rspec-mode / compilation-mode to not send that signal? Or is there something on spring that can be done about it?

dgutov commented 10 months ago

I'm not sure what would be a good solution.

The docs page you linked to suggests using nohup. Have you tried customizing rspec-spec-command to "nohup rspec"?

Also note that most recent versions of Rails don't have Spring in the default config anymore: https://github.com/rails/rails/pull/42997

So it might be worth to re-evaluate whether it brings enough benefit for your particular project.

til commented 10 months ago

I've tried but haven't found a way to use nohup as part of rspec-spec-command without it affecting also the rspec process itself. E.g. like this, all output from rspec gets suppressed:

nohup bin/rspec --format documentation /home/tils/deka-etf/spec/helpers/liquidation_helper_spec.rb:14
nohup: ignoring input and appending output to 'nohup.out'

Regarding the usefulness of Spring, you're right, it's something I should probably reconsider. In this particular project, the difference is currently very noticeable: a simple helper spec takes 5s without Spring, 750ms with Spring, but I suppose the startup time could be improved with some effort (e.g. add bootsnap, which this old project doesn't have yet).

dgutov commented 10 months ago

Yeah, bootsnap is worth a try too.

I've tried but haven't found a way to use nohup as part of rspec-spec-command without it affecting also the rspec process itself

That makes sense. I probably should have tested it first.

On the high level, it seems like you'd want to launch spring in the background using nohup or its equivalent, so this could be an option in Spring itself (because it creates the subprocess during the command's execution), but I haven't found one so far.

In general, I would just recommend launching Spring in a separate terminal with the same env, so that it picks up the rspec launches as well. And here's a corresponding recommendation for when it's difficult to keep the terminal running: https://github.com/rails/spring/issues/343

til commented 10 months ago

bootsnap reduced the startup from 5s to 2.6s – already a huge win! bootsnap says in its README that it works nicely along spring, so until I can reduce startup time further through other means, I'd still like to use spring occasionally I think.

so this could be an option in Spring itself (because it creates the subprocess during the command's execution), but I haven't found one so far.

I haven't found one either. Adding HUP to the list of ignored signals in https://github.com/rails/spring/blob/main/lib/spring/env.rb#L7 has the desired effect, but I don't know if it has implications elsewhere and if it fits into spring's intents to make this configurable.

In general, I would just recommend launching Spring in a separate terminal

Exactly. I should have mentioned, this has been my workflow for many years now: run specs in Emacs, notice slow startup, switch to terminal, run any spec there to start spring, switch back to Emacs. It's only a minor annoyance, which I was hoping to be easy to fix once and for all , but apparently it isn't 😄.

I've tried to find out from where exactly compile causes the HUP signal to be sent to the compilation process, but no success yet. It might be the way that make-process works, which is called through start-file-process-shell-command?

dgutov commented 10 months ago

bootsnap reduced the startup from 5s to 2.6s – already a huge win

Pretty good, indeed.

Adding HUP to the list of ignored signals in https://github.com/rails/spring/blob/main/lib/spring/env.rb#L7 has the desired effect, but I don't know if it has implications elsewhere and if it fits into spring's intents to make this configurable.

I don't know for sure. But the intent might be that, when you close all the open terminals with C-d, spring shuts down as well. Whereas with this alteration its process might still stay around.

I've tried to find out from where exactly compile causes the HUP signal to be sent to the compilation process, but no success yet. It might be the way that make-process works, which is called through start-file-process-shell-command?

It seems that's just how processes work: the compilation runs spring and rspec as a shell command, meaning a shell process is launched to handle it. But then it finishes and the parent process is terminated, and the OS sends SIGHUP to all its children that are still around. See also the explanation here: https://serverfault.com/a/117153, quoting "Advanced Programing in the Unix Environment".

til commented 10 months ago

It seems that's just how processes work

That makes sense. Therefore, trying to suppress the signal being sent from Emacs is not the right approach.

After another look at spring I found an irresistibly simple way to fix it (for me) by monkey patching spring from its user config file:

# ~/.spring.rb:
Spring::IGNORE_SIGNALS << "HUP"

I'll use this approach now, so far it works exactly as needed. Thanks a lot for the guidance!

til commented 10 months ago

For the record, this comment clarifies the intention of spring on this matter: https://github.com/rails/spring/pull/575#issuecomment-439003374, which I think is a good reason to keep it a local config hack instead of trying to make it configurable within spring itself.