Emit note when --future-incompat-report had nothing to report

[Aaron1011]

No description provided.

[rust-highfive]

r? @ehuss

(rust-highfive has picked a reviewer for you, use r? to override)

[alexcrichton]

I would personally think that this wasn't necessary (e.g. rustc doesn't print anything saying "there were 0 warnings"). Could you elaborate a bit on why this should be added? (having a blank PR description and a commit message which is simply the PR title isn't too helpful...)

[Aaron1011]

Sorry, I forgot to link to the discussion on the tracking issue: rust-lang/rust#71249 (comment) and rust-lang/rust#71249 (comment)

Both @ehuss and @jyn514 had wanted an explicit '0 future-incompat warnings' message when --future-incompat-report is passed, to make it clear that the command is working.

[alexcrichton]

Ok thanks for the link, if it's ok with y'all I figure we can discuss it here rather than continuing on the issue over there?

In my opinion there should be no need to print something out like this (e.g. rustc doesn't print out "0 warnings were found"). Could y'all elaborate, though, as to why you feel like a message is necessary? I could see that while testing you might expect it to have a bug and not work, but for a final form of the feature which is expected to work I would think that no output is translateable to "nothing that I need to do"

[jyn514]

I could see that while testing you might expect it to have a bug and not work, but for a final form of the feature which is expected to work I would think that no output is translateable to "nothing that I need to do"

My understanding is that the final product won't have this option at all, and will print out future-incompat warnings unconditionally. I agree at that point printing "0 errors" isn't helpful. But I think explicitly opting into the warnings should confirm that yes it works.

[ehuss]

I believe the (stable) experience is intended to be:

1. Use runs cargo build. This prints a notice that there are future-incompat warnings.
2. User has two options here:
   2a. Run cargo describe-future-incompatibilities --id=xyz
   2b. Run cargo build --future-incompat-report

The concern is someone runs cargo build --future-incompat-report, and it doesn't display anything, that could be confusing.

Personally, I still think the --future-incompat-report flag doesn't seem like the right UX. I'm not entirely clear what the use case is for that where the report command doesn't suffice.

If the intent is that people always pass --future-incompat-report, then yea, printing 0 doesn't make sense. But I don't know why someone would always pass that flag.

[Aaron1011]

I'm not entirely clear what the use case is for that where the report command doesn't suffice.

The use case is running this on CI, which may build dependencies that a user machine typically does not (e.g. platform-specific dependencies like winapi). Instead of making scripts try to extract the report id from a human-readable message, we provide a flag that can be added to every command invocation.

[alexcrichton]

I was not under the impression that Cargo would print "you have future incompat warnings" by default. I thought that users would have to opt-in to the notifications. Seems like it would be best to clarify that point about the expected usage before continuing this discussion since that likely heavily influences this!

[ehuss]

Sorry about the confusion. RFC 2834 describes the behavior where Cargo will always check for future-incompat warnings and print a small notice informing the user. The user can then run the describe command to get a full report. The intent is to warn the developer that one of their dependencies may stop compiling soon, and they need to do something about that. If it isn't enabled all the time, it likely won't have the intended impact of getting projects updated.

A concern is the inability to silence the warning, since it may take a while to resolve it. The RFC included an "Annoyance Modulation", but I'm not sure if that specifically is how it should work.

[alexcrichton]

Ok thanks for the link, I had forgotten this part of the original RFC. If that's the case then this seems fine since it'll likely be rarely run anyway (seems like the majority of what happens is you run cargo build and get a short message to run cargo describe-future-incompat at the end).

[ehuss]

I'm going to go ahead and merge this. I'm also uncertain about the overall UX here, but I think this will be less confusing.

@bors r+

[bors]

📌 Commit f9512ff has been approved by ehuss

[bors]

⌛ Testing commit f9512ff with merge 58a9613...

[bors]

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing 58a9613 to master...