-
Notifications
You must be signed in to change notification settings - Fork 380
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
Allow ListRepositories to load macros not in Workspace #1053
Conversation
Is it safe to assume all repository_macros only contains repository definitions or other repository_macros? In that case, we don't need to specify |
In my personal opinion, I feel that it would be not safe to do this because it opens the potential for more unnecessary recursive calls. Firstly, this function finds all the |
@jayconrod We are adding a new syntax
The |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR keeps failing on Windows. Please fix the test on Windows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jayconrod @blico I don't see further issues. Do you want to take another look?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also modify the repository_macro
doc in readme.rst
to describe the added functionality
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just a few more minor comments.
[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [bazel_gazelle](https://togithub.com/bazelbuild/bazel-gazelle) | http_archive | minor | `v0.22.2` -> `v0.24.0` | --- ### Release Notes <details> <summary>bazelbuild/bazel-gazelle</summary> ### [`v0.24.0`](https://togithub.com/bazelbuild/bazel-gazelle/releases/v0.24.0) [Compare Source](https://togithub.com/bazelbuild/bazel-gazelle/compare/v0.23.0...v0.24.0) This release requires [rules_go 0.29](https://togithub.com/bazelbuild/rules_go/releases/tag/v0.29.0) to support the retirement of `go_tool_library`. #### Changes - .netrc files are now supported for authenticated access for `go_repository` rules ([#​1090](https://togithub.com/bazelbuild/bazel-gazelle/issues/1090)) - There is now a Gazelle extension for authoring BUILD files for the R language, using [rules_r](https://togithub.com/grailbio/rules_r). #### Fixes - language/go: don't let 'go mod download' edit go.sum by [@​jayconrod](https://togithub.com/jayconrod) in [https://github.com/bazelbuild/bazel-gazelle/pull/1015](https://togithub.com/bazelbuild/bazel-gazelle/pull/1015) - Label fix: properly parse external references to the main repository by [@​tfrench-uber](https://togithub.com/tfrench-uber) in [https://github.com/bazelbuild/bazel-gazelle/pull/1006](https://togithub.com/bazelbuild/bazel-gazelle/pull/1006) - gazelle: add -e flag for go list 1.16 by [@​axelberardino](https://togithub.com/axelberardino) in [https://github.com/bazelbuild/bazel-gazelle/pull/1019](https://togithub.com/bazelbuild/bazel-gazelle/pull/1019) - Correctly propagate runfiles from gazelle_bin to gazelle by [@​HALtheWise](https://togithub.com/HALtheWise) in [https://github.com/bazelbuild/bazel-gazelle/pull/1008](https://togithub.com/bazelbuild/bazel-gazelle/pull/1008) - Support Label [@​repo](https://togithub.com/repo) -> @​repo//:repo shorthand by [@​wolfd](https://togithub.com/wolfd) in [https://github.com/bazelbuild/bazel-gazelle/pull/1023](https://togithub.com/bazelbuild/bazel-gazelle/pull/1023) - language/go: rewrite embedResolver to use a tree structure by [@​jayconrod](https://togithub.com/jayconrod) in [https://github.com/bazelbuild/bazel-gazelle/pull/1024](https://togithub.com/bazelbuild/bazel-gazelle/pull/1024) - Respect .bazelignore by [@​Michaelhobo](https://togithub.com/Michaelhobo) in [https://github.com/bazelbuild/bazel-gazelle/pull/1022](https://togithub.com/bazelbuild/bazel-gazelle/pull/1022) - Change merge behavior to remove attrs instead of panicking by [@​wolfd](https://togithub.com/wolfd) in [https://github.com/bazelbuild/bazel-gazelle/pull/1031](https://togithub.com/bazelbuild/bazel-gazelle/pull/1031) - Allow ListRepositories to load macros not in Workspace by [@​tfrench-uber](https://togithub.com/tfrench-uber) in [https://github.com/bazelbuild/bazel-gazelle/pull/1053](https://togithub.com/bazelbuild/bazel-gazelle/pull/1053) - Honor host GOMODCACHE when GO_REPOSITORY_USE_HOST_CACHE by [@​linzhp](https://togithub.com/linzhp) in [https://github.com/bazelbuild/bazel-gazelle/pull/1069](https://togithub.com/bazelbuild/bazel-gazelle/pull/1069) - language/go: add golang.org/x/tools/internal/typeparams to nogo deps list by [@​jayconrod](https://togithub.com/jayconrod) in [https://github.com/bazelbuild/bazel-gazelle/pull/1074](https://togithub.com/bazelbuild/bazel-gazelle/pull/1074) - Fix "exclude" token parsing by [@​gibfahn](https://togithub.com/gibfahn) in [https://github.com/bazelbuild/bazel-gazelle/pull/1073](https://togithub.com/bazelbuild/bazel-gazelle/pull/1073) - Have label properly resolve directives starting with "@​//" by [@​tfrench-uber](https://togithub.com/tfrench-uber) in [https://github.com/bazelbuild/bazel-gazelle/pull/1086](https://togithub.com/bazelbuild/bazel-gazelle/pull/1086) - Correctly propagate runfiles from data dependencies to gazelle by [@​alexeagle](https://togithub.com/alexeagle) in [https://github.com/bazelbuild/bazel-gazelle/pull/1094](https://togithub.com/bazelbuild/bazel-gazelle/pull/1094) - Add bzl_library targets to Gazelle by [@​achew22](https://togithub.com/achew22) in [https://github.com/bazelbuild/bazel-gazelle/pull/760](https://togithub.com/bazelbuild/bazel-gazelle/pull/760) - Escape vars in cgo flags with an extra $ by [@​dierksen](https://togithub.com/dierksen) in [https://github.com/bazelbuild/bazel-gazelle/pull/1107](https://togithub.com/bazelbuild/bazel-gazelle/pull/1107) - autogazelle can build on windows by [@​asuffield](https://togithub.com/asuffield) in [https://github.com/bazelbuild/bazel-gazelle/pull/1083](https://togithub.com/bazelbuild/bazel-gazelle/pull/1083) - and many documentation updates by [@​tanyabouman](https://togithub.com/tanyabouman) **Full Changelog**: bazel-contrib/bazel-gazelle@v0.23.0...v0.24.0 #### `WORKSPACE` code load("@​bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "bazel_gazelle", sha256 = "de69a09dc70417580aabf20a28619bb3ef60d038470c7cf8442fafcf627c21cb", urls = [ "https://mirror.bazel.build/github.com/bazelbuild/bazel-gazelle/releases/download/v0.24.0/bazel-gazelle-v0.24.0.tar.gz", "https://github.com/bazelbuild/bazel-gazelle/releases/download/v0.24.0/bazel-gazelle-v0.24.0.tar.gz", ], ) load("@​bazel_gazelle//:deps.bzl", "gazelle_dependencies") gazelle_dependencies() ### [`v0.23.0`](https://togithub.com/bazelbuild/bazel-gazelle/releases/v0.23.0) [Compare Source](https://togithub.com/bazelbuild/bazel-gazelle/compare/v0.22.3...v0.23.0) #### Changes - Go - `CXXFLAGS` and `CPPFLAGS` `#cgo` directives are separated into `cxxopts` and `cppopts` attributes, respectively. Thanks [@​otan](https://togithub.com/otan). - Dependencies on mapped kinds are now supported. Thanks [@​robfig](https://togithub.com/robfig). - The `gazelle` rule now supports the `update-repos` command. - The `gazelle` rule now has a `data` attribute and expands `$(location)` within arguments. - `go_repository`'s `build_naming_convention` is now considered when resolving external dependencies. If a repository already has build files, this attribute may be set to indicate which naming convention it follows. - `embedsrcs` attributes are generated for packages that contain `//go:embed` directives. - Protobuf - `proto_strip_import_prefix` may be set in the root build file. Thanks [@​linzhp](https://togithub.com/linzhp). #### `WORKSPACE` code load("@​bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "bazel_gazelle", sha256 = "62ca106be173579c0a167deb23358fdfe71ffa1e4cfdddf5582af26520f1c66f", urls = [ "https://mirror.bazel.build/github.com/bazelbuild/bazel-gazelle/releases/download/v0.23.0/bazel-gazelle-v0.23.0.tar.gz", "https://github.com/bazelbuild/bazel-gazelle/releases/download/v0.23.0/bazel-gazelle-v0.23.0.tar.gz", ], ) load("@​bazel_gazelle//:deps.bzl", "gazelle_dependencies") gazelle_dependencies() ### [`v0.22.3`](https://togithub.com/bazelbuild/bazel-gazelle/releases/v0.22.3) [Compare Source](https://togithub.com/bazelbuild/bazel-gazelle/compare/v0.22.2...v0.22.3) #### Bug fixes - `-mode=diff` treats empty build files correctly. (thanks [@​vpanta](https://togithub.com/vpanta)) - Gazelle should walk subdirectories more quickly when indexing is disabled. (thanks [@​blico](https://togithub.com/blico)) - Fixed dependency resolution with mapped kinds. (thanks [@​robfig](https://togithub.com/robfig)) - Compatibility fixes for Go 1.16. #### WORKSPACE code http_archive( name = "bazel_gazelle", sha256 = "222e49f034ca7a1d1231422cdb67066b885819885c356673cb1f72f748a3c9d4", urls = [ "https://mirror.bazel.build/github.com/bazelbuild/bazel-gazelle/releases/download/v0.22.3/bazel-gazelle-v0.22.3.tar.gz", "https://github.com/bazelbuild/bazel-gazelle/releases/download/v0.22.3/bazel-gazelle-v0.22.3.tar.gz", ], ) load("@​bazel_gazelle//:deps.bzl", "gazelle_dependencies") gazelle_dependencies() </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box. --- This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/gapic-config-validator).
[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [bazel_gazelle](https://togithub.com/bazelbuild/bazel-gazelle) | http_archive | minor | `v0.22.2` -> `v0.24.0` | --- ### Release Notes <details> <summary>bazelbuild/bazel-gazelle</summary> ### [`v0.24.0`](https://togithub.com/bazelbuild/bazel-gazelle/releases/v0.24.0) [Compare Source](https://togithub.com/bazelbuild/bazel-gazelle/compare/v0.23.0...v0.24.0) This release requires [rules_go 0.29](https://togithub.com/bazelbuild/rules_go/releases/tag/v0.29.0) to support the retirement of `go_tool_library`. #### Changes - .netrc files are now supported for authenticated access for `go_repository` rules ([#​1090](https://togithub.com/bazelbuild/bazel-gazelle/issues/1090)) - There is now a Gazelle extension for authoring BUILD files for the R language, using [rules_r](https://togithub.com/grailbio/rules_r). #### Fixes - language/go: don't let 'go mod download' edit go.sum by [@​jayconrod](https://togithub.com/jayconrod) in [https://github.com/bazelbuild/bazel-gazelle/pull/1015](https://togithub.com/bazelbuild/bazel-gazelle/pull/1015) - Label fix: properly parse external references to the main repository by [@​tfrench-uber](https://togithub.com/tfrench-uber) in [https://github.com/bazelbuild/bazel-gazelle/pull/1006](https://togithub.com/bazelbuild/bazel-gazelle/pull/1006) - gazelle: add -e flag for go list 1.16 by [@​axelberardino](https://togithub.com/axelberardino) in [https://github.com/bazelbuild/bazel-gazelle/pull/1019](https://togithub.com/bazelbuild/bazel-gazelle/pull/1019) - Correctly propagate runfiles from gazelle_bin to gazelle by [@​HALtheWise](https://togithub.com/HALtheWise) in [https://github.com/bazelbuild/bazel-gazelle/pull/1008](https://togithub.com/bazelbuild/bazel-gazelle/pull/1008) - Support Label [@​repo](https://togithub.com/repo) -> @​repo//:repo shorthand by [@​wolfd](https://togithub.com/wolfd) in [https://github.com/bazelbuild/bazel-gazelle/pull/1023](https://togithub.com/bazelbuild/bazel-gazelle/pull/1023) - language/go: rewrite embedResolver to use a tree structure by [@​jayconrod](https://togithub.com/jayconrod) in [https://github.com/bazelbuild/bazel-gazelle/pull/1024](https://togithub.com/bazelbuild/bazel-gazelle/pull/1024) - Respect .bazelignore by [@​Michaelhobo](https://togithub.com/Michaelhobo) in [https://github.com/bazelbuild/bazel-gazelle/pull/1022](https://togithub.com/bazelbuild/bazel-gazelle/pull/1022) - Change merge behavior to remove attrs instead of panicking by [@​wolfd](https://togithub.com/wolfd) in [https://github.com/bazelbuild/bazel-gazelle/pull/1031](https://togithub.com/bazelbuild/bazel-gazelle/pull/1031) - Allow ListRepositories to load macros not in Workspace by [@​tfrench-uber](https://togithub.com/tfrench-uber) in [https://github.com/bazelbuild/bazel-gazelle/pull/1053](https://togithub.com/bazelbuild/bazel-gazelle/pull/1053) - Honor host GOMODCACHE when GO_REPOSITORY_USE_HOST_CACHE by [@​linzhp](https://togithub.com/linzhp) in [https://github.com/bazelbuild/bazel-gazelle/pull/1069](https://togithub.com/bazelbuild/bazel-gazelle/pull/1069) - language/go: add golang.org/x/tools/internal/typeparams to nogo deps list by [@​jayconrod](https://togithub.com/jayconrod) in [https://github.com/bazelbuild/bazel-gazelle/pull/1074](https://togithub.com/bazelbuild/bazel-gazelle/pull/1074) - Fix "exclude" token parsing by [@​gibfahn](https://togithub.com/gibfahn) in [https://github.com/bazelbuild/bazel-gazelle/pull/1073](https://togithub.com/bazelbuild/bazel-gazelle/pull/1073) - Have label properly resolve directives starting with "@​//" by [@​tfrench-uber](https://togithub.com/tfrench-uber) in [https://github.com/bazelbuild/bazel-gazelle/pull/1086](https://togithub.com/bazelbuild/bazel-gazelle/pull/1086) - Correctly propagate runfiles from data dependencies to gazelle by [@​alexeagle](https://togithub.com/alexeagle) in [https://github.com/bazelbuild/bazel-gazelle/pull/1094](https://togithub.com/bazelbuild/bazel-gazelle/pull/1094) - Add bzl_library targets to Gazelle by [@​achew22](https://togithub.com/achew22) in [https://github.com/bazelbuild/bazel-gazelle/pull/760](https://togithub.com/bazelbuild/bazel-gazelle/pull/760) - Escape vars in cgo flags with an extra $ by [@​dierksen](https://togithub.com/dierksen) in [https://github.com/bazelbuild/bazel-gazelle/pull/1107](https://togithub.com/bazelbuild/bazel-gazelle/pull/1107) - autogazelle can build on windows by [@​asuffield](https://togithub.com/asuffield) in [https://github.com/bazelbuild/bazel-gazelle/pull/1083](https://togithub.com/bazelbuild/bazel-gazelle/pull/1083) - and many documentation updates by [@​tanyabouman](https://togithub.com/tanyabouman) **Full Changelog**: bazel-contrib/bazel-gazelle@v0.23.0...v0.24.0 #### `WORKSPACE` code load("@​bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "bazel_gazelle", sha256 = "de69a09dc70417580aabf20a28619bb3ef60d038470c7cf8442fafcf627c21cb", urls = [ "https://mirror.bazel.build/github.com/bazelbuild/bazel-gazelle/releases/download/v0.24.0/bazel-gazelle-v0.24.0.tar.gz", "https://github.com/bazelbuild/bazel-gazelle/releases/download/v0.24.0/bazel-gazelle-v0.24.0.tar.gz", ], ) load("@​bazel_gazelle//:deps.bzl", "gazelle_dependencies") gazelle_dependencies() ### [`v0.23.0`](https://togithub.com/bazelbuild/bazel-gazelle/releases/v0.23.0) [Compare Source](https://togithub.com/bazelbuild/bazel-gazelle/compare/v0.22.3...v0.23.0) #### Changes - Go - `CXXFLAGS` and `CPPFLAGS` `#cgo` directives are separated into `cxxopts` and `cppopts` attributes, respectively. Thanks [@​otan](https://togithub.com/otan). - Dependencies on mapped kinds are now supported. Thanks [@​robfig](https://togithub.com/robfig). - The `gazelle` rule now supports the `update-repos` command. - The `gazelle` rule now has a `data` attribute and expands `$(location)` within arguments. - `go_repository`'s `build_naming_convention` is now considered when resolving external dependencies. If a repository already has build files, this attribute may be set to indicate which naming convention it follows. - `embedsrcs` attributes are generated for packages that contain `//go:embed` directives. - Protobuf - `proto_strip_import_prefix` may be set in the root build file. Thanks [@​linzhp](https://togithub.com/linzhp). #### `WORKSPACE` code load("@​bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "bazel_gazelle", sha256 = "62ca106be173579c0a167deb23358fdfe71ffa1e4cfdddf5582af26520f1c66f", urls = [ "https://mirror.bazel.build/github.com/bazelbuild/bazel-gazelle/releases/download/v0.23.0/bazel-gazelle-v0.23.0.tar.gz", "https://github.com/bazelbuild/bazel-gazelle/releases/download/v0.23.0/bazel-gazelle-v0.23.0.tar.gz", ], ) load("@​bazel_gazelle//:deps.bzl", "gazelle_dependencies") gazelle_dependencies() ### [`v0.22.3`](https://togithub.com/bazelbuild/bazel-gazelle/releases/v0.22.3) [Compare Source](https://togithub.com/bazelbuild/bazel-gazelle/compare/v0.22.2...v0.22.3) #### Bug fixes - `-mode=diff` treats empty build files correctly. (thanks [@​vpanta](https://togithub.com/vpanta)) - Gazelle should walk subdirectories more quickly when indexing is disabled. (thanks [@​blico](https://togithub.com/blico)) - Fixed dependency resolution with mapped kinds. (thanks [@​robfig](https://togithub.com/robfig)) - Compatibility fixes for Go 1.16. #### WORKSPACE code http_archive( name = "bazel_gazelle", sha256 = "222e49f034ca7a1d1231422cdb67066b885819885c356673cb1f72f748a3c9d4", urls = [ "https://mirror.bazel.build/github.com/bazelbuild/bazel-gazelle/releases/download/v0.22.3/bazel-gazelle-v0.22.3.tar.gz", "https://github.com/bazelbuild/bazel-gazelle/releases/download/v0.22.3/bazel-gazelle-v0.22.3.tar.gz", ], ) load("@​bazel_gazelle//:deps.bzl", "gazelle_dependencies") gazelle_dependencies() </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box. --- This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/gapic-generator-go).
What type of PR is this?
Feature
What package or component does this PR mostly affect?
repo
What does this PR do? Why is it needed?
The PR allows a user to load other repository macros from within a top-level repository macro, so that it does not have to be added to the Workspace file. These macros with a second layer are identified by prepending a "+" to the beginning of the path in the repository_macro directive:
e.g.
# gazelle:repository_macro +repos.bzl%go_repositories
would loadrepos.bzl%go_repositories
and any other files loaded from within the macro.This is needed because the functionality already exists and is legal in bazel: A
go_repository( ... )
rule can be added to a macro, loaded by another macro, and loaded by the workspace. However, gazelle will not resolve this repository, and create a duplicate rule, thereby creating incorrect/duplicate Starlark content.In very large repositories, this is important when dependency management needs to happen in many different file spaces.
This change uses a highly conservative opt-in approach, although since it is legal in Bazel, perhaps this "opt-in" requirement can be removed in the future. With this implementation, ListRepositories will correctly list all of the
go_repository(
imports in that file, whether directly or loaded from another macro, as Bazel would similarly load.For all existing uses (macro files existing as references only from Workspace file), no changes would be made. Only in cases where a user specifically decides to load another macro, and identify this desire in the gazelle directive.
Which issues(s) does this PR fix?
Fixes #1049