Improvements to *JavaHome types

nebhale · nebhale · commit 1b6aec3c161a · 2014-01-06T13:08:54.000Z
Previously the ImmutableJavaHome.do_with() method used the $PWD
qualified version of JAVA_HOME.  This caused problem during some
executions, so both of the *JavaHome types have been changed such that
the fully qualified path is used in the do_with() method.

[#63086152]
diff --git a/lib/java_buildpack/buildpack.rb b/lib/java_buildpack/buildpack.rb
@@ -93,7 +93,7 @@ def release
 
     private
 
-    DEFAULT_BUILDPACK_MESSAGE = '       Java Buildpack source: system'.freeze
+    DEFAULT_BUILDPACK_MESSAGE = '-----> Java Buildpack source: system'.freeze
 
     GIT_DIR = Pathname.new(__FILE__).dirname + '../../.git'
 
@@ -105,8 +105,8 @@ def initialize(app_dir, application)
       log_environment_variables
 
       additional_libraries = Component::AdditionalLibraries.new app_dir
-      mutable_java_home    = Component::MutableJavaHome.new app_dir
-      immutable_java_home  = Component::ImmutableJavaHome.new mutable_java_home
+      mutable_java_home    = Component::MutableJavaHome.new
+      immutable_java_home  = Component::ImmutableJavaHome.new mutable_java_home, app_dir
       java_opts            = Component::JavaOpts.new app_dir
 
       components = JavaBuildpack::Util::ConfigurationUtils.load 'components'
@@ -127,7 +127,7 @@ def diagnose_git_info(print)
       if system("git --git-dir=#{GIT_DIR} status 2>/dev/null 1>/dev/null")
         remote_url = diagnose_remotes
         head_commit_sha = diagnose_head_commit
-        puts "       Java Buildpack source: #{remote_url}##{head_commit_sha}" if print
+        puts "-----> Java Buildpack source: #{remote_url}##{head_commit_sha}" if print
       else
         @logger.debug { DEFAULT_BUILDPACK_MESSAGE }
         puts DEFAULT_BUILDPACK_MESSAGE if print
diff --git a/lib/java_buildpack/component/additional_libraries.rb b/lib/java_buildpack/component/additional_libraries.rb
@@ -15,15 +15,15 @@
 # limitations under the License.
 
 require 'java_buildpack/component'
-require 'java_buildpack/component/qualify_path'
+require 'java_buildpack/util/qualify_path'
 
 module JavaBuildpack::Component
 
   # An abstraction around the additional libraries provided to a droplet by components.
   #
   # A new instance of this type should be created once for the application.
   class AdditionalLibraries < Array
-    include JavaBuildpack::Component
+    include JavaBuildpack::Util
 
     # Creates an instance of the +JAVA_OPTS+ abstraction.
     #
diff --git a/lib/java_buildpack/component/immutable_java_home.rb b/lib/java_buildpack/component/immutable_java_home.rb
@@ -15,6 +15,7 @@
 # limitations under the License.
 
 require 'java_buildpack/component'
+require 'java_buildpack/util/qualify_path'
 
 module JavaBuildpack::Component
 
@@ -23,15 +24,17 @@ module JavaBuildpack::Component
   #
   # A new instance of this type should be created once for the application.
   class ImmutableJavaHome
+    include JavaBuildpack::Util
 
     # Creates a new instance of the java home abstraction
     #
     # @param [MutableJavaHome] delegate the instance of +MutableJavaHome+ to use as a delegate for +root+ calls
-    def initialize(delegate)
+    def initialize(delegate, droplet_root)
       @delegate = delegate
+      @droplet_root = droplet_root
     end
 
-    # Returns the path of +JAVA_HOME+ as an environment variable formatted as +JAVA_HOME="$PWD/<value>"+
+    # Returns the path of +JAVA_HOME+ as an environment variable formatted as +JAVA_HOME=$PWD/<value>+
     #
     # @return [String] the path of +JAVA_HOME+ as an environment variable
     def as_env_var
@@ -44,16 +47,16 @@ def as_env_var
     def do_with
       previous_value = ENV['JAVA_HOME']
       begin
-        ENV['JAVA_HOME'] = root
+        ENV['JAVA_HOME'] = @delegate.root.cleanpath.to_s
         yield
       ensure
         ENV['JAVA_HOME'] = previous_value
       end
     end
 
-    # @return [String] the root of the droplet's +JAVA_HOME+
+    # @return [String] the root of the droplet's +JAVA_HOME+ formatted as +$PWD/<value>+
     def root
-      @delegate.root
+      qualify_path @delegate.root
     end
 
   end
diff --git a/lib/java_buildpack/component/java_opts.rb b/lib/java_buildpack/component/java_opts.rb
@@ -15,15 +15,15 @@
 # limitations under the License.
 
 require 'java_buildpack/component'
-require 'java_buildpack/component/qualify_path'
+require 'java_buildpack/util/qualify_path'
 
 module JavaBuildpack::Component
 
   # An abstraction encapsulating the +JAVA_OPTS+ of an application.
   #
   # A new instance of this type should be created once for the application.
   class JavaOpts < Array
-    include JavaBuildpack::Component
+    include JavaBuildpack::Util
 
     # Creates an instance of the +JAVA_OPTS+ abstraction.
     #
diff --git a/lib/java_buildpack/component/mutable_java_home.rb b/lib/java_buildpack/component/mutable_java_home.rb
@@ -15,35 +15,18 @@
 # limitations under the License.
 
 require 'java_buildpack/component'
-require 'java_buildpack/component/qualify_path'
-require 'java_buildpack/component/immutable_java_home'
 
 module JavaBuildpack::Component
 
-  # An abstraction around the +JAVA_HOME+ path used by the droplet.  This implementation is immutable and should be
-  # passed to any component that is not a jre.
+  # An abstraction around the +JAVA_HOME+ path used by the droplet.  This implementation is mutable and should be
+  # passed to any component that is a jre.
   #
   # A new instance of this type should be created once for the application.
-  class MutableJavaHome < ImmutableJavaHome
-    include JavaBuildpack::Component
+  class MutableJavaHome
 
-    # @!attribute [r] root
+    # @!attribute [rw] root
     #   @return [String] the root of the droplet's +JAVA_HOME+
-    attr_reader :root
-
-    # Creates a new instance of the java home abstraction
-    #
-    # @param [Pathname] droplet_root the root directory of the droplet
-    def initialize(droplet_root)
-      @droplet_root = droplet_root
-    end
-
-    # Sets the root of the droplet's +JAVA_HOME+
-    #
-    # @param [Pathname] value the root of the droplet's +JAVA_HOME+
-    def root=(value)
-      @root = qualify_path value
-    end
+    attr_accessor :root
 
   end
 
diff --git a/lib/java_buildpack/container/groovy.rb b/lib/java_buildpack/container/groovy.rb
@@ -18,6 +18,7 @@
 require 'java_buildpack/container'
 require 'java_buildpack/util/class_file_utils'
 require 'java_buildpack/util/groovy_utils'
+require 'java_buildpack/util/qualify_path'
 require 'pathname'
 require 'set'
 require 'tmpdir'
@@ -27,6 +28,7 @@ module JavaBuildpack::Container
   # Encapsulates the detect, compile, and release functionality for applications running non-compiled Groovy
   # applications.
   class Groovy < JavaBuildpack::Component::VersionedDependencyComponent
+    include JavaBuildpack::Util
 
     def initialize(context)
       super(context) { |candidate_version| candidate_version.check_size(3) }
@@ -40,7 +42,7 @@ def release
       [
           @droplet.java_home.as_env_var,
           @droplet.java_opts.as_env_var,
-          "$PWD/#{(@droplet.sandbox + 'bin/groovy').relative_path_from(@droplet.root)}",
+          qualify_path(@droplet.sandbox + 'bin/groovy', @droplet.root),
           @droplet.additional_libraries.as_classpath,
           relative_main_groovy,
           relative_other_groovy
diff --git a/lib/java_buildpack/container/spring_boot_cli.rb b/lib/java_buildpack/container/spring_boot_cli.rb
@@ -18,12 +18,14 @@
 require 'java_buildpack/component/versioned_dependency_component'
 require 'java_buildpack/container'
 require 'java_buildpack/util/groovy_utils'
+require 'java_buildpack/util/qualify_path'
 
 module JavaBuildpack::Container
 
   # Encapsulates the detect, compile, and release functionality for applications running Spring Boot CLI
   # applications.
   class SpringBootCLI < JavaBuildpack::Component::VersionedDependencyComponent
+    include JavaBuildpack::Util
 
     def compile
       download_tar
@@ -34,7 +36,7 @@ def release
       [
           @droplet.java_home.as_env_var,
           @droplet.java_opts.as_env_var,
-          "$PWD/#{(@droplet.sandbox + 'bin/spring').relative_path_from(@droplet.root)}",
+          qualify_path(@droplet.sandbox + 'bin/spring', @droplet.root),
           'run',
           '--local',
           relative_groovy_files,
diff --git a/lib/java_buildpack/util/play/base.rb b/lib/java_buildpack/util/play/base.rb
@@ -15,11 +15,13 @@
 # limitations under the License.
 
 require 'java_buildpack/util/play'
+require 'java_buildpack/util/qualify_path'
 
 module JavaBuildpack::Util::Play
 
   # Base class for Play application classes.
   class Base
+    include JavaBuildpack::Util
 
     def initialize(droplet)
       @droplet = droplet
@@ -47,7 +49,7 @@ def release
       [
           "PATH=#{@droplet.java_home.root}/bin:$PATH",
           @droplet.java_home.as_env_var,
-          "$PWD/#{start_script.relative_path_from(@droplet.root)}",
+          qualify_path(start_script, @droplet.root),
           java_opts
       ].compact.join(' ')
     end
diff --git a/lib/java_buildpack/util/qualify_path.rb b/lib/java_buildpack/util/qualify_path.rb
@@ -14,15 +14,18 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-require 'java_buildpack/component'
-require 'java_buildpack/component/immutable_java_home'
+require 'java_buildpack/util'
 
-module JavaBuildpack::Component
+module JavaBuildpack::Util
 
-  # Qualifies the path such that is is formatted as +$PWD/<path>+.  Also ensures that the path is relative to the
-  # +@droplet_root+ of the class.
-  def qualify_path(path)
-    "$PWD/#{path.relative_path_from(@droplet_root)}"
+  # Qualifies the path such that is is formatted as +$PWD/<path>+.  Also ensures that the path is relative to a root,
+  # which defaults to the +@droplet_root+ of the class.
+  #
+  # @param [Pathname] path the path to qualify
+  # @param [Pathname] root the root to make relative to
+  # @return [String] the qualified path
+  def qualify_path(path, root = @droplet_root)
+    "$PWD/#{path.relative_path_from(root)}"
   end
 
 end
diff --git a/lib/java_buildpack/util/shell.rb b/lib/java_buildpack/util/shell.rb
@@ -26,7 +26,7 @@ module JavaBuildpack::Util::Shell
   def shell(command)
     Open3.popen3(command) do |stdin, stdout, stderr, wait_thr|
       if wait_thr.value != 0
-        puts "Command '#{command}' has failed"
+        puts "\nCommand '#{command}' has failed"
         puts "STDOUT: #{stdout.gets}"
         puts "STDERR: #{stderr.gets}"
 
diff --git a/spec/droplet_helper.rb b/spec/droplet_helper.rb
@@ -20,7 +20,7 @@
 require 'java_buildpack/component/additional_libraries'
 require 'java_buildpack/component/droplet'
 require 'java_buildpack/component/java_opts'
-require 'java_buildpack/component/mutable_java_home'
+require 'java_buildpack/component/immutable_java_home'
 require 'java_buildpack/util/snake_case'
 require 'pathname'
 
@@ -41,9 +41,8 @@
   let(:sandbox) { droplet.sandbox }
 
   let(:java_home) do
-    java_home      = JavaBuildpack::Component::MutableJavaHome.new app_dir
-    java_home.root = app_dir + '.test-java-home'
-    java_home
+    JavaBuildpack::Component::ImmutableJavaHome.new double('MutableJavaHome', root: app_dir + '.test-java-home'),
+                                                    app_dir
   end
 
   let(:java_opts) do
diff --git a/spec/integration_helper.rb b/spec/integration_helper.rb
@@ -17,11 +17,13 @@
 require 'spec_helper'
 require 'application_helper'
 require 'console_helper'
+require 'logging_helper'
 require 'open3'
 
 shared_context 'integration_helper' do
   include_context 'application_helper'
   include_context 'console_helper'
+  include_context 'logging_helper'
 
   def run(command)
     Open3.popen3(command) do |stdin, stdout, stderr, wait_thr|
diff --git a/spec/java_buildpack/component/immutable_java_home_spec.rb b/spec/java_buildpack/component/immutable_java_home_spec.rb
@@ -19,12 +19,12 @@
 
 describe JavaBuildpack::Component::ImmutableJavaHome do
 
-  let(:delegate) { double('delegate', root: 'test-java-home') }
+  let(:delegate) { double('delegate', root: Pathname.new('test-java-home')) }
 
-  let(:immutable_java_home) { described_class.new delegate }
+  let(:immutable_java_home) { described_class.new delegate, Pathname.new('.') }
 
   it 'should return the JAVA_HOME as an environment variable' do
-    expect(immutable_java_home.as_env_var).to eq('JAVA_HOME=test-java-home')
+    expect(immutable_java_home.as_env_var).to eq('JAVA_HOME=$PWD/test-java-home')
   end
 
   it 'should set JAVA_HOME environment variable' do
@@ -33,8 +33,8 @@
     end
   end
 
-  it 'should return the delegate root' do
-    expect(immutable_java_home.root).to eq('test-java-home')
+  it 'should return the qualified delegate root' do
+    expect(immutable_java_home.root).to eq('$PWD/test-java-home')
   end
 
 end
diff --git a/spec/java_buildpack/component/mutable_java_home_spec.rb b/spec/java_buildpack/component/mutable_java_home_spec.rb
@@ -15,17 +15,18 @@
 # limitations under the License.
 
 require 'spec_helper'
-require 'droplet_helper'
 require 'java_buildpack/component/mutable_java_home'
+require 'pathname'
 
 describe JavaBuildpack::Component::MutableJavaHome do
-  include_context 'droplet_helper'
 
-  let(:mutable_java_home) { described_class.new droplet.root }
+  let(:path) { Pathname.new('foo/bar') }
 
-  it 'should qualify root' do
-    mutable_java_home.root = droplet.root + 'foo/bar'
-    expect(mutable_java_home.root).to eq('$PWD/foo/bar')
+  let(:mutable_java_home) { described_class.new }
+
+  it 'should save root' do
+    mutable_java_home.root = path
+    expect(mutable_java_home.root).to eq(path)
   end
 
 end
diff --git a/spec/java_buildpack/jre/open_jdk_spec.rb b/spec/java_buildpack/jre/open_jdk_spec.rb
@@ -16,12 +16,15 @@
 
 require 'spec_helper'
 require 'component_helper'
+require 'java_buildpack/component/mutable_java_home'
 require 'java_buildpack/jre/open_jdk'
 require 'java_buildpack/jre/memory/weight_balancing_memory_heuristic'
 
 describe JavaBuildpack::Jre::OpenJDK do
   include_context 'component_helper'
 
+  let(:java_home) { JavaBuildpack::Component::MutableJavaHome.new }
+
   let(:memory_heuristic) { double('MemoryHeuristic', resolve: %w(opt-1 opt-2)) }
 
   before do
@@ -43,7 +46,7 @@
   it 'adds the JAVA_HOME to java_home' do
     component
 
-    expect(java_home.root).to eq('$PWD/.java-buildpack/open_jdk')
+    expect(java_home.root).to eq(sandbox)
   end
 
   it 'should add memory options to java_opts' do

Original file line number	Diff line number	Diff line change
`@@ -15,15 +15,15 @@`
`15`	`15`	`# limitations under the License.`
`16`	`16`
`17`	`17`	`require 'java_buildpack/component'`
`18`		`-require 'java_buildpack/component/qualify_path'`
	`18`	`+require 'java_buildpack/util/qualify_path'`
`19`	`19`
`20`	`20`	`module JavaBuildpack::Component`
`21`	`21`
`22`	`22`	`# An abstraction around the additional libraries provided to a droplet by components.`
`23`	`23`	`#`
`24`	`24`	`# A new instance of this type should be created once for the application.`
`25`	`25`	`class AdditionalLibraries < Array`
`26`		`- include JavaBuildpack::Component`
	`26`	`+ include JavaBuildpack::Util`
`27`	`27`
`28`	`28`	`# Creates an instance of the +JAVA_OPTS+ abstraction.`
`29`	`29`	`#`