A backend based on Process.spawn about childprocess HOT 12 CLOSED

If Ruby itself has gotten better about cross-platform process handling, I'd be perfectly content for childprocess to become just a more convenient API on top of the core library instead of it having to handle a lot of the more 'fiddly' stuff that might have been needed back in the day.

from childprocess.

Thank you for your efforts here. Merged in #175.

from childprocess.

Hey @eregon—supportive of the request, though would prefer this be contributed by folks whose use case necessitates it so they can test it in a real-world scenario.

I don't have context on why the original ChildProcess backends were chosen over Process.spawn, but I assume there was some reason. Would be happy to review a tested pull request implementing this functionality.

from childprocess.

I don't have context on why the original ChildProcess backends were chosen over Process.spawn, but I assume there was some reason.

I don't know for sure, but I would bet it's because ChildProcess already existed in the Ruby 1.8 area, and Process.spawn didn't exist then (it was added in Ruby 1.9, just checked).
There were still patch releases of 1.8 in 2010 when the project was started: https://github.com/enkessler/childprocess/tags?after=v0.1.1, so it seems a likely reason.

from childprocess.

I'll try to find some time to make a prototype but if anyone else is interested, go ahead and just mention it.

from childprocess.

Here is a prototype, and it already passes all the specs, at least on Linux: master...eregon:process-spawn

I would think it works fine on Windows too, however the superclass is Unix::Process so there might be some assumptions there.

from childprocess.

#175 already works well on UNIX, but not yet on Windows.
I don't have a Windows dev machine, so that's rather impractical as I'd need to wait for CI to know if something works or not.
Maybe I can try to force it to take the Windows branch locally for faster development.

Are constants like ChildProcess::Unix considered public API or not? I guess not given they are conditionally defined based on the current platform.

from childprocess.

Update: it works on Windows now, there are only 2 failures left: #175 (comment)
And the diff is +166 −2,045.

from childprocess.

A couple thousand fewer lines of code to have to worry about is nice!

The second is the issue that there is no builtin way to kill a process group on Windows.

Would it be possible to incrementally replace the internal implementation? Swap the OSX and Linux parts because core Ruby works fine but keep the Windows specific functionality in place until we can work out those last failures?

from childprocess.

Would it be possible to incrementally replace the internal implementation? Swap the OSX and Linux parts because core Ruby works fine but keep the Windows specific functionality in place until we can work out those last failures?

I would prefer removing all platform-specific code, that's so much better for maintenance and guarantees it works reliably.
Also IIRC it's not trivial to keep the Windows part but not the rest without incurring a lot of complexity and copying classes etc (but I'd need to look into it again).

That would mean no longer supporting killing a process group on Windows (already not supported with process_builder).
I think we should raise an error in that case.
Does anyone use that?
I think it would be a fair enough change for a new major version of the gem given the most likely very low usage.

What do you think?
BTW, this issue keeps coming up, notably because selenium and other gems don't work without setting CHILDPROCESS_POSIX_SPAWN=1 on TruffleRuby (oracle/truffleruby#1525).

from childprocess.

I forgot, another advantage is Process.spawn is faster than fork+exec as currently used by child_process, because that can use vfork() or posix_spawn() for instance which needs to copy less from the parent process.

from childprocess.

Would it be possible to incrementally replace the internal implementation? Swap the OSX and Linux parts because core Ruby works fine but keep the Windows specific functionality in place until we can work out those last failures?

I gave it a try to keep the existing logic for JRuby and Windows backends, but that seems to only results in more test failures, so I don't see the point: https://github.com/eregon/childprocess/tree/process-spawn-non-windows https://github.com/eregon/childprocess/actions/runs/4235173639
Also see my comment at #175 (comment).

Could you merge/review #175 (comment) ?
I think to address that one last failure on Windows we just need to exclude that test on Windows, it doesn't work on master either anyway.

from childprocess.