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

Writing resolved Go modules as K:V JSON for external bookkeeping #1906

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

Buzz-Lightyear
Copy link
Contributor

What type of PR is this?
Feature

What package or component does this PR mostly affect?
go_deps extension

What does this PR do? Why is it needed?
I'd like the go_deps module extension to expose the mapping between every Go module it bazelizes and its resolved version. This would be extremely useful for external bookkeeping (for e.g. we can easily communicate to CI systems that rely on native package manager metadata, precisely what version of each Go module was used in the build).

Which issues(s) does this PR fix?
Fixes #1905

Copy link

google-cla bot commented Aug 29, 2024

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.

go_repository(**go_repository_args)

module_ctx.file("./resolved_go_modules.json", content=json.encode_indent(resolved_go_modules))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Files written by module extensions are emitted into an unspecified temporary directory that should be removed after execution. We would need to instantiate a repo with this info or accept an absolute path via an environment variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, I'm propagating this similar to config.json used by rules_go. I was able to verify that I can access this file as @bazel_gazelle_go_repository_config//:resolved_deps.json. Would this approach work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can also expose this via a separate repo rule if that's preferable. On a related note, while I can pull this into my workspace as @bazel_gazelle_go_repository_config//:resolved_deps.json, bazel mod tidy still thinks this repo is an indirect dependency and removes it from MODULE.bazel (I'm explicitly passing it in as inputs to both a build rule and a sh_binary rule). Is that perhaps a bug? If not, can I add a directive to inform bazel mod tidy that I intend to keep that use_repo call?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsure why Windows build of rules_go is failing but interestingly I see Bazel warn that the repo is an indirect dependency in Gazelle's own MODULE.bazel file:

(16:03:24) WARNING: C:/b/bk-windows-xmds/bazel/bazel-gazelle/tests/bcr/go_work/MODULE.bazel:27:24: The module extension go_deps defined in @gazelle//:extensions.bzl reported incorrect imports of repositories via use_repo():
--
  |  
  | Imported, but reported as indirect dependencies by the extension:
  | bazel_gazelle_go_repository_config, org_golang_google_protobuf

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the failure was intermittent and unrelated to the change

@fmeum
Copy link
Member

fmeum commented Sep 11, 2024

The implementation looks reasonable, I'm just not sure whether this is the right way to export this information and thus don't want to commit to it yet.

Could you perhaps create a discussion on this repo and use it to explain your use case in more detail? I would like to gather more feedback from the community so that we can make more informed design decisions.

@Buzz-Lightyear
Copy link
Contributor Author

Sounds good @fmeum , started #1918

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_deps should expose resolved versions for all Go modules it Bazelizes, for external bookkeeping
2 participants