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

Make # gazelle:proto file work without needing to set different option go_package in .proto files #1765

Merged
merged 18 commits into from
May 20, 2024

Conversation

jeromep-stripe
Copy link
Contributor

@jeromep-stripe jeromep-stripe commented Mar 25, 2024

What type of PR is this?
Feature

What package or component does this PR mostly affect?
language/go

What does this PR do? Why is it needed?
This changes the gazelle's go_proto_library generation behavior.
Previously, in proto file mode, gazelle would generate a separate go_proto_library target for each proto_library target, and these would have the same values for the importpath attribute. This caused build errors in certain situations.

This change makes Gazelle group proto_library targets which would otherwise have the same go_proto_library importpath, into a single go_proto_library using the protos attribute.

Which issues(s) does this PR fix?

Fixes #1724

Other notes for review

  • While testing this PR, I ran bazelisk test //... and saw two tests fail
FAIL: //cmd/gazelle:gazelle_test (see /private/var/tmp/_bazel/7faad61fac7f6d421e59e31d0146c79e/execroot/_main/bazel-out/darwin_arm64-fastbuild/testlogs/cmd/gazelle/gazelle_test/test.log)
INFO: From Testing //cmd/gazelle:gazelle_test:
==================== Test output for //cmd/gazelle:gazelle_test:
could not locate go tool
================================================================================
FAIL: //internal:bazel_test (see /private/var/tmp/_bazel/7faad61fac7f6d421e59e31d0146c79e/execroot/_main/bazel-out/darwin_arm64-fastbuild/testlogs/internal/bazel_test/test.log)
INFO: From Testing //internal:bazel_test:
==================== Test output for //internal:bazel_test:
error: unknown runfile: ./.bazelrc
================================================================================

This looks like an environment issue. Looks like CI is green though, so I think we can ignore this.

  • When generating a single go_proto_library target for multiple proto_library targets, the change creates a combinedImports platformStringsBuilder variable. I'm not sure if my general approach to combining the dependencies is correct, and would appreciate guidance.

@jeromep-stripe jeromep-stripe marked this pull request as ready for review March 25, 2024 12:15
@ouguoc2-stripe
Copy link

@linzhp FYI -- let @jeromep-stripe know what you think.

@ouguoc2-stripe
Copy link

Ping on this @linzhp -- let us know what we can do to help.

@linzhp
Copy link
Contributor

linzhp commented Apr 29, 2024

Oh, sorry, I missed the previous ping. Let me take a look later this week.

@ouguoc2-stripe
Copy link

(Gentle nudge on this; hope you have a good weekend, happy to discuss if you'd like.)

language/go/config.go Outdated Show resolved Hide resolved
language/go/generate.go Outdated Show resolved Hide resolved
language/go/generate.go Outdated Show resolved Hide resolved
language/go/generate.go Outdated Show resolved Hide resolved
@linzhp
Copy link
Contributor

linzhp commented May 5, 2024

Some minor comments. LGTM overall. I also tested it in Uber's Go repo.

@jeromep-stripe
Copy link
Contributor Author

@linzhp ty for the review, I addressed your comments!

language/go/config.go Outdated Show resolved Hide resolved
@linzhp linzhp enabled auto-merge (squash) May 18, 2024 23:37
@linzhp linzhp merged commit 6a27127 into bazel-contrib:master May 20, 2024
15 checks passed
renovate bot referenced this pull request in kreempuff/rules_unreal_engine Aug 1, 2024
[![Mend
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.36.0` -> `v0.38.0` |

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>bazelbuild/bazel-gazelle (bazel_gazelle)</summary>

###
[`v0.38.0`](https://togithub.com/bazelbuild/bazel-gazelle/releases/tag/v0.38.0)

[Compare
Source](https://togithub.com/bazelbuild/bazel-gazelle/compare/v0.37.0...v0.38.0)

#### What's Changed

- Add support for `include()` in `MODULE.bazel` by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1810](https://togithub.com/bazelbuild/bazel-gazelle/pull/1810)
- feat: gazelle_test test rule by
[@&#8203;hunshcn](https://togithub.com/hunshcn) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1785](https://togithub.com/bazelbuild/bazel-gazelle/pull/1785)
- Handle arm64 host platform for MacOS by
[@&#8203;smocherla-brex](https://togithub.com/smocherla-brex) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1817](https://togithub.com/bazelbuild/bazel-gazelle/pull/1817)
- go_repository: add 'clean' build_file_generation by
[@&#8203;TvdW](https://togithub.com/TvdW) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1802](https://togithub.com/bazelbuild/bazel-gazelle/pull/1802)
- fix: support leading ./ in .bazelignore by
[@&#8203;jbedard](https://togithub.com/jbedard) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1828](https://togithub.com/bazelbuild/bazel-gazelle/pull/1828)
- Restore compatibility with Go 1.18 by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1833](https://togithub.com/bazelbuild/bazel-gazelle/pull/1833)
- Remove reliance on specific canonical repo name scheme by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1835](https://togithub.com/bazelbuild/bazel-gazelle/pull/1835)
- temporarily disable `//internal:bazel_test` on Mac to fix CI by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1842](https://togithub.com/bazelbuild/bazel-gazelle/pull/1842)
- update readmes for latest release by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1840](https://togithub.com/bazelbuild/bazel-gazelle/pull/1840)
- \[Gazelle] Fix Duplicate Load Bug by
[@&#8203;ckilian867](https://togithub.com/ckilian867) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1841](https://togithub.com/bazelbuild/bazel-gazelle/pull/1841)
- \[Proto] Require space between 'service' and service name in regex
matching by [@&#8203;ckilian867](https://togithub.com/ckilian867) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1845](https://togithub.com/bazelbuild/bazel-gazelle/pull/1845)
- \[Proto] Keep track of the names of Services, Messages, and Enums by
[@&#8203;ckilian867](https://togithub.com/ckilian867) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1844](https://togithub.com/bazelbuild/bazel-gazelle/pull/1844)
- Always check files in generation tests by
[@&#8203;Whoaa512](https://togithub.com/Whoaa512) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1847](https://togithub.com/bazelbuild/bazel-gazelle/pull/1847)
- Support label using regexp in directive `gazelle:resolve_regexp` by
[@&#8203;lkassar-stripe](https://togithub.com/lkassar-stripe) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1822](https://togithub.com/bazelbuild/bazel-gazelle/pull/1822)
- Add `external/...` prefix to `${SRCDIR}` in external repos by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1850](https://togithub.com/bazelbuild/bazel-gazelle/pull/1850)
- feat(tools): add a tool to automate the generation of go_deps
overrides by [@&#8203;tyler-french](https://togithub.com/tyler-french)
in
[https://github.com/bazelbuild/bazel-gazelle/pull/1677](https://togithub.com/bazelbuild/bazel-gazelle/pull/1677)
- prepare release 0.38 by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1849](https://togithub.com/bazelbuild/bazel-gazelle/pull/1849)

#### New Contributors

- [@&#8203;smocherla-brex](https://togithub.com/smocherla-brex) made
their first contribution in
[https://github.com/bazelbuild/bazel-gazelle/pull/1817](https://togithub.com/bazelbuild/bazel-gazelle/pull/1817)
- [@&#8203;TvdW](https://togithub.com/TvdW) made their first
contribution in
[https://github.com/bazelbuild/bazel-gazelle/pull/1802](https://togithub.com/bazelbuild/bazel-gazelle/pull/1802)
- [@&#8203;ckilian867](https://togithub.com/ckilian867) made their first
contribution in
[https://github.com/bazelbuild/bazel-gazelle/pull/1841](https://togithub.com/bazelbuild/bazel-gazelle/pull/1841)
- [@&#8203;lkassar-stripe](https://togithub.com/lkassar-stripe) made
their first contribution in
[https://github.com/bazelbuild/bazel-gazelle/pull/1822](https://togithub.com/bazelbuild/bazel-gazelle/pull/1822)

**Full Changelog**:
bazel-contrib/bazel-gazelle@v0.37.0...v0.38.0

###
[`v0.37.0`](https://togithub.com/bazelbuild/bazel-gazelle/releases/tag/v0.37.0)

[Compare
Source](https://togithub.com/bazelbuild/bazel-gazelle/compare/v0.36.0...v0.37.0)

#### What's Changed

- Apply map_kind to args as well as rule kinds by
[@&#8203;illicitonion](https://togithub.com/illicitonion) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1722](https://togithub.com/bazelbuild/bazel-gazelle/pull/1722)
- Add a pointer to bzlmod guide by
[@&#8203;sluongng](https://togithub.com/sluongng) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1781](https://togithub.com/bazelbuild/bazel-gazelle/pull/1781)
- \[Extraction] prep for go.mod & go.work FilePath ReplaceDirective work
by [@&#8203;stefanpenner](https://togithub.com/stefanpenner) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1780](https://togithub.com/bazelbuild/bazel-gazelle/pull/1780)
- \[cmd/fetch_repo] make cache corruption failures more clear by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1782](https://togithub.com/bazelbuild/bazel-gazelle/pull/1782)
- Nit: pass -modcacherw in exec.Command. by
[@&#8203;hauserx](https://togithub.com/hauserx) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1784](https://togithub.com/bazelbuild/bazel-gazelle/pull/1784)
- Mention JS extension in Aspect CLI by
[@&#8203;alexeagle](https://togithub.com/alexeagle) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1646](https://togithub.com/bazelbuild/bazel-gazelle/pull/1646)
- \[Feature] bzlmod & go.work by
[@&#8203;stefanpenner](https://togithub.com/stefanpenner) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1731](https://togithub.com/bazelbuild/bazel-gazelle/pull/1731)
- Add GIT_CONFIG_\* env vars to go_repository allow-list by
[@&#8203;mortenmj](https://togithub.com/mortenmj) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1791](https://togithub.com/bazelbuild/bazel-gazelle/pull/1791)
- Reformat with latest buildifier by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1792](https://togithub.com/bazelbuild/bazel-gazelle/pull/1792)
- \[Feature] go.mod FilePath ReplaceDirective Support by
[@&#8203;stefanpenner](https://togithub.com/stefanpenner) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1776](https://togithub.com/bazelbuild/bazel-gazelle/pull/1776)
- Fix README.rst by
[@&#8203;AugustKarlstedt](https://togithub.com/AugustKarlstedt) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1795](https://togithub.com/bazelbuild/bazel-gazelle/pull/1795)
- Update README.rst by
[@&#8203;AugustKarlstedt](https://togithub.com/AugustKarlstedt) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1796](https://togithub.com/bazelbuild/bazel-gazelle/pull/1796)
- Normalise newlines on Windows by
[@&#8203;illicitonion](https://togithub.com/illicitonion) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1798](https://togithub.com/bazelbuild/bazel-gazelle/pull/1798)
- Fix go.work use ROOT moddir by
[@&#8203;hunshcn](https://togithub.com/hunshcn) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1800](https://togithub.com/bazelbuild/bazel-gazelle/pull/1800)
- allow go_visibility directive to change command package's visibility
by [@&#8203;hunshcn](https://togithub.com/hunshcn) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1794](https://togithub.com/bazelbuild/bazel-gazelle/pull/1794)
- Ensure the Gazelle binary is built for the right platform by
[@&#8203;EdSchouten](https://togithub.com/EdSchouten) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1803](https://togithub.com/bazelbuild/bazel-gazelle/pull/1803)
- Add support for `debug_mode` option to `go_deps` by
[@&#8203;davidbyttow](https://togithub.com/davidbyttow) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1806](https://togithub.com/bazelbuild/bazel-gazelle/pull/1806)
- Remove special resolution of go_proto imports by
[@&#8203;linzhp](https://togithub.com/linzhp) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1807](https://togithub.com/bazelbuild/bazel-gazelle/pull/1807)
- address nogo complaints about variable shadowing by
[@&#8203;pmenglund](https://togithub.com/pmenglund) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1808](https://togithub.com/bazelbuild/bazel-gazelle/pull/1808)
- Make `# gazelle:proto file` work without needing to set different
`option go_package` in .proto files by
[@&#8203;jeromep-stripe](https://togithub.com/jeromep-stripe) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1765](https://togithub.com/bazelbuild/bazel-gazelle/pull/1765)
- go_deps: ignore go.work toolchain directive by
[@&#8203;malt3](https://togithub.com/malt3) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1809](https://togithub.com/bazelbuild/bazel-gazelle/pull/1809)
- prepare release 0.37.0 by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[https://github.com/bazelbuild/bazel-gazelle/pull/1812](https://togithub.com/bazelbuild/bazel-gazelle/pull/1812)

#### New Contributors

- [@&#8203;stefanpenner](https://togithub.com/stefanpenner) made their
first contribution in
[https://github.com/bazelbuild/bazel-gazelle/pull/1780](https://togithub.com/bazelbuild/bazel-gazelle/pull/1780)
- [@&#8203;AugustKarlstedt](https://togithub.com/AugustKarlstedt) made
their first contribution in
[https://github.com/bazelbuild/bazel-gazelle/pull/1795](https://togithub.com/bazelbuild/bazel-gazelle/pull/1795)
- [@&#8203;hunshcn](https://togithub.com/hunshcn) made their first
contribution in
[https://github.com/bazelbuild/bazel-gazelle/pull/1800](https://togithub.com/bazelbuild/bazel-gazelle/pull/1800)
- [@&#8203;EdSchouten](https://togithub.com/EdSchouten) made their first
contribution in
[https://github.com/bazelbuild/bazel-gazelle/pull/1803](https://togithub.com/bazelbuild/bazel-gazelle/pull/1803)
- [@&#8203;davidbyttow](https://togithub.com/davidbyttow) made their
first contribution in
[https://github.com/bazelbuild/bazel-gazelle/pull/1806](https://togithub.com/bazelbuild/bazel-gazelle/pull/1806)
- [@&#8203;pmenglund](https://togithub.com/pmenglund) made their first
contribution in
[https://github.com/bazelbuild/bazel-gazelle/pull/1808](https://togithub.com/bazelbuild/bazel-gazelle/pull/1808)
- [@&#8203;jeromep-stripe](https://togithub.com/jeromep-stripe) made
their first contribution in
[https://github.com/bazelbuild/bazel-gazelle/pull/1765](https://togithub.com/bazelbuild/bazel-gazelle/pull/1765)

**Full Changelog**:
bazel-contrib/bazel-gazelle@v0.36.0...v0.37.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, 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 was generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View the
[repository job
log](https://developer.mend.io/github/kreempuff/rules_unreal_engine).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zNjguMTAiLCJ1cGRhdGVkSW5WZXIiOiIzNy40NDAuNyIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants