On Tue, Sep 23, 2008 at 1:52 PM, lacton <[EMAIL PROTECTED]> 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, <[EMAIL PROTECTED]> 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 >> >> >> >> >
