Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can no longer provide alternative implementations of ImageFetcher #1245

Closed
briandealwis opened this issue Aug 3, 2021 · 3 comments · Fixed by #1315
Closed

Can no longer provide alternative implementations of ImageFetcher #1245

briandealwis opened this issue Aug 3, 2021 · 3 comments · Fixed by #1315
Labels
lib Issue or PR applying only to the use as a library. status/ready Issue ready to be worked on. type/bug Issue that reports an unexpected behaviour.
Milestone

Comments

@briandealwis
Copy link
Contributor

Summary

PR #1216 changed ImageFetcher.Fetch() to use an internal FetchOptions type from pack/internal/image. This prevents providing alternative ImageFetcher implementationsas that pack/internal/image` package can't be imported from other modules.

This change prevents Skaffold from updating to use pack v0.20.0 as Skaffold provides an alternative implementation of ImageFetcher.


Reproduction

Steps
  1. create fetcher.go:
package main
import "context"
import "github.com/buildpacks/pack"
import "github.com/buildpacks/pack/internal/image"
import "github.com/buildpacks/imgutil"
type fetcher struct {}
func (f *fetcher) Fetch(_ context.Context, _ string, _ image.FetchOptions) (imgutil.Image, error) { return nil, nil }
var _ pack.ImageFetcher = (*fetcher)(nil)
  1. go mod init example.com/fetch
  2. go mod tidy
  3. go build .
Current behavior

Build fails:

$ go build
package example.com/fetcher
	fetch.go:4:8: use of internal package github.com/buildpacks/pack/internal/image not allowed
Expected behavior

Build should succeed.

@briandealwis briandealwis added status/triage Issue or PR that requires contributor attention. type/bug Issue that reports an unexpected behaviour. labels Aug 3, 2021
@briandealwis
Copy link
Contributor Author

This affects other types in client.go too:

  • the Downloader interface's Download() returns internal/blob.Blob
  • the BuildpackDownloader interface's Download() returns internal/dist.Buildpack

@jromero jromero added lib Issue or PR applying only to the use as a library. status/ready Issue ready to be worked on. and removed status/triage Issue or PR that requires contributor attention. labels Aug 11, 2021
@jromero jromero added this to the 0.21.0 milestone Aug 11, 2021
@aemengo
Copy link
Contributor

aemengo commented Aug 11, 2021

@briandealwis Thanks for creating this issue.

How do we prefer to resolve this? In general, I'm in favor of making options public. Perhaps an ./options.go file at the root of the repository.

Any others have thoughts?

@jromero
Copy link
Member

jromero commented Sep 1, 2021

Tried a few things and this is going to require a bit more elbow grease (restructuring). Sorry but it may take one more release before we can ship this out. :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib Issue or PR applying only to the use as a library. status/ready Issue ready to be worked on. type/bug Issue that reports an unexpected behaviour.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants