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 session gem internally #16

Closed
mattwynne opened this issue Oct 1, 2010 · 25 comments
Closed

Use session gem internally #16

mattwynne opened this issue Oct 1, 2010 · 25 comments

Comments

@mattwynne
Copy link
Member

See http://github.com/ahoward/session - much tidier API and will allow us to extend Aruba's behaviour with interactive steps.

@msassak
Copy link
Member

msassak commented Oct 5, 2010

Aruba v0.2.3 uses BackgroundProcess and has basic support for interactive processes. I'm working now on adding in named processes (and proper background processes) and refactoring to improve step semantics and the internals.

Session looks cool (and we just ran into an odd bug with BackgroundProcess), so maybe we should switch to that?

@aknuds1
Copy link
Contributor

aknuds1 commented Oct 20, 2010

Please switch from background_process at least, as it does not work on Windows!

@msassak
Copy link
Member

msassak commented Oct 20, 2010

The problem is not background_process, the problem is when background_process requires the pty library, but Aruba doesn't actually use that part of background_process. I think changing the requires in Aruba will make it work on Windows. If not then I suppose it's time to use Session.

@aknuds1
Copy link
Contributor

aknuds1 commented Oct 20, 2010

I know that it's really pty that doesn't work on Windows, but the background_process project does state that it is UNIX only.

@msassak
Copy link
Member

msassak commented Oct 20, 2010

background_process uses Kernel#fork, which isn't available on Windows. Crud.

Session uses Open3. Does that work on Windows?

@aknuds1
Copy link
Contributor

aknuds1 commented Oct 20, 2010

Open3 works on Windows under Ruby 1.9, but with Ruby 1.8 you need win32-open3 from what I recall.

@msassak
Copy link
Member

msassak commented Oct 21, 2010

Do the session gem's tests pass on Windows under MRI 1.8 and 1.9? It's probably not worth beginning to move toward it unless they do.

@mattwynne
Copy link
Member Author

Right, I ran Session's tests on 1.8 on Windows and they don't pass. But. It was a two-line hack to get it to support POpen4, and that did work.

It looks like @danlucraft has already done that in a fork:
http://github.com/danlucraft/session/commit/b6d9317fa2ac795569d6f8d8b0caa04d130813c4

I'm not sure what that means for us. Maybe it's just as easy to hack background_process to use POpen4?

@aknuds1
Copy link
Contributor

aknuds1 commented Oct 21, 2010

I've tried running session's tests under Ruby 1.9.2-p0 on Windows Server 2008 R2, and they all fail with this message: 'NotImplementedError: fork() function is unimplemented on this machine'. I thought session was supposed to use Open3.popen3, which doesn't use fork under Ruby 1.9?

@msassak
Copy link
Member

msassak commented Oct 21, 2010

I'm not sure it will be so easy to get background_process to use POpen4, but I'm kind of leaning towards just writing our own process control stuff at this point. In my interactive branch I've encapsulated the background_process stuff into a Process class anyway, so we can just modify that.

As for things working or not with MRI 1.9.2 under Windows, I have no clue.

@mattwynne
Copy link
Member Author

I'd assume that if I can get it working with 1.8.7, 1.9.2 shouldn't be too hard. It might even be the same fix.

@aknuds1
Copy link
Contributor

aknuds1 commented Oct 21, 2010

The issue with session under 1.9 AFAICT is that it insists on using fork() rather than Open3.popen3 (as it advertises?). The fork function is unsupported with both 1.8 and 1.9 on Windows, but Open3.popen3 works under 1.9.

@mattwynne
Copy link
Member Author

This might be a better bet: http://rubygems.org/gems/childprocess

@msassak
Copy link
Member

msassak commented Oct 23, 2010

Looks like we share aims with child process, but so far as I can tell it's currently missing a way to send input to running processes. We should send jarib a note or chat with him on IRC about his plans for the library.

@msassak
Copy link
Member

msassak commented Nov 10, 2010

background_process doesn't work on JRuby, either. /facepalm

See aslakhellesoy/aruba#27 for the sad, sad details.

@msassak
Copy link
Member

msassak commented Dec 1, 2010

I've added support for writing to stdin with childprocess here: https://github.com/msassak/childprocess/tree/support-stdin and tested it with MRI 1.8.7 and JRuby 1.5.2. In the next few days I'll see if what I did is enough to replace background_process in Aruba on those platforms. If someone could run the specs in my childprocess branch under Windows, I'd greatly appreciate it.

