Export copier and create interface

[baude]

This PR exports NewCopier and the Copy() and Close() methods for the copier. We prefer to use this for artifact support.

[baude]

if folks prefer, I could create exported wrapper functions for each of these instead of changing the internal calls to an exported function. Something like:

+// Close open resources.
+func (c *copier) Close() error {
        return c.close()
 }

LMK and I'm happy to do if preferred.

[rhatdan]

@vrothberg PTAL

[vrothberg]

LGTM

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: baude, vrothberg

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [baude,vrothberg]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Luap99]

This doesn't seem to be used anywhere, also in the wrong place? If this is needed it should be defined in copier.go.

And if you have to use the type in order to pass *copier around on the caller side then you should add a a actual check that the interface is satisfied.
Something like var _ Copier = (*copier)(nil) should do that AFAICT.

[mtrmac]

As a general matter of Go API design, I now think libraries exporting objects should not be publishing interface types if at all possible.

(Sure, export an interface type if the caller needs to provide an object, to define what the object is expected to satisfy.)

The reason is that adding a method to an interface breaks all other implementations, so there is really no way to have an API-stable exported interface for an evolving feature set; the only API-stable way to use a such a Go interface which expands over time is to only ever have that one implementation — and at that point there doesn’t need to be an interface at all.

Instead:

• For the purpose of unit testing / mocking: define a private interface in the test subpackage of the consumer of the object
• For the purpose of limiting the method set of the exported type (e.g. if the implementation struct must have public methods to conform to some other interface): instead, export a wrapper struct that only has the truly-public methods, and make the actual implementation a private structure not visible to external callers. (An extreme variant of this is going one step further in https://github.com/containers/image/blob/main/internal/signer/signer.go , where the only publicly-visible method is Close; and all other methods can be called internally from c/image. That’s only necessary if the module-private methods need to be called from multiple subpackages.)

---

Also, if NewCopier is public, the returned type probably should also be public, to allow callers to e.g. store the returned value in a struct field; if the returned type is unnamed, it can only be used via private local := variables — or using an interface type.

[mtrmac]

One further thing to think about is the dependency tree: c/common/libimage, via alltransports.ParseImageName, drags in all of c/storage (and all other transports).

If the OCI artifact code were to be run on the podman machine client side (e.g. on Windows), it might be desirable to split it from libimage into a separate subpackage.

[baude]

closing in favor of #2164