buildr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Assaf Arkin" <ar...@intalio.com>
Subject Re: svn commit: r697901 - in /incubator/buildr/trunk: lib/buildr/java.rb lib/buildr/java/compilers.rb lib/buildr/java/java.rb lib/buildr/java/jruby.rb lib/buildr/java/rjb.rb spec/java_spec.rb
Date Tue, 23 Sep 2008 21:23:18 GMT
On Tue, Sep 23, 2008 at 1:52 PM, lacton <lacton@users.sourceforge.net> wrote:
> Assaf,
>
> Will you tell me what was wrong with the way I tried to factor common
> code between commands.rb, jruby.rb and rjb.rb?

Because I prefer to tightly couple the tools_jar and load methods of
each module, then to tightly couple the modules together.

Generally when I troubleshoot code, I'm either working against rjb.rb
or jruby.rb, depending on which platform I'm using.  It's much more
convenient to have all the code in one place, then spread it out.

And it hides a distinction that exists between these modules: RJB's
tool_jars method brings with it a dependency on setting JAVA_HOME,
JRuby doesn't.  By having two separate methods to begin with, it's
easier when looking at rjb.rb to notice that JAVA_HOME is only
referenced once, and only by tools_jar, and to move it there from the
load method, where it was before.

Assaf

>
> Lacton
>
> On Mon, Sep 22, 2008 at 7:17 PM,  <assaf@apache.org> wrote:
>> Author: assaf
>> Date: Mon Sep 22 10:17:03 2008
>> New Revision: 697901
>>
>> URL: http://svn.apache.org/viewvc?rev=697901&view=rev
>> Log:
>> Removed java/java.rb, not to be confused with java.rb.
>> Java.tools_jar caches results, returns it directly.
>>
>> Removed:
>>    incubator/buildr/trunk/lib/buildr/java/java.rb
>> Modified:
>>    incubator/buildr/trunk/lib/buildr/java.rb
>>    incubator/buildr/trunk/lib/buildr/java/compilers.rb
>>    incubator/buildr/trunk/lib/buildr/java/jruby.rb
>>    incubator/buildr/trunk/lib/buildr/java/rjb.rb
>>    incubator/buildr/trunk/spec/java_spec.rb
>>
>> Modified: incubator/buildr/trunk/lib/buildr/java.rb
>> URL: http://svn.apache.org/viewvc/incubator/buildr/trunk/lib/buildr/java.rb?rev=697901&r1=697900&r2=697901&view=diff
>> ==============================================================================
>> --- incubator/buildr/trunk/lib/buildr/java.rb (original)
>> +++ incubator/buildr/trunk/lib/buildr/java.rb Mon Sep 22 10:17:03 2008
>> @@ -14,7 +14,6 @@
>>  # the License.
>>
>>
>> -require 'buildr/java/java'
>>  require RUBY_PLATFORM == 'java' ? 'buildr/java/jruby' : 'buildr/java/rjb'
>>  require 'buildr/java/compilers'
>>  require 'buildr/java/test_frameworks'
>>
>> Modified: incubator/buildr/trunk/lib/buildr/java/compilers.rb
>> URL: http://svn.apache.org/viewvc/incubator/buildr/trunk/lib/buildr/java/compilers.rb?rev=697901&r1=697900&r2=697901&view=diff
>> ==============================================================================
>> --- incubator/buildr/trunk/lib/buildr/java/compilers.rb (original)
>> +++ incubator/buildr/trunk/lib/buildr/java/compilers.rb Mon Sep 22 10:17:03 2008
>> @@ -55,7 +55,7 @@
>>         check_options options, OPTIONS
>>         cmd_args = []
>>         # tools.jar contains the Java compiler.
>> -        Java.tools_jar { |tools_jar| dependencies << tools_jar }
>> +        dependencies << Java.tools_jar if Java.tools_jar
>>         cmd_args << '-classpath' << dependencies.join(File::PATH_SEPARATOR)
unless dependencies.empty?
>>         source_paths = sources.select { |source| File.directory?(source) }
>>         cmd_args << '-sourcepath' << source_paths.join(File::PATH_SEPARATOR)
unless source_paths.empty?
>>
>> Modified: incubator/buildr/trunk/lib/buildr/java/jruby.rb
>> URL: http://svn.apache.org/viewvc/incubator/buildr/trunk/lib/buildr/java/jruby.rb?rev=697901&r1=697900&r2=697901&view=diff
>> ==============================================================================
>> --- incubator/buildr/trunk/lib/buildr/java/jruby.rb (original)
>> +++ incubator/buildr/trunk/lib/buildr/java/jruby.rb Mon Sep 22 10:17:03 2008
>> @@ -57,6 +57,7 @@
>>  module Java
>>
>>   # Since we already have a JVM loaded, we can use it to guess where JAVA_HOME is.
>> +  # We set JAVA_HOME early so we can use it without calling Java.load first.
>>   ENV['JAVA_HOME'] ||= java.lang.System.getProperty("java.home")
>>
>>   class << self
>> @@ -70,6 +71,15 @@
>>     def classpath
>>       @classpath ||= []
>>     end
>> +
>> +    # Most platforms requires tools.jar to be on the classpath, tools.jar contains
the
>> +    # Java compiler (OS X and AIX are two exceptions we know about, may be more).
>> +    # Guess where tools.jar is from JAVA_HOME, which hopefully points to the JDK,
>> +    # but maybe the JRE.  Return nil if not found.
>> +    def tools_jar #:nodoc:
>> +      @tools_jar ||= ['lib/tools.jar', '../lib/tools.jar'].map { |path| File.expand_path(path,
ENV['JAVA_HOME']) }.
>> +        find { |path| File.exist?(path) }
>> +    end
>>
>>     # Loads the JVM and all the libraries listed on the classpath.  Call this
>>     # method before accessing any Java class, but only call it from methods
>> @@ -90,7 +100,7 @@
>>       add_path = lambda { |path| add_url_method.invoke(sysloader, [java.io.File.new(path).toURI.toURL].to_java(java.net.URL))
}
>>
>>       # Most platforms requires tools.jar to be on the classpath.
>> -      tools_jar { |tools_jar| add_path[tools_jar] }
>> +      add_path[tools_jar] if tools_jar
>>
>>       Buildr.artifacts(classpath).map(&:to_s).each do |path|
>>         file(path).invoke
>>
>> Modified: incubator/buildr/trunk/lib/buildr/java/rjb.rb
>> URL: http://svn.apache.org/viewvc/incubator/buildr/trunk/lib/buildr/java/rjb.rb?rev=697901&r1=697900&r2=697901&view=diff
>> ==============================================================================
>> --- incubator/buildr/trunk/lib/buildr/java/rjb.rb (original)
>> +++ incubator/buildr/trunk/lib/buildr/java/rjb.rb Mon Sep 22 10:17:03 2008
>> @@ -71,13 +71,10 @@
>>     end
>>
>>   end
>> -
>> -
>> +
>>   # On OS X we know where the default JDK is. We can try to guess for other OS.
>> -  case Config::CONFIG['host_os']
>> -  when /darwin/i ; ENV['JAVA_HOME'] ||= '/System/Library/Frameworks/JavaVM.framework/Home'
>> -  end
>> -
>> +  # We set JAVA_HOME early so we can use it without calling Java.load first.
>> +  ENV['JAVA_HOME'] ||= '/System/Library/Frameworks/JavaVM.framework/Home' if Config::CONFIG['host_os']
=~ /darwin/i
>>
>>   class << self
>>
>> @@ -90,15 +87,26 @@
>>     def classpath
>>       @classpath ||= []
>>     end
>> -
>> +
>> +    # Most platforms requires tools.jar to be on the classpath, tools.jar contains
the
>> +    # Java compiler (OS X and AIX are two exceptions we know about, may be more).
>> +    # Guess where tools.jar is from JAVA_HOME, which hopefully points to the JDK,
>> +    # but maybe the JRE.  Return nil if not found.
>> +    def tools_jar #:nodoc:
>> +      @tools_jar ||= begin
>> +        home = ENV['JAVA_HOME'] or fail 'Are we forgetting something? JAVA_HOME
not set.'
>> +        ['lib/tools.jar', '../lib/tools.jar'].map { |path| File.expand_path(path,
home) }.
>> +          find { |path| File.exist?(path) }
>> +      end
>> +    end
>> +
>>     # Loads the JVM and all the libraries listed on the classpath.  Call this
>>     # method before accessing any Java class, but only call it from methods
>>     # used in the build, giving the Buildfile a chance to load all extensions
>>     # that append to the classpath and specify which remote repositories to use.
>>     def load
>>       return self if @loaded
>> -      ENV['JAVA_HOME'] or fail 'Are we forgetting something? JAVA_HOME not set.'
>> -      tools_jar { |tools_jar| classpath << tools_jar }
>> +      classpath << tools_jar if tools_jar
>>
>>       cp = Buildr.artifacts(classpath).map(&:to_s).each { |path| file(path).invoke
}
>>       java_opts = (ENV['JAVA_OPTS'] || ENV['JAVA_OPTIONS']).to_s.split
>>
>> Modified: incubator/buildr/trunk/spec/java_spec.rb
>> URL: http://svn.apache.org/viewvc/incubator/buildr/trunk/spec/java_spec.rb?rev=697901&r1=697900&r2=697901&view=diff
>> ==============================================================================
>> --- incubator/buildr/trunk/spec/java_spec.rb (original)
>> +++ incubator/buildr/trunk/spec/java_spec.rb Mon Sep 22 10:17:03 2008
>> @@ -36,7 +36,7 @@
>>
>>     after do
>>       ENV['JAVA_HOME'] = @old_home
>> -      ENV_JAVA = @old_env_java
>> +      ENV_JAVA.replace @old_env_java
>>     end
>>   end
>>  end
>> @@ -49,6 +49,7 @@
>>
>>   describe 'when JAVA_HOME points to a JDK' do
>>     before do
>> +      Java.instance_eval { @tools_jar = nil }
>>       write 'jdk/lib/tools.jar'
>>       ENV['JAVA_HOME'] = File.expand_path('jdk')
>>     end
>> @@ -56,16 +57,11 @@
>>     it 'should return the path to tools.jar' do
>>       Java.tools_jar.should point_to_path('jdk/lib/tools.jar')
>>     end
>> -
>> -    it 'should accept a block and yield the path to tools.jar' do
>> -      tools_jar_received_by_block = nil
>> -      Java.tools_jar { |tools_jar| tools_jar_received_by_block = tools_jar }
>> -      tools_jar_received_by_block.should point_to_path('jdk/lib/tools.jar')
>> -    end
>>   end
>>
>>   describe 'when JAVA_HOME points to a JRE inside a JDK' do
>>     before do
>> +      Java.instance_eval { @tools_jar = nil }
>>       write 'jdk/lib/tools.jar'
>>       ENV['JAVA_HOME'] = File.expand_path('jdk/jre')
>>     end
>> @@ -73,27 +69,16 @@
>>     it 'should return the path to tools.jar' do
>>       Java.tools_jar.should point_to_path('jdk/lib/tools.jar')
>>     end
>> -
>> -    it 'should accept a block and yield the path to tools.jar' do
>> -      tools_jar_received_by_block = nil
>> -      Java.tools_jar { |tools_jar| tools_jar_received_by_block = tools_jar }
>> -      tools_jar_received_by_block.should point_to_path('jdk/lib/tools.jar')
>> -    end
>>   end
>>
>>   describe 'when there is no tools.jar' do
>>     before do
>> +      Java.instance_eval { @tools_jar = nil }
>>       ENV['JAVA_HOME'] = File.expand_path('jdk')
>>     end
>>
>>     it 'should return nil' do
>> -      Java.tools_jar.should be(nil)
>> -    end
>> -
>> -    it 'should accept a block and not yield to it' do
>> -      block_called = false
>> -      Java.tools_jar { block_called = true }
>> -      block_called.should be(false)
>> +      Java.tools_jar.should be_nil
>>     end
>>   end
>>
>>
>>
>>
>

Mime
View raw message