@mattwynne
Copy link
Member Author

P:\childprocess>git status
# On branch support-stdin
nothing to commit (working directory clean)

P:\childprocess>rake spec
(in P:/childprocess)
All dependencies seem to be installed.
C:/Ruby187/bin/ruby.exe -I lib:spec -S rspec "spec/abstract_io_spec.rb" "spec/ch
ildprocess_spec.rb" "spec/jruby_spec.rb" "spec/unix_spec.rb" "spec/windows_spec.
rb"
........*.....

Pending:
  ChildProcess lets a detached child live on
    # how do we spec this?
    # ./spec/childprocess_spec.rb:65

Finished in 2.39 seconds
14 examples, 0 failures, 1 pending

@msassak
Copy link
Member

msassak commented Dec 3, 2010

I was not expecting that to pass, and what do you know, it shouldn't have! I've pushed code that actually does what it should (only on MRI for now) to my support-stdin branch of childprocess, can you run that on Winders for me?

Using the updates to childprocess I've successfully replaced background_process here: https://github.com/msassak/aruba/tree/childprocess. Hacks abound, but you have to start somewhere. :-)

@aknuds1
Copy link
Contributor

aknuds1 commented Dec 5, 2010

I tried your latest revision of childprocess on Windows/Ruby 1.8.7. There are a couple of problems.

PS C:\Users\Arve\VCSCheckouts\ChildProcess> git status
# On branch support-stdin
nothing to commit (working directory clean)
PS C:\Users\Arve\VCSCheckouts\ChildProcess> rake spec
(in C:/Users/Arve/VCSCheckouts/ChildProcess)
C:/Ruby187/lib/ruby/gems/1.8/gems/jeweler-1.4.0/lib/jeweler/commands/check_dependencies.rb:13:Warning: Gem::Dependency#v
ersion_requirements is deprecated and will be removed on or after August 2010.  Use #requirement
All dependencies seem to be installed.
C:/Ruby187/bin/ruby.exe -I lib:spec -S rspec "spec/abstract_io_spec.rb" "spec/childprocess_spec.rb" "spec/jruby_spec.rb"
 "spec/unix_spec.rb" "spec/windows_spec.rb"

*****************************************************************
DEPRECATION WARNING: you are using deprecated behaviour that will
be removed from rspec-2.0.0.

C:/Ruby187/lib/ruby/gems/1.8/gems/rspec-core-2.0.1/lib/rspec/core/configuration_options.rb:70:in `parse_local_options'

* spec/spec.opts is deprecated.
* please use ./.rspec or ~/.rspec instead.
*****************************************************************
........*F...

Pending:
  ChildProcess lets a detached child live on
    # how do we spec this?
    # ./spec/childprocess_spec.rb:65

Failures:
  1) ChildProcess can redirect stdout, stderr and stdin
     Failure/Error: process.io.stdin.puts "stdin"
     private method `puts' called for nil:NilClass
     # ./spec/childprocess_spec.rb:88

Finished in 1.19 seconds
13 examples, 1 failure, 1 pending
rake aborted!
ruby -I lib:spec -S rspec "spec/abstract_io_spec.rb" "spec/childprocess_spec.rb" "spec/jruby_spec.rb" "spec/unix_spec.rb
" "spec/windows_spec.rb" failed

(See full trace by running task with --trace)

@msassak
Copy link
Member

msassak commented Dec 6, 2010

That's the expected error. The files in childprocess/lib/windows/ need to be modified to get that passing. (The pending spec has always been there.)

@aknuds1
Copy link
Contributor

aknuds1 commented Dec 6, 2010

It's good that it behaved as you expected, then :)

@msassak
Copy link
Member

msassak commented Dec 22, 2010

Thanks to jarib childprocess now supports stdin on MRI, JRuby and Windows, and I've replaced background_process with it in my childprocess branch. I think we should close this ticket and start a new one for testing that branch (or a new release of Aruba that contains the changes) on Windows. @mattwynne and @aknuds1 is that fair?

@msassak
Copy link
Member

msassak commented Dec 22, 2010

I started a ticket at aslakhellesoy/aruba#40 to track testing Aruba with childprocess on Windows.

@aknuds1
Copy link
Contributor

aknuds1 commented Dec 22, 2010

Sounds like a good idea to me, although I've only been peripherally involved so far.

@mattwynne
Copy link
Member Author

Fine by me

This issue was closed.
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

No branches or pull requests

3 participants