Skip to content

Commit

Permalink
go_repository: add 'clean' build_file_generation
Browse files Browse the repository at this point in the history
Before bzlmod, repositories were global, and BUILD files inside upstream
repositories could load from the repositories defined at the root-level
WORKSPACE file.

With the introduction of visibility rules, these result in errors that
cannot trivially be worked around. One pattern that has emerged is
ignoring existing build files by specifying `gazelle:build_file_name
BUILD.bazel`, however this does not work for anything already using
BUILD.bazel filenames.

This does affect real-world users: an example is
github.com/google/go-jsonnet which has a BUILD.bazel file to `genrule`
its AST. The module is pure-go, but the build step contains cpp code,
and when imported under bzlmod-gazelle will not work because the cpp
code is invoked. As a workaround, there is now a 'jsonnet' BCR module.

This patch introduces a `build_file_generation = clean` option, which
removes existing build files before generating new ones. It allows
loading the jsonnet code once again, and also makes it possible to
load some of the complex protobuf repositories.
  • Loading branch information
TvdW committed May 15, 2024
1 parent 1541133 commit 3329c41
Show file tree
Hide file tree
Showing 13 changed files with 85 additions and 6 deletions.
2 changes: 1 addition & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ The following flags are accepted:
+----------------------------------------------------------------------------------------------------------+----------------------------------------------+
| Sets the ``build_extra_args attribute`` for the generated `go_repository`_ rule(s). |
+----------------------------------------------------------------------------------------------------------+----------------------------------------------+
| :flag:`-build_file_generation auto|on|off` | |
| :flag:`-build_file_generation auto|on|off|clean` | |
+----------------------------------------------------------------------------------------------------------+----------------------------------------------+
| Sets the ``build_file_generation`` attribute for the generated `go_repository`_ rule(s). |
+----------------------------------------------------------------------------------------------------------+----------------------------------------------+
Expand Down
2 changes: 2 additions & 0 deletions cmd/fetch_repo/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library", "go_test")
go_library(
name = "fetch_repo_lib",
srcs = [
"clean.go",
"copy_tree.go",
"fetch_repo.go",
"go_mod_download.go",
Expand Down Expand Up @@ -36,6 +37,7 @@ filegroup(
testonly = True,
srcs = [
"BUILD.bazel",
"clean.go",
"copy_tree.go",
"fetch_repo.go",
"fetch_repo_test.go",
Expand Down
37 changes: 37 additions & 0 deletions cmd/fetch_repo/clean.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/* Copyright 2024 The Bazel Authors. All rights reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package main

import (
"io/fs"
"os"
"path/filepath"
)

func cleanBuildFiles(path string) error {
return filepath.Walk(*dest, func(path string, info fs.FileInfo, err error) error {
if err != nil {
return err
}
if info.IsDir() {
return nil
}
if info.Name() == "BUILD" || info.Name() == "BUILD.bazel" {
return os.Remove(path)
}
return nil
})
}
7 changes: 7 additions & 0 deletions cmd/fetch_repo/fetch_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ var (
importpath = flag.String("importpath", "", "Go importpath to the repository fetch")
path = flag.String("path", "", "absolute or relative path to a local go module")
dest = flag.String("dest", "", "destination directory")
clean = flag.Bool("clean", false, "remove existing bazel build files")

// Repository flags
remote = flag.String("remote", "", "The URI of the remote repository. Must be used with the --vcs flag.")
Expand Down Expand Up @@ -122,4 +123,10 @@ func main() {
log.Fatal(err)
}
}

if *clean {
if err := cleanBuildFiles(*dest); err != nil {
log.Fatal(err)
}
}
}
5 changes: 4 additions & 1 deletion internal/bzlmod/go_deps.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ 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"`.
doc = """One of `"auto"`, `"on"` (default), `"off"`, `"clean"`.
Whether Gazelle should generate build files for the Go module.
Expand All @@ -60,11 +60,14 @@ _GAZELLE_ATTRS = {
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.
""",
values = [
"auto",
"off",
"on",
"clean",
],
),
"build_extra_args": attr.string_list(
Expand Down
12 changes: 9 additions & 3 deletions internal/go_repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ def _go_repository_impl(ctx):
go_env_cache = str(ctx.path(Label("@bazel_gazelle_go_repository_cache//:go.env")))
if not ctx.attr.urls:
fetch_repo = str(ctx.path(Label("@bazel_gazelle_go_repository_tools//:bin/fetch_repo{}".format(executable_extension(ctx)))))
generate = ctx.attr.build_file_generation == "on"
generate = ctx.attr.build_file_generation in ["on", "clean"]
_gazelle = "@bazel_gazelle_go_repository_tools//:bin/gazelle{}".format(executable_extension(ctx))
if generate:
gazelle_path = ctx.path(Label(_gazelle))
Expand Down Expand Up @@ -193,6 +193,10 @@ def _go_repository_impl(ctx):
else:
fail("one of urls, commit, tag, or version must be specified")

# Clean existing build files if requested
if ctx.attr.build_file_generation == "clean":
fetch_repo_args += ["-clean"]

env = read_cache_env(ctx, go_env_cache)
env_keys = [
# keep sorted
Expand Down Expand Up @@ -515,15 +519,17 @@ go_repository = repository_rule(
),
"build_file_generation": attr.string(
default = "auto",
doc = """One of `"auto"`, `"on"`, `"off"`.
doc = """One of `"auto"`, `"on"`, `"off"`, `"clean"`.
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.""",
directory. In `"clean"` mode, Gazelle will first remove any existing build
files.""",
values = [
"on",
"auto",
"off",
"clean",
],
),
"build_naming_convention": attr.string(
Expand Down
1 change: 1 addition & 0 deletions internal/go_repository_tools_srcs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ GO_REPOSITORY_TOOLS_SRCS = [
Label("//cmd/autogazelle:client_unix.go"),
Label("//cmd/autogazelle:server_unix.go"),
Label("//cmd/fetch_repo:BUILD.bazel"),
Label("//cmd/fetch_repo:clean.go"),
Label("//cmd/fetch_repo:copy_tree.go"),
Label("//cmd/fetch_repo:fetch_repo.go"),
Label("//cmd/fetch_repo:go_mod_download.go"),
Expand Down
2 changes: 1 addition & 1 deletion repository.md
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,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"`.<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. | String | optional | `"auto"` |
| <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_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
7 changes: 7 additions & 0 deletions tests/bcr/go_mod/MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,12 @@ go_deps.archive_override(
],
)

# Test a module that ships with its own BUILD files and first needs rewriting
go_deps.gazelle_override(
build_file_generation = "clean",
path = "github.com/google/go-jsonnet",
)

# Transitive dependencies have to be listed here explicitly.
go_deps.module(
indirect = True,
Expand Down Expand Up @@ -130,6 +136,7 @@ use_repo(
"com_github_datadog_sketches_go",
"com_github_envoyproxy_protoc_gen_validate",
"com_github_fmeum_dep_on_gazelle",
"com_github_google_go_jsonnet",
"com_github_google_safetext",
"com_github_stretchr_testify",
"org_golang_google_protobuf",
Expand Down
3 changes: 3 additions & 0 deletions tests/bcr/go_mod/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ require (
github.com/cloudflare/circl v1.3.7
github.com/envoyproxy/protoc-gen-validate v1.0.1
github.com/fmeum/dep_on_gazelle v1.0.0
github.com/google/go-jsonnet v0.20.0
github.com/google/safetext v0.0.0-20220905092116-b49f7bc46da2
github.com/stretchr/testify v1.6.1
golang.org/x/sys v0.15.0
Expand All @@ -27,9 +28,11 @@ require (
github.com/pmezard/go-difflib v1.0.0 // indirect
golang.org/x/text v0.9.0 // indirect
google.golang.org/protobuf v1.32.0 // indirect
gopkg.in/yaml.v2 v2.2.7 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
rsc.io/quote v1.5.2 // indirect
rsc.io/sampler v1.3.0 // indirect
sigs.k8s.io/yaml v1.1.0 // indirect
)

replace example.org/hello => ../../fixtures/hello
6 changes: 6 additions & 0 deletions tests/bcr/go_mod/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ github.com/google/go-cmp v0.5.1/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/go-jsonnet v0.20.0 h1:WG4TTSARuV7bSm4PMB4ohjxe33IHT5WVTrJSU33uT4g=
github.com/google/go-jsonnet v0.20.0/go.mod h1:VbgWF9JX7ztlv770x/TolZNGGFfiHEVx9G6ca2eUmeA=
github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0=
github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
github.com/google/safetext v0.0.0-20220905092116-b49f7bc46da2 h1:SJ+NtwL6QaZ21U+IrK7d0gGgpjGGvd2kz+FzTHVzdqI=
Expand Down Expand Up @@ -117,6 +119,8 @@ google.golang.org/protobuf v1.32.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHh
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b h1:QRR6H1YWRnHb4Y/HeNFCTJLFVxaq6wH4YuVdsUOr75U=
gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v2 v2.2.7 h1:VUgggvou5XRW9mHwD/yXxIYSMtY0zoKQf/v226p2nyo=
gopkg.in/yaml.v2 v2.2.7/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
Expand All @@ -126,3 +130,5 @@ rsc.io/quote v1.5.2 h1:w5fcysjrx7yqtD/aO+QwRjYZOKnaM9Uh2b40tElTs3Y=
rsc.io/quote v1.5.2/go.mod h1:LzX7hefJvL54yjefDEDHNONDjII0t9xZLPXsUe+TKr0=
rsc.io/sampler v1.3.0 h1:7uVkIFmeBqHfdjD+gZwtXXI+RODJ2Wc4O7MPEh/QiW4=
rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=
sigs.k8s.io/yaml v1.1.0 h1:4A07+ZFc2wgJwo8YNlQpr1rVlgUDlxXHhPJciaPY5gs=
sigs.k8s.io/yaml v1.1.0/go.mod h1:UJmg0vDUVViEyp3mgSv9WPwZCDxu4rQW1olrI1uml+o=
1 change: 1 addition & 0 deletions tests/bcr/go_mod/pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ go_test(
"@com_github_datadog_sketches_go//ddsketch",
"@com_github_envoyproxy_protoc_gen_validate//validate",
"@com_github_fmeum_dep_on_gazelle//:dep_on_gazelle",
"@com_github_google_go_jsonnet//:go-jsonnet",
"@com_github_google_safetext//yamltemplate",
"@com_github_stretchr_testify//require:go_default_library",
"@my_rules_go//go/runfiles",
Expand Down
6 changes: 6 additions & 0 deletions tests/bcr/go_mod/pkg/pkg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/bmatcuk/doublestar/v4"
"github.com/cloudflare/circl/dh/x25519"
"github.com/fmeum/dep_on_gazelle"
"github.com/google/go-jsonnet"
"github.com/google/safetext/yamltemplate"
"github.com/stretchr/testify/require"

Expand All @@ -38,6 +39,11 @@ func TestBuildFileGeneration(t *testing.T) {
yamltemplate.HTMLEscapeString("<b>foo</b>")
}

func TestCleanBuildFileGeneration(t *testing.T) {
// github.com/google/go-jsonnet@v0.20.0 requires fully replacing the BUILD files it provides
jsonnet.Version()
}

func TestGeneratedFilesPreferredOverProtos(t *testing.T) {
_, _ = ddsketch.NewDefaultDDSketch(0.01)
}
Expand Down

0 comments on commit 3329c41

Please sign in to comment.