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: incorrect dev-dependencies compilation #2024

Closed
wants to merge 1 commit into from
Closed

fix: incorrect dev-dependencies compilation #2024

wants to merge 1 commit into from

Conversation

DeveloperC286
Copy link

Cargo has a bug where it is incorrectly including dependencies/features from dev-dependency.

rust-lang/cargo#1796
rust-lang/cargo#7916

A fix has been applied but it is in nightly and behind a feature flag.

Issue

Cargo picks up serde_derive from serde's dev-dependency and successfully compiles, when it should fail as serde_derive is missing.
A project can then compile, pass all tests etc but when cargo install is used it will fail to compile.

e.g.

  > git clone https://gitlab.com/DeveloperC/cli_chess
  > cd cli_chess
  > git checkout 533102fd5ef73b9ff0bff1f0153ed370ee9ec147
  > cargo build
   Compiling memchr v2.3.4
   Compiling libc v0.2.80
   Compiling proc-macro2 v1.0.24
   Compiling unicode-xid v0.2.1
   Compiling lazy_static v1.4.0
   Compiling log v0.4.11
   Compiling regex-syntax v0.6.21
   Compiling syn v1.0.53
   Compiling quick-error v1.2.3
   Compiling serde_derive v1.0.117
   Compiling cfg-if v0.1.10
   Compiling termcolor v1.1.2
   Compiling serde v1.0.117
   Compiling unicode-width v0.1.8
   Compiling thread_local v1.0.1
   Compiling humantime v1.3.0
   Compiling aho-corasick v0.7.15
   Compiling quote v1.0.7
   Compiling atty v0.2.14
   Compiling terminal_size v0.1.15
   Compiling regex v1.4.2
   Compiling env_logger v0.7.1
   Compiling console v0.13.0
   Compiling pretty_env_logger v0.4.0
   Compiling cli_chess v0.1.0 (/home/rust/Documents/Git/Public/cli_chess)
    Finished dev [unoptimized + debuginfo] target(s) in 9.65s
  > cargo build --release
   Compiling libc v0.2.80
   Compiling memchr v2.3.4
   Compiling proc-macro2 v1.0.24
   Compiling unicode-xid v0.2.1
   Compiling lazy_static v1.4.0
   Compiling syn v1.0.53
   Compiling regex-syntax v0.6.21
   Compiling log v0.4.11
   Compiling serde_derive v1.0.117
   Compiling quick-error v1.2.3
   Compiling cfg-if v0.1.10
   Compiling termcolor v1.1.2
   Compiling serde v1.0.117
   Compiling unicode-width v0.1.8
   Compiling thread_local v1.0.1
   Compiling humantime v1.3.0
   Compiling aho-corasick v0.7.15
   Compiling quote v1.0.7
   Compiling atty v0.2.14
   Compiling terminal_size v0.1.15
   Compiling regex v1.4.2
   Compiling env_logger v0.7.1
   Compiling console v0.13.0
   Compiling pretty_env_logger v0.4.0
   Compiling cli_chess v0.1.0 (/home/rust/Documents/Git/Public/cli_chess)
    Finished release [optimized] target(s) in 9.08s
  > cargo install --version 0.1.0 cli_chess
    Updating crates.io index
  Installing cli_chess v0.1.0
   Compiling libc v0.2.94
   Compiling memchr v2.4.0
   Compiling regex-syntax v0.6.25
   Compiling log v0.4.14
   Compiling quick-error v1.2.3
   Compiling cfg-if v1.0.0
   Compiling serde v1.0.125
   Compiling termcolor v1.1.2
   Compiling unicode-width v0.1.8
   Compiling lazy_static v1.4.0
   Compiling humantime v1.3.0
   Compiling aho-corasick v0.7.18
   Compiling atty v0.2.14
   Compiling termios v0.3.3
   Compiling terminal_size v0.1.16
   Compiling regex v1.5.4
   Compiling env_logger v0.7.1
   Compiling console v0.11.3
   Compiling pretty_env_logger v0.4.0
   Compiling cli_chess v0.1.0
error: cannot find derive macro `Serialize` in this scope
 --> /home/rust/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/cli_chess-0.1.0/src/movement/mod.rs:8:51
  |
8 | #[derive(Debug, PartialEq, Eq, Hash, Clone, Copy, Serialize)]
  |                                                   ^^^^^^^^^

...

