From bdfc567c1872f621acebf94057016c8fb49b8e59 Mon Sep 17 00:00:00 2001 From: Sebastian Reitenbach Date: Mon, 25 Sep 2017 12:05:00 +0200 Subject: [PATCH 1/3] OpenBSD doesn't have /etc/environment, therefore there is no need to fiddle with it. The default case statement, "do nothing" is all fine here. Facter::Util::Resolution.with_env is long time gone since Facter > 2.x. That was even longer before introduced by me. Remove that OpenBSD special case in the java_version fact, and assume that $PATH is properly set in order to find the 'java' binary. Additionally confine the java_default_home and java_libjvm_path to OpenBSD kernel as well. Add some flesh to the java_default_home fact to allow it to find the java binary. --- lib/facter/java_default_home.rb | 15 +++++++++++---- lib/facter/java_libjvm_path.rb | 2 +- lib/facter/java_version.rb | 16 +--------------- manifests/config.pp | 2 +- 4 files changed, 14 insertions(+), 21 deletions(-) diff --git a/lib/facter/java_default_home.rb b/lib/facter/java_default_home.rb index 330f037..04ae923 100644 --- a/lib/facter/java_default_home.rb +++ b/lib/facter/java_default_home.rb @@ -11,13 +11,20 @@ # Notes: # None Facter.add(:java_default_home) do - confine :kernel => 'Linux' + confine :kernel => [ 'Linux', 'OpenBSD' ] setcode do if Facter::Util::Resolution.which('readlink') - java_bin = Facter::Util::Resolution.exec('readlink -e /usr/bin/java').strip + java_bin = Facter::Util::Resolution.exec('readlink -e /usr/bin/java').to_s.strip if java_bin.empty? - nil - elsif java_bin =~ %r(/jre/) + java_bin = Facter::Util::Resolution.exec('readlink -e /usr/local/bin/java').to_s.strip + if java_bin.empty? + java_bin = Facter::Util::Resolution.which('java').to_s.strip + if java_bin.empty? + nil + end + end + end + if java_bin =~ %r(/jre/) java_default_home = File.dirname(File.dirname(File.dirname(java_bin))) else java_default_home = File.dirname(File.dirname(java_bin)) diff --git a/lib/facter/java_libjvm_path.rb b/lib/facter/java_libjvm_path.rb index 14fd78c..792d494 100644 --- a/lib/facter/java_libjvm_path.rb +++ b/lib/facter/java_libjvm_path.rb @@ -11,7 +11,7 @@ # Notes: # None Facter.add(:java_libjvm_path) do - confine :kernel => "Linux" + confine :kernel => [ "Linux", "OpenBSD" ] setcode do java_default_home = Facter.value(:java_default_home) java_libjvm_file = Dir.glob("#{java_default_home}/jre/lib/**/libjvm.so") diff --git a/lib/facter/java_version.rb b/lib/facter/java_version.rb index e7040e5..4392bfe 100644 --- a/lib/facter/java_version.rb +++ b/lib/facter/java_version.rb @@ -21,21 +21,7 @@ Facter.add(:java_version) do # Additionally, facter versions prior to 2.0.1 only support # positive matches, so this needs to be done manually in setcode. setcode do - unless [ 'openbsd', 'darwin' ].include? Facter.value(:operatingsystem).downcase - version = nil - if Facter::Util::Resolution.which('java') - Facter::Util::Resolution.exec('java -Xmx12m -version 2>&1').lines.each { |line| version = $~[1] if /^.+ version \"(.+)\"$/ =~ line } - end - version - end - end -end - -Facter.add(:java_version) do - confine :operatingsystem => 'OpenBSD' - has_weight 100 - setcode do - Facter::Util::Resolution.with_env("PATH" => '/usr/local/jdk-1.7.0/jre/bin:/usr/local/jre-1.7.0/bin') do + unless [ 'darwin' ].include? Facter.value(:operatingsystem).downcase version = nil if Facter::Util::Resolution.which('java') Facter::Util::Resolution.exec('java -Xmx12m -version 2>&1').lines.each { |line| version = $~[1] if /^.+ version \"(.+)\"$/ =~ line } diff --git a/manifests/config.pp b/manifests/config.pp index 5c7f3fd..6237a8b 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -44,7 +44,7 @@ class java::config ( ) { } } } - 'OpenBSD', 'FreeBSD', 'Suse': { + 'FreeBSD', 'Suse': { if $java::use_java_home != undef { file_line { 'java-home-environment': path => '/etc/environment', From e50eb646143f674ac97adcdf88ed38214b9d114f Mon Sep 17 00:00:00 2001 From: Sebastian Reitenbach Date: Tue, 26 Sep 2017 23:35:15 +0200 Subject: [PATCH 2/3] do not use unportable readlink utility to find the java_default_home, use Ruby in a more portable fashion. adapt tests as well --- lib/facter/java_default_home.rb | 21 ++++----- spec/classes/java_spec.rb | 2 +- spec/unit/facter/java_default_home_spec.rb | 54 ++++++++++------------ spec/unit/facter/java_version_spec.rb | 4 +- 4 files changed, 36 insertions(+), 45 deletions(-) diff --git a/lib/facter/java_default_home.rb b/lib/facter/java_default_home.rb index 04ae923..cc47a4d 100644 --- a/lib/facter/java_default_home.rb +++ b/lib/facter/java_default_home.rb @@ -3,27 +3,22 @@ # Purpose: get absolute path of java system home # # Resolution: -# Uses `readlink` to resolve the path of `/usr/bin/java` then returns subsubdir +# Find the real java binary, and return the subsubdir # # Caveats: -# Requires readlink +# java binary has to be found in $PATH # # Notes: # None Facter.add(:java_default_home) do confine :kernel => [ 'Linux', 'OpenBSD' ] setcode do - if Facter::Util::Resolution.which('readlink') - java_bin = Facter::Util::Resolution.exec('readlink -e /usr/bin/java').to_s.strip - if java_bin.empty? - java_bin = Facter::Util::Resolution.exec('readlink -e /usr/local/bin/java').to_s.strip - if java_bin.empty? - java_bin = Facter::Util::Resolution.which('java').to_s.strip - if java_bin.empty? - nil - end - end - end + java_bin = Facter::Util::Resolution.which('java').to_s.strip + if java_bin.empty? + nil + else + # We might have found a symlink instead of the real binary + java_bin = File.realpath(java_bin) if java_bin =~ %r(/jre/) java_default_home = File.dirname(File.dirname(File.dirname(java_bin))) else diff --git a/spec/classes/java_spec.rb b/spec/classes/java_spec.rb index 00a4e3f..47bfbd0 100644 --- a/spec/classes/java_spec.rb +++ b/spec/classes/java_spec.rb @@ -236,7 +236,7 @@ describe 'java', :type => :class do context 'select jdk for OpenBSD' do let(:facts) { {:osfamily => 'OpenBSD', :architecture => 'x86_64'} } it { is_expected.to contain_package('java').with_name('jdk') } - it { is_expected.to contain_file_line('java-home-environment').with_line('JAVA_HOME=/usr/local/jdk/') } + it { is_expected.to_not contain_file_line('java-home-environment') } end context 'select jre for OpenBSD' do diff --git a/spec/unit/facter/java_default_home_spec.rb b/spec/unit/facter/java_default_home_spec.rb index 41be89e..a5fce07 100644 --- a/spec/unit/facter/java_default_home_spec.rb +++ b/spec/unit/facter/java_default_home_spec.rb @@ -1,47 +1,43 @@ require "spec_helper" describe Facter::Util::Fact do - before { - Facter.clear - Facter.fact(:kernel).stubs(:value).returns('Linux') - } describe "java_default_home" do - context 'returns java home path when readlink present' do - context 'when java is in HOME/jre/bin/java' do + before(:each) { + Facter.clear + Facter.fact(:kernel).stubs(:value).returns('Linux') + } + + context 'returns java home path when java found in PATH' do + context "when java is in /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java" do + it do - java_path_output = <<-EOS -/usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java - EOS - Facter::Util::Resolution.expects(:which).with("readlink").returns(true) - Facter::Util::Resolution.expects(:exec).with("readlink -e /usr/bin/java").returns(java_path_output) - expect(Facter.value(:java_default_home)).to eql "/usr/lib/jvm/java-7-openjdk-amd64" + File.delete('./java') if File.exist?('./java') + File.symlink('/usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java', './java') + Facter::Util::Resolution.expects(:which).with("java").returns("./java") + expect(File.readlink('./java')).to eq('/usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java') + expect(Facter.value(:java_default_home)).to eql '/usr/lib/jvm/java-7-openjdk-amd64' + File.delete('./java') if File.exist?('./java') end end - context 'when java is in HOME/bin/java' do + + context "when java is in /usr/lib/jvm/oracle-java8-jre-amd64/bin/java" do + it do - java_path_output = <<-EOS -/usr/lib/jvm/oracle-java8-jre-amd64/bin/java - EOS - Facter::Util::Resolution.expects(:which).with("readlink").returns(true) - Facter::Util::Resolution.expects(:exec).with("readlink -e /usr/bin/java").returns(java_path_output) - expect(Facter.value(:java_default_home)).to eql "/usr/lib/jvm/oracle-java8-jre-amd64" + File.delete('./java') if File.exist?('./java') + File.symlink('/usr/lib/jvm/oracle-java8-jre-amd64/bin/java', './java') + Facter::Util::Resolution.expects(:which).with("java").returns("./java") + expect(File.readlink('./java')).to eq('/usr/lib/jvm/oracle-java8-jre-amd64/bin/java') + expect(Facter.value(:java_default_home)).to eql '/usr/lib/jvm/oracle-java8-jre-amd64' + File.delete('./java') if File.exist?('./java') end end end - context 'returns nil when readlink is present but java is not' do - it do - java_path_output = "" - Facter::Util::Resolution.expects(:which).with("readlink").returns(true) - Facter::Util::Resolution.expects(:exec).with("readlink -e /usr/bin/java").returns(java_path_output) - expect(Facter.value(:java_default_home)).to be_nil - end - end - context 'returns nil when readlink not present' do + context 'returns nil when java not present' do it do Facter::Util::Resolution.stubs(:exec) - Facter::Util::Resolution.expects(:which).with("readlink").at_least(1).returns(false) + Facter::Util::Resolution.expects(:which).with("java").at_least(1).returns(false) expect(Facter.value(:java_default_home)).to be_nil end end diff --git a/spec/unit/facter/java_version_spec.rb b/spec/unit/facter/java_version_spec.rb index 02af656..961bd2d 100644 --- a/spec/unit/facter/java_version_spec.rb +++ b/spec/unit/facter/java_version_spec.rb @@ -7,7 +7,7 @@ describe Facter::Util::Fact do describe "java_version" do context 'returns java version when java present' do - context 'on OpenBSD', :with_env => true do + context 'on OpenBSD' do before do Facter.fact(:operatingsystem).stubs(:value).returns("OpenBSD") end @@ -61,7 +61,7 @@ Java HotSpot(TM) 64-Bit Server VM (build 24.71-b01, mixed mode) end context 'returns nil when java not present' do - context 'on OpenBSD', :with_env => true do + context 'on OpenBSD' do before do Facter.fact(:operatingsystem).stubs(:value).returns("OpenBSD") end From e1df6498c799e1fef77a0cf0850028eb7269527a Mon Sep 17 00:00:00 2001 From: Will Meek Date: Wed, 15 Nov 2017 12:14:26 +0000 Subject: [PATCH 3/3] Fix facter unit test when using realpath --- lib/facter/java_default_home.rb | 9 +++---- spec/unit/facter/java_default_home_spec.rb | 31 +++++++++++++--------- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/lib/facter/java_default_home.rb b/lib/facter/java_default_home.rb index cc47a4d..c6b653d 100644 --- a/lib/facter/java_default_home.rb +++ b/lib/facter/java_default_home.rb @@ -17,12 +17,11 @@ Facter.add(:java_default_home) do if java_bin.empty? nil else - # We might have found a symlink instead of the real binary - java_bin = File.realpath(java_bin) - if java_bin =~ %r(/jre/) - java_default_home = File.dirname(File.dirname(File.dirname(java_bin))) + java_path = File.realpath(java_bin) + if java_path =~ %r(/jre/) + java_default_home = File.dirname(File.dirname(File.dirname(java_path))) else - java_default_home = File.dirname(File.dirname(java_bin)) + java_default_home = File.dirname(File.dirname(java_path)) end end end diff --git a/spec/unit/facter/java_default_home_spec.rb b/spec/unit/facter/java_default_home_spec.rb index a5fce07..a111fef 100644 --- a/spec/unit/facter/java_default_home_spec.rb +++ b/spec/unit/facter/java_default_home_spec.rb @@ -1,5 +1,14 @@ require "spec_helper" +def unlink_and_delete(filename) + if File.symlink?(filename) + File.unlink(filename) + end + if File.exist?(filename) + File.delete(filename) + end +end + describe Facter::Util::Fact do describe "java_default_home" do @@ -10,26 +19,24 @@ describe Facter::Util::Fact do context 'returns java home path when java found in PATH' do context "when java is in /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java" do - it do - File.delete('./java') if File.exist?('./java') - File.symlink('/usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java', './java') - Facter::Util::Resolution.expects(:which).with("java").returns("./java") - expect(File.readlink('./java')).to eq('/usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java') + unlink_and_delete('./java_test') + File.symlink('/usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java', './java_test') + Facter::Util::Resolution.expects(:which).with("java").returns("./java_test") + File.expects(:realpath).with('./java_test').returns('/usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java') expect(Facter.value(:java_default_home)).to eql '/usr/lib/jvm/java-7-openjdk-amd64' - File.delete('./java') if File.exist?('./java') + unlink_and_delete('./java_test') end end context "when java is in /usr/lib/jvm/oracle-java8-jre-amd64/bin/java" do - it do - File.delete('./java') if File.exist?('./java') - File.symlink('/usr/lib/jvm/oracle-java8-jre-amd64/bin/java', './java') - Facter::Util::Resolution.expects(:which).with("java").returns("./java") - expect(File.readlink('./java')).to eq('/usr/lib/jvm/oracle-java8-jre-amd64/bin/java') + unlink_and_delete('./java_test') + File.symlink('/usr/lib/jvm/oracle-java8-jre-amd64/bin/java', './java_test') + Facter::Util::Resolution.expects(:which).with("java").returns("./java_test") + File.expects(:realpath).with('./java_test').returns('/usr/lib/jvm/oracle-java8-jre-amd64/bin/java') expect(Facter.value(:java_default_home)).to eql '/usr/lib/jvm/oracle-java8-jre-amd64' - File.delete('./java') if File.exist?('./java') + unlink_and_delete('./java_test') end end end