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

Comments

@toothrot
Copy link
Contributor

@toothrot 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

@andybons

This comment has been minimized.

Copy link
Member

@andybons andybons commented Sep 9, 2019

What kind of speedup do we get from this optimization?

@dmitshur

This comment has been minimized.

Copy link
Member

@dmitshur dmitshur commented Sep 9, 2019

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

@bradfitz

This comment has been minimized.

Copy link
Member

@bradfitz 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

This comment has been minimized.

Copy link
Member

@bradfitz 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

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Sep 26, 2019

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 added 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
Projects
None yet
5 participants
You can’t perform that action at this time.