cargo-semver-checks
cargo-semver-checks copied to clipboard
Error message on previously nonexistent crate is a bit unhelpful
Steps to reproduce the bug with the above code
Run cargo-semver-checks
on a workspace in which at least one crate has been created since the previous revision.
Actual Behaviour
At the end of its output, you get an error message like this:
Error: possibly due to errors: [
failed to parse /home/nickm/src/arti/target/semver-checks/git-arti_v1_1_5_reconstructed/9e415240a0616f26cc766dabcad14f2f66b1c472/Cargo.toml: no `package` table,
]
Caused by:
package `tor-geoip` not found in /home/nickm/src/arti/target/semver-checks/git-arti_v1_1_5_reconstructed/9e415240a0616f26cc766dabcad14f2f66b1c472
This was confusing to me for several reasons:
- At first, I though that this was a failure in the cargo-semver-checks process, and that I had to add
--exclude tor-geoip
in order to get valid output for my other crates. - I thought that
- It looked like an internal error.
Expected Behaviour
If this message has to appear at the end of the output, I would prefer a message more along the lines of:
Warning: could not find the following crates in the baseline revision.
(This is not a semver problem if the crates did not previously exist.)
* `tor-geoip`
* `tor-keymgr`
If the message can reasonably occur in the middle of the output, I'd prefer something like:
Parsing tor-geoip v0.1.0 (current)
Could not find tor-geoip in baseline!
Cannot check tor-geoip ??? -> 0.1.0: Baseline not found.
Generated System Information
[1082]$ cargo semver-checks --bugreport
#### Software version
cargo-semver-checks 0.22.1
#### Operating system
Linux 6.3.8-200.fc38.x86_64
#### Command-line
```bash
/home/nickm/.cargo/bin/cargo-semver-checks semver-checks --bugreport
cargo version
> cargo -V
cargo 1.70.0 (ec8a8a0ca 2023-04-25)
Compile time information
- Profile: release
- Target triple: x86_64-unknown-linux-gnu
- Family: unix
- OS: linux
- Architecture: x86_64
- Pointer width: 64
- Endian: little
- CPU features: fxsr,sse,sse2
- Host: x86_64-unknown-linux-gnu
### Build Configuration
_No response_
### Additional Context
I'm happy to try hacking this myself if you can point me to the right parts of the code to look at, but I might be a little slow.
Completely agreed that this error message leaves a lot to be desired. Thank you for flagging it and offering to look into it!
There are definitely a few similar edge cases here:
- newly-created lib crate has no previous version on crates.io
- existing crate newly gains a lib target (previously only a bin crate), so the prior version on crates.io has no lib target
- all prior versions of the crate on crates.io have been yanked, so the baseline rustdoc generation fails since it can't generate a lockfile
For each of these, we need (probably in this order):
- a test case
- suitable error handling that detects that these edge cases happened
- a way to propagate this error in the library API of this crate (probably in the
CrateReport
type) - a good error message when that error happens via the CLI
As a separate issue, for --workspace
runs we may perhaps want to write a summary of which crates got checked (w/ outcomes) and which got skipped for which reasons (explicitly excluded, no lib target, no valid baseline for reason XYZ, etc.). This summary could perhaps be generated from the Report
type.
Feel free to tackle as few or as many of these as you'd like. Just please keep PRs small for fast and easy reviews and ask early and often if you feel stuck on anything :)
Pointers to code
For the tests, I'd imagine we'd like to either add them in a file like the existing rustdoc edge cases test, or as a separate GitHub Actions job like the many we already have: https://github.com/obi1kenobi/cargo-semver-checks/blob/main/.github/workflows/ci.yml
For the "bin crate newly gained a lib target" test case, you can use cargo-semver-checks
itself: it first gained a lib target in v0.18.2.
For the code that checks crates.io for prior versions to pick as a baseline, you can find it here. You may need to change the API to return something better than anyhow::Result<PathBuf>
in order to fully communicate the variety of ways something might have gone wrong.
Cargo hit this issue when importing rustfix
into rust-lang/cargo
: https://github.com/rust-lang/cargo/pull/13005#issuecomment-1819817814.
I don't think we want a fix here, but indeed the error message could be better. I am also glad that it failed then silent succeeded :)
@nmathewson any chance you're still interested in taking a look at resolving this issue? Happy to mentor if so!
@nmathewson any chance you're still interested in taking a look at resolving this issue? Happy to mentor if so!
Hi! Theoretically someday, but probably not soon... if anybody else wants to pick this up, they should feel free.
I don't think we want a fix here
When explicitly checking a crate I agree a better error message rather than a fix might be better. But for workspaces when adding a new crate it's inconvenient that it'll cause semver-checks to fail for the first time until the crate is published. It may be more appropriate to be a warning.