Ensembl / Bio-DB-HTS

Git repo for Bio::DB::HTS module on CPAN, providing Perl links into HTSlib
Apache License 2.0
24 stars 16 forks source link

Split HTSLIB_DIR into HTSLIB_LIBDIR and HTSLIB_INCDIR #30

Closed mmokrejs closed 7 years ago

mmokrejs commented 8 years ago

Hi, I am trying to create a package into Gentoo Linux.

The current INSTALL.pl logic does not work on Gentoo Linux, which tries to get around the samtools/htslib mess by installing the headers and libs into subdirectories (see at the end for a list of files). This setup is not detected by your INSTALL.pl script. Here is what happens:

>>> Emerging (1 of 1) dev-perl/Bio-DB-HTS-2.3::science
 * Bio-DB-HTS-2.3.tar.gz SHA256 SHA512 WHIRLPOOL size ;-) ...                                                                                                                                                                                                                                                           [ ok ]
>>> Unpacking source...
>>> Unpacking Bio-DB-HTS-2.3.tar.gz to /scratch/var/tmp/portage/dev-perl/Bio-DB-HTS-2.3/work
>>> Source unpacked in /scratch/var/tmp/portage/dev-perl/Bio-DB-HTS-2.3/work
>>> Preparing source in /scratch/var/tmp/portage/dev-perl/Bio-DB-HTS-2.3/work/Bio-DB-HTS-2.3 ...
>>> Source prepared.
>>> Configuring source in /scratch/var/tmp/portage/dev-perl/Bio-DB-HTS-2.3/work/Bio-DB-HTS-2.3 ...
 * Using Module::Build
 * perl Build.PL --installdirs=vendor --libdoc= --destdir=/scratch/var/tmp/portage/dev-perl/Bio-DB-HTS-2.3/image/ --create_packlist=1

This module requires htslib (http://htslib/org)
Install it if you have not done so already.

This script will attempt to locate htslib by looking for hts.h and libhts.a in:

  1. --htslib command line argument
  2. HTSLIB_DIR environment variable
  3. --prefix command line argument (which also sets installation location)
  4. Alien::HTSlib dependency resolver
  5. pkg-config (extra directories can be set in PKG_CONFIG_PATH environment variable)
  6. common library locations: /usr/local, /usr/share, /opt/local

 * ERROR: dev-perl/Bio-DB-HTS-2.3::science failed (configure phase):
>>> Emerging (1 of 1) dev-perl/Bio-DB-HTS-2.3::science
 * Bio-DB-HTS-2.3.tar.gz SHA256 SHA512 WHIRLPOOL size ;-) ...                                                                                                                                                                                                                                                           [ ok ]
>>> Unpacking source...
>>> Unpacking Bio-DB-HTS-2.3.tar.gz to /scratch/var/tmp/portage/dev-perl/Bio-DB-HTS-2.3/work
>>> Source unpacked in /scratch/var/tmp/portage/dev-perl/Bio-DB-HTS-2.3/work
>>> Preparing source in /scratch/var/tmp/portage/dev-perl/Bio-DB-HTS-2.3/work/Bio-DB-HTS-2.3 ...
>>> Source prepared.
>>> Configuring source in /scratch/var/tmp/portage/dev-perl/Bio-DB-HTS-2.3/work/Bio-DB-HTS-2.3 ...
 * Using Module::Build
 * perl Build.PL --installdirs=vendor --libdoc= --destdir=/scratch/var/tmp/portage/dev-perl/Bio-DB-HTS-2.3/image/ --create_packlist=1

