feat: implement coarse versioning in go#5225
Hidden character warning
Conversation
another-rex
left a comment
another-rex
left a comment
There was a problem hiding this comment.
Looks good! Just some minor questions.
| } | ||
|
|
||
| func (e apkEcosystem) Coarse(_ string) (string, error) { | ||
| // TODO(michaelkedar): semantic.AlpineVersion currently breaks transitivity rules |
There was a problem hiding this comment.
What happens if coarse is not supported when querying at the moment? Is it just everything for a specific package has to be loaded into memory?
There was a problem hiding this comment.
| ) | ||
|
|
||
| func TestCoarseMonotonicityLarge(t *testing.T) { | ||
| if os.Getenv("RUN_COARSE_LARGE_TEST") != "1" { |
There was a problem hiding this comment.
Can you document this env somewhere.
Also ideally add this to the Makefile as a help option similar to osv-scanner, though I think that can wait till we moved everything over to go.
There was a problem hiding this comment.
Added some instructions to CONTRIBUTING.md
I don't know about adding this specifically to the Makefile - it's a bit specific and generating the data takes quite a while.
| EmptyAs *string // If not nil, treats empty parts as the given string instead of removing them. If nil, removes them. | ||
| } | ||
|
|
||
| var implicitRegex = regexp.MustCompile(`\d+|\D+`) |
There was a problem hiding this comment.
what does implicit mean here...?
There was a problem hiding this comment.
Renamed it implicitSplitRegex and added a comment that it's for splitting transitions between letters and numbers
2 participants
Re-implements the
EE:XXXXXXXX.YYYYYYYY.ZZZZZZZZcomparable version string generation in Go from Python.The implementations are mostly the same as the Python ones, but the semantic submodule in scalibr is generally much more lenient in accepting technically invalid versions (dunno if we should be being more strict about this).
Differences are mostly due to how we want to handle invalid versions, which I don't think really appears in the OSV database. I'll do a pass over the AffectedVersions in the database once the worker is migrated to Go to make sure this is all consistent anyway.
I created a program & test that compiles every single unique version (from
affected[].versionsandaffected[].ranges[]) in every single OSV record, and verifies thatParseandCoarseboth error on the same strings, andCoarsemaintains monotonicity. This test doesn't run by default because a) the versions list is 22MB big and b) it takes a while to generate and run.I've also added some fuzzing tests to help catch edge cases if we want to run them for a bit. It's already helped me find a few edge cases in dpkg and packagist, which is nice. The regular tests run the fuzzers with only the explicit seed corpus. To do full fuzzing, you need to run it manually.
APK is currently not implemented due to transitivity issues in scalibr that I'm looking to fix (google/osv-scalibr#1932)