Attention:
The frontline support team will be slow on the forum because we are occupied with the GATK Workshop on March 21st and 22nd 2019. We will be back and more available to answer questions on the forum on March 25th 2019.

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.