-
Notifications
You must be signed in to change notification settings - Fork 9
feat: rust analysis support #252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
84a77a9 to
3e9bd2c
Compare
3e9bd2c to
5d3670f
Compare
…rules are not met
|
@ruromero please review when you have time. |
ruromero
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @soul2zimate
I have added a first round of comments. After you refactor and answer some of my questions I'd like to review again.
src/main/java/io/github/guacsec/trustifyda/providers/RustProvider.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/guacsec/trustifyda/providers/RustProvider.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/guacsec/trustifyda/providers/RustProvider.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/guacsec/trustifyda/providers/RustProvider.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/guacsec/trustifyda/tools/Ecosystem.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/guacsec/trustifyda/providers/RustProvider.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/guacsec/trustifyda/providers/RustProvider.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/guacsec/trustifyda/providers/RustProvider.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/guacsec/trustifyda/providers/CargoProvider.java
Outdated
Show resolved
Hide resolved
e835253 to
759ffc4
Compare
759ffc4 to
7a00ef6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
I have added 2 small suggestions.
Overall Design
After looking deeper in to how you addressed the problem I think it provides a great opportunity to introduce a complex but powerful feature for monorepo projects that we could consider adding to all the other package managers.
If I understood correctly your approach you resolve all the tree from the root down to the members but the dependencies are added to the root, meaning that
root.deps = [member1.dep1, member1.dep2, member2.dep3, ...]
That creates an incorrect SBOM because we don't know which deps each member has and it can be that different members introduce the same dep with different version or that a member depends on another member.
But we can use that to our benefit and consider the possibility to create a complex SBOM where we will have something like:
root
member1
dep1, dep2
member2
dep1, dep3
...
The stack analysis and generated SBOMs will be much detailed. We can also use the component analysis to underline the members with vulnerabilities.
Regarding the root name/version in virtual workspaces, we can use the folder name and set a placeholder version or the workspace.version if defined.
What do you think?
Description
feat: rust analysis support
Related issue (if any): https://issues.redhat.com/browse/TC-3459
Checklist
Additional information
Assisted-by: Claude Code