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

go_binary: Add support to set a default GODEBUG per binary #4168

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zecke
Copy link
Contributor

@zecke zecke commented Nov 9, 2024

Go supports setting default GODEBUG values in the go.mod file or per directive in the main package at the right location. This currently does not work with rules_go/gazelle.

Introduce an explicit godebug_default attribute on go_binary so that gazelle can easily manage it.

Supports bazel-contrib/bazel-gazelle!1945

What type of PR is this?

Uncomment one line below and remove others.

Bug fix
Feature
Documentation
Other

What does this PR do? Why is it needed?

Which issues(s) does this PR fix?

Fixes #

Other notes for review

Go supports setting default GODEBUG values in the go.mod file or per
directive in the main package at the right location. This currently does
not work with rules_go/gazelle.

Introduce an explicit godebug_default attribute on go_binary so that
gazelle can easily manage it.

Supports bazel-contrib/bazel-gazelle!1945
Copy link
Member

@fmeum fmeum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this, but I think that an attribute on go_binary is the wrong location for this feature. Having potentially different semantics for the same line of code depending on the downstream consumer it is built for seems like too much of a foot gun.

Go language version and GODEBUG could be made configurable on a Go SDK and, with Bzlmod, directly via go.mod.

@zecke
Copy link
Contributor Author

zecke commented Nov 10, 2024

Thanks for working on this, but I think that an attribute on go_binary is the wrong location for this feature. Having potentially different semantics for the same line of code depending on the downstream consumer it is built for seems like too much of a foot gun.

My idea was to handle this like //go:embed. Have gazelle parse the .go files files and generate the go_binary based on the directives observed. Do you think something like rules_go's builder should scan the .go file to extract the directives from the main package?

//go:debug http2server=0
package main

const foo = 123

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

Successfully merging this pull request may close these issues.

2 participants