Bioconductor / Rhtslib

HTSlib high-throughput sequencing library as an R package
https://bioconductor.org/packages/Rhtslib
11 stars 12 forks source link

installation fails in conda environment that already contains htslib headers #16

Closed jgarthur closed 4 years ago

jgarthur commented 4 years ago

While installing Rhtslib 1.18.0 within a conda environment, I hit the following error:

make[1]: *** [Makefile.Rhtslib:128: hts.o] Error 1
make[1]: Leaving directory '/tmp/RtmpJaq3Wa/R.INSTALL2cc0e9944d8ce/Rhtslib/src/htslib-1.7'
make: *** [Makevars.common:24: htslib] Error 2

After a little digging, I found that the headers from my [conda environment]/include/htslib were being used rather than the ones for this package. Temporarily moving those conda htslib headers fixed it.

I suspect the issue is around here: https://github.com/Bioconductor/Rhtslib/blob/master/src/htslib-1.7/Makefile.Rhtslib#L127-L131 The -I. needs to come before $(CFLAGS). In conda at least, the default CFLAGS within lib/R/etc/Makeconf specifies -I/path/to/conda/env/include

hpages commented 4 years ago

The -I. needs to come before $(CFLAGS). In conda at least, the default CFLAGS within lib/R/etc/Makeconf specifies -I/path/to/conda/env/include

-I is a preprocessor directive so should go in CPPFLAGS rather than CFLAGS. FWIW this is what I get on my machine:

hpages@spectre:~$ R CMD config CFLAGS
-g -O2 -Wall
hpages@spectre:~$ R CMD config CPPFLAGS
-I/usr/local/include

and I've seen something similar on all the systems where I've configured/installed R.

So the way your CFLAGS is set doesn't seem right to me (but at the same time I don't know anything about conda).

Note that the authors of htslib's Makefile also seem to assume that CFLAGS will never contain -I directives (based on how they define the $(CC) $(CFLAGS) -I. $(CPPFLAGS) ... rule) so I'm not too keen on changing that.

H.

jgarthur commented 4 years ago

@hpages I think you are right here about my/Conda's CFLAGS being set strangely. That said, moving the -I. to the front seems safe, so you might consider it if many others hit this problem. Feel free to close the issue, thanks

hpages commented 4 years ago

I agree it "seems safe". We'll see if others hit this problem but so far you're the first and only one that I know of. Thanks!