error: aborting due to 7 previous errors

error: failed to compile `cli_chess v0.1.0`, intermediate artifacts can be found at `/tmp/cargo-installeZHAVU`

Caused by:
  could not compile `cli_chess`

To learn more, run the command again with --verbose.

This is very deceptive, I have had a number of projects for months which compile and test successfully but unbeknownst to me attempting to install them from crates.io resulted in compilation failure.

Workaround

While you can use nightly and the features flags cargo +nightly test -Z features=dev_dep to fix this issue and get compilation to fail.

e.g.

  > git clone https://gitlab.com/DeveloperC/cli_chess
  > cd cli_chess
  > git checkout 533102fd5ef73b9ff0bff1f0153ed370ee9ec147
  > cargo +nightly build -Z features=dev_dep
warning: flag `-Z features` has been stabilized in the 1.51 release, and is no longer necessary
  The new feature resolver is now available by specifying `resolver = "2"` in Cargo.toml.
  See https://doc.rust-lang.org/nightly/cargo/reference/features.html#feature-resolver-version-2 for more information.

   Compiling serde v1.0.117
   Compiling cli_chess v0.1.0 (/home/rust/Documents/Git/Public/cli_chess)
error: cannot find derive macro `Serialize` in this scope
 --> src/movement/mod.rs:8:51
  |
8 | #[derive(Debug, PartialEq, Eq, Hash, Clone, Copy, Serialize)]
  |                                                   ^^^^^^^^^

...

warning: unused import: `serde::Serialize`
 --> src/movement/mod.rs:3:5
  |
3 | use serde::Serialize;
  |     ^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

...

error: aborting due to 7 previous errors; 5 warnings emitted

error: could not compile `cli_chess`

To learn more, run the command again with --verbose.

I believe serde_derive from serde's dev-dep can just be removed.
Because calling cargo test without the "derive" feature causes code to not be compiled so some tests still fail to compile.
Also no where in the CICD etc is cargo test/build etc called without the "derive" feature.

Removing serde_derive from dev-dependency will cause compilation failure using cargo build(desirable).

  > git diff
diff --git a/Cargo.toml b/Cargo.toml
index 1cbc03d..92eae30 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -15,7 +15,7 @@ categories = ["games", "command-line-utilities"]
 log = "0.4.11"
 pretty_env_logger = "0.4.0"
 console = "0.13.0"
-serde = "1.0.117"
+serde = { path = "../../serde/serde" }

 [dev-dependencies]
  > cargo build
   Compiling libc v0.2.80
   Compiling memchr v2.3.4
   Compiling lazy_static v1.4.0
   Compiling log v0.4.11
   Compiling regex-syntax v0.6.21
   Compiling quick-error v1.2.3
   Compiling cfg-if v0.1.10
   Compiling termcolor v1.1.2
   Compiling serde v1.0.125 (/home/rust/Documents/Git/serde/serde)
   Compiling unicode-width v0.1.8
   Compiling thread_local v1.0.1
   Compiling humantime v1.3.0
   Compiling aho-corasick v0.7.15
   Compiling atty v0.2.14
   Compiling terminal_size v0.1.15
   Compiling regex v1.4.2
   Compiling env_logger v0.7.1
   Compiling console v0.13.0
   Compiling pretty_env_logger v0.4.0
   Compiling cli_chess v0.1.0 (/home/rust/Documents/Git/Public/cli_chess)
error: cannot find derive macro `Serialize` in this scope
 --> src/movement/mod.rs:8:51
  |
8 | #[derive(Debug, PartialEq, Eq, Hash, Clone, Copy, Serialize)]
  |                                                   ^^^^^^^^^

...

warning: unused import: `serde::Serialize`
 --> src/movement/mod.rs:3:5
  |
3 | use serde::Serialize;
  |     ^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

...

error: aborting due to 7 previous errors; 5 warnings emitted

error: could not compile `cli_chess`

To learn more, run the command again with --verbose.

Cargo has a bug where it is incorrectly including dependencies/features from dev-dependency.

rust-lang/cargo#1796
rust-lang/cargo#7916

A fix has been applied but it is in nightly and behind a feature flag.

## Issue
Cargo picks up serde_derive from serde's dev-dependency and successfully compiles, when it should fail as serde_derive is missing.
A project can then compile, pass all tests etc but when `cargo install` is used it will fail to compile.

