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

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

Closed
FiloSottile opened this issue Dec 14, 2018 · 17 comments
Closed

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

FiloSottile opened this issue Dec 14, 2018 · 17 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@FiloSottile
Copy link
Contributor

#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.

@FiloSottile FiloSottile added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. release-blocker labels Dec 14, 2018
@FiloSottile FiloSottile added this to the Go1.12 milestone Dec 14, 2018
@bcmills
Copy link
Contributor

bcmills commented Dec 14, 2018

(CC @rsc)

@rsc
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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

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
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
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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@bcmills
Copy link
Contributor

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
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
Copy link
Contributor

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
Copy link
Contributor

rsc commented Jan 17, 2019

SGTM.

@kalyanac
Copy link

kalyanac commented Feb 2, 2019

One instance where this is a problem is the startup scripts for GCE VMs. Go Profiler agent integration test uses a startup template script to build and run go code. When the startup script runs, $HOME is not set.

To avoid breaking existing code, setting GOCACHE to /tmp/.cache would be preferable.

@lyda
Copy link

lyda commented May 2, 2019

Ugh. This also caused breakage in my personal environment. And looking through this issue I see a number of people warned it would.

Really disappointing.

@bradfitz
Copy link
Contributor

bradfitz commented May 2, 2019

@lyda, did it help you find that your personal environment was missing $HOME? How did lacking HOME not mess up dozens of other tools?

@lyda
Copy link

lyda commented May 2, 2019

It was an automated deployment system I use for stuff at home. It's worked fine for years w/o HOME being set for lots of tools.

Lots of processes on a normal unix system do not have HOME set. cron jobs, things started by init, cgi scripts, git post-receive hooks, things started by inetd and, in this case, scripts run by webhook.

Above there was talk about fixing the golang docker container for CI/CD systems - but not everyone uses docker for CI/CD. There are unix platforms that do not support docker - for instance for work I do CI/CD on FreeBSD (I call it my retrocomputing gig - but hey, it works so why not?).

I just don't understand what I've read in this issue. Caches like this are normally nice to have things. And if this cache is needed, the unix world has had pretty standard ways of finding a temp dir for ages. You can add the fancy new XDG var, but TMPDIR and /tmp have been the normal way to get temp space. And with go 1.11 you all had brought in modules and removed the need for any environment variables.

But now we're back to a var being needed. A different one. Which is fine, I'll update my system and build scripts for home and work for the next week or so as they break. It just seems so... unnecessary.

It's honestly just odd. You'd removed this need for environment vars which was great because it was always a minor annoyance with Go. Victory! But then promptly went and required one again. Is there some sort of environment var quota for the Go compiler team? Like, if it ever gets down to zero something bad has to happen - Go 3 gets designed by a committee of Haskell and FORTRAN developers.

@golang golang locked and limited conversation to collaborators May 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

7 participants