reference: implement Sort() #3777
Conversation
Codecov ReportBase: 57.06% // Head: 57.06% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #3777 +/- ##
=======================================
Coverage 57.06% 57.06%
=======================================
Files 105 106 +1
Lines 10806 10838 +32
=======================================
+ Hits 6166 6185 +19
- Misses 3952 3964 +12
- Partials 688 689 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
Should Sort() have a Fuzzer for oss-fuzz to test? |
|
Perhaps 🤔 that said, there's no fuzzers currently running on this package in containerd (where this comes from). Perhaps it would be good to add them once (IF depends in wether the maintainers accept that) this package is moved to a separate module; see #3587 (comment) |
|
That sounds like a good plan, it also means you don't need to worry about whatever #3780 does to the fuzzers in this repo. |
|
The code LGTM, though the commit history is going to confuse the heck out of future code archaeologists. The "Fix reference ordering" commit is entirely greenfield code, and the two "Move x to y package" commits do nothing of the sort. WDYT about squashing this PR down to one commit with a description like "reference: implement Sort()" @thaJeztah? |
Yes, works for me; I had my branch squashed locally, then last minute wasn't sure wether or not I should squash, or to try to keep contributor history, but I guess it's ok to squash. I could include a link to the original implementation in the commit message. |
This upstreams `Sort()` as originally implemented in containerd in https://github.com/containerd/containerd/0886ceaea2470edc7339dfc5ebe0e3257ae84d06 From that commit: > Fix reference ordering in CRI image store > > Currently image references end up being stored in a > random order due to the way maps are iterated through > in Go. This leads to inconsistent identifiers being > resolved when a single reference is needed to identify > an image and the ordering of the references is used for > the selection. > > Sort references in a consistent and ranked manner, > from higher information formats to lower. > > Note: A `name + tag` reference is considered higher > information than a `name + digest` reference since a > registry may be used to resolve the digest from a > `name + tag` reference. Co-Authored-by: Derek McGowan <derek@mcg.dev> Co-Authored-by: Maksym Pavlenko <pavlenko.maksym@gmail.com> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
43e8027 to
1052518
Compare
This forward-ports the following patches;
Patches were slightly modified to account for different import paths.