Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bring over autoconf from samtools #576

Merged
merged 9 commits into from Jun 13, 2017
Merged

bring over autoconf from samtools #576

merged 9 commits into from Jun 13, 2017

Conversation

mcshane
Copy link
Contributor

@mcshane mcshane commented Mar 24, 2017

Still todo:

  • --enable-gsl / --with-gsl
  • --disable-plugins and make sure disambiguated from htslib plugins

Plus there may be a bunch of stuff I am missing...

Closes #573, closes #481

@jkbonfield jkbonfield mentioned this pull request May 8, 2017
@daviesrob
Copy link
Member

I've just added my additions to this PR. They add configure options for plugins and libgsl, and rearrange Makefile slightly so that variables get set at the correct times. I've also updated INSTALL and made travis test both configure and non-configure builds.

mcshane and others added 8 commits June 9, 2017 15:19
bcftools needs to export all of its symbols so that plugins can use them.
There are two ways to do this on MacOS - pass -export_dynamic to the
linker, or -rdynamic to the compiler.  Unfortunately -export_dynamic
isn't supported by older linkers on the Mac, causing build failures.

Using -rdynamic works a bit better.  gcc is fine, but on old systems
clang issues an "argument unused during compilation: '-rdynamic'"
warning.  On newer systems, clang accepts the -rdynamic option without
complaining.  As the warning is non-fatal (as long as -Werror is not
in use) the build completes.

These options only appear to be necessary with clang if link-time
optimization is turned on.  Enabling -flto without -rdynamic or
-Wl,-export_dynamic results in undefined symbols on new MacOS systems.
Allow plugins to be enabled/disabled via configure, and add options to
set the install directory and default plugin search path.

If plugins are enabled:
 * The plugins are built and installed
 * `make test` will test the plugins
 * 'plugin' will appear in the bcftools usage output
 * `bcftools plugin` will work as normal

If plugins are disabled:
 * The plugins are not built
 * `make test` does not try to test the plugins
 * 'plugin' is hidden from the bcftools usage
 * `bcftools plugin` prints a short message to say that plugins are
   disabled, and exits non-zero.

If configure is not used, the default is to enable plugins.

The Makefile is rearranged slightly so that default values and overrides
from config.mk work correctly.  The platform test in the Makefile is
changed so that it can be overridden by configure, so that the
correct result should be used when cross-compiling.
Add a configure option to allow linking against the GNU Scientific
Library, enabling the polysomy subcommand.  Includes library checks
for libgsl and cblas.  The cblas library may be called libcblas or
libgslcblas. The default is to look for both of these, but a
--with-cblas option is also included to allow the cblas library name
to be selected manually.

A message about the change of license to GPL is printed out if configure
is run successfully with --enable-libgsl.

Travis in autoconf mode is updated to use --enable-libgsl.
@mcshane
Copy link
Contributor Author

mcshane commented Jun 9, 2017

Thanks alot @daviesrob! I've rebased against develop and squashed a few of the early commits.

I get the following warnings when running autoheader and autoconf. Anything to worry about?

configure.ac:81: warning: AC_CONFIG_SUBDIRS: you should use literals
../../lib/autoconf/status.m4:1099: AC_CONFIG_SUBDIRS is expanded from...
m4/ax_with_htslib.m4:55: AX_WITH_HTSLIB is expanded from...
configure.ac:81: the top level

@mcshane
Copy link
Contributor Author

mcshane commented Jun 9, 2017

Also, @daviesrob, the stuff with -Wl,-export_dynamic was brought in with b8d7e3b (see also link to samtools-help message in that commit) to solve #388. Would bringing back -rdynamic bring that issue up again?

@daviesrob
Copy link
Member

The warnings are due to AX_WITH_HTSLIB passing a variable ($HTSDIR) to AC_CONFIG_SUBDIRS. It means bcftools' ./configure --help won't print help messages for an embedded HTSlib's configure. Otherwise it's nothing to worry about.

On -rdynamic, as far as I can make out it's actually slightly better supported than -Wl,-export_dynamic. Our Mac mini claims:
gcc version 4.2.1 (Based on Apple Inc. build 5658) (LLVM build 2336.11.00)
Apple clang version 4.0 (tags/Apple/clang-421.0.60) (based on LLVM 3.1svn)
@(#)PROGRAM:ld PROJECT:ld64-133.3
ld doesn't accept -Wl,-export_dynamic. gcc is fine with -rdynamic while clang prints a warning about the option being ignored.

On newer installations the linker knows about -Wl,-export_dynamic and both gcc and clang will pass on the option if started with -rdynamic. Given this, I think using -rdynamic is currently the least bad option. It's also what htslib does if you try to build that with plugins.

It might be worth adding a test to configure to see if -rdynamic works, but the only platform I've found so far where it doesn't is illumos, and that has other issues too.

@jmarshall
Copy link
Member

You can configure with ./configure -Wno-syntax to silence that warning — see the commit message and README.md change in samtools/samtools@30bc5e2.

@mcshane mcshane merged commit d595e63 into develop Jun 13, 2017
@mcshane mcshane deleted the feature/autoconf branch June 13, 2017 07:26
@pd3
Copy link
Member

pd3 commented Jun 14, 2017

Great stuff, thanks for this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problem building with an installed htslib bcftools 1.3.1 hardcoded to build against htslib 1.3.1
4 participants