some bindgen wrapper.c objects fail with htslib 1.11 upgrade

[brainstorm]

Related with samtools/htslib#1143 and #274.

[coveralls]

Pull Request Test Coverage Report for Build 365004060

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.05%) to 93.913%

---

Totals
Change from base Build 361447066:	0.05%
Covered Lines:	10800
Relevant Lines:	11500

---

💛 - Coveralls

[brainstorm]

There might be an issue upstream with the OSX runner?: actions-rs/cargo#113

[brainstorm]

The current error is:

error: could not find `Cargo.toml` in `/Users/runner/work/rust-htslib/rust-htslib` or any parent directory

So cross is perhaps targetting the wrong directory?... I guess it should run on /Users/runner/work/rust-htslib instead?

On the other hand, the OSX runner is using exactly the same github action parameters as the rest of the .github/workflows/rust.yml 🤔

[mbargull]

@brainstorm, it is missing the previous steps the other jobs have; most importantly actions/checkout@v2 which is why it doesn't find anything ;).

[brainstorm]

Oh, bummer, thanks for letting me know, I'll fix that in the next few hours (AFK now).

[mbargull]

2020-11-06T12:11:34.8328140Z   cargo:warning=/Users/runner/work/rust-htslib/rust-htslib/target/release/build/hts-sys-f01b434aaaf7d469/out/htslib/sam.c:2349:21: error: implicit declaration of function 'usleep' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
2020-11-06T12:11:34.8329330Z   cargo:warning=                    usleep(10000);
2020-11-06T12:11:34.8329680Z   cargo:warning=                    ^
2020-11-06T12:11:34.8331320Z   cargo:warning=/Users/runner/work/rust-htslib/rust-htslib/target/release/build/hts-sys-f01b434aaaf7d469/out/htslib/sam.c:2369:21: error: implicit declaration of function 'usleep' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
2020-11-06T12:11:34.8332480Z   cargo:warning=                    usleep(10000);
2020-11-06T12:11:34.8332780Z   cargo:warning=                    ^

usleep is also defined in unistd.h. Since this is in sam.c, you probably need samtools/htslib#1145.

[brainstorm]

2020-11-06T12:11:34.8328140Z   cargo:warning=/Users/runner/work/rust-htslib/rust-htslib/target/release/build/hts-sys-f01b434aaaf7d469/out/htslib/sam.c:2349:21: error: implicit declaration of function 'usleep' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
2020-11-06T12:11:34.8329330Z   cargo:warning=                    usleep(10000);
2020-11-06T12:11:34.8329680Z   cargo:warning=                    ^
2020-11-06T12:11:34.8331320Z   cargo:warning=/Users/runner/work/rust-htslib/rust-htslib/target/release/build/hts-sys-f01b434aaaf7d469/out/htslib/sam.c:2369:21: error: implicit declaration of function 'usleep' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
2020-11-06T12:11:34.8332480Z   cargo:warning=                    usleep(10000);
2020-11-06T12:11:34.8332780Z   cargo:warning=                    ^

usleep is also defined in unistd.h. Since this is in sam.c, you probably need samtools/htslib#1145.

@mbargull Good guess, but I don't think so, that commit was included in this PR and I just forwarded the submodule to the latest commit from upstream, same effect:

  = note: /usr/local/Cellar/musl-cross/0.9.9/libexec/bin/../lib/gcc/x86_64-linux-musl/9.2.0/../../../../x86_64-linux-musl/bin/ld: /Users/runner/work/rust-htslib/rust-htslib/target/x86_64-unknown-linux-musl/release/deps/rust_htslib-d7992bb2120bd4d3.rust_htslib.cornzu32-cgu.3.rcgu.o: in function `rust_htslib::bcf::tests::test_remove_alleles':
          rust_htslib.cornzu32-cgu.3:(.text._ZN11rust_htslib3bcf5tests19test_remove_alleles17h583ad6b1a593720dE+0xfd): undefined reference to `_wrap_kbs_init'
          /usr/local/Cellar/musl-cross/0.9.9/libexec/bin/../lib/gcc/x86_64-linux-musl/9.2.0/../../../../x86_64-linux-musl/bin/ld: rust_htslib.cornzu32-cgu.3:(.text._ZN11rust_htslib3bcf5tests19test_remove_alleles17h583ad6b1a593720dE+0x10e): undefined reference to `_wrap_kbs_insert'
          /usr/local/Cellar/musl-cross/0.9.9/libexec/bin/../lib/gcc/x86_64-linux-musl/9.2.0/../../../../x86_64-linux-musl/bin/ld: rust_htslib.cornzu32-cgu.3:(.text._ZN11rust_htslib3bcf5tests19test_remove_alleles17h583ad6b1a593720dE+0x132): undefined reference to `_wrap_kbs_destroy'
          collect2: error: ld returned 1 exit status

