From 90e247199e2383f1b036bd8fbded82c2b03eae2f 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 330f037e..04ae9234 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 14fd78ca..792d494c 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 e7040e5f..4392bfe0 100644 --- a/lib/facter/java_version.rb +++ b/lib/facter/java_version.rb @@ -21,21 +21,7 @@ # 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 5c7f3fd8..6237a8b7 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -44,7 +44,7 @@ } } } - 'OpenBSD', 'FreeBSD', 'Suse': { + 'FreeBSD', 'Suse': { if $java::use_java_home != undef { file_line { 'java-home-environment': path => '/etc/environment', From 0111827f77745f2ae2fe42da89f8130c43af9f2d Mon Sep 17 00:00:00 2001 From: Sebastian Reitenbach Date: Tue, 26 Sep 2017 23:29:29 +0200 Subject: [PATCH 2/3] update metadata.json to include OpenBSD support --- metadata.json | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/metadata.json b/metadata.json index 79cf9dbb..667f08cf 100644 --- a/metadata.json +++ b/metadata.json @@ -77,6 +77,13 @@ "operatingsystemrelease": [ "11" ] + }, + { + "operatingsystem": "OpenBSD", + "operatingsystemrelease": [ + "6.1", + "6.2" + ] } ], "requirements": [ From f1353d482580408c0ded47a0ee7067e2f37afbfd Mon Sep 17 00:00:00 2001 From: Sebastian Reitenbach Date: Tue, 26 Sep 2017 23:35:15 +0200 Subject: [PATCH 3/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 04ae9234..cc47a4de 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 00a4e3f2..47bfbd0a 100644 --- a/spec/classes/java_spec.rb +++ b/spec/classes/java_spec.rb @@ -236,7 +236,7 @@ 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 41be89e4..a5fce079 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 02af6560..961bd2df 100644 --- a/spec/unit/facter/java_version_spec.rb +++ b/spec/unit/facter/java_version_spec.rb @@ -7,7 +7,7 @@ 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 @@ 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