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

vendor: github.com/containerd/console 8f6c4e4 #2238

Merged
merged 1 commit into from
Feb 5, 2024

Conversation

crazy-max
Copy link
Member

@crazy-max crazy-max commented Feb 2, 2024

As discussed internally with @milas @laurazard @thaJeztah, the containerd/console terminal check was causing errors to get logged on Windows if the output was redirected (piping to another command with more).

> docker buildx build . | more
failed to get console mode for stdout: The handle is invalid.
failed to get console mode for stdout: The handle is invalid.
[+] Building 0.7s (2/3)                                          docker:default
 => [internal] load build definition from Dockerfile                       0.0s
 => => transferring dockerfile: 4.66kB                                     0.0s
 => resolve image config for docker.io/docker/dockerfile:1                 0.7s
 => [auth] docker/dockerfile:pull token for registry-1.docker.io           0.0s

Bump github.com/containerd/console to containerd/console@8f6c4e4 to include containerd/console#66 that fixes this issue.

@crazy-max
Copy link
Member Author

We could test this with client-side integration tests on Windows related to #2206 (comment)

@crazy-max

This comment was marked as outdated.

@thaJeztah
Copy link
Member

@crazy-max would github.com/moby/term still be an alternative for this, or is that complicated?

@crazy-max
Copy link
Member Author

@crazy-max would github.com/moby/term still be an alternative for this, or is that complicated?

No because BuildKit progress ui that buildx depends on also uses containerd/console so it still prints one failed to get console mode: https://github.com/moby/buildkit/blob/b09a9f1afef847b71aa01faf1b6ff9c019c75b1f/util/progress/progressui/display.go#L172

@thaJeztah
Copy link
Member

Ah, gotcha, thx.

We really need to look at some of these. I have a feeling we have multiple packages / implementations do very similar things

@crazy-max crazy-max changed the title fix 'failed to get console mode' errors on Windows vendor: github.com/containerd/console 8f6c4e4 Feb 5, 2024
go.mod Outdated Show resolved Hide resolved
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

left a suggestion (adding a comment to provide context)

full diff: containerd/console@v1.0.3...8f6c4e4

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
@crazy-max crazy-max merged commit 30ae5ce into docker:master Feb 5, 2024
63 checks passed
@crazy-max crazy-max deleted the fix-win-console branch February 5, 2024 12:10
@crazy-max crazy-max added this to the v0.12.2 milestone Feb 5, 2024
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