You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@buildr.apache.org by Daniel Spiewak <dj...@gmail.com> on 2009/04/17 20:42:31 UTC
Native Kernel#system on JRuby
I think we should consider this:
http://github.com/djspiewak/buildr/commit/51280abe5d37c2800ae53828bc0bb3ba5b51d946
The reason being that JRuby's Kernel#system method is fundamentally broken
(because it is based on java.lang.Runtime.exec). They've managed to piece
things together so that it's rarely an issue, but problems do surface when
doing things like the following:
system 'scala'
If you run this from JRuby, your terminal will freeze and refuse to accept
any input (except for escape sequences like Ctrl+C). This limitation in
Kernel#system basically prevents any sort of working REPL support (git://
github.com/djspiewak/buildr.git / interactive-shell). In order to get
around this, I propose that Buildr should rewrite the Kernel#system
method *when
running under JRuby* so that it uses the platform's native `system` function
(from the C stdlib). The commit linked above (51280ab) contains the
necessary implementation. Note that this commit is basically untested, but
it seems to work on Mac for everything I've tried so far.
Advantages: Buildr's external process dispatch on JRuby will be more
predictable and basically in line with what happens on MRI. This will
probably resolve/avoid some bugs and certainly opens up a whole new series
of possibilities (like the interactive shell support).
Disadvantages: It introduces a new gem dependency on JRuby (ffi-0.3.2) and
actually overwrites a core Ruby method. It's the latter that has me the
most concerned. Kernel#system is quite foundational to Buildr and Ruby in
general, so casually monkey-patching in a new implementation is not a step
which should be taken lightly.
Thoughts?
Daniel
Re: Native Kernel#system on JRuby
Posted by Daniel Spiewak <dj...@gmail.com>.
Actually, this would mean gathering the information necessary to completely
populate a ProcessStatus object (for $?). Seems like a bit too much trouble
at the moment (since my patch is working for what I need). I'll probably do
this eventually, but not at the moment.
Daniel
On Fri, Apr 17, 2009 at 9:04 PM, Daniel Spiewak <dj...@gmail.com> wrote:
> I suppose changing sh would be a better solution. The only disadvantage
> there is we need to make sure that anything which would otherwise use system
> for invoking potentially-problematic executables should instead use sh.
> Optimally, the JRuby guys should eventually adopt something like this for
> the canonical system implementation, but until then...
>
> I'll make the necessary changes in my fork.
>
> Daniel
>
>
> On Fri, Apr 17, 2009 at 8:17 PM, Assaf Arkin <ar...@intalio.com> wrote:
>
>> On Fri, Apr 17, 2009 at 11:42 AM, Daniel Spiewak <djspiewak@gmail.com
>> >wrote:
>>
>> > I think we should consider this:
>> >
>> >
>> http://github.com/djspiewak/buildr/commit/51280abe5d37c2800ae53828bc0bb3ba5b51d946
>> >
>> > The reason being that JRuby's Kernel#system method is fundamentally
>> broken
>> > (because it is based on java.lang.Runtime.exec). They've managed to
>> piece
>> > things together so that it's rarely an issue, but problems do surface
>> when
>> > doing things like the following:
>> >
>> > system 'scala'
>> >
>> > If you run this from JRuby, your terminal will freeze and refuse to
>> accept
>> > any input (except for escape sequences like Ctrl+C). This limitation in
>> > Kernel#system basically prevents any sort of working REPL support
>> (git://
>> > github.com/djspiewak/buildr.git / interactive-shell). In order to get
>> > around this, I propose that Buildr should rewrite the Kernel#system
>> > method *when
>> > running under JRuby* so that it uses the platform's native `system`
>> > function
>> > (from the C stdlib). The commit linked above (51280ab) contains the
>> > necessary implementation. Note that this commit is basically untested,
>> but
>> > it seems to work on Mac for everything I've tried so far.
>> >
>> > Advantages: Buildr's external process dispatch on JRuby will be more
>> > predictable and basically in line with what happens on MRI. This will
>> > probably resolve/avoid some bugs and certainly opens up a whole new
>> series
>> > of possibilities (like the interactive shell support).
>> >
>> > Disadvantages: It introduces a new gem dependency on JRuby (ffi-0.3.2)
>> and
>> > actually overwrites a core Ruby method. It's the latter that has me the
>> > most concerned. Kernel#system is quite foundational to Buildr and Ruby
>> in
>> > general, so casually monkey-patching in a new implementation is not a
>> step
>> > which should be taken lightly.
>>
>>
>> I also have a bad feeling about changing Kernel#system, but we can
>> certainly
>> change sh, which is brought over from Rake.
>>
>> Assaf
>>
>>
>> >
>> >
>> > Thoughts?
>> >
>> > Daniel
>> >
>>
>
>
Re: Native Kernel#system on JRuby
Posted by Daniel Spiewak <dj...@gmail.com>.
I suppose changing sh would be a better solution. The only disadvantage
there is we need to make sure that anything which would otherwise use system
for invoking potentially-problematic executables should instead use sh.
Optimally, the JRuby guys should eventually adopt something like this for
the canonical system implementation, but until then...
I'll make the necessary changes in my fork.
Daniel
On Fri, Apr 17, 2009 at 8:17 PM, Assaf Arkin <ar...@intalio.com> wrote:
> On Fri, Apr 17, 2009 at 11:42 AM, Daniel Spiewak <djspiewak@gmail.com
> >wrote:
>
> > I think we should consider this:
> >
> >
> http://github.com/djspiewak/buildr/commit/51280abe5d37c2800ae53828bc0bb3ba5b51d946
> >
> > The reason being that JRuby's Kernel#system method is fundamentally
> broken
> > (because it is based on java.lang.Runtime.exec). They've managed to
> piece
> > things together so that it's rarely an issue, but problems do surface
> when
> > doing things like the following:
> >
> > system 'scala'
> >
> > If you run this from JRuby, your terminal will freeze and refuse to
> accept
> > any input (except for escape sequences like Ctrl+C). This limitation in
> > Kernel#system basically prevents any sort of working REPL support (git://
> > github.com/djspiewak/buildr.git / interactive-shell). In order to get
> > around this, I propose that Buildr should rewrite the Kernel#system
> > method *when
> > running under JRuby* so that it uses the platform's native `system`
> > function
> > (from the C stdlib). The commit linked above (51280ab) contains the
> > necessary implementation. Note that this commit is basically untested,
> but
> > it seems to work on Mac for everything I've tried so far.
> >
> > Advantages: Buildr's external process dispatch on JRuby will be more
> > predictable and basically in line with what happens on MRI. This will
> > probably resolve/avoid some bugs and certainly opens up a whole new
> series
> > of possibilities (like the interactive shell support).
> >
> > Disadvantages: It introduces a new gem dependency on JRuby (ffi-0.3.2)
> and
> > actually overwrites a core Ruby method. It's the latter that has me the
> > most concerned. Kernel#system is quite foundational to Buildr and Ruby
> in
> > general, so casually monkey-patching in a new implementation is not a
> step
> > which should be taken lightly.
>
>
> I also have a bad feeling about changing Kernel#system, but we can
> certainly
> change sh, which is brought over from Rake.
>
> Assaf
>
>
> >
> >
> > Thoughts?
> >
> > Daniel
> >
>
Re: Native Kernel#system on JRuby
Posted by Assaf Arkin <ar...@intalio.com>.
On Fri, Apr 17, 2009 at 11:42 AM, Daniel Spiewak <dj...@gmail.com>wrote:
> I think we should consider this:
>
> http://github.com/djspiewak/buildr/commit/51280abe5d37c2800ae53828bc0bb3ba5b51d946
>
> The reason being that JRuby's Kernel#system method is fundamentally broken
> (because it is based on java.lang.Runtime.exec). They've managed to piece
> things together so that it's rarely an issue, but problems do surface when
> doing things like the following:
>
> system 'scala'
>
> If you run this from JRuby, your terminal will freeze and refuse to accept
> any input (except for escape sequences like Ctrl+C). This limitation in
> Kernel#system basically prevents any sort of working REPL support (git://
> github.com/djspiewak/buildr.git / interactive-shell). In order to get
> around this, I propose that Buildr should rewrite the Kernel#system
> method *when
> running under JRuby* so that it uses the platform's native `system`
> function
> (from the C stdlib). The commit linked above (51280ab) contains the
> necessary implementation. Note that this commit is basically untested, but
> it seems to work on Mac for everything I've tried so far.
>
> Advantages: Buildr's external process dispatch on JRuby will be more
> predictable and basically in line with what happens on MRI. This will
> probably resolve/avoid some bugs and certainly opens up a whole new series
> of possibilities (like the interactive shell support).
>
> Disadvantages: It introduces a new gem dependency on JRuby (ffi-0.3.2) and
> actually overwrites a core Ruby method. It's the latter that has me the
> most concerned. Kernel#system is quite foundational to Buildr and Ruby in
> general, so casually monkey-patching in a new implementation is not a step
> which should be taken lightly.
I also have a bad feeling about changing Kernel#system, but we can certainly
change sh, which is brought over from Rake.
Assaf
>
>
> Thoughts?
>
> Daniel
>
Re: Native Kernel#system on JRuby
Posted by Daniel Spiewak <dj...@gmail.com>.
http://github.com/djspiewak/buildr/tree/jruby-system I just went ahead and
made a separate branch, it was easier than updating the list every time I
changed something.
Daniel
On Fri, Apr 17, 2009 at 3:38 PM, Daniel Spiewak <dj...@gmail.com> wrote:
> Amendment:
> http://github.com/djspiewak/buildr/commit/144b4b0eb9159ef6f42f135e47a6bc8c9297edce
>
> I forgot that Kernel#system actually needs to return Boolean, not Fixnum.
>
> Daniel
>
>
> On Fri, Apr 17, 2009 at 1:42 PM, Daniel Spiewak <dj...@gmail.com>wrote:
>
>> I think we should consider this:
>> http://github.com/djspiewak/buildr/commit/51280abe5d37c2800ae53828bc0bb3ba5b51d946
>>
>> The reason being that JRuby's Kernel#system method is fundamentally broken
>> (because it is based on java.lang.Runtime.exec). They've managed to piece
>> things together so that it's rarely an issue, but problems do surface when
>> doing things like the following:
>>
>> system 'scala'
>>
>> If you run this from JRuby, your terminal will freeze and refuse to accept
>> any input (except for escape sequences like Ctrl+C). This limitation in
>> Kernel#system basically prevents any sort of working REPL support (git://
>> github.com/djspiewak/buildr.git / interactive-shell). In order to get
>> around this, I propose that Buildr should rewrite the Kernel#system method
>> *when running under JRuby* so that it uses the platform's native `system`
>> function (from the C stdlib). The commit linked above (51280ab) contains
>> the necessary implementation. Note that this commit is basically untested,
>> but it seems to work on Mac for everything I've tried so far.
>>
>> Advantages: Buildr's external process dispatch on JRuby will be more
>> predictable and basically in line with what happens on MRI. This will
>> probably resolve/avoid some bugs and certainly opens up a whole new series
>> of possibilities (like the interactive shell support).
>>
>> Disadvantages: It introduces a new gem dependency on JRuby (ffi-0.3.2) and
>> actually overwrites a core Ruby method. It's the latter that has me the
>> most concerned. Kernel#system is quite foundational to Buildr and Ruby in
>> general, so casually monkey-patching in a new implementation is not a step
>> which should be taken lightly.
>>
>> Thoughts?
>>
>> Daniel
>>
>
>
Re: Native Kernel#system on JRuby
Posted by Daniel Spiewak <dj...@gmail.com>.
Amendment:
http://github.com/djspiewak/buildr/commit/144b4b0eb9159ef6f42f135e47a6bc8c9297edce
I forgot that Kernel#system actually needs to return Boolean, not Fixnum.
Daniel
On Fri, Apr 17, 2009 at 1:42 PM, Daniel Spiewak <dj...@gmail.com> wrote:
> I think we should consider this:
> http://github.com/djspiewak/buildr/commit/51280abe5d37c2800ae53828bc0bb3ba5b51d946
>
> The reason being that JRuby's Kernel#system method is fundamentally broken
> (because it is based on java.lang.Runtime.exec). They've managed to piece
> things together so that it's rarely an issue, but problems do surface when
> doing things like the following:
>
> system 'scala'
>
> If you run this from JRuby, your terminal will freeze and refuse to accept
> any input (except for escape sequences like Ctrl+C). This limitation in
> Kernel#system basically prevents any sort of working REPL support (git://
> github.com/djspiewak/buildr.git / interactive-shell). In order to get
> around this, I propose that Buildr should rewrite the Kernel#system method
> *when running under JRuby* so that it uses the platform's native `system`
> function (from the C stdlib). The commit linked above (51280ab) contains
> the necessary implementation. Note that this commit is basically untested,
> but it seems to work on Mac for everything I've tried so far.
>
> Advantages: Buildr's external process dispatch on JRuby will be more
> predictable and basically in line with what happens on MRI. This will
> probably resolve/avoid some bugs and certainly opens up a whole new series
> of possibilities (like the interactive shell support).
>
> Disadvantages: It introduces a new gem dependency on JRuby (ffi-0.3.2) and
> actually overwrites a core Ruby method. It's the latter that has me the
> most concerned. Kernel#system is quite foundational to Buildr and Ruby in
> general, so casually monkey-patching in a new implementation is not a step
> which should be taken lightly.
>
> Thoughts?
>
> Daniel
>