This module requires htslib (http://htslib/org)
Install it if you have not done so already.

This script will attempt to locate htslib by looking for hts.h and libhts.a in:

  1. --htslib command line argument
  2. HTSLIB_DIR environment variable
  3. --prefix command line argument (which also sets installation location)
  4. Alien::HTSlib dependency resolver
  5. pkg-config (extra directories can be set in PKG_CONFIG_PATH environment variable)
  6. common library locations: /usr/local, /usr/share, /opt/local

 * ERROR: dev-perl/Bio-DB-HTS-2.3::science failed (configure phase):

Here is a list of files from htslib and their installation place:

# equery files htslib
 * Searching for htslib ...
 * Contents of sci-libs/htslib-1.3:
/usr
/usr/bin
/usr/bin/bgzip
/usr/bin/htsfile
/usr/bin/tabix
/usr/include
/usr/include/htslib
/usr/include/htslib/bgzf.h
/usr/include/htslib/cram.h
/usr/include/htslib/faidx.h
/usr/include/htslib/hfile.h
/usr/include/htslib/hts.h
/usr/include/htslib/hts_defs.h
/usr/include/htslib/kbitset.h
/usr/include/htslib/kfunc.h
/usr/include/htslib/khash.h
/usr/include/htslib/khash_str2int.h
/usr/include/htslib/klist.h
/usr/include/htslib/knetfile.h
/usr/include/htslib/kseq.h
/usr/include/htslib/ksort.h
/usr/include/htslib/kstring.h
/usr/include/htslib/regidx.h
/usr/include/htslib/sam.h
/usr/include/htslib/synced_bcf_reader.h
/usr/include/htslib/tbx.h
/usr/include/htslib/vcf.h
/usr/include/htslib/vcf_sweep.h
/usr/include/htslib/vcfutils.h
/usr/lib
/usr/lib/debug
/usr/lib/debug/usr
/usr/lib/debug/usr/bin
/usr/lib/debug/usr/bin/bgzip.debug
/usr/lib/debug/usr/bin/htsfile.debug
/usr/lib/debug/usr/bin/tabix.debug
/usr/lib/debug/usr/lib64
/usr/lib/debug/usr/lib64/libhts.so.1.3.debug
/usr/lib64
/usr/lib64/libhts.so -> libhts.so.1.3
/usr/lib64/libhts.so.1 -> libhts.so.1.3
/usr/lib64/libhts.so.1.3
/usr/lib64/pkgconfig
/usr/lib64/pkgconfig/htslib.pc
/usr/share
/usr/share/doc
/usr/share/doc/htslib-1.3
/usr/share/doc/htslib-1.3/NEWS.bz2
/usr/share/doc/htslib-1.3/README.bz2
/usr/share/man
/usr/share/man/man1
/usr/share/man/man1/htsfile.1.bz2
/usr/share/man/man1/tabix.1.bz2
/usr/share/man/man5
/usr/share/man/man5/faidx.5.bz2
/usr/share/man/man5/sam.5.bz2
/usr/share/man/man5/vcf.5.bz2

Maybe I could pursue the PKG_CONFIG_PATH way?

rishidev commented 8 years ago

The script does look in standard locations but /usr isn't one of those currently listed.

Could try adding it to the line in Build.PL and let me know how that goes? qw{ /usr/local /usr/share /opt/local },

rishidev commented 8 years ago

I've added /usr to the build locations, which should fix things on Gentoo.

Our build process already has a lot of options and so I'm reticent to introduce more. As you spotted, the PKG_CONFIG_PATH environment variable can also be used to pick things up in non-standard locations.

mmokrejs commented 8 years ago

Could try adding it to the line in Build.PL and let me know how that goes? qw{ /usr/local /usr/share /opt/local },

That did not help.

I see a problem elsewhere:

$ perl -d Build.PL --installdirs=vendor --libdoc= --destdir=/scratch/var/tmp/portage/dev-perl/Bio-DB-HTS-2.3/image/ --create_packlist=1 --prefix=/usr

...

Module::Build::HTS::find_hts(Build.PL:98):
98:             if ($dir and $self->find_hts_in_install_dir($dir)) {
  DB<2> 
Module::Build::HTS::find_hts_in_install_dir(Build.PL:144):
144:        my ($self, $root) = @_;
  DB<2> 
Module::Build::HTS::find_hts_in_install_dir(Build.PL:146):
146:        chomp($root);
  DB<2> 
Module::Build::HTS::find_hts_in_install_dir(Build.PL:147):
147:        $root =~ s{/$}{};
  DB<2> 
Module::Build::HTS::find_hts_in_install_dir(Build.PL:148):
148:        $root =~ s{/(lib|include|include/htslib)$}{};
  DB<2> 
Module::Build::HTS::find_hts_in_install_dir(Build.PL:150):
150:        my $hts_lib     = "$root/lib";
  DB<2> 
Module::Build::HTS::find_hts_in_install_dir(Build.PL:151):
151:        my $hts_include = "$root/include/htslib";
  DB<2> 
Module::Build::HTS::find_hts_in_install_dir(Build.PL:152):
152:        if (-f "$hts_lib/libhts.a" && -f "$hts_include/hts.h") {
  DB<2> 
Module::Build::HTS::find_hts_in_install_dir(Build.PL:158):
158:            return 0;
  DB<2> 
Module::Build::HTS::find_hts(Build.PL:98):
98:             if ($dir and $self->find_hts_in_install_dir($dir)) {
  DB<2> 
Module::Build::HTS::find_hts(Build.PL:98):
98:             if ($dir and $self->find_hts_in_install_dir($dir)) {
  DB<2> p $dir
/usr/lib64

  DB<3> 
Module::Build::HTS::find_hts_in_install_dir(Build.PL:144):
144:        my ($self, $root) = @_;
  DB<3> 
Module::Build::HTS::find_hts_in_install_dir(Build.PL:146):
146:        chomp($root);
  DB<3> 
Module::Build::HTS::find_hts_in_install_dir(Build.PL:147):
147:        $root =~ s{/$}{};
  DB<3> 
Module::Build::HTS::find_hts_in_install_dir(Build.PL:148):
148:        $root =~ s{/(lib|include|include/htslib)$}{};
  DB<3> 
Module::Build::HTS::find_hts_in_install_dir(Build.PL:150):
150:        my $hts_lib     = "$root/lib";
  DB<3> p $root 
/usr/lib64
  DB<4> 
Module::Build::HTS::find_hts_in_install_dir(Build.PL:151):
151:        my $hts_include = "$root/include/htslib";
  DB<4>  
Module::Build::HTS::find_hts_in_install_dir(Build.PL:152):
152:        if (-f "$hts_lib/libhts.a" && -f "$hts_include/hts.h") {
  DB<4> p $hts_lib
/usr/lib64/lib
  DB<5> p $hts_include
/usr/lib64/include/htslib
  DB<7> p "$hts_include/hts.h"
/usr/lib64/include/htslib/hts.h
  DB<8> c

This module requires htslib (http://htslib/org)
Install it if you have not done so already.

This script will attempt to locate htslib by looking for hts.h and libhts.a in:

  1. --htslib command line argument
  2. HTSLIB_DIR environment variable
  3. --prefix command line argument (which also sets installation location)
  4. Alien::HTSlib dependency resolver
  5. pkg-config (extra directories can be set in PKG_CONFIG_PATH environment variable)
  6. common library locations: /usr/local, /usr/share, /opt/local

 at Build.PL line 166.
        Module::Build::HTS::die_hts_not_found(Module::Build::HTS=HASH(0x2f9c800)) called at Build.PL line 105
        Module::Build::HTS::find_hts(Module::Build::HTS=HASH(0x2f9c800)) called at Build.PL line 57
Debugged program terminated.  Use q to quit or R to restart,
use o inhibit_exit to avoid stopping after program termination,
h q, h R or h o to get additional info.
  DB<8> 

And here the correct answers.

$ ls -latr /usr/include/htslib/hts.h
-rw-r--r-- 1 root root 21879 Oct  6  2015 /usr/include/htslib/hts.h
$ ls -altr /usr/lib64/libhts.so*
-rwxr-xr-x 1 root root 600152 Jul 28 12:13 /usr/lib64/libhts.so.1.3
lrwxrwxrwx 1 root root     13 Jul 28 12:13 /usr/lib64/libhts.so.1 -> libhts.so.1.3
lrwxrwxrwx 1 root root     13 Jul 28 12:13 /usr/lib64/libhts.so -> libhts.so.1.3
$

$ pkg-config --print-variables htslib
includedir
libdir
$ pkg-config --variable=includedir htslib
/usr/include
$ pkg-config --libs htslib
-lhts
$ pkg-config --variable=libdir htslib
/usr/lib64
$ 

Guessing the path to both header and library from a single input value is wrong (it end up in $root in the above perl code). It works for cases like: /usr/lib/libhts.so and /usr/include/hts.h but not for more complicated layouts like above. I propose introduction of --htsinc (in addition to existing --htslib). I assume both anticipate actually a directory path.

Provided you support environment variables for passing input, this below is my favorite:

HTSLIB_LIBDIR="${EPREFIX}"/"$(get_libdir)" HTSLIB_INCDIR="${EPREFIX}"/usr/include/htslib" ./Build.PL

rishidev commented 7 years ago

There are quite a few existing methods of customising the install procedure in place, each which gets testing and maintenance. Adding these additional ones will introduce more overhead and confusion, and pointing to a separately downloaded HTSLib is always available as an option.

mmokrejs commented 7 years ago

I just do not agree but what else can I say.

The problem is:

  DB<5> p $hts_include
/usr/lib64/include/htslib
  DB<7> p "$hts_include/hts.h"
/usr/lib64/include/htslib/hts.h
  DB<8> c

I did a lot of debugging and I proposed a fix. You need to have two variables, one for headers path and the other for libs path. They cannot be deduced from a common prefix in all scenarios. IMHO it has nothing to do with "quite a few existing methods of customising the install procedure in place, each which gets testing and maintenance.". You just do not have proper tests in place. ;-) My 2c.

mmokrejs commented 7 years ago

The package in Gentoo links the objects only because of luck because the /usr/lib64 (where the libhts.{a,so} files exist) is included by compiler/linker automatically: https://cgit.gentoo.org/proj/sci.git/log/dev-perl/Bio-DB-HTS

The addition of -L/usr/lib is pretty useless (luckily not causing harm in this case) thanks to the code appending blindly '/lib/' to the HTSLIB_DIR content. I hope this demonstrates better that HTSLIB_DIR should be replaced by HTSLIB_LIBDIR and HTSLIB_INCDIR. Thank you.

SarahBeecroft commented 3 years ago

Hey, I don't know if this is directly relevent to the above thread but I had a similar issue with installing HTSlib/Faidx for VEP and turns out I needed sudo cpan Archive::Extract and then it worked! So just posting here for other people in case it's a fix for them