Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

JRuby compatibility - replace popen with a tempfile #12

Merged
merged 2 commits into from

3 participants

@kares

It's a bit shamy closure-compiler does not work on JRuby (due to fork).

Now the #10 is not the right way as the compiler Java code does a System.exit once done and thus it'll exit the current JVM process which is not desired.

If the popen is replaced with a Tempfile that is passed as a --js argument while invoking the compiler using "`" it works nicely on JRuby as well as Ruby + no Windows hustle necessary.

@jashkenas
Owner

The patch looks good -- can you make sure that it unlinks (deletes) the tempfile after it's done with it?

@kares

That should to it, thanks for the review.

@igrigorik

+1 for merge. Need JRuby support.

@jashkenas
Owner

Whoops ... sorry to have left this on the floor. @kares: for the record, what evironments have you tested this patch on?

@kares

@jashkenas i've only tested on linux, test pass for me using the following rubies:

ruby 1.8.7 (2011-02-18 patchlevel 334)
ruby 1.9.2p180 (2011-02-18 revision 30909)
jruby 1.6.2 (ruby-1.8.7-p330)

@kares

just to be sure I installed ruby on a vm with win7.
all test passed with ruby 1.8.7 (2011-02-18 patchlevel 334)

@jashkenas jashkenas merged commit 3e4d763 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 25, 2011
  1. @kares
Commits on May 26, 2011
  1. @kares

    ensure tempfile is deleted

    kares authored
This page is out of date. Refresh to see the latest.
View
1  lib/closure-compiler.rb
@@ -12,5 +12,4 @@ module Closure
end
-require 'stringio'
require 'closure/compiler'
View
43 lib/closure/compiler.rb
@@ -1,4 +1,5 @@
-require 'closure/popen'
+require 'stringio'
+require 'tempfile'
module Closure
@@ -20,28 +21,28 @@ def initialize(options={})
# JavaScript as a string or yields an IO object containing the response to a
# block, for streaming.
def compile(io)
- result, error = nil, nil
- status = Closure::Popen.popen(command) do |stdin, stdout, stderr|
- if io.respond_to? :read
- while buffer = io.read(4096) do
- stdin.write(buffer)
- end
- else
- stdin.write(io.to_s)
+ tempfile = Tempfile.new('closure_compiler')
+ if io.respond_to? :read
+ while buffer = io.read(4096) do
+ tempfile.write(buffer)
end
- stdin.close
- if Closure::Popen::WINDOWS
- stderr.close
- result = stdout.read
- error = "Stderr cannot be read on Windows."
- else
- out_thread = Thread.new { result = stdout.read }
- err_thread = Thread.new { error = stderr.read }
- out_thread.join and err_thread.join
- end
- yield(StringIO.new(result)) if block_given?
+ else
+ tempfile.write(io.to_s)
+ end
+ tempfile.flush
+
+ begin
+ result = `#{command} --js #{tempfile.path}`
+ rescue Exception
+ raise Error, "compression failed"
+ ensure
+ tempfile.close!
end
- raise Error, error unless status.success?
+ unless $?.exitstatus.zero?
+ raise Error, result
+ end
+
+ yield(StringIO.new(result)) if block_given?
result
end
alias_method :compress, :compile
View
66 lib/closure/popen.rb
@@ -1,66 +0,0 @@
-module Closure
-
- # A slightly modified version of Ruby 1.8's Open3, that doesn't use a
- # grandchild process, and returns the pid of the external process.
- module Popen
-
- WINDOWS = RUBY_PLATFORM.match(/(win|w)32$/)
- ONE_NINE = RUBY_VERSION >= "1.9"
- if WINDOWS
- if ONE_NINE
- require 'open3'
- else
- require 'rubygems'
- require 'win32/open3'
- end
- end
-
- def self.popen(cmd)
- if WINDOWS
- error = nil
- Open3.popen3(cmd) do |stdin, stdout, stderr, wait_thread|
- yield(stdin, stdout, stderr) if block_given?
- stdout.read unless stdout.closed? or stdout.eof?
- unless stderr.closed?
- stderr.rewind
- error = stderr.read
- end
- return wait_thread.value if wait_thread.is_a? Thread
- end
- else
- # pipe[0] for read, pipe[1] for write
- pw, pr, pe = IO.pipe, IO.pipe, IO.pipe
-
- pid = fork {
- pw[1].close
- STDIN.reopen(pw[0])
- pw[0].close
-
- pr[0].close
- STDOUT.reopen(pr[1])
- pr[1].close
-
- pe[0].close
- STDERR.reopen(pe[1])
- pe[1].close
-
- exec(cmd)
- }
-
- pw[0].close
- pr[1].close
- pe[1].close
- pi = [pw[1], pr[0], pe[0]]
- pw[1].sync = true
- begin
- yield(*pi) if block_given?
- ensure
- pi.each{|p| p.close unless p.closed?}
- end
- Process.waitpid pid
- end
- $?
- end
-
- end
-end
View
10 test/unit/test_closure_compiler.rb
@@ -34,12 +34,10 @@ def test_block_syntax
end
def test_jar_and_java_specifiation
- if !Closure::Popen::WINDOWS
- jar = Dir['vendor/closure-compiler-*.jar'].first
- java = `which java`.strip
- compiler = Compiler.new(:java => java, :jar_file => jar)
- assert compiler.compress(ORIGINAL) == COMPILED_SIMPLE
- end
+ jar = Dir['vendor/closure-compiler-*.jar'].first
+ java = `which java`.strip
+ compiler = Compiler.new(:java => java, :jar_file => jar)
+ assert compiler.compress(ORIGINAL) == COMPILED_SIMPLE
end
def test_exceptions
Something went wrong with that request. Please try again.