[brainstorm]

I believe that the wrapper from PR #107 by @johanneskoester is now interfering with the MUSL static build for htslib 1.11.

EDIT: I guess that we need to regenerate the osx bindgen bindings.

[mbargull]

@mbargull Good guess, but I don't think so, that commit was included in this PR and I just forwarded the submodule to the latest commit from upstream, same effect:

  = note: /usr/local/Cellar/musl-cross/0.9.9/libexec/bin/../lib/gcc/x86_64-linux-musl/9.2.0/../../../../x86_64-linux-musl/bin/ld: /Users/runner/work/rust-htslib/rust-htslib/target/x86_64-unknown-linux-musl/release/deps/rust_htslib-d7992bb2120bd4d3.rust_htslib.cornzu32-cgu.3.rcgu.o: in function `rust_htslib::bcf::tests::test_remove_alleles':
          rust_htslib.cornzu32-cgu.3:(.text._ZN11rust_htslib3bcf5tests19test_remove_alleles17h583ad6b1a593720dE+0xfd): undefined reference to `_wrap_kbs_init'
          /usr/local/Cellar/musl-cross/0.9.9/libexec/bin/../lib/gcc/x86_64-linux-musl/9.2.0/../../../../x86_64-linux-musl/bin/ld: rust_htslib.cornzu32-cgu.3:(.text._ZN11rust_htslib3bcf5tests19test_remove_alleles17h583ad6b1a593720dE+0x10e): undefined reference to `_wrap_kbs_insert'
          /usr/local/Cellar/musl-cross/0.9.9/libexec/bin/../lib/gcc/x86_64-linux-musl/9.2.0/../../../../x86_64-linux-musl/bin/ld: rust_htslib.cornzu32-cgu.3:(.text._ZN11rust_htslib3bcf5tests19test_remove_alleles17h583ad6b1a593720dE+0x132): undefined reference to `_wrap_kbs_destroy'
          collect2: error: ld returned 1 exit status

The build I was referring to definitely didn't have the submodule updated to include the sam.c unistd.h change ;) (samtools/htslib@24389b0...450c912).
But anyway, the error now is of different nature. Maybe you could try to build with the bindgen feature: https://github.com/rust-bio/rust-htslib/blob/v0.33.0/hts-sys/build.rs#L201 . Just a hunch that the symbol names might differ between the pre-generated ones and freshly build ones. (But I'm not versed in this at all, so I'm not not able to pin point the issue and help further, unfortunately.)

[brainstorm]

As suspected/expected, the manually re-generated bindings.rs throw away symbols that are needed (threadpool and faidx-related, see last commit and CI build).

@pmarks, let me know how you generated those last time and how can I generate them now, I'm very curious to know what's going on here now :)

[pmarks]

@brainstorm

You can find the generated bindings in the target/debug/build/hts-sys-<hash>/outs directory after running cargo b --all-features (which enables automatic binding generation)

You can also generate binding with the bindgen CLI. Note -- the latest bindgen rename the isize field of bam1_core_t to isize_, due to this change so we'll need to make those changes and update the bindgen version & bindings in a coordinated change.

Here's the cmd-line:
cargo install bindgen
bindgen --no-doc-comments --blacklist-function strtold --blacklist-type max_align_t wrapper.h -o <appropriate bindings file>

[brainstorm]

I just tried by cargo b --all-features as suggested, but I'm facing the same error with those 3 wrapper kbs methods: https://github.com/rust-bio/rust-htslib/pull/275/checks?check_run_id=1388897739#step:7:1542

You can also generate binding with the bindgen CLI.

Would that alternative way to generate bindings help?

[pmarks]

I think the issue is that those wrapper methods are in hts-sys/wrapper.c and that file isn't in the FILES list here:

rust-htslib/hts-sys/build.rs

Line 18 in 000eed9

const FILES: &[&str] = &[

[pmarks]

LGTM once the build is fixed.

[brainstorm]

I think the issue is that those wrapper methods are in hts-sys/wrapper.c and that file isn't in the FILES list here:

rust-htslib/hts-sys/build.rs

Line 18 in 000eed9

const FILES: &[&str] = &[

But the wrapper.c does not come from htslib itself, it's the bindgen interface for rust-htslib (introduced by #107).

Besides, isn't wrapper.c handled already here, outside of FILES?:

https://github.com/rust-bio/rust-htslib/blob/master/hts-sys/build.rs#L197

[sstadick]

I'm not familiar with the release cycles of rust-htslib, but is there a timeline for the next release that will include these changes?

(I'm not trying to be pushy! I just want to know whether it is worth holding up some releases of my own to wait for it, or to proceed with linux only releases for now.)

[brainstorm]

@johanneskoester Has been the release master so far ;)