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

Fix configure.ac buglet when used with autoconf 2.70 #1198

Merged
merged 1 commit into from
Jan 4, 2021

Conversation

jmarshall
Copy link
Member

As detected in Debian bug 978835, HTSlib's ./configure fails when built with autoconf 2.70 (released in December). It is a byproduct of autoconf 2.70 being more careful about cross compilation:

*** More macros use config.sub and config.guess internally.

As a consequence of improved support for cross compilation [snip]

Using the standard $host_alias variable name in our shared library check was fun and intended to be mostly already correct if we moved to using AC_CANONICAL_HOST at the top of the configure script, but causes problems when autoconf 2.70 implicitly uses AC_CANONICAL_HOST below our check. The simple fix is to use a variable name that's not used by autoconf, as there are reasons not to move to using AC_CANONICAL_HOST just yet.

As autoconf 2.70 implicitly uses AC_CANONICAL_HOST, it requires (and its autoreconf --install installs) config.guess and config.sub. Ignore those, and ignore install-sh as well for good measure.

Autoconf 2.70 is more careful about cross compilation, so with this
version using AC_FUNC_MMAP implies AC_CANONICAL_HOST and hence computes
$build/build_alias/host/host_alias/etc. Setting $host_alias ourselves
interferes with that. Hat tip Matthias Klose (via debbug#978835).

As autoconf 2.70 implicitly uses AC_CANONICAL_HOST, it requires (and its
autoreconf --install installs) config.guess and config.sub. Ignore those,
and ignore install-sh as well for good measure.
@jkbonfield
Copy link
Contributor

Thanks for this John.

I see our Travis and Cirrus configs use autoreconf. The INSTALL file however tries to explicitly suggest doing it by hand using autoheader and autoconf. I'm guessing this is no longer workable with autoconf 2.7.0 as the changes document explicitly states using autoreconf --install. Is this true? (I note we don't have a --install and it still works though, so maybe it's getting them from elsewhere and we don't now have a requirement to use autoreconf.)

@jmarshall
Copy link
Member Author

Correct. I have a second draft PR coming up that makes it “2.70-esque native”, with the recommendation that there's no point applying it until autoconf 2.70 is widespread.

@jkbonfield
Copy link
Contributor

Ok thanks.

@jkbonfield jkbonfield merged commit 78441c9 into samtools:develop Jan 4, 2021
@jmarshall jmarshall deleted the autoconf-2.70-compat branch January 4, 2021 10:12
@jmarshall
Copy link
Member Author

See also PR #1199, which expands on the reasons not to move to using AC_CANONICAL_HOST just yet, not until autoconf 2.70 is widespread at which point HTSlib's release-making scripts, instructions, and other infrastructure will have to learn about config.guess etc anyway.

Currently Travis/Cirrus/etc work fine because they are using autoconf 2.69 and the resulting configure script doesn't invoke any of these three support scripts. I guess we could at our leisure change the CI scripts to use autoreconf --install which will do the same thing as presently with 2.69 but hopefully start copying the support scripts and just carry on working when the CI environments update to 2.70.

ihnorton added a commit to TileDB-Inc/TileDB-VCF that referenced this pull request Apr 28, 2021
autoconf@2.69 pin is due to changes in autoconf 2.70, compat for which is fixed
in htlslib here: `samtools/htslib#1198
 remove after updating to htslib>=1.12
ihnorton added a commit to TileDB-Inc/TileDB-VCF that referenced this pull request Apr 28, 2021
ihnorton added a commit to TileDB-Inc/TileDB-VCF that referenced this pull request Apr 28, 2021
* Patch htslib 1.10 to fix build with autoconf 2.70+

details in: samtools/htslib#1198

* Also autoreconf htslib after patching configure.ac

* Add automake brew dependency to ci

* Add patch to Dockerfile-cli

Co-authored-by: Seth Shelnutt <Shelnutt2@gmail.com>
ZenithalHourlyRate added a commit to ZenithalHourlyRate/archriscv-packages that referenced this pull request Sep 9, 2021
felixonmars pushed a commit to felixonmars/archriscv-packages that referenced this pull request Sep 9, 2021
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.

None yet

2 participants