e.g.
```
  > git clone https://gitlab.com/DeveloperC/cli_chess
  > cd cli_chess
  > git checkout 533102fd5ef73b9ff0bff1f0153ed370ee9ec147
  > cargo build
   Compiling memchr v2.3.4
   Compiling libc v0.2.80
   Compiling proc-macro2 v1.0.24
   Compiling unicode-xid v0.2.1
   Compiling lazy_static v1.4.0
   Compiling log v0.4.11
   Compiling regex-syntax v0.6.21
   Compiling syn v1.0.53
   Compiling quick-error v1.2.3
   Compiling serde_derive v1.0.117
   Compiling cfg-if v0.1.10
   Compiling termcolor v1.1.2
   Compiling serde v1.0.117
   Compiling unicode-width v0.1.8
   Compiling thread_local v1.0.1
   Compiling humantime v1.3.0
   Compiling aho-corasick v0.7.15
   Compiling quote v1.0.7
   Compiling atty v0.2.14
   Compiling terminal_size v0.1.15
   Compiling regex v1.4.2
   Compiling env_logger v0.7.1
   Compiling console v0.13.0
   Compiling pretty_env_logger v0.4.0
   Compiling cli_chess v0.1.0 (/home/rust/Documents/Git/Public/cli_chess)
    Finished dev [unoptimized + debuginfo] target(s) in 9.65s
  > cargo build --release
   Compiling libc v0.2.80
   Compiling memchr v2.3.4
   Compiling proc-macro2 v1.0.24
   Compiling unicode-xid v0.2.1
   Compiling lazy_static v1.4.0
   Compiling syn v1.0.53
   Compiling regex-syntax v0.6.21
   Compiling log v0.4.11
   Compiling serde_derive v1.0.117
   Compiling quick-error v1.2.3
   Compiling cfg-if v0.1.10
   Compiling termcolor v1.1.2
   Compiling serde v1.0.117
   Compiling unicode-width v0.1.8
   Compiling thread_local v1.0.1
   Compiling humantime v1.3.0
   Compiling aho-corasick v0.7.15
   Compiling quote v1.0.7
   Compiling atty v0.2.14
   Compiling terminal_size v0.1.15
   Compiling regex v1.4.2
   Compiling env_logger v0.7.1
   Compiling console v0.13.0
   Compiling pretty_env_logger v0.4.0
   Compiling cli_chess v0.1.0 (/home/rust/Documents/Git/Public/cli_chess)
    Finished release [optimized] target(s) in 9.08s
  > cargo install --version 0.1.0 cli_chess
    Updating crates.io index
  Installing cli_chess v0.1.0
   Compiling libc v0.2.94
   Compiling memchr v2.4.0
   Compiling regex-syntax v0.6.25
   Compiling log v0.4.14
   Compiling quick-error v1.2.3
   Compiling cfg-if v1.0.0
   Compiling serde v1.0.125
   Compiling termcolor v1.1.2
   Compiling unicode-width v0.1.8
   Compiling lazy_static v1.4.0
   Compiling humantime v1.3.0
   Compiling aho-corasick v0.7.18
   Compiling atty v0.2.14
   Compiling termios v0.3.3
   Compiling terminal_size v0.1.16
   Compiling regex v1.5.4
   Compiling env_logger v0.7.1
   Compiling console v0.11.3
   Compiling pretty_env_logger v0.4.0
   Compiling cli_chess v0.1.0
error: cannot find derive macro `Serialize` in this scope
 --> /home/rust/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/cli_chess-0.1.0/src/movement/mod.rs:8:51
  |
8 | #[derive(Debug, PartialEq, Eq, Hash, Clone, Copy, Serialize)]
  |                                                   ^^^^^^^^^

...

error: aborting due to 7 previous errors

error: failed to compile `cli_chess v0.1.0`, intermediate artifacts can be found at `/tmp/cargo-installeZHAVU`

Caused by:
  could not compile `cli_chess`

To learn more, run the command again with --verbose.
```

This is very deceptive, I have had a number of projects for months which compile and test successfully but unbeknownst to me attempting to install them from crates.io resulted in compilation failure.

## Workaround
While you can use nightly and the features flags `cargo +nightly test -Z features=dev_dep` to fix this issue and get compilation to fail.

