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

Add hint of progress to the output of docker build #24978

Merged
merged 1 commit into from
Aug 18, 2016

Conversation

yongtang
Copy link
Member

- What I did

This fix tries to address the issue raised in #24912 where docker build only consists of the current step without overall total steps.

- How I did it

This fix adds the overall total steps so that end user could follow
the progress of the docker build.

- How to verify it

An additonal test has been added to cover the changes.

- Description for the changelog

Add hint of progress (e.g., Step 1/5) to the output of docker build so that the total steps is known during the build progress.

- A picture of a cute animal (not mandatory but encouraged)

This fix fixes #24912.

Signed-off-by: Yong Tang yong.tang.github@outlook.com

This fix tries to address the issue raised in 24912 where docker
build only consists of the current step without overall total steps.

This fix adds the overall total steps so that end user could follow
the progress of the docker build.

An additonal test has been added to cover the changes.

This fix fixes 24912.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang yongtang force-pushed the 24912-build-with-progress branch from c7e7e24 to 35418c1 Compare July 24, 2016 15:14
@AkihiroSuda
Copy link
Member

This change breaks compatibility for those who greps the stream with a regexp like Step ([0-9]+) :.

So I think "Step 1 : FOOBAR (1/5)" is better for compatibility.

@thaJeztah
Copy link
Member

ping @duglin wdyt?

@duglin
Copy link
Contributor

duglin commented Jul 25, 2016

At first I thought we'd have to turn the Dockerfile processing into a 2-pass parsing thing, so I'm glad that I forgot that we read it all into memory and can know the number of steps easily. I'll try to find some time over the next day or so to review it. But, overall, I don't have an issue with the idea of doing this, I just personally don't see much value in it, but if others do it might help to have them explain why this would help aside from "just to know".

@thaJeztah
Copy link
Member

I just personally don't see much value in it

Yeah, it won't give any clue as to how long the remaining steps will take, so it's not adding a lot, other than you know how many steps it takes until completion

@tianon
Copy link
Member

tianon commented Aug 11, 2016

Perhaps instead just print out the "Total Steps" right at the start?

ie:

$ docker build .
Sending build context to Docker daemon 8.192 kB
Total Steps: 9
Step 1 : FROM alpine:3.4
...
Step 9 : CMD sh
 ---> Running in 2c2a1c7f3aa1
 ---> fdbc4e5a9e55
Removing intermediate container 2c2a1c7f3aa1
Successfully built fdbc4e5a9e55

@duglin
Copy link
Contributor

duglin commented Aug 11, 2016

I have a slight preference for the Step x/y form, just so I don't have to scroll up to see the total.

@thaJeztah
Copy link
Member

I agree with @duglin

@thaJeztah
Copy link
Member

moving this to code review

@tianon
Copy link
Member

tianon commented Aug 18, 2016

LGTM

1 similar comment
@duglin
Copy link
Contributor

duglin commented Aug 18, 2016

LGTM

@duglin duglin merged commit 282b0af into moby:master Aug 18, 2016
@yongtang yongtang deleted the 24912-build-with-progress branch August 19, 2016 01:26
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.

docker build should hint some kind of progress
7 participants