Skip to content

Add String method so quieted output displays properly - #1090

Merged
thaJeztah merged 1 commit into
docker:masterfrom
zkry:1089-quiet-stream-output-fix
May 29, 2018
Merged

thaJeztah merged 1 commit into
docker:masterfrom
zkry:1089-quiet-stream-output-fix

Conversation

@zkry

@zkry zkry commented May 29, 2018

Copy link
Copy Markdown
Contributor

Fixes #1089 as well as addresses moby/moby#36812

- What I did
It turns out that the when the --stream flag is provided, a struct with a few different fields is used and when trying to print it in the quiet case, it uses a default string representation of the struct. I added a custom String method to only display the relevant data.

- Description for the changelog
Fixed bug displaying garbage output for build command when --stream and --quiet flags combined

Fixes docker#1089

Signed-off-by: Zachary Romero <zacromero3@gmail.com>
@vdemeester

Copy link
Copy Markdown
Collaborator

We should wait for https://jenkins.dockerproject.org/job/docker/job/cli/job/PR-1090 to be done (but somehow there is no node 😭)

@vdemeester vdemeester left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

SGTM 🐸

@thaJeztah

Copy link
Copy Markdown
Member

Before this patch:

cat <<EOF> Dockerfile
FROM busybox
RUN echo foo
EOF

docker build --quiet --stream .
&{%!!(MISSING)s(chan error=0xc42001a960) sha256:a0656916e1769300a230126221760c5d997870d249445f34e7884017a2bf1454
 %!!(MISSING)s(*bytes.Buffer=&{[] 0 0 [0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]}) %!!(MISSING)s(chan struct {}=0xc42001a9c0) {%!!(MISSING)s(int32=0) %!!(MISSING)s(uint32=0)}}

With this patch:

cat <<EOF> Dockerfile
FROM busybox
RUN echo foo
EOF

docker build --quiet --stream .
sha256:a0656916e1769300a230126221760c5d997870d249445f34e7884017a2bf1454

@thaJeztah thaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks!

}

func (bw *bufferedWriter) String() string {
return fmt.Sprintf("%s", bw.Writer)

@thaJeztah thaJeztah May 29, 2018 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Actually, wondering if there's a better solution to convert it to a string 🤔 (at least could be simplified to fmt.Sprint(bw.Writer))

@silvin-lubecki silvin-lubecki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@thaJeztah

Copy link
Copy Markdown
Member

Yeah, let's leave it as-is for now

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.

5 participants