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

Compile error when depending on rust-bio HEAD/master #201

Closed
RagnarGrootKoerkamp opened this issue Nov 30, 2021 · 7 comments
Closed

Compile error when depending on rust-bio HEAD/master #201

RagnarGrootKoerkamp opened this issue Nov 30, 2021 · 7 comments

Comments

@RagnarGrootKoerkamp
Copy link
Contributor

I'm getting this error when building rust-bio-tools against the latest version of rust-bio:

error[E0277]: the trait bound `rust_htslib::bam::Record: SequenceRead` is not satisfied
   --> src/bam/collapse_reads_to_fragments/calc_consensus.rs:190:10
    |
190 | impl<'a> CalcConsensus<'a, bam::Record> for CalcOverlappingConsensus<'a> {
    |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `SequenceRead` is not implemented for `rust_htslib::bam::Record`
    |
   ::: src/common.rs:14:32
    |
14  | pub trait CalcConsensus<'a, R: SequenceRead> {
    |                                ------------ required by this bound in `CalcConsensus`

It looks like SequenceRead is implemented for a fastq record, but not for bam records.

@FelixMoelder
Copy link
Member

Hey @RagnarGrootKoerkamp, I just updated to the latest version of rust-bio but could not reproduce your issue.
There were a few breaking changes that needed to be fixed (see #202) but your error was not showing up.
I also set bio = {git ="https://github.com/rust-bio/rust-bio", branch="master"} in Cargo.toml but that also works well.
Could you give some more details to reproduce this? Have you also changed the version of rust-htslib?

@RagnarGrootKoerkamp
Copy link
Contributor Author

RagnarGrootKoerkamp commented Dec 1, 2021

Oh I see. It actually seems to be using rust-bio-types at HEAD. Sorry for the confusion:

bio-types = {git ="https://github.com/rust-bio/rust-bio-types", branch="master"}

Gives the error, but bio-types = "0.12.1" does not.

Edit: Huh; this doesn't make sense, since the bump to 0.12.1 is the last commit. I'll investigate more. -> Still, this is exactly the change that causes the problem. This is using bio = "0.39" as you just added it.

@FelixMoelder
Copy link
Member

FelixMoelder commented Dec 1, 2021

That's indeed a bit weird.
As you say it works when setting bio-types = "0.12.1" but not when setting the git branch.
The SequenceRead trait is implemented in rust-htslib (https://github.com/rust-bio/rust-htslib/blob/a21aff289bc03c7549afc7a958084ed57e8c93f2/src/bam/record.rs#L1114) and has not been touched for 3 years.
Also the release history in rust-htslib is confusing and seems to be messed up.

Why exactly are you trying to use the git-repo directly? I assume you are trying to depend on the branch of your current PR in rust-bio-types?

@johanneskoester Any idea on this?

@RagnarGrootKoerkamp
Copy link
Contributor Author

Yes, I have open PRs on all of rust-bio-types, rust-bio, and rust-bio-tools now, so I'm using bio = {path = "../rust-bio"} and bio-types={path="../rust-bio-types"} locally in the Cargo.toml files. (I'm not actually using the git versions; local versions are easier, but have the same problem.)

If there is a better solution that'd be nice anyway, since it's a bit tedious to keep a local version of Cargo.toml outside of git.

@RagnarGrootKoerkamp
Copy link
Contributor Author

Ah, maybe the issue is that when using the github version, there are somehow multiple definitions of SequenceRead, coming from different versions of bio-types? I don't have enough experience with Rust to know how managing duplicate package dependencies work, but it could be that it doesn't collapse a git dependency to other versions.

@RagnarGrootKoerkamp
Copy link
Contributor Author

RagnarGrootKoerkamp commented Dec 1, 2021

Looking at cargo tree -d suggest that duplicate versions when using the git branch is indeed the issue.

bio-types v0.12.1
├── bio v0.39.0 (*)
└── rust-htslib v0.38.2 (*)

bio-types v0.12.1 (https://github.com/rust-bio/rust-bio-types?branch=master#d78b8ca4)
└── rust-bio-tools v0.32.0 (.../git/rust-bio-tools)

There's rust-lang/rust#89143 to improve the error message to hint that there may be multiple versions.

Here's a report of someone else running into the same problem, with possible fixes: https://github.com/Malax/libcnb.rs/issues/90, in case we'd like to avoid this problem in the future.

@RagnarGrootKoerkamp
Copy link
Contributor Author

Here's the 'proper' way to use local versions of crates, for future reference:

Just add this at the bottom of Cargo.toml:

[patch.crates-io]
bio = { path = "../rust-bio" }
bio-types = { path = "../rust-bio-types" }

You'll still need to not commit that hunk, but at least it's in one place, and it overrides the bio dependency everywhere where it uses the same major version (if I understand the docs correctly).

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

No branches or pull requests

2 participants