samtools / bcftools

This is the official development repository for BCFtools. See installation instructions and other documentation here http://samtools.github.io/bcftools/howtos/install.html
http://samtools.github.io/bcftools/
Other
649 stars 240 forks source link

make USE_GPL=1 clean all fails because GSL_LIBS already defined #2040

Closed tobiasrausch closed 7 months ago

tobiasrausch commented 9 months ago

Hi,

Small fix for:

make USE_GPL=1 clean all

GSL_LIBS ?= -lgsl -lcblas didn't work for me because GSL_LIBS was already defined in the Makefile beforehand.

Best, Tobias

daviesrob commented 8 months ago

Sorry to be a while getting on to this.

The proposed solution isn't quite correct as it doesn't account for values set by configure. Currently, enabling libgsl through configure does work, as can be seen using make print-GSL_LIBS:

$ ./configure --enable-libgsl
[ ... output snipped ... ]
checking for gsl/gsl_version.h... yes
checking for library containing cblas_dgemm... -lcblas
checking for gsl_multifit_gradient in -lgsl... yes
checking for library containing regcomp... none required
configure: creating ./config.status
config.status: creating config.mk
[ ... output snipped ... ]

$ make print-GSL_LIBS
GSL_LIBS=-lgsl -lcblas

As noted, either not running configure, or running it without --enable-libgsl and then trying to enable libgsl using make USE_GPL=1 doesn't work:

$ ./configure
[ ... output omitted ... ]

$ make print-GSL_LIBS
GSL_LIBS=

$ make USE_GPL=1 print-GSL_LIBS
GSL_LIBS=

The proposed solution fixes that, but when you run configure it adds some unwanted entries to GSL_LIBS:

$ ./configure --enable-libgsl
[ ... output omitted ... ]

$ make print-GSL_LIBS
GSL_LIBS=-lgsl -lcblas -lgsl -lcblas

In this case it's harmless, but it may clash with the decisions made by configure in some cases.

I think a solution that gives the intended behaviour might be:

ifdef USE_GPL
    main.o : EXTRA_CPPFLAGS += -DUSE_GPL
    OBJS += polysomy.o peakfit.o
    #ifndef GSL_LIBS                                                             
        GSL_LIBS += -lgsl -lcblas
    #endif                                                                       
endif

as ifndef matches both unset and empty (before expansion) variables. With that I get:

$ ./configure
[ ... output omitted ... ]
$ make print-GSL_LIBS
GSL_LIBS=
$ make USE_GPL=1 print-GSL_LIBS
GSL_LIBS=-lgsl -lcblas

$ ./configure --enable-libgsl
[ ... output omitted ... ]
$ make print-GSL_LIBS
GSL_LIBS=-lgsl -lcblas
$ make USE_GPL=1 print-GSL_LIBS
GSL_LIBS=-lgsl -lcblas

... which is what we want to happen.

tobiasrausch commented 8 months ago

Thanks. I missed the configure part (sorry) but your solution seems to work fine.

pd3 commented 7 months ago

Thank you both!