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

x/build/devapp: handle module dependencies more intelligently in Dockerfile #34192

Open
toothrot opened this issue Sep 9, 2019 · 5 comments
Open
Labels
Builders x/build issues (builders, bots, dashboards) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@toothrot
Copy link
Contributor

toothrot commented Sep 9, 2019

The x/build/devapp Dockerfile contains extra steps to optimize the speed of repeated docker builds when the go.mod or go.sum files have not changed. It would be nice to have a more automated way of doing this that doesn't require us to remember to add some our dependencies manually inside of our Dockerfile.

See golang.org/cl/193878.

@toothrot toothrot added Builders x/build issues (builders, bots, dashboards) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 9, 2019
@toothrot toothrot added this to the Unreleased milestone Sep 9, 2019
@andybons
Copy link
Member

andybons commented Sep 9, 2019

What kind of speedup do we get from this optimization?

@dmitshur
Copy link
Contributor

dmitshur commented Sep 9, 2019

/cc @bradfitz Might be most familiar with the effect of this optimization.

@bradfitz
Copy link
Contributor

bradfitz commented Sep 9, 2019

The point is to speed up incremental development.

Checking now, it looks like it's ~11 seconds to do an increment "make docker-prod" after doing a minor change to x/build/devapp/*.go.

But without it, incremental builds are ~22 seconds, assuming you're on a really fast network (Google corp, wired). Because without those lines you pull lots of source from proxy.golang.org.

@bradfitz
Copy link
Contributor

bradfitz commented Sep 9, 2019

Btw, we used to have such a tool: gitlock. The current TODOs in our Dockerfiles come from removing that, in our move to using modules. (#26872)

See also: #27719

@gopherbot
Copy link

Change https://golang.org/cl/197302 mentions this issue: cmd/tip: don't install cmd/tip from the internet

gopherbot pushed a commit to golang/build that referenced this issue Sep 26, 2019
Set the working directory in the Dockerfile to be inside the
golang.org/x/build module before running go install on cmd/tip.
Otherwise, it just installs the latest cmd/tip from the internet
rather than the intended local version. (All other Dockerfiles
in x/build had this line, except cmd/tip.)

Remove duplicate and unneeded WORKDIR instruction from two other
Dockerfiles to improve consistency.

Fix a rare race condition setting p.err when os.MkdirAll fails.

Add a log message when a new server has been started successfully
and the side switches. This will make logs easier to read.

Fixes golang/go#34526
Updates golang/go#34192

Change-Id: Iab8124f5c872fb87844e8e2f9b31637ce395f11b
Reviewed-on: https://go-review.googlesource.com/c/build/+/197302
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
codebien pushed a commit to codebien/build that referenced this issue Nov 13, 2019
Set the working directory in the Dockerfile to be inside the
golang.org/x/build module before running go install on cmd/tip.
Otherwise, it just installs the latest cmd/tip from the internet
rather than the intended local version. (All other Dockerfiles
in x/build had this line, except cmd/tip.)

Remove duplicate and unneeded WORKDIR instruction from two other
Dockerfiles to improve consistency.

Fix a rare race condition setting p.err when os.MkdirAll fails.

Add a log message when a new server has been started successfully
and the side switches. This will make logs easier to read.

Fixes golang/go#34526
Updates golang/go#34192

Change-Id: Iab8124f5c872fb87844e8e2f9b31637ce395f11b
Reviewed-on: https://go-review.googlesource.com/c/build/+/197302
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Builders x/build issues (builders, bots, dashboards) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants