INCOMPATIBLE: Remove the implementation of the ostree transport #2821
Conversation
ostree/ostree_transport.go
Outdated
| } | ||
| // | ||
| // Deprecated: The ostree: implementation has been removed, and any attempt to use this transport fails. | ||
| var Transport = transports.NewStubTransport("ostree") |
There was a problem hiding this comment.
What is the point of keeping this? If we don't support any ostree code would not be better to outright cause compile failures for all users? Why would we move all errors to runtime? Sure it break API but removing the actual function already breaks any potential user so I would prefer a proper clean break no?
There was a problem hiding this comment.
I don’t feel strongly about this.
I think there is, in principle, some value to keeping the transport available, because code matching transports,, similar to https://github.com/containers/common/blob/f71a7a6eca1d1c2846e00b2cda013c9b37af7cc5/libimage/pull.go#L237 , can continue to compile for a while; keeping the symbols around reduces the urgency of updating it.
OTOH it is almost certainly a purely hypothetical situation.
There was a problem hiding this comment.
Right but I wonder if this not ends up causing more technical debt that way. If nobody updates the places where it is referenced they just end up with defunct code which IMO is worse.
Maybe I do to much rust coding but I like to have changes fail to compile and not find out only at runtime in tests (in the best case) or production (at the worst case) that something changed in an incompatible way.
I don't see any podman references to this the ostree package so I am not that worried about the upgrade path. (I know there are more c/image users so that is an selfish view)
And if things failed to compile already removing it would not make it worse? I guess there is the case about users of the already stub implementation?
There was a problem hiding this comment.
Right but I wonder if this not ends up causing more technical debt that way.
There should be a deprecation warning; i.e. this gives the users choices.
And if things failed to compile already removing it would not make it worse?
Yes. That’s a clear argument for dropping immediately anyway. But then again, we did have an user “only” 2 years ago…
Currently, ostree does not compile: > ostree/ostree_dest.go:115:28: cannot use d (variable of type *ostreeImageDestination) as private.ImageDestinationInternalOnly value in argument to impl.AddCompat: *ostreeImageDestination does not implement private.ImageDestinationInternalOnly (missing method NoteOriginalOCIConfig) (typecheck) > d.Compat = impl.AddCompat(d) This has been broken since b941c6b (Nov 18 2024, > 4 months). Previously: - 807381f (broken almost 4 months), - 2e33bf7 (broken > 7 months) Overall, it seems extremely likely that noone is using the containers_image_ostree build tag (off by default since March 2019). So, finally, give up, and delete the code. Most importantly, this allows us to delete the > 3-year-old-frozen dependency, and risky CGo code. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
|
Updated to outright remove the Also, |
We are not opting into the ostree backend, and it doesn't build: containers/image#2821 . So, stop referencing the dependency. Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
We are not opting into the ostree backend, and it doesn't build: containers/image#2821 . So, stop referencing the dependency. Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
|
/approve |
We are not opting into the ostree backend, and it doesn't build: containers/image#2821 . So, stop referencing the dependency. Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Currently, ostree does not compile:
This has been broken since b941c6b (Nov 18 2024, > 4 months).
Previously:
Overall, it seems extremely likely that noone is using the
containers_image_ostreebuild tag (off by default since March 2019).So, finally, give up, and delete the code. Most importantly, this allows us to delete the > 3-year-old-frozen dependency, and risky CGo code.
Cc: @giuseppe , @mheon , @rhatdan (general RFC / awareness), @joonas-fi (previously reporting a build failure in #1917 ).