[Feature] bzlmod & go.work #1731

stefanpenner · 2024-02-06T22:14:59Z

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?

Explores a possible approach for adding go.work support to the go bzlmod extension.

adds go_deps.from_file(go_work = "//:go.work")
adds ability to op-out of version conflict failures go_deps.from_file(go_work = "//:go.work, fail_on_version_conflict = False)
parses go.work files to discover used go modules and replace statements
generates repos for all deps specified in go.mod's referenced by the //:go.work
handle replace statements (both at the go.work and the go.mod level)
Errors with actionable error message if duplicate dependencies with differing versions arise (Question: can we do better?)

Which issues(s) does this PR fix?

Fixes #1720 #1549

Other notes for review

Although this does work, it should be considered a prototype intended for discussion and feedback. If we decide to pursue this or something similar, expect this PR to evolve rapidly. Although it has some tests, it will require more. Additionally, some architectural changes may improve readability.

An additional consideration, a complementary PR to teach gazelle about go.work + go.mods would help mitigate the dependency bleed problem this PR has. We should discuss this as well.

google-cla · 2024-02-06T22:15:03Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

stefanpenner · 2024-02-06T22:16:09Z

Although I have signed the CLA already, I have two user accounts in the commits. Let me fix that...

MODULE.bazel

internal/bzlmod/go_deps.bzl

stefanpenner · 2024-02-07T18:56:52Z

TODO (from call with team):

only support from_file(go_work = ...) from root modules. (check is_root property of the module)
double check that multiple from_file calls is disallowed
mandate that from_file is called with only one go_work, or go_mod attribute, and not none.
make tests/bcr use go.work
add additional unit test support
tidy-up

stefanpenner · 2024-02-09T18:49:43Z

Debugging BCR tests, I realized this also needs to work properly with Let bazel_deps replace Go deps #1526 ... so Adding a todo.

.bazelci/presubmit.yml

.bazelversion

internal/bzlmod/go_deps.bzl

internal/bzlmod/go_mod.bzl

stefanpenner · 2024-04-16T20:27:07Z

I've been thinking about how the resolutions work, and I want to confirm a few more more advanced scenarios. Let's hold off on merging for the moment.

I should be able to finish the testing by tomorrow, and report back. :)

internal/bzlmod/go_deps.bzl

internal/bzlmod/go_mod.bzl

fmeum · 2024-04-16T20:30:59Z

Sounds good, remaining comments are all just nits in changed code.

stefanpenner · 2024-04-17T19:59:46Z

I believe I have a reasonable resolution to the issue I noticed, working to implement the fix now. Let's see how it goes :)

* adds go_deps.from_file(go_work = "//:go.work") * adds ability to op-out of version conflict failures go_deps.from_file(go_work = "//:go.work, fail_on_version_conflict = False) * parses go.work files to discover used go modules and replace statements * generates repos for all deps specified in go.mod's referenced by the //:go.work * handle replace statements (both at the go.work and the go.mod level) * Errors with actionable error message if duplicate dependencies with differing versions arise (Question: can we do better?) * expand BCR test coverage to include both go.work and go.mod scenarios Caveats: * allows for dependency bleed between different go modules in the workspace - this is sorta by design for this stepping stone, but is ideal * doesn't support replace statements in go.work yet (but that shouldn't be to hard to add in a follow up)

stefanpenner · 2024-04-18T18:30:56Z

I believe I have a reasonable resolution to the issue I noticed, working to implement the fix now. Let's see how it goes :)

TL;DR: Within a go.work, if module A depends on an external module B (not in go.work), and B in turn depends on module C which is included in the go.work, then the version of C referenced by B will differ from the version of C that is directly included in the go.work.

Our team has been describing this the "the sandwich problem".

In general, a well formed workspace must not contain the the sandwich problem as even in a perfectly functioning system this scenario will be a source of headaches and confusion.

I did spend some time seeing if we can easily address this but with my limited understanding of gazelle I couldn't come up with a reasonable solution that didn't result in a whole new feature. That being said, someone with more expertise may have a good idea here that I'm missing.

Given this, my recommendation is:

proceed with this PR
separately explore how to enable this capability within gazelle and then add this capability in the future.

fmeum · 2024-04-18T18:50:42Z

Agreed, let's merge this and as a follow-up:

Resolve the problem you mentioned.
Add docs to our Bzlmod guide.

fmeum · 2024-04-18T18:50:56Z

Thanks for working on this large project!

Docs for bazel-contrib/bazel-gazelle#1731

stefanpenner changed the title ~~[Exploration] bzlmod & go.work~~ [For Discussion] bzlmod & go.work Feb 6, 2024

stefanpenner commented Feb 6, 2024

View reviewed changes

MODULE.bazel Outdated Show resolved Hide resolved

stefanpenner commented Feb 6, 2024

View reviewed changes

internal/bzlmod/go_deps.bzl Outdated Show resolved Hide resolved

stefanpenner commented Feb 6, 2024

View reviewed changes

internal/bzlmod/go_deps.bzl Outdated Show resolved Hide resolved

stefanpenner force-pushed the hack-duplicate-go-deps-from-file branch 4 times, most recently from e522ebb to 86da7c5 Compare February 7, 2024 17:57

stefanpenner force-pushed the hack-duplicate-go-deps-from-file branch 5 times, most recently from 82b70b3 to f7a7d32 Compare February 8, 2024 17:30

fmeum mentioned this pull request Feb 10, 2024

Support for Bzlmod in a monorepo #1658

Open

stefanpenner force-pushed the hack-duplicate-go-deps-from-file branch 5 times, most recently from 30af2af to dc31480 Compare February 15, 2024 21:59