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

cmd/go: builds fail if neither $HOME nor $GOCACHE are set #29267

Closed
FiloSottile opened this Issue Dec 14, 2018 · 13 comments

Comments

Projects
None yet
3 participants
@FiloSottile
Copy link
Member

FiloSottile commented Dec 14, 2018

#29243 (comment)

Are we committed to not letting users build code unless they set $HOME or $GOCACHE? I'd expect this to break quite a few Docker images, which probably didn't want to keep the cache around anyway. Also, I run some machines with a read-only $HOME, which I guess won't be able to build Go code anymore.

It already broke tip.golang.org and stuff inside Google, too.

@bcmills in #29243 (comment)

It's what we announced in the 1.11 release notes, and nobody complained about it then.

We could perhaps put the cache in os.TempDir() if we can't find it some other place, though.

I'd argue that https://golang.org/doc/go1.11#gocache does not communicate that if you don't set $HOME, or if it's not writeable, builds will fail. To an outsider it seems to just say you can't force $GOCACHE off manually anymore.

I personally would expect go to still work, and simply not persist the cache.

Anyway, we are seeing breakage, so this should be an explicit decision. I think we should not break build environments as much as possible: modules should make go more flexible, not less.

@bcmills

This comment has been minimized.

Copy link
Member

bcmills commented Dec 14, 2018

(CC @rsc)

@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Dec 18, 2018

Let's make sure we figure out how the "standard" Go docker images keep being useful to the many users they have. That may mean working with the people who maintain those images to get the base images to set GOCACHE=/tmp/gocache by default, so every user doesn't have to.

@FiloSottile

This comment has been minimized.

Copy link
Member

FiloSottile commented Dec 18, 2018

The golang images actually set $HOME

$ docker run golang bash -c 'echo $HOME'
/root

but I don't think that solves the problem. I personally run most of my systems with read-only $HOME, and just like the Google internal CI as well as tip.golang.org broke because we don't set $HOME, surely others will. I guess we can wait and see how many people break in beta.

As for Docker, keeping the cache around in the image will probably be undesirable because minimal size images are considered important, so we will see some go clean -cache bolierplate emerge like rm -vrf /var/cache/apk/* did before they introduced apk add --no-cache. However, this sounds more like an argument to allow GOCACHE=off to be honest.

@bcmills

This comment has been minimized.

Copy link
Member

bcmills commented Dec 18, 2018

keeping the cache around in the image will probably be undesirable because minimal size images are considered important

Does Docker not clean the contents of /tmp automatically? (I would find that pretty surprising, but I'm not that familiar with Docker.)

@FiloSottile

This comment has been minimized.

Copy link
Member

FiloSottile commented Dec 18, 2018

It doesn't.

$ cat > Dockerfile
FROM alpine
RUN echo _o/ > /tmp/hi
$ docker build .
Sending build context to Docker daemon  2.048kB
Step 1/2 : FROM alpine
latest: Pulling from library/alpine
4fe2ade4980c: Pull complete
Digest: sha256:621c2f39f8133acb8e64023a94dbdf0d5ca81896102b9e57c0dc184cadaf5528
Status: Downloaded newer image for alpine:latest
 ---> 196d12cf6ab1
Step 2/2 : RUN echo _o/ > /tmp/hi
 ---> Running in 9ce0e929ea2e
Removing intermediate container 9ce0e929ea2e
 ---> f68c5e00ee65
Successfully built f68c5e00ee65
$ docker run --rm f68c5e00ee65 cat /tmp/hi
_o/
@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Dec 19, 2018

I still think the base golang images need to set things up so that the cache is available, either with a writable HOME or with an explicit GOCACHE variable that's writable. A sequence of RUN commands in the Dockerfile need to not rebuild everything every time. That will just lead people to believe - incorrectly! - that go is very slow at building. The go command is correct to fail loudly when there is no cache.

As far as cleaning up the cache, @bradfitz says that now that there are multi-stage dockerfiles, so you can separate build and prod-run images, it's less important for build to clean up. Maybe we don't need to worry about cleaning up.

@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Dec 19, 2018

Should we repurpose this bug to get the golang docker image-build scripts updated in preparation for Go 1.12?

@FiloSottile

This comment has been minimized.

Copy link
Member

FiloSottile commented Dec 19, 2018

The golang images already have a writeable $HOME AFAICT.

The part I wanted a decision on is whether we are ok breaking build environments that have a write-only or unset $HOME, of which we already have some examples (tip.golang.org, Google's internal stuff, and my personal infrastructure).

(Sorry for derailing with the discussion about Docker image sizes, multi-stage Dockerfiles are a reasonable mitigation.)

@FiloSottile

This comment has been minimized.

Copy link
Member

FiloSottile commented Dec 19, 2018

#29341 is unrelated but mentions another environment with $HOME unset.

@bcmills

This comment has been minimized.

Copy link
Member

bcmills commented Dec 19, 2018

A write-only or unset HOME seems like a pretty rare thing to have — and note that in the case of #29341 it will also cause go build to fail unless GOPATH is set, so it requires environment customization already.

Requiring GOCACHE in addition doesn't seem like a huge burden, bearing in mind that a read-only HOME already pretty firmly places us in power-user territory.

@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Jan 16, 2019

Since it's not a release blocker, milestoning forward.

@rsc rsc modified the milestones: Go1.12, Go1.13 Jan 16, 2019

@bcmills

This comment has been minimized.

Copy link
Member

bcmills commented Jan 17, 2019

Is there anything left to be done on this issue?

Given the relative lack of discussion since the betas have been cut, perhaps the improved diagnostics are enough to resolve the usability concerns.

@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Jan 17, 2019

SGTM.

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