Running picard

Hi,

I'm currently trying to package htsjdk 1.138 and picard 1.138 for Debian. I've a question and a few bugs to report.

The question

It seems that picard cannot be run on Linux if the jar file is inside a directory with a '+' character in its name. Look at this session:

(sid-amd64-sbuild)[email protected]:~/picard-tools$ CLASSPATH=/usr/share/java/htsjdk.jar:/usr/share/java/guava.jar:dist/picard.jar java picard.cmdline.PicardCommandLine FilterVcf -h
USAGE: FilterVcf [options]
[... help text removed ...]
(sid-amd64-sbuild)[email protected]:~/picard-tools$ cd ..
(sid-amd64-sbuild)[email protected]:~$ mv picard-tools picard-tools+1
(sid-amd64-sbuild)[email protected]:~$ cd picard-tools+1/
(sid-amd64-sbuild)[email protected]:~/picard-tools+1$ CLASSPATH=/usr/share/java/htsjdk.jar:/usr/share/java/guava.jar:dist/picard.jar java picard.cmdline.PicardCommandLine FilterVcf -h
WARNING 2015-09-08 07:36:07 ClassFinder could not read entriesjava.io.FileNotFoundException: /home/vdanjean/picard-tools 1/dist/picard.jar (No such file or directory)
USAGE: PicardCommandLine <program name> [-h]

Available Programs:
--------------------------------------------------------------------------------------

'FilterVcf' is not a valid command. See PicardCommandLine --help for more information.
(sid-amd64-sbuild)[email protected]:~/picard-tools+1$

This is annoying because the testsuite fails due to this problem on Debian autobuilder (the directory in which the package is compiled has a '+' in its name).

Do you know how I can fix that?

Then, few typo/bug you might be interesting in fixing:

1) in htsjdk, there is a typo in the property name to include the version in the manifest:

--- a/build.xml
+++ b/build.xml
@@ -177,7 +177,7 @@
             <fileset dir="${classes}" includes="htsjdk/tribble/**/*.*"/>
             <fileset dir="${classes}" includes="htsjdk/variant/**/*.*"/>
             <manifest>
-                <attribute name="Implementation-Version" value="${hts-version}(${repository.revision})"/>
+                <attribute name="Implementation-Version" value="${htsjdk-version}(${repository.revision})"/>
                 <attribute name="Implementation-Vendor" value="Broad Institute"/>
             </manifest>
         </jar>

2) in picard, there is a bug in the IlluminaDataProviderTest: this method does not have the same number of arguments that the provider give. I fixed that by modifying the method signature. Perhaps, you might prefer fix the provider data.

--- a/src/tests/java/picard/illumina/parser/IlluminaDataProviderTest.java
+++ b/src/tests/java/picard/illumina/parser/IlluminaDataProviderTest.java
@@ -259,10 +259,12 @@ public class IlluminaDataProviderTest {
     }

     @Test(dataProvider = "badData", expectedExceptions = {PicardException.class, IllegalArgumentException.class})
