add two facts: libjvm and java executable paths #117

faxm0dem · 2015-05-05T13:23:49Z

It's complicated to automatically determine the location of libjvm.so. Neither openjdk nor oracle packages provide hints for ld.so.conf.d. The path to the java executable is just a byproduct than I thought could be useful.

riton · 2015-05-05T13:36:43Z

👍 for the libjvm.so path.

elyscape · 2015-05-05T22:05:17Z

lib/facter/java_libjvm.rb

+      java_path = Facter.value(:java_path)
+      java_package = Facter::Util::Resolution.exec('dpkg -S '+java_path).split(':').first
+      #Facter::Util::Resolution.exec('dpkg -L '+java_package+' | grep libjvm.so\$').lines.first.strip
+      Facter::Util::Resolution.exec('dpkg -L '+java_package).lines.find_all { |l| l =~ /libjvm.so$/}.first.strip


Instead of doing find_all.first, why not just do find?

faxm0dem · 2015-05-06T09:00:09Z

Thanks for these constructive comments. I'll integrate them ASAP but currently I'm fighting with rspec: I just realized I was only testing the debian part in the test for libjvm fact, and now I'm trying to find a way to test both osfamilies. If you have any idea on how to test that without the confine :osfamily getting in the way, I'd be very grateful.

Edit: I just found this which should help
Edit2: This link was irrelevant. We can simply use before blocks here

faxm0dem · 2015-05-06T14:49:13Z

It would maybe be more useful to have the dirname() of libjvm.so so that it could be used directly for LD_LIBRARY_PATH what do you think?

riton · 2015-05-06T14:50:49Z

I agree that the dirname() would be more useful for later use in ld.so.conf.d mechanisms (even if puppetlabs-stdlib already provides a dirname function)

faxm0dem · 2015-05-19T09:19:45Z

I decided to go with the java_libjvm_path fact.
Can @elyscape or anybody else kindly review this?
I'd be happy to rebase this to a single commit if needed

faxm0dem · 2015-07-16T10:38:39Z

Hi is there anything else I can do to have this merged?

hunner · 2015-11-05T19:00:39Z

Hi! Could you explain why the path to libjvm.so is useful? Is this something that is generally needed, or is there some special case that this is for? Also is it possible to use some generic method like ldd instead of rpm and dpkg?

faxm0dem · 2015-11-06T09:33:26Z

Sure thing!
The main use-case for this is to add the path to ld.so.conf so that the dynamic linker will find it.
We're using it for instance for syslog-ng which has a java destination since 3.7.1.

So why do we need to do this?

no java distribution that I know of puts libjvm.so in the right place
no java distribution I know of provides a /etc/ld.so.conf.d/java.conf

Unfortunately, I have no better way to find its location apart from using the local packager or find.
Ideas welcome!

igalic · 2015-11-07T09:32:07Z

lib/facter/java_path.rb

+Facter.add(:java_path) do
+  setcode do
+    if Facter::Util::Resolution.which('readlink')
+      Facter::Util::Resolution.exec('readlink -e /usr/bin/java').strip


this will not work on Darwin, or FreeBSD or OpenBSD

Patches welcome!
But you're right the code should probably be confined

... or Windows. Please just add the confine to :kernel => "Linux" if you do not want to support other platforms (which is totally fine with me!)

faxm0dem · 2015-11-27T12:59:41Z

I confined the java_path fact to linux and fixed the unit tests.
I'd be delighted if any rspec-guru could review the latter, especially 7510f4c

Cheers

DavidS · 2015-11-27T13:41:46Z

lib/facter/java_libjvm_path.rb

+    if java_path.empty?
+      nil
+    elsif Facter::Util::Resolution.which('dpkg')
+      java_package = Facter::Util::Resolution.exec('dpkg -S '+java_path).split(':').first


You missed one string concatenation here.

faxm0dem · 2015-11-27T14:43:56Z

thanks!

DavidS · 2015-11-27T14:47:15Z

The tests look OK to me. There is still a minor comment outstanding on the coding style.

On a higher level, I'm wondering whether there is a reason not to use Dir.glob("#{basedir}/lib/**/libjvm.so") to find the libjvm.so (with basedir being the parent of dirname(readlink -f /usr/bin/java))? That should be much quicker than having to hit the package database.

DavidS · 2015-11-27T14:48:15Z

On a more organizational note, pleas add a short description of the facts to the README (there's already some described there) and squash everything into a single commit, to keep the history clean!

faxm0dem · 2015-11-27T16:54:18Z

I was unaware of the existence of ** and have to admit I wanted to avoid to implement this myself.
That light being shed on my knowledge, I will do some research on the performance implications of pkg vs. recursive glob

faxm0dem · 2015-11-30T12:17:37Z

I added the implementation proposed by @DavidS as default resolution, so I can keep the rpm/deb based implementation in case the former is failing.
I now have to find a way to integrate this into the unit tests, and as of now have no idea on how to do that.
Anybody have an idea on how to rspec facts with multiple resolutions?

DavidS · 2015-11-30T13:23:42Z

In what case would you expect the rpm/dpkg based variant to succeed when the globbing one doesn't?

I've also read a bit through the java docs, and it seems that moving the dirname(dirname(bin)) into java_path and calling it java_home would match people's expectations better? e.g. http://gedankenverlust.blogspot.de/2012/05/java-environment-variables-definitive.html :

User variable JAVA_HOME: de-facto convention used by some Java programs, especially Tomcat, to point to the JDK base folder.

Thank you for your patience and support in getting the best possible solution here!

faxm0dem · 2015-11-30T14:15:06Z

here's what I found googling:

JAVA_HOME: must point to installation directory of JDK.

JRE_HOME: must point to installation directory of JRE.

CLASSPATH: contains libraries path which JVM will look for.

The java executable being provided by jre, JRE_HOME would IMHO be more suited instead of JAVA_HOME for locating the java binary. That being said, having both variables available would probably be useful.

Problem is, I'm no java expert, so maybe we should ask one :)

DavidS · 2015-11-30T14:44:52Z

So I talked to two of our java devs and the base directory we're talking about is the java "home". Given that we're talking about the system default and to avoid any misunderstandings I'd recommend calling the fact "java_default_home" or "java_system_home" and return the dirname(dirname(readlink(usr/bin/java))). That way it is useful on its own.

faxm0dem · 2015-11-30T14:56:31Z

is this what you'd like?

Example

java_default_home => '/usr/lib/jvm/java-8-openjdk-amd64/jre'

DavidS · 2015-11-30T14:57:53Z

java_default_home => '/usr/lib/jvm/java-8-openjdk-amd64'

All the other directories below it (/lib, /bin, /jre/lib and /jre/bin) are the same across java versions.

faxm0dem · 2015-11-30T15:19:30Z

I wouldn't use home then as ppl would mistake it for java_home which is home of the JDK, not its parent

DavidS · 2015-11-30T15:27:00Z

the jre_home is usually java_home/jre

faxm0dem · 2015-11-30T17:35:27Z

I'll go for this

java_default_home => '/usr/lib/jvm/java-8-openjdk-amd64'

as for the multiple resolutions, I think it's always a good thing to have a fallback method, especially now that it's already there and tested. If you prefer to keep things simple though, I'll remove the deb/rpm based ones.

faxm0dem · 2015-12-01T10:00:54Z

@DavidS please advise on the last open quesion: do you want to keep the alternate package resolutions of libjvm.so or do you prefer to drop them?

DavidS · 2015-12-01T10:55:19Z

@faxm0dem Personally I'd prefer to leave the code out as long as there is not a immediate use-case for it.

* dirname containing base directory of java * e.g. java binary is `${::java_default_home}/jre/bin/java` * dirname containing `libjvm.so` * Most people will use this for LD_LIBRARY_PATH

faxm0dem · 2015-12-01T12:07:09Z

@DavidS all right, please review

add two facts: libjvm and java executable paths

DavidS · 2015-12-01T13:19:29Z

Thank you for your work!

elyscape reviewed May 5, 2015
View reviewed changes

tphoney added needs-docs needs-squash labels Nov 5, 2015

igalic reviewed Nov 7, 2015
View reviewed changes

faxm0dem force-pushed the f/libjvm_fact branch 2 times, most recently from 8562fd9 to 7510f4c Compare November 27, 2015 12:27

DavidS reviewed Nov 27, 2015
View reviewed changes

DavidS added needs-work and removed needs-docs labels Nov 30, 2015

add two facts: java_libjvm_path java_default_home

9c15489

* dirname containing base directory of java * e.g. java binary is `${::java_default_home}/jre/bin/java` * dirname containing `libjvm.so` * Most people will use this for LD_LIBRARY_PATH

faxm0dem force-pushed the f/libjvm_fact branch from dd97178 to 9c15489 Compare December 1, 2015 11:13

DavidS added a commit that referenced this pull request Dec 1, 2015

Merge pull request #117 from ccin2p3/f/libjvm_fact

4764484

add two facts: libjvm and java executable paths

DavidS merged commit 4764484 into puppetlabs:master Dec 1, 2015

DavidS removed needs-squash labels Dec 1, 2015

jordanbreen28 added the feature label Jun 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add two facts: libjvm and java executable paths #117

add two facts: libjvm and java executable paths #117

faxm0dem commented May 5, 2015

riton commented May 5, 2015

elyscape May 5, 2015

faxm0dem May 6, 2015

faxm0dem commented May 6, 2015

faxm0dem commented May 6, 2015

riton commented May 6, 2015

faxm0dem commented May 19, 2015

faxm0dem commented Jul 16, 2015

hunner commented Nov 5, 2015

faxm0dem commented Nov 6, 2015

igalic Nov 7, 2015

faxm0dem Nov 7, 2015

DavidS Nov 12, 2015

faxm0dem commented Nov 27, 2015

DavidS Nov 27, 2015

faxm0dem commented Nov 27, 2015

DavidS commented Nov 27, 2015

DavidS commented Nov 27, 2015

faxm0dem commented Nov 27, 2015

faxm0dem commented Nov 30, 2015

DavidS commented Nov 30, 2015

faxm0dem commented Nov 30, 2015

DavidS commented Nov 30, 2015

faxm0dem commented Nov 30, 2015

DavidS commented Nov 30, 2015

faxm0dem commented Nov 30, 2015

DavidS commented Nov 30, 2015

faxm0dem commented Nov 30, 2015

faxm0dem commented Dec 1, 2015

DavidS commented Dec 1, 2015

faxm0dem commented Dec 1, 2015

DavidS commented Dec 1, 2015

add two facts: libjvm and java executable paths #117

add two facts: libjvm and java executable paths #117

Conversation

faxm0dem commented May 5, 2015

riton commented May 5, 2015

elyscape May 5, 2015

Choose a reason for hiding this comment

faxm0dem May 6, 2015

Choose a reason for hiding this comment

faxm0dem commented May 6, 2015

faxm0dem commented May 6, 2015

riton commented May 6, 2015

faxm0dem commented May 19, 2015

faxm0dem commented Jul 16, 2015

hunner commented Nov 5, 2015

faxm0dem commented Nov 6, 2015

igalic Nov 7, 2015

Choose a reason for hiding this comment

faxm0dem Nov 7, 2015

Choose a reason for hiding this comment

DavidS Nov 12, 2015

Choose a reason for hiding this comment

faxm0dem commented Nov 27, 2015

DavidS Nov 27, 2015

Choose a reason for hiding this comment

faxm0dem commented Nov 27, 2015

DavidS commented Nov 27, 2015

DavidS commented Nov 27, 2015

faxm0dem commented Nov 27, 2015

faxm0dem commented Nov 30, 2015

DavidS commented Nov 30, 2015

faxm0dem commented Nov 30, 2015

DavidS commented Nov 30, 2015

faxm0dem commented Nov 30, 2015

Example

DavidS commented Nov 30, 2015

faxm0dem commented Nov 30, 2015

DavidS commented Nov 30, 2015

faxm0dem commented Nov 30, 2015

faxm0dem commented Dec 1, 2015

DavidS commented Dec 1, 2015

faxm0dem commented Dec 1, 2015

DavidS commented Dec 1, 2015