Change Rust/Cargo detector to be lock file based
This is a significant change to Rust component detection to solely use the Cargo.lock file to detect component usage (e.g., no longer processing Cargo.toml).
As discussed in #116 there are several issues with the existing Cargo.toml processing that this change sidesteps by reporting all crates from Cargo.lock.
This change also explicitly includes the package registry in the identifier for components. I have not investigated what implications there may be for this to downstream tools, but because Cargo has support for multiple registries the package name and version number is insufficient to identify the package (because of multiple registries or a local package with the same name as one published in the registry).
In general, the sole use of the Cargo.lock file should strictly increase the number of crates reported, although I did observe some reductions that were due to this change no longer reporting some local/path crates as components (because they had a version and path specified) but I believe this was unintentional/unexpected.
There was also significant duplication in the code between v1 and v2 detectors, but in rewriting the logic to process the lock file I was able to use the same logic for both formats.
I expect this to need a careful review and some changes before landing, but I wanted to send out the PR to make some progress on getting these issues resolved.
Fixes #116
Hey @arlosi and @danielframpton, is there anything else blocking this pr? It would be nice to have component detection in good shape for rust. Thanks!
It needs review from the maintainers. Neither @danielframpton nor I can approve it.
Thanks @arlosi,
@jcfiorenzano @cobya do you think this can be merged? Do you miss something before it can be approved?
I have simplified this PR to focus on the source of dependencies (lock vs toml) deferring the question of multiple registries to see if we can land the highest priority part this change.
@danielframpton are the verification tests for rust still sufficient, or do they need updating as well?
Snapshot tests are expected to fail due to changes in the detector.
@danielframpton Do you have time to address the comments and resolve conflicts?
@danielframpton Do we believe this PR is on track for September?
@danielframpton thanks for deconflicting this PR ❤️
I just wanted to double check with you that you're good with this being merged?
Thanks @JamieMagee!
I am, I have responded to all the feedback and updated across the code analysis fixes. Let me know if there is anything I have missed.
👋 Hi! It looks like you modified some files in the Detectors folder.
You may need to bump the detector versions if any of the following scenarios apply:
- The detector detects more or fewer components than before
- The detector generates different parent/child graph relationships than before
- The detector generates different
devDependenciesvalues than before
If none of the above scenarios apply, feel free to ignore this comment 🙂