e.g.
```
  > git clone https://gitlab.com/DeveloperC/cli_chess
  > cd cli_chess
  > git checkout 533102fd5ef73b9ff0bff1f0153ed370ee9ec147
  > cargo +nightly build -Z features=dev_dep
warning: flag `-Z features` has been stabilized in the 1.51 release, and is no longer necessary
  The new feature resolver is now available by specifying `resolver = "2"` in Cargo.toml.
  See https://doc.rust-lang.org/nightly/cargo/reference/features.html#feature-resolver-version-2 for more information.

   Compiling serde v1.0.117
   Compiling cli_chess v0.1.0 (/home/rust/Documents/Git/Public/cli_chess)
error: cannot find derive macro `Serialize` in this scope
 --> src/movement/mod.rs:8:51
  |
8 | #[derive(Debug, PartialEq, Eq, Hash, Clone, Copy, Serialize)]
  |                                                   ^^^^^^^^^

...

warning: unused import: `serde::Serialize`
 --> src/movement/mod.rs:3:5
  |
3 | use serde::Serialize;
  |     ^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

...

error: aborting due to 7 previous errors; 5 warnings emitted

error: could not compile `cli_chess`

To learn more, run the command again with --verbose.
```

I believe serde_derive from serde's dev-dep can just be removed.
Because calling `cargo test` without the "derive" feature causes code to not be compiled so some tests still fail to compile.
Also no where in the CICD etc is cargo test/build etc called without the "derive" feature.

Removing serde_derive from dev-dependency will cause compilation failure using cargo build(desirable).

```
  > git diff
diff --git a/Cargo.toml b/Cargo.toml
index 1cbc03d..92eae30 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -15,7 +15,7 @@ categories = ["games", "command-line-utilities"]
 log = "0.4.11"
 pretty_env_logger = "0.4.0"
 console = "0.13.0"
-serde = "1.0.117"
+serde = { path = "../../serde/serde" }

 [dev-dependencies]
  > cargo build
   Compiling libc v0.2.80
   Compiling memchr v2.3.4
   Compiling lazy_static v1.4.0
   Compiling log v0.4.11
   Compiling regex-syntax v0.6.21
   Compiling quick-error v1.2.3
   Compiling cfg-if v0.1.10
   Compiling termcolor v1.1.2
   Compiling serde v1.0.125 (/home/rust/Documents/Git/serde/serde)
   Compiling unicode-width v0.1.8
   Compiling thread_local v1.0.1
   Compiling humantime v1.3.0
   Compiling aho-corasick v0.7.15
   Compiling atty v0.2.14
   Compiling terminal_size v0.1.15
   Compiling regex v1.4.2
   Compiling env_logger v0.7.1
   Compiling console v0.13.0
   Compiling pretty_env_logger v0.4.0
   Compiling cli_chess v0.1.0 (/home/rust/Documents/Git/Public/cli_chess)
error: cannot find derive macro `Serialize` in this scope
 --> src/movement/mod.rs:8:51
  |
8 | #[derive(Debug, PartialEq, Eq, Hash, Clone, Copy, Serialize)]
  |                                                   ^^^^^^^^^

...

warning: unused import: `serde::Serialize`
 --> src/movement/mod.rs:3:5
  |
3 | use serde::Serialize;
  |     ^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

...

error: aborting due to 7 previous errors; 5 warnings emitted

error: could not compile `cli_chess`

To learn more, run the command again with --verbose.
```
@taiki-e
Copy link
Contributor

taiki-e commented May 8, 2021

This patch does not fix the problem. Cargo ignores the development dependencies of the dependencies when building the dependencies.

The problem is that insta (dev-dependency of cli_chess) enables the derive feature of serde, but the dependencies of cli_chess 0.1.0 do not.
Therefore, this is a bug in cli_chess 0.1.0, not in serde.

but it is in nightly and behind a feature flag.

No. You can fix this problem by enabling the v2 resolver at Rust 1.51+.

Also, you can use the --no-dev-deps or --remove-dev-deps flags provided by cargo-hack if you want to do the same thing with older compilers. Like futures or tokio's CI.

@DeveloperC286
Copy link
Author

Hi @taiki-e thank you for your quick response and the links to all that additional information.

You are correct, removing the Crate insta or adding resolver = "2" in the Cargo.toml will cause the correct feature resolution and the compilation will fail.

I retested trying to build cli_chess using a local version of serde without this patch and it still failed to compile, should have tested that first.

Thanks for you time 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants