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.

Sign up for GitHub

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

Jump to bottom

refactor: unify Library and Package structs #6633

Merged
merged 35 commits into from
May 7, 2024
Merged

refactor: unify Library and Package structs #6633

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).

@knqyf263 knqyf263 self-assigned this May 6, 2024
@knqyf263 knqyf263 force-pushed the refactor/libraries branch from 3d9da72 to 725e4fd Compare May 6, 2024 07:55
@knqyf263 knqyf263 marked this pull request as ready for review May 6, 2024 08:29
DmitriyLewen
DmitriyLewen reviewed May 7, 2024
View reviewed changes
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
rpc/common/service.proto
@@ -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
@knqyf263 @DmitriyLewen
refactor: replace lib with pkg
ff481eb 
Co-authored-by: DmitriyLewen <[email protected]>
@knqyf263 @DmitriyLewen
refactor: replace lib with pkg
398aeaf 
Co-authored-by: DmitriyLewen <[email protected]>
@knqyf263 @DmitriyLewen
refactor: replace lib with pkg
48cbd27 
Co-authored-by: DmitriyLewen <[email protected]>
@knqyf263 @DmitriyLewen
refactor: replace lib with pkg
3718e95 
Co-authored-by: DmitriyLewen <[email protected]>
@knqyf263
refactor(gradle): use sort.Sort
020e3b5 
Signed-off-by: knqyf263 <[email protected]>
@knqyf263 @DmitriyLewen
refactor: replace lib with pkg
4e33bc7 
Co-authored-by: DmitriyLewen <[email protected]>
@knqyf263 @DmitriyLewen
refactor: replace lib with pkg
2468e08 
Co-authored-by: DmitriyLewen <[email protected]>
@knqyf263 @DmitriyLewen
refactor: replace lib with pkg
84cf0be 
Co-authored-by: DmitriyLewen <[email protected]>
@knqyf263 @DmitriyLewen
refactor: replace lib with pkg
05aa799 
Co-authored-by: DmitriyLewen <[email protected]>
@knqyf263 @DmitriyLewen
refactor: replace lib with pkg
793c85d 
Co-authored-by: DmitriyLewen <[email protected]>
@knqyf263 @DmitriyLewen
refactor: replace lib with pkg
964e98f 
Co-authored-by: DmitriyLewen <[email protected]>
@knqyf263 @DmitriyLewen
refactor: replace lib with pkg
8598e4f 
Co-authored-by: DmitriyLewen <[email protected]>
@knqyf263 @DmitriyLewen
refactor: replace lib with pkg
129fff0 
Co-authored-by: DmitriyLewen <[email protected]>
@knqyf263 @DmitriyLewen
refactor: replace lib with pkg
9dcccd5 
Co-authored-by: DmitriyLewen <[email protected]>
@knqyf263 @DmitriyLewen
refactor: replace lib with pkg
52cb6d1 
Co-authored-by: DmitriyLewen <[email protected]>
@knqyf263 @DmitriyLewen
refactor: replace lib with pkg
bff98d2 
Co-authored-by: DmitriyLewen <[email protected]>
@knqyf263 @DmitriyLewen
refactor: replace lib with pkg
d416271 
Co-authored-by: DmitriyLewen <[email protected]>
@knqyf263 @DmitriyLewen
refactor: replace lib with pkg
4c3ddab 
Co-authored-by: DmitriyLewen <[email protected]>
knqyf263 and others added 9 commits May 7, 2024 14:05
@knqyf263 @DmitriyLewen
refactor: replace lib with pkg
ce70874 
Co-authored-by: DmitriyLewen <[email protected]>
@knqyf263 @DmitriyLewen
refactor: replace lib with pkg
82f95d3 
Co-authored-by: DmitriyLewen <[email protected]>
@knqyf263 @DmitriyLewen
refactor: replace lib with pkg
b02d21f 
Co-authored-by: DmitriyLewen <[email protected]>
@knqyf263 @DmitriyLewen
refactor: replace lib with pkg
e854249 
Co-authored-by: DmitriyLewen <[email protected]>
@knqyf263
refactor(hex): use sort.Sort
07da4e2 
Signed-off-by: knqyf263 <[email protected]>
@knqyf263
refactor(pnpm): use sort.Sort
5f43652 
Signed-off-by: knqyf263 <[email protected]>
@knqyf263
refactor(npm): replace lib with pkg
d284d01 
Signed-off-by: knqyf263 <[email protected]>
@knqyf263
refactor(yarn): use sort.Sort
fee8c55 
Signed-off-by: knqyf263 <[email protected]>
@knqyf263
refactor: renaming
f83ca44 
Signed-off-by: knqyf263 <[email protected]>
@knqyf263
Copy link
Collaborator Author

knqyf263 commented May 7, 2024

@DmitriyLewen I've addressed your comments.

DmitriyLewen
DmitriyLewen approved these changes May 7, 2024
View reviewed changes
Copy link
Contributor

@DmitriyLewen DmitriyLewen left a comment •
edited
Loading

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
@knqyf263 @DmitriyLewen
refactor: unify Library and Package structs (#6633)
c2b46d3 
Signed-off-by: knqyf263 <[email protected]>
Co-authored-by: DmitriyLewen <[email protected]>
Co-authored-by: DmitriyLewen <[email protected]>
@nscuro nscuro mentioned this pull request May 20, 2024
Closed
2 tasks
@DmitriyLewen DmitriyLewen mentioned this pull request Jun 13, 2024
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DmitriyLewen DmitriyLewen DmitriyLewen approved these changes

@chen-keinan chen-keinan Awaiting requested review from chen-keinan

Assignees

@knqyf263 knqyf263

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@knqyf263 @DmitriyLewen