-
Notifications
You must be signed in to change notification settings - Fork 210
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
NVSHAS-9584: Update tar.go with using upstream package tarutil #1722
Conversation
kyledong-suse
commented
Dec 19, 2024
•
edited
Loading
edited
- Remove unmaintained functions in tar.go
- Import upstream package tarutil in tar.go
- Add upstream package tarutil in vendor
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.
Generally looks good to me. Some questions about its implementation detail.
func (s *ScanUtil) readRunningPackages(id string, pid int, prefix, kernel string, pidHost bool) ([]utils.TarFileInfo, bool) { | ||
var files []utils.TarFileInfo | ||
func (s *ScanUtil) readRunningPackages(pid int, prefix, kernel string, pidHost bool) (tarutil.FilesMap, bool) { | ||
files := make(tarutil.FilesMap) |
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.
tarutil.FilesMap is just type FilesMap map[string][]byte
Do we need github.com/coreos/clair/pkg/tarutil
for it?
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.
I agree that we don't need github.com/coreos/clair/pkg/tarutil
. I'll create type FilesMap map[string][]byte
for tar processing
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.
After discussed with Jay, I aware that scanner uses most of the tar functions. So it's better to keep this tar.go and update it with using the upstream tarutil.
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.
There seems to be some confusion here. Let's talk through this.
77fc23e
to
45dd755
Compare
6b026fb
to
ed21821
Compare
1. Remove unmaintained functions in tar.go 2. Import upstream package tarutil in tar.go 3. Add upstream package tarutil in vendor Remove tarutil, create type FilesMap map[string][]byte instead Update tar.go with using upstream package tarutil
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.
I have concern about tar.go because its copyright from third party is being removed. Please ensure the change is done correctly before merging this PR.
1. Remove unmaintained tar.go 2. Add upstream package github.com/coreos/clair in vendor 3. Create extract.go for NV implemented tar realted functions
Need to separate this PR into 2 part,
|