rrwick / Unicycler

hybrid assembly pipeline for bacterial genomes
GNU General Public License v3.0
547 stars 131 forks source link

Incorrect parsing of java 1.8 version in 0.3.1 #17

Closed andersgs closed 7 years ago

andersgs commented 7 years ago

Hi Ryan.

When running java -version on our system we get the following:

openjdk version "1.8.0_131"
OpenJDK Runtime Environment (build 1.8.0_131-b11)
OpenJDK 64-Bit Server VM (build 25.131-b11, mixed mode)

In your current code, it parses out the version to be openjdk, leading unicycler to fail the java dependency with a ? and the comment too old.

Perhaps some regex would help here:

import re
# some subprocess code here to run java -version and capture stdout to a 
# variable called java_version
java_pat = re.compile(r'([01])\.([1-8])\.([0-9])_([0-9]{0,3})')
obs_java_version = java_pat.findall(java_version.decode())
# On our system, you get two hits here:
# [('1', '8', '0', '131'), ('1', '8', '0', '131')]
# Some further processing of the output from java -version to pick out just
# the first line before applyting the regex might work best. But, each tuple 
# already has your major and sub-versions already parsed out.

Cheers.

A.

andersgs commented 7 years ago

I have monkey patched my version with the following code:

def java_path_and_version(java_path):
    java_path = shutil.which(java_path)
    if java_path is None:
        return '', '', 'not found'
    java_pat = re.compile(r'([01])\.([1-8])\.([0-9])_([0-9]{0,3})')
    command = [java_path, '-version']
    process = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
    out, _ = process.communicate()
    version = java_pat.findall(out.decode())[0]

    major_version,minor_version,sub_version,fine_version = version

    # Make sure Java is 1.7+
    try:
        major_version = int(major_version)
        if major_version < 1:
            status = 'too old'
        else:
            minor_version = int(minor_version)
            if minor_version < 7:
                status = 'too old'
            else:
                status = 'good'
    except (ValueError, IndexError):
        version, status = '?', 'too old'

    return java_path, "{major}.{minor}".format(major = major_version,minor = minor_version), status

It works, but there is probably a more elegant way of doing this...

rrwick commented 7 years ago

Thanks, this should now work the current version (v0.4.0). I just added a pretty simplistic fix: checking for "openjdk" as well as "java".

I'll assume it works and close this issue, but let me know if anything's still not right!

A-BN commented 6 years ago

Hello, First I'd like to thank you for all your work on unicycler and other software (everything in Wick et al., Microbial Genomics 2017 in fact).

On the matter of this issue_17, after installing the open-jdk version 9 on ubuntu, we still have this error because java -version returns openjdk version "9-internal" and is not parsed correctly.

Cheers,

A-BN