use containerd/reference/docker instead of docker/distribution#43278
use containerd/reference/docker instead of docker/distribution#43278thaJeztah wants to merge 1 commit intomoby:masterfrom
Conversation
|
Ah, some fixes needed in a mock; |
|
So, the error is because the moby/vendor/github.com/docker/distribution/registry.go Lines 96 to 99 in 7b9275c But I feel silly now; not sure I understand; so the failure is because I swapped the imports. Now, that makes perfect sense if the returned type was a "type", but it's an interface, so this line: moby/distribution/push_v2_test.go Lines 640 to 643 in 7b9275c
So.. how do they differ? Well.. they don't. The containerd Here's the docker/distribution definition: moby/vendor/github.com/docker/distribution/reference/reference.go Lines 63 to 68 in 7b9275c moby/vendor/github.com/docker/distribution/reference/reference.go Lines 108 to 112 in 7b9275c And here's the containerd definition: moby/vendor/github.com/containerd/containerd/reference/docker/reference.go Lines 126 to 130 in 7b9275c So, I would expect things to fail if those definitions depended on some type, but they're defining an interface that returns a generic ( A smaller example; https://go.dev/play/p/cRwQGtWCZi9 Detailspackage main
import "fmt"
type Named interface {
Name() string
}
type NamedTwo interface {
Name() string
}
type Registry interface {
Named() Named
}
// implementNamed implements Named, NamedTwo interface.
type implementNamed struct{ name string }
func (n *implementNamed) Name() string {
return n.name
}
type oneRegistry struct{}
func (r *oneRegistry) Named() Named {
return &implementNamed{"one"}
}
type twoRegistry struct{}
func (r *twoRegistry) Named() NamedTwo {
return &implementNamed{"two"}
}
func main() {
oneReg := oneRegistry{}
twoReg := twoRegistry{}
expectNamed(oneReg.Named())
expectNamed(twoReg.Named())
expectNamedTwo(oneReg.Named())
expectNamedTwo(twoReg.Named())
type thingWithNamed struct {
named Named
}
// This works; even though twoReg.Named() returns a NamedTwo
foo := thingWithNamed{}
foo.named = oneReg.Named()
fmt.Println(foo.named.Name())
foo.named = twoReg.Named()
fmt.Println(foo.named.Name())
type thingWithRegistry struct {
registry Registry
}
bar := thingWithRegistry{}
bar.registry = &oneReg
// But this doesn't work:
// ./main.go:60:15: cannot use &twoReg (type *twoRegistry) as type Registry in assignment:
// *twoRegistry does not implement Registry (wrong type for Named method)
// have Named() NamedTwo
// want Named() Named
// bar.registry = &twoReg
}
func expectNamed(in Named) {
fmt.Println("expectNamed: " + in.Name())
}
func expectNamedTwo(in NamedTwo) {
fmt.Println("expectNamedTwo: " + in.Name())
} |
3dc29b9 to
0d5356b
Compare
| // Named() must return a distref.Named here, not containerd's Named interface; | ||
| // while both interfaces are identical, Golang considers it an incompatible | ||
| // type otherwise: | ||
| // | ||
| // distribution/push_v2_test.go:404:4: cannot use repo (type *mockRepo) as type distribution.Repository in field value: | ||
| // *mockRepo does not implement distribution.Repository (wrong type for Named method) | ||
| // have Named() docker.Named | ||
| // want Named() "github.com/docker/docker/vendor/github.com/docker/distribution/reference".Named | ||
| func (m *mockRepo) Named() distref.Named { |
There was a problem hiding this comment.
As it looks like the "interface" issues really only affects this Mock, I decided to import the "old" package here. "production" code doesn't need the old interface
Not sure if there's a better solution for this; perhaps we should replace the distribution.Repository interface with something local if possible (haven't dug too deeply into that yet)
|
Relevant to what we discussed in the maintainers meeting, it appears this was copied into containerd specifically for containerd/containerd#3554 (via containerd/containerd#3728). Was there ever any communication with either party about maintaining it in a separate (smaller) place? Now there are two copies that have diverged slightly and it'd be neat if we could help get them back in sync. 🙈 |
0d5356b to
b7f49f2
Compare
containerd now maintains a copy of the docker/distribution code under the github.com/containerd/containerd/reference/docker package. Let's switch to using that package, to remove the (direct) dependency on github.com/docker/distribution. Signed-off-by: Sebastiaan van Stijn <[email protected]>
b7f49f2 to
8bf92da
Compare
splitting this off from #40961
similar PR can be found in moby/buildkit#2666, which removes the dependency on this package from BuildKit (only remaining use in the code now)
containerd now maintains a copy of the docker/distribution code under the
github.com/containerd/containerd/reference/docker package.
Let's switch to using that package, to remove the (direct) dependency on
github.com/docker/distribution.
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)