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.

Sign up for GitHub

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

Update go_repository defaults to only use Go code #1888

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

Update go_repository defaults to only use Go code #1888

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 1 addition & 70 deletions internal/bzlmod/default_gazelle_overrides.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,67 +15,10 @@
visibility("private")

DEFAULT_BUILD_FILE_GENERATION_BY_PATH = {
"github.com/envoyproxy/protoc-gen-validate": "on",
"github.com/google/safetext": "on",
"github.com/grpc-ecosystem/grpc-gateway/v2": "on",
"google.golang.org/grpc": "on",
"github.com/grpc-ecosystem/grpc-gateway/v2": "update",
}

DEFAULT_DIRECTIVES_BY_PATH = {
"github.com/argoproj/argo-workflows/v3": [
"gazelle:proto disable",
],
"github.com/argoproj/argo-events": [
"gazelle:proto disable",
"gazelle:go_naming_convention import_alias",
],
"github.com/census-instrumentation/opencensus-proto": [
"gazelle:proto disable",
],
"github.com/envoyproxy/protoc-gen-validate": [
"gazelle:build_file_name BUILD.bazel",
],
"github.com/cockroachdb/errors": [
"gazelle:proto disable",
],
"github.com/colinmarc/hdfs/v2": [
"gazelle:go_naming_convention import_alias",
"gazelle:proto disable",
],
"github.com/containerd/containerd": [
"gazelle:proto disable",
],
"github.com/containerd/containerd/api": [
"gazelle:proto disable",
],
"github.com/containerd/ttrpc": [
"gazelle:proto disable",
],
"github.com/gogo/googleapis": [
"gazelle:proto disable",
],
"github.com/gogo/protobuf": [
"gazelle:proto disable",
],
"github.com/google/cel-go": [
"gazelle:go_naming_convention go_default_library",
],
"github.com/google/gnostic": [
"gazelle:proto disable",
],
"github.com/google/gnostic-models": [
"gazelle:proto disable",
],
"github.com/google/safetext": [
"gazelle:build_file_name BUILD.bazel",
"gazelle:build_file_proto_mode disable_global",
],
"github.com/googleapis/gax-go/v2": [
"gazelle:proto disable",
],
"github.com/googleapis/gnostic": [
"gazelle:proto disable",
],
"github.com/grpc-ecosystem/grpc-gateway": [
"gazelle:resolve go github.com/grpc-ecosystem/grpc-gateway/internal //internal:go_default_library",
"gazelle:go_naming_convention import_alias",
Expand All @@ -86,18 +29,6 @@ DEFAULT_DIRECTIVES_BY_PATH = {
# in go files are generated by gogo proto. Resolving to the gogo proto target preserves the behavior of Go modules.
"gazelle:resolve go github.com/mwitkow/go-proto-validators @com_github_mwitkow_go_proto_validators//:validators_gogo",
],
"google.golang.org/grpc": [
"gazelle:proto disable",
],
"google.golang.org/protobuf": [
"gazelle:proto disable",
],
"k8s.io/api": [
"gazelle:proto disable",
],
"k8s.io/apiextensions-apiserver": [
"gazelle:proto disable",
],
"k8s.io/apimachinery": [
"gazelle:go_generate_proto false",
"gazelle:proto_import_prefix k8s.io/apimachinery",
Expand Down
39 changes: 15 additions & 24 deletions internal/bzlmod/go_deps.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,27 +47,23 @@ https://github.com/bazelbuild/bazel-gazelle/tree/master/internal/bzlmod/default_

_GAZELLE_ATTRS = {
"build_file_generation": attr.string(
default = "on",
doc = """One of `"auto"`, `"on"` (default), `"off"`, `"clean"`.
default = "clean",
doc = """One of `"clean"` (default), `"update"`, `"off"`
Whether Gazelle should generate build files for the Go module.
Although "auto" is the default globally for build_file_generation,
if a `"gazelle_override"` or `"gazelle_default_attributes"` tag is present
for a Go module, the `"build_file_generation"` attribute will default to "on"
since these tags indicate the presence of `"directives"` or `"build_extra_args"`.
In `"clean"` mode (default), Gazelle will ignore bazel files from the repository,
and create new files.
In `"auto"` mode, Gazelle will run if there is no build file in the Go
module's root directory.
In `"clean"` mode, Gazelle will first remove any existing build files.
In `"update"` mode, Gazelle will attempt to use and update the existing files
in the repository.
In `"off"` mode, Gazelle will leave the BUILD files from the repository intact.
""",
values = [
"auto",
"off",
"on",
"clean",
"update",
"off",
],
),
"build_extra_args": attr.string_list(
Expand Down Expand Up @@ -123,35 +119,30 @@ def _get_override_or_default(specific_overrides, gazelle_default_attributes, def
# 1st: Check for user-provided specific overrides. If a specific override is found,
# all of its attributes will be applied (even if left to the tag's default). This is to allow
# users to override the gazelle_default_attributes tag back to the tag's default.
#
# This will also cause "build_file_generation" to default to "on" if a specific override is found.
specific_override = specific_overrides.get(path)
if specific_override and hasattr(specific_override, attribute_name):
if specific_override != None and hasattr(specific_override, attribute_name):
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason for the explicit none check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An empty list is False, but we do want to consider that to be an override.

In the default list of overrides I'm adding

"github.com/bazelbuild/buildtools": [],

Since buildtools doesn't like proto:disable. However, the None check is needed since this is still considered False

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The above is partially incorrect: buildtools doesn't even have proto code used by gazelle. That said, the rest is still true: the None check is needed so that we can specify an empty list, which otherwise would get reverted back to the default (proto:disable).

return getattr(specific_override, attribute_name)

# 2nd. Check for default attributes provided by the user. This must be done before checking for
# gazelle's defaults path overrides to prevent Gazelle from overriding a user-specified flag.
#
# This will also cause "build_file_generation" to default to "on" if default attributes are found.
global_override_value = getattr(gazelle_default_attributes, attribute_name, None)
if global_override_value:
if global_override_value != None:
return global_override_value

# 3rd: Check for default overrides for specific path.
default_path_override = default_path_overrides.get(path)
if default_path_override:
if default_path_override != None:
return default_path_override

# 4th. Return the default value if no override was found.
# This will cause "build_file_generation" to default to "auto".
return default_value

def _get_directives(path, gazelle_overrides, gazelle_default_attributes):
return _get_override_or_default(gazelle_overrides, gazelle_default_attributes, DEFAULT_DIRECTIVES_BY_PATH, path, [], "directives")
return _get_override_or_default(gazelle_overrides, gazelle_default_attributes, DEFAULT_DIRECTIVES_BY_PATH, path, ["gazelle:proto disable"], "directives")

def _get_build_file_generation(path, gazelle_overrides, gazelle_default_attributes):
# The default value for build_file_generation is "auto" if no override is found, but will default to "on" if an override is found.
return _get_override_or_default(gazelle_overrides, gazelle_default_attributes, DEFAULT_BUILD_FILE_GENERATION_BY_PATH, path, "auto", "build_file_generation")
# The default value for build_file_generation is "clean" if no override is found
return _get_override_or_default(gazelle_overrides, gazelle_default_attributes, DEFAULT_BUILD_FILE_GENERATION_BY_PATH, path, "clean", "build_file_generation")

def _get_build_extra_args(path, gazelle_overrides, gazelle_default_attributes):
return _get_override_or_default(gazelle_overrides, gazelle_default_attributes, DEFAULT_BUILD_EXTRA_ARGS_BY_PATH, path, [], "build_extra_args")
Expand Down
18 changes: 6 additions & 12 deletions internal/go_repository.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def _go_repository_impl(ctx):
# https://docs.bazel.build/versions/main/skylark/repository_rules.html#when-is-the-implementation-function-executed
go_env_cache = str(ctx.path(Label("@bazel_gazelle_go_repository_cache//:go.env")))
fetch_repo = str(ctx.path(Label("@bazel_gazelle_go_repository_tools//:bin/fetch_repo{}".format(executable_extension(ctx)))))
generate = ctx.attr.build_file_generation in ["on", "clean"]
generate = ctx.attr.build_file_generation in ["clean", "update"]
_gazelle = "@bazel_gazelle_go_repository_tools//:bin/gazelle{}".format(executable_extension(ctx))
if generate:
gazelle_path = ctx.path(Label(_gazelle))
Expand Down Expand Up @@ -295,8 +295,6 @@ def _go_repository_impl(ctx):
existing_build_file = name
break

generate = generate or (not existing_build_file and ctx.attr.build_file_generation == "auto")

if generate:
# Build file generation is needed. Populate Gazelle directive at root build file
build_file_name = existing_build_file or build_file_names[0]
Expand Down Expand Up @@ -553,18 +551,14 @@ go_repository = repository_rule(
file systems.""",
),
"build_file_generation": attr.string(
default = "auto",
doc = """One of `"auto"`, `"on"`, `"off"`, `"clean"`.
default = "clean",
doc = """One of `"clean"` (default), `"update"`, `"off"`.

Whether Gazelle should generate build files in the repository. In `"auto"`
mode, Gazelle will run if there is no build file in the repository root
directory. In `"clean"` mode, Gazelle will first remove any existing build
files.""",
Whether Gazelle should generate build files in the repository.""",
values = [
"on",
"auto",
"off",
"clean",
"update",
"off",
],
),
"build_naming_convention": attr.string(
Expand Down
2 changes: 1 addition & 1 deletion language/go/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ type moduleRepo struct {

var (
validBuildExternalAttr = []string{"external", "vendored"}
validBuildFileGenerationAttr = []string{"auto", "on", "off"}
validBuildFileGenerationAttr = []string{"clean", "update", "off"}
validBuildFileProtoModeAttr = []string{"default", "legacy", "disable", "disable_global", "package"}
)

Expand Down
2 changes: 1 addition & 1 deletion repository.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ go_repository(
| <a id="go_repository-build_directives"></a>build_directives | A list of directives to be written to the root level build file before Calling Gazelle to generate build files. Each string in the list will be prefixed with `#` automatically. A common use case is to pass a list of Gazelle directives. | List of strings | optional | `[]` |
| <a id="go_repository-build_external"></a>build_external | One of `"external"`, `"static"` or `"vendored"`.<br><br>This sets Gazelle's `-external` command line flag. In `"static"` mode, Gazelle will not call out to the network to resolve imports.<br><br>**NOTE:** This cannot be used to ignore the `vendor` directory in a repository. The `-external` flag only controls how Gazelle resolves imports which are not present in the repository. Use `build_extra_args = ["-exclude=vendor"]` instead. | String | optional | `"static"` |
| <a id="go_repository-build_extra_args"></a>build_extra_args | A list of additional command line arguments to pass to Gazelle when generating build files. | List of strings | optional | `[]` |
| <a id="go_repository-build_file_generation"></a>build_file_generation | One of `"auto"`, `"on"`, `"off"`, `"clean"`.<br><br>Whether Gazelle should generate build files in the repository. In `"auto"` mode, Gazelle will run if there is no build file in the repository root directory. In `"clean"` mode, Gazelle will first remove any existing build files. | String | optional | `"auto"` |
| <a id="go_repository-build_file_generation"></a>build_file_generation | One of `"clean"` (default), `"update"`, `"off"`.<br><br>Whether Gazelle should generate build files in the repository. | String | optional | `"clean"` |
| <a id="go_repository-build_file_name"></a>build_file_name | Comma-separated list of names Gazelle will consider to be build files. If a repository contains files named `build` that aren't related to Bazel, it may help to set this to `"BUILD.bazel"`, especially on case-insensitive file systems. | String | optional | `"BUILD.bazel,BUILD"` |
| <a id="go_repository-build_file_proto_mode"></a>build_file_proto_mode | One of `"default"`, `"legacy"`, `"disable"`, `"disable_global"` or `"package"`.<br><br>This sets Gazelle's `-proto` command line flag. See [Directives] for more information on each mode. | String | optional | `""` |
| <a id="go_repository-build_naming_convention"></a>build_naming_convention | Sets the library naming convention to use when resolving dependencies against this external repository. If unset, the convention from the external workspace is used. Legal values are `go_default_library`, `import`, and `import_alias`.<br><br>See the `gazelle:go_naming_convention` directive in [Directives] for more information. | String | optional | `"import_alias"` |
Expand Down
22 changes: 2 additions & 20 deletions tests/bcr/go_mod/MODULE.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,31 +38,12 @@ go_deps.config(
# Validate a go.mod replace directive works.
go_deps.from_file(go_mod = "//:go.mod")
go_deps.gazelle_default_attributes(
build_file_generation = "on",
build_file_generation = "clean",
directives = [
"gazelle:proto disable",
],
)

# By defining `gazelle_default_attributes`, we also must individually
# specify certain overrides from internal/bzlmod/default_gazelle_overrides.bzl
#
# "build_file_generation" defaults to "on" because we provided a "gazelle_override"
# (which contains either directives or build extra args).
go_deps.gazelle_override(
directives = [
"gazelle:build_file_name BUILD.bazel",
"gazelle:build_file_proto_mode disable_global",
],
path = "github.com/google/safetext",
)
go_deps.gazelle_override(
directives = [
"gazelle:build_file_name BUILD.bazel",
],
path = "github.com/envoyproxy/protoc-gen-validate",
)

# Verify that the gazelle:go_naming_convention directive in an override is
# respected.
go_deps.module(
Expand All @@ -88,6 +69,7 @@ go_deps.module_override(

# Test an archive override from a known archive.
go_deps.gazelle_override(
build_file_generation = "off",
directives = [
"gazelle:go_naming_convention go_default_library",
],
Expand Down
6 changes: 2 additions & 4 deletions tests/bcr/go_work/MODULE.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,14 @@ go_deps.config(
# Validate a full go workspace
go_deps.from_file(go_work = "//:go.work")
go_deps.gazelle_default_attributes(
build_file_generation = "on",
build_file_generation = "update",
directives = [
"gazelle:proto disable",
],
)

# By defining `gazelle_default_attributes`, we also must individually
# specify certain overrides from internal/bzlmod/default_gazelle_overrides.bzl
#
# "build_file_generation" defaults to "on" because we provided a "gazelle_override"
# (which contains either directives or build extra args).
go_deps.gazelle_override(
directives = [
"gazelle:build_file_name BUILD.bazel",
Expand Down Expand Up @@ -84,6 +81,7 @@ go_deps.module_override(

# Test an archive override from a known archive.
go_deps.gazelle_override(
build_file_generation = "off",
directives = [
"gazelle:go_naming_convention go_default_library",
],
Expand Down
2 changes: 1 addition & 1 deletion tools/override-generator/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func parseArgs(stderr io.Writer, osArgs []string) (*mainArgs, error) {
flag.StringVar(&a.defName, "def_name", "", "name of the macro definition")
flag.StringVar(&a.outputFile, "output", "", "path to the output file")
flag.StringVar(&a.gazelleRepoName, "gazelle_repo_name", "@bazel_gazelle", "name of the gazelle repo to load go_deps, (default: @bazel_gazelle)")
flag.StringVar(&a.defaultBuildFileGeneration, "default_build_file_generation", "auto", "the default value for build_file_generation attribute")
flag.StringVar(&a.defaultBuildFileGeneration, "default_build_file_generation", "clean", "the default value for build_file_generation attribute")
flag.StringVar(&a.defaultBuildFileProtoMode, "default_build_file_proto_mode", "default", "the default value for build_file_proto_mode attribute")
flag.Parse(osArgs)

Expand Down
14 changes: 7 additions & 7 deletions tools/override-generator/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func TestBzlmodOverride(t *testing.T) {

go_repository(
name = "com_github_apache_thrift",
build_file_generation = "auto",
build_file_generation = "update",
build_file_proto_mode = "default",
importpath = "github.com/apache/thrift",
sum = "h1:cMd2aj52n+8VoAtvSvLn4kDC3aZ6IAkBuqWQ2IDu7wo=",
Expand All @@ -36,7 +36,7 @@ func TestBzlmodOverride(t *testing.T) {
go_repository(
name = "com_github_apache_thrift",
build_extra_args = ["-go_naming_convention_external=go_default_library"],
build_file_generation = "on",
build_file_generation = "update",
build_file_proto_mode = "disable",
importpath = "github.com/apache/thrift",
sum = "h1:cMd2aj52n+8VoAtvSvLn4kDC3aZ6IAkBuqWQ2IDu7wo=",
Expand All @@ -46,7 +46,7 @@ func TestBzlmodOverride(t *testing.T) {

go_deps.gazelle_override(
build_extra_args = ["-go_naming_convention_external=go_default_library"],
build_file_generation = "on",
build_file_generation = "update",
directives = ["gazelle:proto disable"],
path = "github.com/apache/thrift",
)`,
Expand Down Expand Up @@ -95,7 +95,7 @@ func TestBzlmodOverride(t *testing.T) {
"gazelle:resolve go github.com/ClickHouse/clickhouse-go/v2/external @com_github_clickhouse_clickhouse_go_v2//external",
],
build_extra_args = ["-go_naming_convention_external=go_default_library"],
build_file_generation = "on",
build_file_generation = "update",
build_file_proto_mode = "disable",
importpath = "github.com/ClickHouse/clickhouse-go/v2",
sum = "h1:Nbl/NZwoM6LGJm7smNBgvtdr/rxjlIssSW3eG/Nmb9E=",
Expand All @@ -105,7 +105,7 @@ func TestBzlmodOverride(t *testing.T) {

go_deps.gazelle_override(
build_extra_args = ["-go_naming_convention_external=go_default_library"],
build_file_generation = "on",
build_file_generation = "update",
directives = [
"gazelle:resolve go github.com/ClickHouse/clickhouse-go/v2/external @com_github_clickhouse_clickhouse_go_v2//external",
"gazelle:proto disable",
Expand All @@ -121,7 +121,7 @@ func TestBzlmodOverride(t *testing.T) {
"-go_prefix=golang.org/x/tools",
"-exclude=**/testdata",
],
build_file_generation = "on",
build_file_generation = "update",
build_file_proto_mode = "disable",
importpath = "golang.org/x/tools/cmd/goimports",
patch_args = ["-p1"],
Expand Down Expand Up @@ -154,7 +154,7 @@ func TestBzlmodOverride(t *testing.T) {
"-go_prefix=golang.org/x/tools",
"-exclude=**/testdata",
],
build_file_generation = "on",
build_file_generation = "update",
directives = ["gazelle:proto disable"],
path = "golang.org/x/tools/cmd/goimports",
)`,
Expand Down