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

cargo build = spurious errors, prevent cargo from merging the same dependency with different features #4323

Closed
Dushistov opened this issue Jul 25, 2017 · 7 comments

Comments

@Dushistov
Copy link

Dushistov commented Jul 25, 2017

Steps to reproduce:

$ cargo new feature_merge
$ cargo add nmea
$ cargo check > /dev/null 2>&1 && echo "success"
success
$ cargo add cexpr
$ cargo check > /dev/null 2>&1 || echo "failed"
failed
$ cargo rm cexpr && cargo check > /dev/null 2>&1 && echo "success"
success

I remove/add the dependency without any code modification and this influences the build result.

The source of problem is that cexpr depends on nom like this:

nom = {version = "^3", features = ["verbose-errors"] }

while nmea describes the dependency like this:

nom = "3.1.0"

even if change nmea's way to specify dependency to this:

nom = { version = "3.1.0", features = [] }

I still got compile time error.

This cause a very strange behavior that I desribed here:

you built crate X (cargo build return OK), then you go to
crate Y, add [dependecy.crate X] path = path/to/crate X, run cargo build
and your build failed, you try to understand what is going on, go to path/to/crate X
run cargo build and again all Ok.

Any way to prevent such kind of errors?

Related SO questions 1 2

@alexcrichton
Copy link
Member

Can you gist the errors involved here or perhaps the manifests involved? This may be a case of buggy upstream crates? From a first read it's not clear that this is a bug in Cargo at least

@Dushistov
Copy link
Author

Dushistov commented Jul 25, 2017

@alexcrichton

Can you gist the errors involved here or perhaps the manifests involved?

Not sure what you want,
I attached result of commands in the first post:

cargo new feature_merge && cd feature_merge && cargo add nmea && cargo add cexpr && cargo check

feature_merge.zip

it produces (if you run cargo check) such errors:

error[E0599]: no method named `description` found for type `nom::Err<&[u8]>` in the current scope
   --> /home/evgeniy/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/nmea-0.0.6/src/parse.rs:100:44
    |
100 |                      IError::Error(e) => e.description().to_string(),
    |                                            ^^^^^^^^^^^
    |
    = help: items from traits can only be used if the trait is in scope
    = note: the following trait is implemented but not in scope, perhaps add a `use` for it:
            candidate #1: `use std::error::Error;`

if you remove cexpr from dependicies it builds without errors.

From a first read it's not clear that this is a bug in Cargo at least

I try to reformulate: author of cexpr crate build, test and publish its crate,
I as mantainer of nmea crate build, test and publish my crate.

Somebody combines cexpr + nmea without any call of any line from these crates,
just add them to dependicies and get compile time errors.

Is it job of cargo to combine working crate A + working crate B (without any system dependicies, just pure Rust) and produce working crate C,
or at least report clear error?

@Dushistov
Copy link
Author

In other words you have

  1. crate Main that depends on A and B
  2. A depends on nom with default features = ["std", "stream"]
  3. B depends on nom with features = ["verbose-errors"]

these obviously mean that A not tested (and potentionaly not compiled) with nom with not default features, why then give nom to A with features = ["verbose-errors"]?

@kennytm
Copy link
Member

kennytm commented Jul 25, 2017

This should be a bug in nom. Features are supposed to be additive, so adding a feature shouldn't make a downstream crate fail to compile. ~cargo has done nothing wrong~

The problem here is that, when using "simple errors", the type nom::simple_errors::Err<E> is simply a type alias of nom::ErrorKind<E>, while with "verbose errors" the type nom::verbose_errors::Err<E> is a dedicated enum, so the "with-feature" and "without-feature" interfaces are incompatible.

nom fixing this may require a major version bump to 4.0 since API is changed, which means neither nmea nor cexpr can automatically pick up the fix 🤷‍♂️.


BTW if you change all e.description().to_string() to e.to_string() then nmea can be compiled with nom/verbose-errors feature.

@Dushistov
Copy link
Author

Dushistov commented Jul 25, 2017

@kennytm

Features are supposed to be additive

Thanks, I don't know this, is it described somewhere?

The problem here is that

The problem for me as a user that it is very hard to find source of problem,
here my initial question, I need a day (basicaly of course because of slow rustc) to find that cexpr cause compile time error (I comment code, remove crate dependency, rebuild and repeat loop).

It would be nice from cargo to report what features set it used for each dependency at least.
Using such functionality, you just compare two variants (with build failure and without) and see what is wrong.

@kennytm
Copy link
Member

kennytm commented Jul 25, 2017

@Dushistov

Thanks, I don't know this, is it described somewhere?

Unfortunately it is currently some sort of "hidden rule" and is not documented loudly. This is a known problem.

There was rust-lang/rfcs#1841 trying to enforce this on crates.io, but is eventually closed since banning a crate is too aggressive and prone to error. The idea behind is still valid though.

It would be nice from cargo to report what features set it used for each dependency at least.

For now, you extract the information via cargo check -v:

$ cargo check -v
...
   Compiling nom v3.2.0
     Running `rustc --crate-name nom «snip» --cfg 'feature="default"' --cfg 'feature="memchr"' --cfg 'feature="std"' --cfg 'feature="stream"' «snip»`

we know that the set of features enabled for nom is ["default", "memchr", "std", "stream"].

@Dushistov
Copy link
Author

@kennytm

Thanks, I missed that cargo check -v prints features.

Not usability frieandly, but shell tools should fix this complexity for me,
and I going to ask feature with features from sfackler/cargo-tree

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

3 participants