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

build: prefix baked-in environment variables with GO_ or similar #38368

Open
ydnar opened this issue Apr 10, 2020 · 7 comments
Open

build: prefix baked-in environment variables with GO_ or similar #38368

ydnar opened this issue Apr 10, 2020 · 7 comments

Comments

@ydnar
Copy link

@ydnar ydnar commented Apr 10, 2020

Per request here: #38361 (comment)

Currently a go binary is built with the value of the PKG_CONFIG variable as-defined at build-time, and uses that variable to find the pkg-config binary:

https://github.com/golang/go/blob/master/src/make.bash#L53

A coincidental change in Homebrew broke cgo builds for anyone using Homebrew to manage their Go installation.

Given that pkg-config is critical for building Go packages with cgo, maybe it makes sense to rename the variable to GO_PKG_CONFIG so it’s unlikely to be clobbered by upstream build systems. Or, going further, prefix any other environment variables baked into the Go binary with GO?

@ianlancetaylor ianlancetaylor changed the title Prefix baked-in environment variables with GO_ or similar build: prefix baked-in environment variables with GO_ or similar Apr 10, 2020
@andybons andybons added this to the Unplanned milestone Apr 10, 2020
@andybons
Copy link
Member

@andybons andybons commented Apr 10, 2020

@aclements
Copy link
Member

@aclements aclements commented Apr 12, 2020

Can you say more about how the Homebrew change broke cgo builds? It's actually fairly standard for PKG_CONFIG to refer to the path to pkg-config. That's not a Go-specific convention. I don't fully understand what that change is doing, but if it broke Go, I would have expected it to break a lot of other things, too.

@ydnar
Copy link
Author

@ydnar ydnar commented Apr 12, 2020

Homebrew shipped an update to its core that set the PKG_CONFIG variable to the path to its shimmed pkg-config that had a specific environment dependency (HOMEBREW_OPT), which isn’t defined at runtime. The go binary that Homebrew shipped had a PKG_CONFIG variable pointing to a script that failed to run, breaking cgo for anyone that used that distribution.

Curious: what’s the purpose of baking the path to pkg-config into the binary?

Edit: if you want to see the carnage: Homebrew/brew#5068 (comment)

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Apr 12, 2020

The Homebrew change was that someone set the PKG_CONFIG environment variable to something other than the default of pkg-config while building the binaries. That change was then baked into cmd/go by make.bash and cmd/dist.

The argument in this issue is that other build systems may set environment variables like PKG_CONFIG and CC, and it can be confusing that we build those into the binaries. It's the right thing to do for individual builds, but not so much for builds that are distributed to other people.

One way to make this less potentially confusing would be to use GO_ prefix for all the environment variables that we bake into the Go tools. At least we can arguably claim that environment variables starting with GO_ can affect the Go build.

I think the environment variables that bake in defaults are PKG_CONFIG, CC, CC_FOR_TARGET, CXX, and CXX_FOR_TARGET. And also various names that already start with GO, like GO386, GOARM, etc., and also GO_EXTLINK_ENABLED and GO_LDSO. And of course GOOS and GOARCH.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Apr 12, 2020

@ydnar For people who aren't distributing their binaries, it's reasonable to permit setting PKG_CONFIG at build time to set the default that the binaries will use later. We do that for most of the external tools invoked by cmd/go, though perhaps we've missed some.

@ydnar
Copy link
Author

@ydnar ydnar commented Apr 12, 2020

@ianlancetaylor agree.

Of the variables you mentioned above, which are [paths] used by go or cgo at runtime like PKG_CONFIG?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Apr 12, 2020

The ones that are paths are PKG_CONFIG, CC, CC_FOR_TARGET, CXX, and CXX_FOR_TARGET.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.