Fixing an error when the crate's name mismatches it will panic #170
Conversation
… unwrap(), instead we can use unwrap_or() to avoid being panic.
| self.name | ||
| == krates | ||
| .get_package_name_from_cargo_metadata_package_id(&package_id) | ||
| .unwrap() | ||
| .unwrap_or(package_id.to_string()) | ||
| //.unwrap() | ||
| && self.req.matches( | ||
| &krates | ||
| .get_package_version_from_cargo_metadata_package_id( |
There was a problem hiding this comment.
@yijunyu Thanks for the fix, did you find the root cause of the problem? Why is the get_package_name_from_cargo_metadata_package_id function failing to return the package name?
@jmcconnell26 Any ideas?
There was a problem hiding this comment.
I'm not sure, @yijunyu, could you provide the package_id that fails / a link to a repository to scan that fails?
If so, I can try and write a few more test cases to catch behaviour like this.
I would like to go back at some point, and add some further error handling to this part of the code.
There was a problem hiding this comment.
I've added a print statement before the line,
59 println!("package_id: {:?}", package_id.to_string());
60 self.name
61 == krates
62 .get_package_name_from_cargo_metadata_package_id(&package_id)
63 .unwrap()It turns out that the crate that caused the error is wasm-bindgen.
This can be confirmed if you run cargo-geiger with this crate at https://github.com/rustwasm/wasm-bindgen.
I was trying to debug into this, the error is likely from .cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/krates-0.5.0/src/lib.rs, but I am not too sure about it.
Indeed if I used the unwrap_or_default instead, it will skip the wasm-bindgen crate, which is not what we wanted.
What will be the better way to fix the error?
There was a problem hiding this comment.
Thanks for the info @yijunyu! Taking a look at the issue, something seems strange with the dependencies being added to the graph, but only when running with wasm-bindgen as the root of the crate. If wasm-bindgen is added as a dependency in a Cargo.toml, it looks to generate the report correctly.
I'll take a deeper look at this, and try to figure out what is causing the root problem.
There was a problem hiding this comment.
Thanks for looking into this: we will test it again later.
There was a problem hiding this comment.
@jmcconnell26 Any news? Just curious, no stress 😃
There was a problem hiding this comment.
No problem :) Take your time and thanks for #178!
There was a problem hiding this comment.
I've been trying to replicate this behaviour in a new test project, and have been stumped. I can see it happening in the wasm-bindgen crate, but attempting to create a new crate with the same error fails.
@anderejd, if its OK with you, I'd like to land a few PR's cleaning up the error handling, and hopefully that will lead me to the root of the problem.
There was a problem hiding this comment.
Sound great to me, go for it!
|
Could this be caused by renaming dependencies in Cargo.toml? rust-lang/cargo#5653 |
|
Or perhaps related to patching crate versions in Cargo.toml? |
|
Hey all - this has been hanging around Just to get some closure on this - We've had bunch of cargo bumps and we are using the latest 0.58.0 as dep (#230) that ended into current 0.11.2 released today as well as git version. Sadly - The error message has been already addressed elsewhere by @jmcconnell26 It cleaned out those unwraps in general including this as indicated here Just for the sake of completenes I ran with the latest 0.11.2 cargo-geiger that uses 0.58.0 cargo
(I get bunch of warnings but those are unrelated ofc.) I will close this PR so it's not hanging dead - Many thanks anyways @yijunyu for the effort Also everyone else - thanks for the effort anyways 💜 |
This bug fix comes from Huawei's application of
cargo-geigerto an internal product. After the fix, the tool can run to the end and produce the Safe Ratio report now.