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

[Feature] go.mod FilePath ReplaceDirective Support #1776

Merged

Conversation

stefanpenner
Copy link
Contributor

@stefanpenner stefanpenner commented Apr 5, 2024

PR Description

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 builds on the go.work support PR and the new rctx.watch_files support to add support for FilePath ReplaceDirective, in go.mod and go.work files when using bzlmod go_deps extension.

For example, now the following native go capability works in the bazel world:

module github.com/bazelbuild/bazel-gazelle/tests/bcr/go_mod

go 1.21

require (
	example.org/hello v1.0.0
        // ... snip ...
)

// ... snip ...

replace example.org/hello => ../../../fixtures/hello

Which issues(s) does this PR fix?

Fixes #1775

Status

  • make it "work"
  • ensure relative and absolute pathing works
    • /Users/user/path/to/mod
    • ../../path/to/mod
  • test with nightly bazel + rctx.watch_files and verify it fixes "rebuilds"
  • expand bcr tests
    • go.mod
    • go.work
  • even less hack and even more quality
  • fix tests that fail on the bazelbuild/bazel-gazelle CI

@stefanpenner stefanpenner changed the title Go deps replace with module path [Feature] go.mod FilePath ReplaceDirective Support Apr 5, 2024
@stefanpenner stefanpenner force-pushed the go-deps-replace-with-module-path branch 2 times, most recently from 634ee12 to 62868d1 Compare April 5, 2024 21:34
@fmeum
Copy link
Member

fmeum commented Apr 6, 2024

@stefanpenner Could you extract the parts of the Go code changes that are essentially a pure refactoring (e.g. extracting out functions)? We could merge those right away and simplify the overall diff.

@fmeum
Copy link
Member

fmeum commented Apr 6, 2024

The overall approach looks good to me. Kudos to @Wyverald for giving us the required API.

@stefanpenner stefanpenner force-pushed the go-deps-replace-with-module-path branch 7 times, most recently from f7c40ba to acaa88e Compare April 8, 2024 22:46
@stefanpenner stefanpenner marked this pull request as ready for review April 10, 2024 16:20
@stefanpenner
Copy link
Contributor Author

@stefanpenner Could you extract the parts of the Go code changes that are essentially a pure refactoring (e.g. extracting out functions)? We could merge those right away and simplify the overall diff.

@fmeum sure, let me get that started

@stefanpenner stefanpenner force-pushed the go-deps-replace-with-module-path branch 2 times, most recently from 820489c to e599cdf Compare April 10, 2024 20:16
@stefanpenner
Copy link
Contributor Author

@fmeum Extracted PR -> #1780. I'm a little stretched for time, so once that lands I'll rebase this one so it incorporates those changes.

@stefanpenner stefanpenner force-pushed the go-deps-replace-with-module-path branch 2 times, most recently from 68b6e55 to 28f5602 Compare April 12, 2024 16:39
@stefanpenner
Copy link
Contributor Author

stefanpenner commented Apr 12, 2024

@fmeum Extracted PR -> #1780. I'm a little stretched for time, so once that lands I'll rebase this one so it incorporates those changes.

Now that #1780 has landed, I've propagated the change through #1731 and this PR. Once #1731 lands, I'll do the same again here which should cleanup the diff considerably.

@stefanpenner stefanpenner force-pushed the go-deps-replace-with-module-path branch 7 times, most recently from 4174ca5 to c204598 Compare April 18, 2024 18:58
@stefanpenner stefanpenner force-pushed the go-deps-replace-with-module-path branch from 70ade39 to efbbaa2 Compare April 29, 2024 20:35
@stefanpenner stefanpenner requested a review from fmeum April 29, 2024 21:03
@stefanpenner
Copy link
Contributor Author

@fmeum thanks for the review, I've updated & rebased the PR. I left one discussion point for us, but from my vantage point we are looking pretty good. Let me know if there is anything else I can dig into to help move this forward.

internal/go_repository.bzl Outdated Show resolved Hide resolved
internal/go_repository.bzl Outdated Show resolved Hide resolved
internal/go_repository.bzl Outdated Show resolved Hide resolved
internal/go_repository.bzl Outdated Show resolved Hide resolved
internal/go_repository.bzl Show resolved Hide resolved
internal/bzlmod/semver.bzl Outdated Show resolved Hide resolved
internal/bzlmod/go_mod.bzl Outdated Show resolved Hide resolved
internal/bzlmod/go_mod.bzl Show resolved Hide resolved
internal/bzlmod/go_deps.bzl Outdated Show resolved Hide resolved
@stefanpenner stefanpenner force-pushed the go-deps-replace-with-module-path branch 7 times, most recently from 9304905 to 06779fa Compare May 1, 2024 12:27
@stefanpenner stefanpenner requested a review from fmeum May 1, 2024 12:48
@stefanpenner stefanpenner force-pushed the go-deps-replace-with-module-path branch 3 times, most recently from 5ce68df to 2da0494 Compare May 1, 2024 12:52
@stefanpenner
Copy link
Contributor Author

@fmeum thanks for the review, I've made the updates. Feel free to take another look, and let me know if anything else needs attention.

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 the contribution!

@tyler-french @linzhp Do you have any concerns or comments? I guess that this feature won't really be used at Uber, so testing it there may not be that helpful.

internal/bzlmod/go_deps.bzl Outdated Show resolved Hide resolved
@stefanpenner stefanpenner force-pushed the go-deps-replace-with-module-path branch from 2da0494 to 21b9b48 Compare May 1, 2024 15:19
@stefanpenner stefanpenner requested a review from fmeum May 1, 2024 15:21
@linzhp
Copy link
Contributor

linzhp commented May 2, 2024

yeah, we don't use go.work, although we do use replace in go.mod. So no concerns

@fmeum fmeum merged commit 918a4c4 into bazel-contrib:master May 2, 2024
15 checks passed
stefanpenner added a commit to stefanpenner/rules_go that referenced this pull request May 6, 2024
stefanpenner added a commit to stefanpenner/rules_go that referenced this pull request May 6, 2024
fmeum pushed a commit to bazel-contrib/rules_go that referenced this pull request May 6, 2024
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://github.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://github.com/bazelbuild/bazel-gazelle/releases/tag/v0.38.0)

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

#### New Contributors

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

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

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

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

#### New Contributors

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

Successfully merging this pull request may close these issues.

go.mod FilePath ReplaceDirective is missing when using the go-deps bzlmod extension
3 participants