Refactor start command to use Rackup instead of Puma

ayushn21 commented 5 months ago

This is a 🙋 feature or enhancement.

Summary

This PR refactors the start command to use Rackup instead of Puma. This way we can run Bridgetown with any Rack compliant server and aren't restricted to Puma.

Rackup will automatically detect from the Gemfile and start it. Our out-of-the-box Puma setup still works as is.

To do:

[x] Remove/amend usages of options[:using_puma].
[x] Remove hardcoding of host/port and use configuration.
[x] Move "Now serving at:" logging to start file and use info from Rackup.
[x] Test with another server like Falcon.
[x] Test without a server in the Gemfile (Should start with WEBrick I think).
- Starts with WEBrick, but live reload causes errors. I think we can ignore this since we setup Puma by default.
[x] Generic testing.

More details

I had to refactor the way we start the Bridgetown build and aux processes to make it work with this setup. We're now forking a process which runs the build in the background. That in turn starts the aux processes like the frontend watcher. As such I now track PIDs in files so we can kill the processes we start despite stuff happening in different processes.

Context

Fixed https://github.com/bridgetownrb/bridgetown/issues/860

jaredcwhite commented 5 months ago

@ayushn21 I spent a little time with this and it feels pretty solid! Excited to see the progress.

ayushn21 commented 5 months ago

I've disabled live reload when using servers other than Puma. It's causing a few issues. Probably a can we can kick further down the road I think.

ayushn21 commented 5 months ago

Hey @jaredcwhite could you stress test this PR a bit please? I'm too close to it to test it properly 🙈

jaredcwhite commented 5 months ago

I think it's looking really great @ayushn21. 🙌 I have a couple of small fixes on hand regarding when the frontend process isn't running (it crashes on Ctrl+C due to missing PID) and just some process messaging on startup. Mind if I push up and we can do a final review, or do you want to step through it ahead of time?

jaredcwhite commented 5 months ago

I took the liberty of pushing up one fix, as there was a bug where the watcher in SSR mode wasn't returning from a method before setting up an INT trap, causing automated tests to hang and run indefinitely! 😬

ayushn21 commented 5 months ago

Mind if I push up and we can do a final review, or do you want to step through it ahead of time?

Go ahead and push it :).

I took the liberty of pushing up one fix, as there was a bug where the watcher in SSR mode

Awesome thanks! I was stumped by that when I was working on this but was too tired to figure it out at the time so I kicked the can down the road. Thanks for fixing it!

jaredcwhite commented 5 months ago

@ayushn21 All right, I pushed up a couple of things…binding to 0.0.0.0 instead of localhost means other devices on the same network can access the site (I use this all the time, like for testing on my phone), and only stopping the frontend bundling if it's actually running (and not in production). Also still want to set the process name so when I run ps I can see a nice readout for the Bridgetown process, next to Puma.

bridgetownrb / bridgetown