Skip to content
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

refactor: unify Library and Package structs #6633

Merged
merged 35 commits into from
May 7, 2024

Conversation

knqyf263
Copy link
Collaborator

@knqyf263 knqyf263 commented May 6, 2024

Description

Previously, when go-dep-parser was a separate library, it defined a distinct Library struct to represent parsed packages. Trivy then converted this to the Package struct. However, since go-dep-parser has been merged into Trivy, there is no longer a need to define separate structs, and they can be unified. This eliminates the need for duplicate definitions of Relationship and other related elements. Additionally, the sorting methods for Package and Library were slightly different, but by using the same Package struct, these are now unified as well.

Furthermore, the internal terminology has been streamlined to consistently use "package" throughout the codebase. Previously, OS packages and language-specific packages were distinguished and referred to as "package" and "library" respectively. Going forward, both will be referred to as "package".

Related PRs

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've followed the conventions in the PR title.
  • I've added tests that prove my fix is effective or that my feature works.
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
@knqyf263 knqyf263 self-assigned this May 6, 2024
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
@knqyf263 knqyf263 marked this pull request as ready for review May 6, 2024 08:29
pkg/dependency/parser/c/conan/parse.go Outdated Show resolved Hide resolved
pkg/dependency/parser/conda/environment/parse.go Outdated Show resolved Hide resolved
pkg/dependency/parser/golang/sum/parse.go Outdated Show resolved Hide resolved
pkg/dependency/parser/gradle/lockfile/parse_test.go Outdated Show resolved Hide resolved
pkg/dependency/parser/hex/mix/parse_test.go Outdated Show resolved Hide resolved
pkg/fanal/analyzer/language/php/composer/composer.go Outdated Show resolved Hide resolved
pkg/fanal/analyzer/language/python/packaging/packaging.go Outdated Show resolved Hide resolved
pkg/fanal/analyzer/language/python/poetry/poetry.go Outdated Show resolved Hide resolved
pkg/fanal/applier/docker.go Outdated Show resolved Hide resolved
@@ -27,7 +27,7 @@ message PackageInfo {
message Application {
string type = 1;
string file_path = 2;
repeated Package libraries = 3;
repeated Package packages = 3;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC it's better to use new number, even if we just rename the field.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renaming should be ok as protobuf internally uses number, not name. I'll check it just in case.

knqyf263 and others added 18 commits May 7, 2024 13:59
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
knqyf263 and others added 9 commits May 7, 2024 14:05
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
@knqyf263
Copy link
Collaborator Author

knqyf263 commented May 7, 2024

@DmitriyLewen I've addressed your comments.

Copy link
Contributor

@DmitriyLewen DmitriyLewen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To save us time, I refactored some missing places:
2c73669
b2d35b1

Take a look, please.
If these changes are good for you - you can merge this PR.

@knqyf263
Copy link
Collaborator Author

knqyf263 commented May 7, 2024

Thanks!

@knqyf263 knqyf263 enabled auto-merge May 7, 2024 12:19
@knqyf263 knqyf263 added this pull request to the merge queue May 7, 2024
Merged via the queue into aquasecurity:main with commit 3eecfc6 May 7, 2024
12 checks passed
@knqyf263 knqyf263 deleted the refactor/libraries branch May 7, 2024 12:49
knqyf263 added a commit that referenced this pull request May 20, 2024
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
Co-authored-by: DmitriyLewen <dmitriy.lewen@smartforce.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants