Verbose output for cargo upgrade

[djc]

When I run cargo upgrade --compatible ignore --incompatible allow (after running cargo update separately), I get a table of dependencies whose version in Cargo.toml doesn't match the locked version. In this case, all of the new req versions are older than the version requirements. Then, I get a long list of unchanged crates at the end.

IMO the output should focus on things that are actually affected, and thus:

• In the case of --compatible ignore, no rows should be printed if new req is the same as old req
• It would be better to omit the list of unchanged crates, which can be quite long and doesn't provide very useful information

[epage]

I get a table of dependencies whose version in Cargo.toml doesn't match the locked version

We are not looking at the lock file at all and instead doing this based on what is the latest on crates.io. We are making the assumption that the lock file was modified in lock step with the manifest and so those lines would most likely be upgrades that could be performed.

IMO the output should focus on things that are actually affected, and thus:

The output has two goals

• Build trust. I would run cargo upgrade and was unsure if it did anything, if it was working, etc
• Experiment with getting most of the benefits of cargo outdated

This is why we list all unchanged crates; to show what we had considered but skipped. So long as we still show the note, I can see us skipping this one as users will have a hint that some information was elided.

I decided to show rows where there was the potential for upgrade by default. Think of it the other way, when running cargo upgrade. You'll see everything upgraded plus that there are potential upgrades that we did not perform. Its the same case the other way, we are informing users about upgrades we did not perform but could (since we are assuming the lock file is out of date if the manifest is)

[djc]

Well, maybe it should be taking the lock file into account? With the current way it's setup, I get many lines of output none of which are actionable (because my lockfile has already been updated to grab all the latest semver-compatible upgrades). In a single crate setting, it might make more sense to inform users about upgrades we did not perform but could, but at workspace granularity the output is just overwhelming and hard to understand.

[epage]

Well, maybe it should be taking the lock file into account? With the current way it's setup, I get many lines of output none of which are actionable (because my lockfile has already been updated to grab all the latest semver-compatible upgrades).

Was this from running cargo update, deleting the lock file, or the lock file isn't committed?

The current area of exploration is "what if cargo-upgrade replaced cargo-update", so if you run cargo-update, then that case is lower priority for resolving.

[djc]

From running cargo update.

It's still actionable for me because even if there's one command to do everything I would still likely want to update compatible and incompatible versions independently.

[epage]

I feel like this was resolved across #849, #850, and #851, so I'm closing this. If there is something I missed, let me know.

[djc]

Thanks for following up so quickly! Have these changes been released yet? Do you have a plan for releasing them?

[epage]

Its released since it isn't a breaking change. There are some related improvements that I'm holding off on as they are breaking and am wanting more generally settle other breaking discussions.

[djc]

Okay, so here's that I got from cargo-edit 0.11.11:

djc-2021 banned domains $ cargo upgrade --compatible=ignore --incompatible=allow
    Updating 'https://github.com/rust-lang/crates.io-index' index
    Checking virtual workspace's dependencies
name                               old req compatible latest new req
====                               ======= ========== ====== =======
async-compression                  0.3     0.3.15     0.4.0  0.4    
opentelemetry-http                 0.7     0.7.0      0.8.0  0.8    
opentelemetry-semantic-conventions 0.10    0.10.0     0.11.0 0.11   
palette                            0.6.1   0.6.1      0.7.1  0.7.1  

So far, so good! Honestly, for this case I'd still prefer to only get the <old req> -> <new req> lines that cargo update gives me -- IMO the compatible and latest columns are useless to me for this scenario.

    Checking certifier's dependencies
    Checking dns's dependencies
    Checking email's dependencies
    Checking epoxide's dependencies
    Checking epp's dependencies
    Checking instagram's dependencies
    Checking linktree's dependencies
    Checking macros's dependencies
    Checking migrate's dependencies
    Checking proteus's dependencies
    Checking proxy's dependencies
    Checking rde's dependencies
    Checking smtp's dependencies
    Checking store's dependencies
    Checking stripe's dependencies
    Checking tiktok's dependencies
    Checking utils's dependencies
    Checking whois's dependencies

This seems overly verbose and unnecessary?

   Upgrading recursive dependencies
    Updating bumpalo v3.12.1 -> v3.12.0
    Updating libc v0.2.143 -> v0.2.144
    Updating quote v1.0.26 -> v1.0.27
    Updating serde v1.0.162 -> v1.0.163
    Updating serde_derive v1.0.162 -> v1.0.163
    Updating tokio v1.28.0 -> v1.28.1

This shouldn't happen -- I requested --compatible=ignore.

note: Re-run with `--verbose` to show more dependencies
  compatible: 34 packages

I think the compatible line should just be skipped, there's no useful information here.

  git: instant-epp, instant-smtp, rdap_types

This seems useful. At the same time, here I would be inclined to put them in the table, because one consistent way of giving me the data is better than having two separate ways.

  latest: 44 packages

This also seems low value.

  local: 12 packages

I also don't think there's much value here.

[epage]

IMO the compatible and latest columns are useless to me for this scenario.

As it runs in multiple modes, I think consistency in modes is important for processing the information

Checking certifier's dependencies
...

This seems overly verbose and unnecessary?

The main value for this is

• building confidence that we are upgrading everything. I think between some bugs and confusing UX, in the past I had questions of what cargo upgrade was upgrading
• Making it clear to the user that this is a workspace-level command (less common), rather than a package-level command (more common)
• If we keep the table, finding the right way to frame the message so people don't misunderstand when a crate isn't listed in one of these lines

This shouldn't happen -- I requested --compatible=ignore.

This was the breaking change that I referred to holding off on

compatible: 34 packages
I think the compatible line should just be skipped, there's no useful information here.

...

latest: 44 packages
This also seems low value.

I held off on completely not showing it when I remembered the coalescing conversation during MSRV to more slowly change this because

• I also got feedback from others about not skimping on the output
• This is also about building confidence with the users that this is doing the right thing

git: instant-epp, instant-smtp, rdap_types
This seems useful. At the same time, here I would be inclined to put them in the table, because one consistent way of giving me the data is better than having two separate ways.

Except we can't really show much about git because we don't process updating these dependencies and this takes up much less space

Something that might not be obvious is that I made the max number of crates to show before coalescing to be 3 or 4. That is something I'm willing to tweak (originally, I always coalesced).

I also don't think there's much value here.

I feel the same about this as about git.

I'll have to think about hiding these two when verbosity is off (my automatic solution).

[djc]

The main value for this is

* building confidence that we are upgrading everything.  I think between some bugs and confusing UX, in the past I had questions of what `cargo upgrade` was upgrading
* Making it clear to the user that this is a workspace-level command (less common), rather than a package-level command (more common)
* If we keep the table, finding the right way to frame the message so people don't misunderstand when a crate isn't listed in one of these lines

cargo update doesn't output this and works on a workspace basis before. I think the workings of this command should be as analogous to cargo update as possible.

This was the breaking change that I referred to holding off on

Ah, I was not aware of this because in non-testing scenarios I only run cargo upgrade after running cargo update.

[djc]

I held off on completely not showing it when I remembered the coalescing conversation during MSRV to more slowly change this because

* I also got feedback from others about not skimping on the output
* This is also about building confidence with the users that this is doing the right thing

Counter arguments here: in the vast majority of cases, the Cargo.toml files this is changing are under source control, so reverting them is trivial. There's also already a --dry-run for users who are feeling more cautious.

[epage]

cargo update doesn't output this and works on a workspace basis before. I think the workings of this command should be as analogous to cargo update as possible.

cargo update works on the Cargo.lock file. cargo upgrade could be implemented to work like cargo check (runs on current package, could accept --workspace) and originally was, so I think its helpful to help set those expectations.

Counter arguments here: in the vast majority of cases, the Cargo.toml files this is changing are under source control, so reverting them is trivial. There's also already a --dry-run for users who are feeling more cautious.

It wasn't about doing too much but too little.