refactor: introduce cargo as a dependency

[Frederick888]

So this is the version with cargo as a dependency, which tends to be a "more native" one as suggested in rust-lang/cargo#4309.

Changes:

1. Introduces cargo as a dependency
2. Supports cargo workspaces (closes Support cargo workspaces #28)
3. Supports embedded dependencies, see issuecomment-320433502 (fixes doesn't work when dependency is in path #50)
4. Replaces RM with Removed (closes What does "RM" mean in the output? #46)
5. Build/development dependencies are included (closes Provide option to check build dependencies as well #20, fixes cargo outdated just says 'RM' for everything. #49)
6. Target-specific dependencies are included
7. Feature-related --all-features --features --no-default-features as other cargo subcommands
8. New output format

There might should still be some bugs and some implementations are a bit silly, but it's already fine to me for a new release. And by the way @kbknapp, is it ok to add me to the author list as well?

Example 1

Run cargo outdated -r openssl against rust-lang/cargo@a60d185c (0.20.0)

Name                      Project  Compat   Latest   Kind    Platform
gdi32-sys->winapi         0.2.8    Removed  Removed  Normal  ---
gdi32-sys->winapi-build   0.1.1    Removed  Removed  Build   ---
openssl                   0.9.13   0.9.15   0.9.15   Normal  cfg(unix)
openssl->libc             0.2.23   0.2.29   0.2.29   Normal  ---
openssl->openssl-sys      0.9.13   0.9.15   0.9.15   Normal  ---
openssl-sys->gcc          0.3.50   0.3.51   0.3.51   Build   ---
openssl-sys->gdi32-sys    0.2.0    Removed  Removed  Normal  cfg(windows)
openssl-sys->libc         0.2.23   0.2.29   0.2.29   Normal  ---
openssl-sys->user32-sys   0.2.0    Removed  Removed  Normal  cfg(windows)
user32-sys->winapi        0.2.8    Removed  Removed  Normal  ---
user32-sys->winapi-build  0.1.1    Removed  Removed  Build   ---

Example 2

Run cargo outdated -d 3 against Manishearth/rust-clippy@681a303

Name                    Project  Compat  Latest   Kind         Platform
clippy_lints->semver    0.6.0    ---     0.7.0    Normal       ---
duct                    0.8.2    ---     0.9.1    Development  ---
duct->error-chain       0.8.1    ---     Removed  Normal       ---
duct->shared_child      0.2.1    ---     0.3.2    Normal       ---
error-chain->backtrace  0.3.2    ---     Removed  Normal       ---

Example 3

Run cargo outdated --features color against https://gist.github.com/Frederick888/306bf41c0aef2f4ec409add082ee5d0b

Name             Project  Compat  Latest   Kind         Platform
clap             2.20.0   2.20.5  2.26.0   Normal       ---
clap->bitflags   0.7.0    ---     0.9.1    Normal       ---
clap->libc       0.2.18   0.2.29  Removed  Normal       ---
clap->term_size  0.2.1    0.2.3   0.3.0    Normal       ---
clap->vec_map    0.6.0    ---     0.8.0    Normal       ---
num_cpus         1.6.0    ---     1.6.2    Development  ---
num_cpus->libc   0.2.18   0.2.29  0.2.29   Normal       ---
pkg-config       0.3.8    0.3.9   0.3.9    Build        ---
term             0.4.5    ---     0.4.6    Normal       ---
term_size->libc  0.2.18   0.2.29  0.2.29   Normal       cfg(not(target_os = "windows"))

[alexcrichton]

Thanks for working on this @Frederick888!

I ran this locally and got a bunch of lines that look like:

url->percent-encoding   1.0.0    ---     Removed  Normal  ---

This was inside of rustup.rs, but I was wondering if maybe you know what's going on there? (specifically the "Removed" bit)

[Frederick888]

@alexcrichton It means that a node in the dependency tree is removed after an update.

In the case of rustup, I deem it's because after the semver requirements of module download are replaced by wildcards, hyper is updated to 1.11.x and url is no longer a dependency of hyper, hence the whole sub tree is marked as Removed

---

Edit: Actually I think printing out a JSON or even a tree like cargo-tree could make much more sense in this case. But that's kinda the last piece of improvement. I am keener on getting rid of the structs e.g. Manifest to make it more native for the time being.

[alexcrichton]

Ah ok! 👍

[Frederick888]

@alexcrichton Actually I wonder whether I could replace Manifest with TomlManifest or not to modify the semver requirements as the fields of TomlManifest are private...

Is it possible to modify the manifest through cargo and write it back?

[alexcrichton]

Unfortunately writing back a manifest may not be possible right now with Cargo's public API, but if you've got suggestion for modifying Cargo's API we're always more than willing to take PRs!

[SergioBenitez]

I tried running this on a few of my projects. First, there appears to be some issues running this against Rocket. When I run this inside Rocket's lib/, I get:

Rocket/lib ❯ cargo outdated
error: failed to read `/var/folders/jg/0xyyp7ys2752_121dvfp4nrm0000gn/T/cargo-outdated.RAsybF6nEGgy/Cargo.toml`

Caused by:
  No such file or directory (os error 2)

I then tried running this against a project with some outdated dependencies:

rocket = "0.2.2"
rocket_codegen = "0.2.2"
rand = "0.3"

I was expecting for the output to contain three lines, one for each outdated dependency, but instead the output contained 87 (!) lines, apparently one for each outdated transitive dependency. This seems like bad default behavior. There's nothing I can do about outdated transitive dependencies, so showing me those as outdated by default only makes it hard to see what I can control. I suggest showing all outdated transitive dependencies optionally and disabling display by default. Perhaps by default you can display a message saying something like, "Outdated transitive dependencies detected. For the full list, run cargo outdated --transitive."

[Keats]

Since it was generally accepted to add a native cargo outdated to Cargo (rust-lang/cargo#4309), would it make more sense to do a PR on Cargo directly?

I would also like to hide the transitive dependencies completely by default and maybe add a flag to show them: it just pollutes the output imo.
Is it possible to get the repo url from the manifest/Cargo for a crate? I use that all the time in yarn output to look at the changelog before updating.

Nitpick: Project -> Current for the header

[Frederick888]

@SergioBenitez Ah, I think I know what the problem is. Before using cargo the lib/ was treated as the project root but now cargo finds the real project root automatically. I'd try to make some time to fix it.

In terms of the default behaviour, showing the transitive dependencies is actually the default one of some other package managers as well, e.g. composer. But personally I reckon it's more intuitive to print only the direct ones by default. However my concern is that this could potentially introduce some inconsistencies. For example, if the users selects another root via -r, I deem that transitive dependencies should then be shown in this case. Sometimes too many defaults and hypotheses from developers would make it unwieldy instead.

---

@Keats I think it's still a bit early to merge this function to the development cycle of cargo. It's currently much too immature and need more feedback from users to be stabilised.

[Frederick888]

@SergioBenitez I have just pushed a fixed to your issue here. Could you give it a try?

[mitskevich]

I'm getting following error with freshly compiled feature/cargo-depend branch:

$ RUST_BACKTRACE=1 cargo outdated -v
  Parsing... current workspace
Resolving... current workspace
  Parsing... compat workspace
thread 'main' panicked at 'byte index 45 is out of bounds of `/Users/sam/Playground/project/tokio-modbus`', src/libcore/str/mod.rs:2171:8
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
   1: std::panicking::default_hook::{{closure}}
   2: std::panicking::default_hook
   3: std::panicking::rust_panic_with_hook
   4: std::panicking::begin_panic_new
   5: std::panicking::begin_panic_fmt
   6: rust_begin_unwind
   7: core::panicking::panic_fmt
   8: core::str::slice_error_fail
   9: core::str::traits::<impl core::slice::SliceIndex<str> for core::ops::range::RangeTo<usize>>::index::{{closure}}
  10: <core::option::Option<T>>::unwrap_or_else
  11: core::str::traits::<impl core::slice::SliceIndex<str> for core::ops::range::RangeTo<usize>>::index
  12: core::str::traits::<impl core::ops::index::Index<core::ops::range::RangeTo<usize>> for str>::index
  13: cargo_outdated::cargo_ops::temp_project::manifest_paths::manifest_paths_recursive
  14: cargo_outdated::cargo_ops::temp_project::manifest_paths::manifest_paths_recursive
  15: cargo_outdated::cargo_ops::temp_project::manifest_paths
  16: cargo_outdated::cargo_ops::temp_project::TempProject::from_workspace
  17: cargo_outdated::execute
  18: cargo_outdated::main
  19: __rust_maybe_catch_panic
  20: std::rt::lang_start
  21: main

My Cargo.toml:

[package]
name = "modbus-operator"
version = "0.1.0"

[dependencies]
bodyparser = "0.7"
bytes = "0.4"
chrono = { version = "0.4", features = ["serde"] }
clap = "2.23"
colored = "1.4"
decimal = "1.0"
env_logger = "0.4"
error-chain = "0.10"
eui48 = "0.3"
futures = "0.1"
futures-cpupool = "0.1"
iron = "0.5"
log = "0.3"
num = "0.1"
postgres = { version = "0.15", features = ["with-chrono"] }
postgres-derive = "0.3"
r2d2 = "0.7"
r2d2_postgres = "0.13"
rand = "0.3"
router = "0.5"
serde = "1.0"
serde_derive = "1.0"
serde_json = "1.0"
tera = "0.10"
time = "0.1"
tokio-core = "0.1"
tokio-io = "0.1"
tokio-modbus = { path="../tokio-modbus" }
# change back to version once 0.1.2 is out
tokio-proto = { git = "https://github.com/tokio-rs/tokio-proto", rev = "8fb8e482dcd55cf02ceee165f8e08eee799c96d3" }
tokio-service = "0.1"
tokio-timer = "0.1"
toml = "0.4"
percent-encoding = "1.0"

System: MacOS Sierra
Cargo: cargo 0.21.0 (5b4b8b2ae 2017-08-12)
Rustc: rustc 1.20.0 (f3d6973f4 2017-08-27)

UPDATE:

I made a quick fix in temp_project.rs:

diff --git a/src/cargo_ops/temp_project.rs b/src/cargo_ops/temp_project.rs
index ca291bf..7146d78 100644
--- a/src/cargo_ops/temp_project.rs
+++ b/src/cargo_ops/temp_project.rs
@@ -307,7 +307,7 @@ fn manifest_paths(elab: &ElaborateWorkspace) -> CargoResult<Vec<PathBuf>> {
         if !members.contains(pkg_id) {
             let pkg = &elab.pkgs[pkg_id];
             let pkg_path = pkg.root().to_string_lossy();
-            if &pkg_path[..workspace_path.len()] == workspace_path {
+            if pkg_path.len() > workspace_path.len() && &pkg_path[..workspace_path.len()] == workspace_path {
                 manifest_paths.push(pkg.manifest_path().to_owned());
             }
         }

And now I'm getting this error:

$ RUST_BACKTRACE=1 cargo outdated -v
  Parsing... current workspace
Resolving... current workspace
  Parsing... compat workspace
 Updating... compat workspace
error: failed to load source for a dependency on `tokio-modbus`

Caused by:
  Unable to update file:///var/folders/xl/7ggvf3xn6n55sw105l0mkq5w0000gn/T/tokio-modbus

Caused by:
  failed to read `/var/folders/xl/7ggvf3xn6n55sw105l0mkq5w0000gn/T/tokio-modbus/Cargo.toml`

Caused by:
  No such file or directory (os error 2)

[SergioBenitez]

@Frederick888 Okay, that seemed to fix the problem. A couple of notes/questions, however:

1. What does "supporting workspaces" mean?

   My expectation was that I could run cargo outdated at the root of a workspace and have the tool check all dependencies used by any crate in the workspace. Instead, I get: "error: manifest path /Rocket/Cargo.toml is a virtual manifest, but this command requires running against an actual package in this workspace". Is there a way to get the functionality I'm looking for? If not, what workspace support has been added?

2. Why does running cargo outdated result in crates being downloaded?

   When I run cargo outdated, the tool downloads some crates. What's going on there? Why is this happening? Maybe there's a good reason, and if there is, some message indicating the reason should be printed.

3. --all-features should be enabled by default

   Because I'm using this tool to find outdated dependencies, it's very unlikely that I somehow don't care about optional dependencies. Much like my previous comment about --depth 1 being the default, I think --all-features should be the default as well. It's what I'm using the tool for, and the tool shouldn't make it harder to get that done.

[Frederick888]

@mitskevich Thanks for the fix. I modified it a bit and have pushed a new commit here.

The new error seems to be caused by an uncopied manifest file, i.e. tokio-modbus/Cargo.toml in your case. But I wasn't able to reproduce the error locally. I tried it on Rocket, which has also such a dependency and it worked totally fine. Is it possible for you make a more complete example?

---

@SergioBenitez

1. You still need to enter the specific crate root to check the updates. Previously cargo-outdated would completely ignore the workspace and member stuff so it crashed in such scenarios. But yeah, it makes sense to loop through all the members and possibly even better to have a Member column in the output.

2. cargo-outdated was designed to run cargo update against the project to check updates so it's cargo update that's downloading the crates. Maybe it doesn't sound quite programmatic but the idea originally from @kbknapp is fairly concise and works great. Personally I'm not very familiar with the internal stuff of cargo and perhaps there's a better way to query the registry...? (it's time to @alexcrichton lol) But definitely the messages from cargo update should not be suppressed when -v is used. I'd fix it asap.

3. Great suggestion, thanks! And for the default depth, actually there is a discussion going on here: Default to only showing direct dependencies #53

[Keats]

1. You still need to enter the specific crate root to check the updates. Previously cargo-outdated would completely ignore the workspace and member stuff so it crashed in such scenarios. But yeah, it makes sense to loop through all the members and possibly even better to have a Member column in the output.

How about changing grouping by member? An output like the following for one of my projects:

site
================
Name          Current  Compat  Latest    Kind         Platform
clap             2.20.0     2.20.5    2.26.0   Normal       ---

rendering
================
Name          Current  Compat  Latest    Kind         Platform
clap             2.20.0     2.20.5    2.26.0   Normal       ---

Adding a column doesn't necessarily make sense as some members might have the same packages with different versions for some reasons

[Frederick888]

@Keats That's an awesome idea! I like it.

[mitskevich]

@Frederick888 Here is an example that consistently breaks latest version of cargo outdated for me (with the same error as in my previous comment): https://github.com/mitskevich/cargo-outdated-example. Error is happening only when I run it in ./modbus-operator project folder. When I remove tokio-modbus dependency, cargo outdated works correctly.

If you will not be able to reproduce it, please let me know, I'll check what is happening in more details later.

[Frederick888]

@mitskevich Ah, I see. Currently a Cargo.toml would be copied to the temporary directory if:

1. it's in the sub-folder of the working one, e.g. a relative path foo_bar = { path = "local_dependency/foo_bar" } dependency
2. it's in the same workspace of the working one

Otherwise there's simply no relationship between modbus-operator and tokio-modbus in your case, hence cargo won't take it as a member of the project. I suggest you create a virtual manifest in their parent directory, or use absolute path instead for now. And this does warn me of some other possible issues, e.g. path = "local_dependency/../../../../../somewhere".

Perhaps I should resolve all the relative paths to absolute ones first so all such craps could be gone.

[Frederick888]

@mitskevich I think your issue should be fixed by a8491e0. Could you please check it?

[mitskevich]

@Frederick888 Yep, cargo-depend branch build works correctly on all my projects now. Thanks!

[Frederick888]

So as it stands now, most of the discovered "bugs" have been fixed in this PR. Since other ongoing discussions in this thread are basically related to improvements and new features, I think I'd merge this PR, tag a new release (although I can't publish it to crates.io) and file independent issues for them. I would be a bit busy in the next one or two months and this could make it more friendly to potential PRs from others.