Juan Hernandez has posted comments on this change.
Change subject: packaging: Find and check JVM during setup (#834436)
......................................................................
Patch Set 3: (7 inline comments)
....................................................
File packaging/fedora/setup/engine-setup.py
Line 1872:
controller.MESSAGES.append(output_messages.ERR_FAILED_START_SERVICE %
"rhevm-notifierd")
Line 1873:
Line 1874: def checkJavaVersion(version):
Line 1875: # Check that the version is supported:
Line 1876: if not version.startswith(basedefs.JAVA_VERSION):
Agree. Should I do that in this change?
Line 1877: logging.debug("Java version \"%s\" is not supported, it
should start with \"%s\"." % (version, basedefs.JAVA_VERSION))
Line 1878: return False
Line 1879:
Line 1880: # If we are here it is an acceptable java version:
Line 1872:
controller.MESSAGES.append(output_messages.ERR_FAILED_START_SERVICE %
"rhevm-notifierd")
Line 1873:
Line 1874: def checkJavaVersion(version):
Line 1875: # Check that the version is supported:
Line 1876: if not version.startswith(basedefs.JAVA_VERSION):
We can change this when Java 1.8 is released and we test/update the rest of the
code to work with it.
Line 1877: logging.debug("Java version \"%s\" is not supported, it
should start with \"%s\"." % (version, basedefs.JAVA_VERSION))
Line 1878: return False
Line 1879:
Line 1880: # If we are here it is an acceptable java version:
Line 1893: logging.debug("The Java launcher \"%s\" isn't executable." %
javaLauncher)
Line 1894: return False
Line 1895:
Line 1896: # Invoke the Java launcher to check what is the version number:
Line 1897: javaProcess = subprocess.Popen([javaLauncher, "-version"],
stdout=subprocess.PIPE, stderr=subprocess.PIPE)
It should work. I will change it.
Line 1898: javaOut, javaErr = javaProcess.communicate()
Line 1899: javaExit = javaProcess.wait()
Line 1900: if javaExit != 0:
Line 1901: logging.debug("The Java launcher \"%s\" fails with code %d."
% (javaLauncher, javaExit))
Line 1913: logging.debug("The java version \"%s\" is not supported." %
javaVersion)
Line 1914: return False
Line 1915:
Line 1916: # Check that it is an OpenJDK:
Line 1917: match = re.search(r'^OpenJDK .*$', str(javaErr), re.MULTILINE)
Yes, we only develop and test using OpenJDK.
Line 1918: if not match:
Line 1919: logging.debug("The Java launcher \"%s\" is not OpenJDK." %
javaLauncher)
Line 1920: return False
Line 1921:
Line 1921:
Line 1922: # It passed all the checks, so it is valid JVM:
Line 1923: return True
Line 1924:
Line 1925: def checkJdk(jvmPath):
We don't need the java compiler. This just checks if the given directory
contains it. Later we use this check to prefer the JRE (no compiler) over the
JDK (compiler included).
Line 1926: # We assume that this JVM path has already been checked and that
it
Line 1927: # contains a valid JVM, so we only need to check that it also
Line 1928: # contains a Java compiler:
Line 1929: javaCompiler = os.path.join(jvmPath, "bin", "javac")
Line 1949: targetPath = targetName
Line 1950: else:
Line 1951: targetPath = os.path.join(javaDir, targetName)
Line 1952: if not os.path.islink(targetPath):
Line 1953: jvmLinks.append(filePath)
We only resolve the link in order to make sure that it doesn't point to another
link, after that we use the link itself. The reason for this check is that in
some distributions some links point to other links, in particular to
"alternatives" links, and we want to avoid them. For example, in Fedora:
/usr/lib/jvm/java-1.7.0 -> /etc/alternatives/java_sdk_1.7.0
Why do we want to avoid the "alternatives" symlinks? Because the user can
change them to some other unsupported JRE. We already had this problem with the
IBM J9 VM.
Line 1954:
Line 1955: # For each possible JVM path check that it really contain a JVM
and
Line 1956: # that the version is supported:
Line 1957: jvmLinks = [x for x in jvmLinks if checkJvm(x)]
Line 1964: jvmLinks = jreLinks
Line 1965:
Line 1966: # Sort the list by the lenght of the link in order to use the
Line 1967: # shorter one:
Line 1968: jvmLinks.sort(key=lambda jvmLink: len(jvmLink))
Nothing, really. Just wanted to sort the list to avoid having an unpredictable
result. The length is not probably the better order. Any suggestion?
Alphabetical maybe?
Line 1969:
Line 1970: # We need at least one value in the list:
Line 1971: if not jvmLinks:
Line 1972: logging.error("Can't find any supported JVM.")
--
To view, visit http://gerrit.ovirt.org/7549
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib279ea8111825ec35a25f9ba6ee37b481ed4354f
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Alex Lourie <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Moran Goldboim <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
