Replace stereoscope with using go-containerregistry directly #836
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -41,61 +41,30 @@ require ( | |||
|
|
|||
| require ( | |||
| dario.cat/mergo v1.0.0 // indirect | |||
There was a problem hiding this comment.
Awesome to see so many dependencies gone!
| } | ||
|
|
||
| type FileMap struct { | ||
| fileNodeTrie *trie.PathTrie |
There was a problem hiding this comment.
Also, how much memory do we expect to save on average with using a trie structure here?
There was a problem hiding this comment.
I can check, but I doubt it's a significant amount, this is mainly used to allow us to iterate through directories.
There was a problem hiding this comment.
Can you explain a bit more what you mean by "iterate through directories"?
There was a problem hiding this comment.
E.g. if we want to scan a node_modules folder, we need to iterate through the contents of the directory.
It's mainly used for when we want to refactor and implement the fs.FS interface.
There was a problem hiding this comment.
got it! this trie dependency is fine for now, but if memory usage differences is negligible I wonder if this can be just simplified to a sorted array of paths that we can binary search on to enable fast lookups and directory listings.
There was a problem hiding this comment.
Looks like most of the memory usage is from extracting the tar.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #836 +/- ##
==========================================
+ Coverage 59.78% 60.40% +0.61%
==========================================
Files 136 142 +6
Lines 11268 9032 -2236
==========================================
- Hits 6737 5456 -1281
+ Misses 4102 3109 -993
- Partials 429 467 +38 ☔ View full report in Codecov by Sentry. |
| } | ||
|
|
||
| type FileMap struct { | ||
| fileNodeTrie *trie.PathTrie |
There was a problem hiding this comment.
Can you explain a bit more what you mean by "iterate through directories"?
| @@ -124,7 +124,7 @@ | |||
| "position": { | |||
| "filename": "\u003cAny value\u003e", | |||
| "offset": -1, | |||
| "line": 840, | |||
| "line": 839, | |||
There was a problem hiding this comment.
Do we want to set the line number to a fixed value? so we don't need to update it again when go version changes.
There was a problem hiding this comment.
I think it's alright for now, this allows us to check whether the line result in govulncheck is returning correctly. And we shouldn't be updating go that often anyway, so updating it when we do should be fine.
…rmission issue, fix bug in AllFiles returning directories.
Replace stereoscope with using the lower level go-containerregistry library directly.
Stereoscope brings in a large list of dependencies to support functions that is not necessary for osv-scanner. This also gives us more low level control of the extraction and scanning.
It's possible to switch to a new fs.FS lockfile interface without too much work.