Skip to content

do not render Building when no build is needed#10620

Merged
glours merged 1 commit intodocker:v2from
ndeloof:Building
Jun 8, 2023
Merged

do not render Building when no build is needed#10620
glours merged 1 commit intodocker:v2from
ndeloof:Building

Conversation

@ndeloof
Copy link
Copy Markdown
Contributor

@ndeloof ndeloof commented May 25, 2023 •
edited
Loading

What I did
Don't setup buildkit's progress printer is we have no build to run, to prevent [+] Building 0/0 to be displayed.

progress.Printer doesn't offer an option to select the stream used to render status vs build logs

Related issue
fixes #10619

(not mandatory) A picture of a cute animal, if possible in relation to what you did

@ndeloof ndeloof requested review from a team, StefanScherer, glours, laurazard, milas, nicksieger and ulyssessouza and removed request for a team May 25, 2023 13:38
@ndeloof ndeloof mentioned this pull request May 25, 2023
Closed
@codecov
Copy link
Copy Markdown

codecov Bot commented May 25, 2023 •
edited
Loading

Codecov Report

Patch coverage: 94.44% and project coverage change: -0.05 ⚠️

Comparison is base (6f6e163) 59.55% compared to head (466e1d3) 59.51%.

Additional details and impacted files 
@@            Coverage Diff             @@
##               v2   #10620      +/-   ##
==========================================
- Coverage   59.55%   59.51%   -0.05%     
==========================================
  Files         107      107              
  Lines        9438     9442       +4     
==========================================
- Hits         5621     5619       -2     
- Misses       3241     3245       +4     
- Partials      576      578       +2
Impacted Files Coverage Δ
pkg/compose/watch.go 24.05% <0.00%> (ø)
pkg/compose/build.go 73.33% <100.00%> (+0.27%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ndeloof ndeloof force-pushed the Building branch 2 times, most recently from 8822df4 to b6fa268 Compare May 26, 2023 12:25
@zlianon
Copy link
Copy Markdown

zlianon commented May 28, 2023

I hope it makes it to the next version because it kind of annoys me

milas
milas approved these changes May 31, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@milas milas left a comment

Choose a reason for hiding this comment

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

LGTM - only suggestion would be to TestPrepareProjectForBuild to assert on buildRequired (not in every case, but at least 1x for true/false)

@milas milas changed the title prevent buildkt's progress to render Building when no built is needed do not render Building when no build is needed May 31, 2023
glours
glours approved these changes Jun 8, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@glours glours left a comment

Choose a reason for hiding this comment

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

LGTM

@glours glours merged commit 32cf776 into docker:v2 Jun 8, 2023
@NiklasBr
Copy link
Copy Markdown

NiklasBr commented Oct 13, 2023

Which version is this available in? Currently with Compose v2.22.0-desktop.2 we are seeing this output.

@glours
Copy link
Copy Markdown
Contributor

glours commented Oct 13, 2023

@NiklasBr it was released last June in https://github.com/docker/compose/releases/tag/v2.19.0
Can you open an issue with a reproducible example please?

@NiklasBr
Copy link
Copy Markdown

NiklasBr commented Oct 15, 2023

This is reproducible on both Mac OS 13 with Docker Desktop 4.24 and Windows 11 WSL2 and Docker Desktop 4.24.2

A docker-compose.yaml file like so:

services:
  system-cli:
    image: docker.io/system-cli:latest

A Dockerfile like so:

ARG PHP_VERSION="8.1"
ARG DEBIAN_VERSION="bookworm"

FROM php:${PHP_VERSION}-cli-${DEBIAN_VERSION}

WORKDIR /var/www

COPY --from=composer:latest /usr/bin/composer /usr/bin/composer

And then run the following command:

docker compose run --rm system-cli composer

Output becomes:

[+] Building 0.0s (0/0)                                                                                                                                                                                                                                                  docker:default
[+] Building 0.0s (0/0)                                                                                                                                                                                                                                                  docker:default
   ______
  / ____/___  ____ ___  ____  ____  ________  _____
 / /   / __ \/ __ `__ \/ __ \/ __ \/ ___/ _ \/ ___/
/ /___/ /_/ / / / / / / /_/ / /_/ (__  )  __/ /
\____/\____/_/ /_/ /_/ .___/\____/____/\___/_/
                    /_/
Composer version 2.6.5 2023-10-06 10:11:52

Usage:
  command [options] [arguments]

Options:
...

@NiklasBr NiklasBr mentioned this pull request Nov 9, 2023
Closed
@ndeloof ndeloof deleted the Building branch December 20, 2023 13:34
@diemondashley80-source
Copy link
Copy Markdown

diemondashley80-source commented Jan 16, 2026

ndeloof:Building

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@glours glours glours approved these changes

@nicksieger nicksieger Awaiting requested review from nicksieger nicksieger was automatically assigned from docker/compose-maintainers

@StefanScherer StefanScherer Awaiting requested review from StefanScherer StefanScherer was automatically assigned from docker/compose-maintainers

@ulyssessouza ulyssessouza Awaiting requested review from ulyssessouza ulyssessouza was automatically assigned from docker/compose-maintainers

@laurazard laurazard Awaiting requested review from laurazard laurazard was automatically assigned from docker/compose-maintainers

+1 more reviewer

@milas milas milas approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] compose run display [+] Building X.Ys (n/m)on stdout even if there is nothing to build

6 participants

@ndeloof @zlianon @NiklasBr @glours @diemondashley80-source @milas