Alon Bar-Lev has posted comments on this change.
Change subject: packaging: Find and check JVM during setup (#834436)
......................................................................
Patch Set 3: (5 inline comments)
Good work!
I don't like this is done at setup time, but can live with this :)))
In future we will discuss the option to make this configurable at runtime or
executing some distro specific command if available.
Thanks!
....................................................
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):
And above? eg. 1.8.0?
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 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)
Are you sure we do not support sun/oracle jdk?
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):
Why do we need java compiler?
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)
I don't think there is any reason to resolve the link, why not use them as-is?
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))
I don't understand what length has to do with this.
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
