Skip to content

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Oct 1, 2025

The ResolveDefaultContext function is only used internally by the CLI, and has no known external users, except for this test in buildx. It was exported in cli@f820766 to allow (unit) testing, but did not document that it was only exported for this purpose.

This patch rewrites the test to allow deprecating / removing the function in the CLI.

The ResolveDefaultContext function is only used internally by the CLI,
and has no known external users, except for this test in buildx. It was
exported in [cli@f820766] to allow (unit) testing, but did not document
that it was only exported for this purpose.

This patch rewrites the test to allow deprecating / removing the function
in the CLI.

[cli@f820766]: docker/cli@f820766

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@crazy-max
Copy link
Member

@thaJeztah
Copy link
Member Author

Yeah; github is having an incident with macOS runners.

Also worth noting (but not sure if used here) that macOS 13 is being deprecated, and they added macos-15-intel runners now; docker/cli#6530

Comment on lines +19 to +21
// FIXME(thaJeztah): the context-store is not initialized until "Initialize" is called.
err = dockerCLI.Initialize(cliflags.NewClientOptions())
require.NoError(t, err)
Copy link
Member Author

Choose a reason for hiding this comment

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

I want to have a look at this; the whole CLI (and CLI-plugin) bootstrapping is really involved. IMO, the CLI should be in a usable state after NewDockerCLI, but need to check if everything CAN be initialised in that constructor due to some of the oddities with flag parsing and top-level flags;

If we can't initialise in the NewDockerCli constructor, perhaps we can add some lazy constructor for the ContextStore() to at least prevent it returning nil (and panic in this test 😂)

@thaJeztah
Copy link
Member Author

🎉 OK, all happy now; should be ready to merge ❤️

@crazy-max crazy-max merged commit 6430db9 into docker:master Oct 1, 2025
265 of 267 checks passed
@thaJeztah thaJeztah deleted the no_ResolveDefaultContext branch October 1, 2025 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants