Skip to content

Commit 3976c80

Browse files
brainstormjuliangehringjohanneskoester
authored
Update to htslib 1.10.2 (#184)
* Pull 1.10.2 submodule. Initial pass on i32->i64 type conversion, will eventually fix issue #183 * Fixes issue #109 while I'm at it, long hanging fruit fix * Fix compression levels * Going through https://github.com/samtools/htslib/blob/develop/README.large_positions.md to spot 64 bit reference position changes and sam_* function renaming. Only 4 errors left, but need to closely check types afterwards very carefully. Specially for the binary formats where this change does not apply and (sub)fields that do not necessarily require big representations * Repoint submodule to upstream release tag * Switch from rust-bio hosted htslib to upstream's samtools htslib. * Try to amend submodule * Handle submodules manually on TravisCI, since it is failing now: https://travis-ci.org/rust-bio/rust-htslib/builds/645772614 * Odd balls that would need review marked with XXX. New htslib seems to require libcurl * Add curl-sys crate for hplugin_curl, aws s3 et al * Add conditional libcurl compilation flags * Be nice with cargo fmt [ci skip] * Try installing libcurl meta-package instead of the openssl dependent one?: https://travis-ci.org/rust-bio/rust-htslib/builds/645790138#L1529 * Does not work with libcurl-dev metapackage: https://travis-ci.org/rust-bio/rust-htslib/jobs/645812050#L166, rollback and try with bionic ubuntu distro. It does compile locally on OSX * Add matrix compile support for TravisCI, I would like to know why it compiles fine in OSX and fails on Linux now... * PPA source no longer needed for ubuntu bionic? [ci skip] * Bionic packages htslib with gnutls instead of openssl: https://packages.ubuntu.com/source/disco/htslib ... and it even comes with pre-shipped libcurl4-gnutls-dev, so no need to install it explicitly?: https://travis-ci.org/rust-bio/rust-htslib/jobs/645816116#L160 * sed is not gsed on OSX... not entirely sure this substitution is 100% needed * Add libssl-dev to the mix for the openssl-sys: https://travis-ci.org/rust-bio/rust-htslib/jobs/645825022#L1000. Testsuite seems to run partially now: https://travis-ci.org/rust-bio/rust-htslib/jobs/645825022#L623 * Add build-essential in hopes to fix libcurl issue, incorporate #185, fix one testsuite warning * Add curl on default features for hts-sys * Point to the right htslib upstream repo, thanks @dlaehnemann [skip ci] * Appease OSX musl-cross toolchain requirements: https://travis-ci.org/rust-bio/rust-htslib/jobs/645907695#L1222 * TravisCI timesout installing brew deps: https://travis-ci.org/rust-bio/rust-htslib/jobs/647667984#L212 ... Not sure if travis_wait will work with homebrew addons?: https://docs.travis-ci.com/user/common-build-problems/#build-times-out-because-no-output-was-received... trying * Try workaround for https://twitter.com/braincode/status/1226103002550849542 * Do not update homebrew (time intensive), switch from travis_wait to verbose to keep the build output being generated, hopefully * brew install --verbose is not verbose enough for travis, reverting travis_wait and bumping to 40min for brew deps install and musl-cross * Explicitly install system libcurl on linux and osx * This .cargo/config makes it test right locally: rust-lang/rust#60149 (comment), only 5 tests failing now and no linking errors from libcurl (on OSX, at least) :) * Cargo.toml missing quote misshap * Same linker flags should apply for Linux builds? build-essential not necessary * tid is i64 now, CRAM vs BAM test l_data is off by 3 and m_data is off too * There should be no need for travis_wait with -v * Debugging htslib structs with @jkbonfield guidance... * Disable osx and musl-cross homebrew on the TravisCI matrix since it times out anyway, start looking into l_extranul and alignment issues for CRAM/BAM. Add curl_sys crate in hts-sys/src/lib.rs. * Mystery solved for test_read_cram, testing some of the struct fields, explicitly state that l_data should differ according to #184 (comment) * Extract selective CRAM/BAM field comparison to test helper function * Formatting * Temporarily commenting the hfile format category detection makes .bed filetype tests pass but .BAI idx locator fails * Format detection seems to be broken upstream on htslib, see #184 (comment) * Revert to libcurl4-openssl-dev instead of gnutls. All TravisCI script targets compiling and passing tests otherwise * Bump up curl-sys version * Experimenting musl cross-compilation with rustembedded cross. Ideally this would get rid of the homebrew musl-cross issue (too long compile times). /cc @nlhepler * Remove assert_ne since cannot guarantee that l_data is different on all bam/crams * Add minimum deps needed for rustembedded cross docker container * Run docker in TravisCI, even if it is only supported on linux jobs :/ * Add CFLAGS suggested by @nlhepler * Separate musl from GNU toolchains on Rust cross * Bump up bindgen, vendorize openssl, further experimentation with compile flags * Deprecate the mixed GNU/MUSL container * Add minimal explanation on how to use the new docker files [ci skip] * cross 0.2.0 with openssl vendored. Progress but more htslib compile time issues remain still * Make sure libzma is present in both containers [ci skip] * Focusing on LLVMv8 GNU cross build first, musl will come later on (if possible w/ htslib) * Two more usize that should be u64 * Amend and settle BED header debate * Rename test name accordingly * Transition to cross instead of cargo for builds and tests * All GNU/clang/llvm8 builds and tests seem to succeed, transitioning to musl testing now... * Remove stray (repeated) musl build * Remove @FiloSottile musl-cross homebrew formula since it takes too long to compile and timeouts CI, using clux/muslrust gets me a bit farther. Remove local rustup musl target... since htslib 1.10, the introduction of openssl complicated local compilation w/ musl greatly, unfortunately we have to resort to docker these days instead :/ * Experiment with toolchain setup in https://gitlab.com/rust_musl_docker/image. Remove vendored openssl since it's shipped in cross containers. * Remove build.rs flags noise and different tests, containers should handle them transparently depending on the defined environment within, thanks @nlhepler * Indicate dependencies and build flags required for OSX [ci skip]. * enable musl builds * add libclang * minor * dbg * revert * dbg * dbg * Revert cross/docker madness, bottled up musl-cross myself over here: https://github.com/brainstorm/homebrew-musl-cross/commit/9304bf9cd6bc2b784083948771977d502030815a, high sierra bottle coming up soon. Cargo build script draft * Add in openssl to hts-sys * Make clippy boolean logic happy * Remove homebrew OSX check since we'll just use rustembedded cross Co-authored-by: Julian Gehring <[email protected]> Co-authored-by: Johannes Köster <[email protected]>
1 parent 0364460 commit 3976c80

25 files changed

+226
-133
lines changed

.cargo/config

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
[target.x86_64-apple-darwin]
2+
rustflags = [
3+
"-C", "link-arg=-undefined",
4+
"-C", "link-arg=dynamic_lookup",
5+
]
6+
7+
[target.x86_64-unknown-linux-musl]
8+
rustflags = [
9+
"-C", "link-arg=-undefined",
10+
"-C", "link-arg=dynamic_lookup",
11+
]
12+
linker = "x86_64-linux-musl-gcc"

.github/workflows/rust.yml

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,6 @@ jobs:
3939
override: true
4040
components: clippy
4141

42-
- name: Install system dependencies
43-
run: |
44-
sudo apt-get install --yes zlib1g-dev libbz2-dev musl musl-dev musl-tools
45-
4642
- name: Lint with clippy
4743
uses: actions-rs/clippy-check@v1
4844
with:
@@ -67,7 +63,7 @@ jobs:
6763

6864
- name: Install system dependencies
6965
run: |
70-
sudo apt-get install --yes zlib1g-dev libbz2-dev musl musl-dev musl-tools
66+
sudo apt-get install --yes zlib1g-dev libbz2-dev musl musl-dev musl-tools clang libc6-dev
7167
7268
- name: Run cargo-tarpaulin
7369
uses: actions-rs/[email protected]
@@ -79,3 +75,17 @@ jobs:
7975
with:
8076
github-token: ${{ secrets.GITHUB_TOKEN }}
8177
path-to-lcov: ./lcov.info
78+
79+
- name: Test musl build without default features
80+
uses: actions-rs/cargo@v1
81+
with:
82+
use-cross: true
83+
command: build
84+
args: --target x86_64-unknown-linux-musl --no-default-features
85+
86+
- name: Test musl build with all features
87+
uses: actions-rs/cargo@v1
88+
with:
89+
use-cross: true
90+
command: build
91+
args: --target x86_64-unknown-linux-musl --all-features

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
.vscode/
2+
.idea
23
*~
34
target
45
Cargo.lock

.gitmodules

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
[submodule "htslib"]
22
path = hts-sys/htslib
3-
url = https://github.com/rust-bio/htslib.git
3+
url = https://github.com/samtools/htslib.git

Cargo.toml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,11 @@ snafu = ">= 0.5.0, <= 0.6.0"
3333
hts-sys = { version = "^1.9", path = "hts-sys", default-features = false }
3434

3535
[features]
36-
default = ["bzip2", "lzma"]
36+
default = ["bzip2", "lzma", "curl"]
3737
bzip2 = ["hts-sys/bzip2"]
3838
lzma = ["hts-sys/lzma"]
39+
curl = ["hts-sys/curl"]
40+
openssl = ["hts-sys/openssl"]
3941
serde = ["serde_base", "serde_bytes"]
4042

4143
[dev-dependencies]

Cross.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
[target.x86_64-unknown-linux-musl]
2+
image = "brainstorm/rust_musl_docker:stable-latest-libcurl"
3+
[target.x86_64-unknown-linux-gnu]
4+
image = "brainstorm/cross-x86_64-unknown-linux-gnu:libcurl-openssl"

README.md

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,18 @@ If you only want to use the library, there is no need to clone the repository. G
1919

2020
## Requirements
2121

22-
To compile this crate you need the development headers of zlib, bzip2 and xz.
22+
To compile this crate you need the development headers of zlib, bzip2 and xz. For instance, in Debian systems one needs the following dependencies:
23+
24+
```shell
25+
$ sudo apt-get install zlib1g-dev libbz2-dev liblzma-dev clang
26+
```
27+
28+
On OSX, this will take a significant amount of time due to musl cross compiling toolchain:
29+
30+
```shell
31+
$ brew install FiloSottile/musl-cross/musl-cross
32+
$ brew install bzip2 zlib xz curl-openssl
33+
```
2334

2435
## Usage
2536

docker/Dockerfile.gnu

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
FROM rustembedded/cross:x86_64-unknown-linux-gnu
2+
3+
ENV LIBCLANG_PATH /usr/lib/x86_64-linux-gnu
4+
ENV LLVM_CONFIG_PATH /usr/bin
5+
RUN apt-get update && \
6+
apt-get install -y libcurl4-openssl-dev zlib1g-dev libbz2-dev liblzma-dev clang-8 && \
7+
ln -sf /usr/bin/llvm-config-8 /usr/bin/llvm-config && \
8+
ln -sf /usr/lib/x86_64-linux-gnu/libclang-8.so.1 /usr/lib/x86_64-linux-gnu/libclang.so.1

docker/Dockerfile.musl

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
FROM rustembedded/cross:x86_64-unknown-linux-musl
2+
3+
ENV PKG_CONFIG_ALLOW_CROSS 1
4+
ENV OPENSSL_LIB_DIR /usr/lib/x86_64-linux-gnu
5+
ENV OPENSSL_INCLUDE_DIR /usr/include/openssl
6+
RUN apt-get update && \
7+
apt-get install -y libssl-dev libcurl4-openssl-dev zlib1g-dev libbz2-dev liblzma-dev musl musl-dev musl-tools linux-libc-dev linux-headers-4.15.0-20-generic

docker/README.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# cross rustembedded containers
2+
3+
Allows to compile (rust-)htslib in a variety of environments and architectures via [rustembedded cross](https://github.com/rust-embedded/cross).
4+
5+
## Quickstart
6+
7+
```shell
8+
$ cd docker
9+
$ docker build -t brainstorm/cross-x86_64-unknown-linux-musl:libcurl-openssl . -f Dockerfile.musl
10+
$ docker build -t brainstorm/cross-x86_64-unknown-linux-gnu:libcurl-openssl . -f Dockerfile.gnu
11+
```
12+
13+
Then to build and test rust-htslib with the above containers, proceed as you would with `cargo`, using `cross` instead, i.e:
14+
15+
```shell
16+
$ cross build --target x86_64-unknown-linux-musl
17+
```

hts-sys/Cargo.toml

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ description = "This library provides HTSlib bindings."
88
readme = "README.md"
99
keywords = ["htslib", "bam", "bioinformatics", "pileup", "sequencing"]
1010
license = "MIT"
11-
repository = "https://github.com/rust-bio/rust-htslib.git"
11+
repository = "https://github.com/samtools/htslib.git"
1212
documentation = "https://docs.rs/rust-htslib"
1313
edition = "2018"
1414

@@ -21,14 +21,18 @@ libc = "0.2"
2121
libz-sys = "1.0"
2222
bzip2-sys = { version = "0.1", optional = true }
2323
lzma-sys = { version = "0.1", optional = true }
24+
curl-sys = { version = "0.4.26", optional = true }
25+
openssl-sys = { version = "0.9.54", optional = true }
2426

2527
[features]
26-
default = ["bzip2", "lzma"]
28+
default = ["bzip2", "lzma", "curl"]
2729
bzip2 = ["bzip2-sys"]
2830
lzma = ["lzma-sys"]
31+
openssl = ["openssl-sys"]
32+
curl = ["curl-sys"]
2933

3034
[build-dependencies]
3135
fs-utils = "1.1"
32-
bindgen = { version = "0.52.0", default-features = false, features = ["runtime"] }
36+
bindgen = { version = "0.53.1", default-features = false, features = ["runtime"] }
3337
cc = "1.0"
3438
glob = "0.3.0"

hts-sys/build.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@ use std::fs;
1313
use std::path::PathBuf;
1414
use std::process::Command;
1515

16-
fn sed_htslib_makefile(out: &PathBuf, patterns: &Vec<&str>, feature: &str) {
16+
fn sed_htslib_makefile(out: &PathBuf, patterns: &[&str], feature: &str) {
1717
for pattern in patterns {
18-
if Command::new("sed")
18+
if !Command::new("sed")
1919
.current_dir(out.join("htslib"))
2020
.arg("-i")
2121
.arg("-e")
@@ -24,7 +24,6 @@ fn sed_htslib_makefile(out: &PathBuf, patterns: &Vec<&str>, feature: &str) {
2424
.status()
2525
.unwrap()
2626
.success()
27-
!= true
2827
{
2928
panic!("failed to strip {} support", feature);
3029
}
@@ -63,10 +62,18 @@ fn main() {
6362
cfg.include(inc);
6463
}
6564

65+
let use_curl = env::var("CARGO_FEATURE_CURL").is_ok();
66+
if !use_curl {
67+
let curl_patterns = vec!["s/ -lcurl//", "/#define HAVE_LIBCURL/d"];
68+
sed_htslib_makefile(&out, &curl_patterns, "curl");
69+
} else if let Ok(inc) = env::var("DEP_CURL_INCLUDE").map(PathBuf::from) {
70+
cfg.include(inc);
71+
}
72+
6673
let tool = cfg.get_compiler();
6774
let (cc_path, cflags_env) = (tool.path(), tool.cflags_env());
6875
let cc_cflags = cflags_env.to_string_lossy().replace("-O0", "");
69-
if Command::new("make")
76+
if !Command::new("make")
7077
.current_dir(out.join("htslib"))
7178
.arg(format!("CC={}", cc_path.display()))
7279
.arg(format!("CFLAGS={}", cc_cflags))
@@ -75,7 +82,6 @@ fn main() {
7582
.status()
7683
.unwrap()
7784
.success()
78-
!= true
7985
{
8086
panic!("failed to build htslib");
8187
}

hts-sys/htslib

Submodule htslib updated 200 files

hts-sys/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ extern crate libz_sys;
1111
extern crate bzip2_sys;
1212
#[cfg(feature = "lzma")]
1313
extern crate lzma_sys;
14+
#[cfg(feature = "curl")]
15+
extern crate curl_sys;
1416

1517
// include on-the-fly generated bindings
1618
include!(concat!(env!("OUT_DIR"), "/bindings.rs"));

src/bam/buffer.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ impl RecordBuffer {
8686
let to_remove = self
8787
.inner
8888
.iter()
89-
.take_while(|rec| rec.pos() < window_start as i32)
89+
.take_while(|rec| rec.pos() < window_start as i64)
9090
.count();
9191
for _ in 0..to_remove {
9292
self.inner.pop_front();
@@ -111,7 +111,7 @@ impl RecordBuffer {
111111
record.cache_cigar();
112112
}
113113

114-
if pos >= end as i32 {
114+
if pos >= end as i64 {
115115
self.overflow = Some(record);
116116
break;
117117
} else {

0 commit comments

Comments
 (0)