-    public void testIlluminaDataProviderMissingDatas(final int lane,
-                                                     final IlluminaDataType[] actualDts,
-                                                     final String illuminaConfigStr,
-                                                     final File basecallsDirectory)
+    public void testIlluminaDataProviderMissingDatas(
+            final String testName, final int lane, final int size,
+            final List<Integer> tiles,
+            final IlluminaDataType[] actualDts,
+            final String illuminaConfigStr,
+            final File basecallsDirectory)
             throws Exception {
         final IlluminaDataProviderFactory factory = new IlluminaDataProviderFactory(basecallsDirectory, lane, new ReadStructure(illuminaConfigStr), bclQualityEvaluationStrategy, actualDts);
         factory.makeDataProvider();

And, to conclude, I would say I've very surprised that htsjdk is required to be chekouted and compiled locally in the picard build process. I would have think that picard would only use an external htsjdk.jar (for Debian, I modified the build system to do that, as JARs are required to be installed only once, system-wide, in /usr/share/java).

So, If you have any clue about the '+' problem, I would be very happy to know.

Regards,
Vincent

Issue · Github
by Sheila

Issue Number
163
State
closed
Last Updated
Milestone
Array
Closed By
chandrans

Comments

  • vdanjeanvdanjean FranceMember

    I think I find the bug. It is linked to http://bugs.java.com/bugdatabase/view_bug.do?bug_id=4466485
    Here is the patch I will apply:

    --- a/src/java/picard/cmdline/ClassFinder.java
    +++ b/src/java/picard/cmdline/ClassFinder.java
    @@ -30,7 +30,8 @@ import java.io.File;
     import java.io.IOException;
     import java.net.URL;
     import java.net.URLClassLoader;
    -import java.net.URLDecoder;
    +import java.net.URI;
    +import java.net.URISyntaxException;
     import java.util.*;
     import java.util.zip.ZipEntry;
     import java.util.zip.ZipFile;
    @@ -63,7 +64,7 @@ public class ClassFinder {
             // but the jarPath is remembered so that the iteration over the classpath skips anything other than
             // the jarPath.
             jarPath = jarFile.getCanonicalPath();
    -        final URL[] urls = {new URL("file", "", jarPath)};
    +        final URL[] urls = {new File(jarPath).toURI().toURL()};
             loader = new URLClassLoader(urls, Thread.currentThread().getContextClassLoader());
         }
    
    @@ -95,9 +96,14 @@ public class ClassFinder {
             while (urls.hasMoreElements()) {
                 try {
                     String urlPath = urls.nextElement().getFile();
    -                urlPath = URLDecoder.decode(urlPath, "UTF-8");
    -                if ( urlPath.startsWith("file:") ) {
    -                    urlPath = urlPath.substring(5);
    +                // convert URL to URI
    +                // http://bugs.java.com/bugdatabase/view_bug.do?bug_id=4466485
    +                // using URLDecode does not work if urlPath has a '+' character
    +                try {
    +                    URI uri = new URI(urlPath);
    +                   urlPath = uri.getPath();
    +                } catch (URISyntaxException e) {
    +                    log.warn("Cannot convert to URI the " + urlPath + " URL");
                     }
                     if (urlPath.indexOf('!') > 0) {
                         urlPath = urlPath.substring(0, urlPath.indexOf('!'));
    

    Regards
    Vincent

  • SheilaSheila Broad InstituteMember, Broadie, Moderator admin

    @vdanjean
    Hi Vincent,

    That is good news. https://www.broadinstitute.org/gatk/guide/article?id=1267
    I will look into the + issue and get back to you.

    Thanks,
    Sheila

  • vdanjeanvdanjean FranceMember

    @Sheila
    Hi Sheila,

    Thanks for pointing me to https://www.broadinstitute.org/gatk/guide/article?id=1267
    I've no problem to create and generate patch. It is just that the document ends with:

    [...]and you can simply e-mail them as an attachment to us.
    

    So, my question is, what is the emails to use for the htsjdk library and the picard software?

    Regards,
    Vincent

  • SheilaSheila Broad InstituteMember, Broadie, Moderator admin

    @vdanjean
    Hi Vincent,

    Oh, that is a slight mistake. There is no need to email, simply post a comment here letting us know the github link.

    As for the + issue, I got some help from Yossi, a team member. This is due to the fact that we use a URL decoder (picard/cmdline/ClassFinder.java:98) which replaces the '+' with a ' '.

    I think the best thing to do is not use a +.

    -Sheila

  • vdanjeanvdanjean FranceMember

    @Sheila

    Ok. Will go to github.
    Note that my first comment here propose a patch for the '+' issue. I will put it in github too.

  • vdanjeanvdanjean FranceMember

    Just created 3 pull requests on GitHub (2 for picard and 1 for htsjdk)

  • SheilaSheila Broad InstituteMember, Broadie, Moderator admin

    @vdanjean
    Wonderful Vincent! I will ask someone to review asap.

    Thanks for being so prompt!

    -Sheila

Sign In or Register to comment.