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

Use Process class to spawn rackup #11

Merged
merged 8 commits into from
Jan 15, 2013
Merged

Use Process class to spawn rackup #11

merged 8 commits into from
Jan 15, 2013

Conversation

viking
Copy link
Contributor

@viking viking commented Jan 14, 2013

Ruby 1.9 and later comes with better support for spawning child processes (like rackup). I've switched Guard::RackRunner to use the Process class instead of direct system calls. This allows guard-rack to not have to bother with PID files.

This is not backward compatible with Ruby 1.8. If this is a deal breaker, I can work on adding back the PID file support.

@dblock
Copy link
Owner

dblock commented Jan 15, 2013

I don't think we want to drop 1.8 compat. I have used posix-spawn in the past with great success, it has a nice process API, shouldn't be too much of a stretch?

Is UI.debug something coming from Rack?

@dblock
Copy link
Owner

dblock commented Jan 15, 2013

You're saying this is not backward compatible with Ruby 1.8, but https://travis-ci.org/dblock/guard-rack/jobs/4155002 disagrees :) This probably means that specs don't actually test the functionality and we're stubbing too much.

@viking
Copy link
Contributor Author

viking commented Jan 15, 2013

UI.debug prints things out when you use the -d flag with guard start. The spec passes in 1.8 because I'm using expectations on the Process class. Ruby 1.8 has a Process class, but not a Process.spawn function.

@dblock
Copy link
Owner

dblock commented Jan 15, 2013

Alright, please try with posix-spawn. If it's too much work I'll sleep on whether I think Ruby 1.8 support should be cut (I think not right now).

@viking
Copy link
Contributor Author

viking commented Jan 15, 2013

Tried just dropping in posix-spawn, but it doesn't behave the same way. Process.wait times out after trying to kill the process. I'll see if I can figure out what's happening.

@viking
Copy link
Contributor Author

viking commented Jan 15, 2013

OK, got it worked out. If the command string has a space, it executes the command with sh -c, which returns the wrong PID value. Changed build_rack_command to return an array of arguments instead of a joined string.

@dblock
Copy link
Owner

dblock commented Jan 15, 2013

Last thing, please update https://github.com/dblock/guard-rack/blob/master/CHANGELOG.md - add a "Next Release". Thanks.

@dblock dblock merged commit f5f9b40 into dblock:master Jan 15, 2013
@dblock
Copy link
Owner

dblock commented Jan 15, 2013

I have merged the changes, and noticed that JRuby implementation broke. After some research I found spoon which uses libffi and works on everything. I also wrote an integration spec so that we can see whether a process is actually spawned (testing is awesome, but working is better).

Can you please check the gem and my last two commits after me (just need another pair of eyes) in case I missed anything, before I cut a release?

f769dd8
201d879

@viking
Copy link
Contributor Author

viking commented Jan 15, 2013

Looks good to me :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants