Skip to content

test: relax validation of TEST_BUILDKIT_IMAGE to reduce direct dependencies - #1818

Closed
thaJeztah wants to merge 1 commit into
docker:masterfrom
thaJeztah:reduce_direct_dependencies
Closed

thaJeztah wants to merge 1 commit into
docker:masterfrom
thaJeztah:reduce_direct_dependencies

Conversation

@thaJeztah

Copy link
Copy Markdown
Member

commit 2d124e0 (#1770) added some integration-tests, and as part of those tests added github.com/distribution/distribution/v3 as a direct dependency.

github.com/distribution/distribution/v3 is unreleased, and is still receiving backward breaking changes. While this dependency is already used elsewhere, it looks like the only use in buildx was for a sanity check of the custom TEST_BUILDKIT_IMAGE; we can probably relax that check to avoid adding the direct dependency.

…encies

commit 2d124e0 added some integration-tests,
and as part of those tests added github.com/distribution/distribution/v3 as a
direct dependency.

github.com/distribution/distribution/v3 is unreleased, and is still receiving
backward breaking changes. While this dependency is already used elsewhere,
it looks like the only use in buildx was for a sanity check of the custom
TEST_BUILDKIT_IMAGE; we can probably relax that check to avoid adding the
direct dependency.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Comment thread tests/integration_test.go
buildkitImage = ref.String()
}
if img, ok := os.LookupEnv("TEST_BUILDKIT_IMAGE"); ok && img != "" {
t.Log("using TEST_BUILDKIT_IMAGE", img)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if needed, but thought to make it more visible if a custom image is used, in that odd case an invalid image reference is passed.

@thaJeztah

Copy link
Copy Markdown
Member Author

@jedevc @tonistiigi PTAL (let me know if you feel strongly about this; the alternative was to use the same version as BuildKit (v2.8.2), but when looking what it was used for, it seemed a very minimal "added value"

@tonistiigi

Copy link
Copy Markdown
Member

This looks fine but how does it matter if it is a direct or indirect dependency? Looks like this came from latest compose update.

We can also just use the regular reference pkg that is already used in many places.

@jedevc

jedevc commented May 22, 2023

Copy link
Copy Markdown
Collaborator

Ugh, that's frustrating. Yeah, I 100% meant to use docker/distribution, which we already use in other places in the codebase, have opened an alternative PR to update that.

@thaJeztah
thaJeztah deleted the reduce_direct_dependencies branch May 22, 2023 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants