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

chore(bazel): add MODULE.bazel files for bzlmod #1245

Merged
merged 6 commits into from
Jul 30, 2024

Conversation

mmorel-35
Copy link
Contributor

@mmorel-35 mmorel-35 commented Feb 23, 2024

Related to #1204

Signed-off-by: Matthieu MOREL matthieu.morel35@gmail.com

@mmorel-35 mmorel-35 force-pushed the bzlmod branch 5 times, most recently from f7fca2c to c23ff51 Compare February 23, 2024 18:23
@mmorel-35 mmorel-35 marked this pull request as ready for review February 23, 2024 18:24
@mmorel-35 mmorel-35 force-pushed the bzlmod branch 4 times, most recently from 9d2ac99 to ada6c6d Compare February 23, 2024 19:03
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
@Wyverald
Copy link
Member

ping @vladmos

@fmeum
Copy link
Contributor

fmeum commented Mar 26, 2024

Something to be aware of: The release versioning used by this repo is incompatible with how Go modules are versioned. It would be great if this could be resolved before this module is added to the BCR. This could otherwise result in quite a bit of pain for Go projects using the repo as a Go module and also picking it up as a transitive bazel_dep.

MODULE.bazel Outdated
bazel_dep(name = "gazelle", version = "0.34.0", repo_name = "bazel_gazelle")
bazel_dep(name = "protobuf", version = "23.1", repo_name = "com_google_protobuf")
bazel_dep(name = "rules_go", version = "0.45.1", repo_name = "io_bazel_rules_go")
bazel_dep(name = "rules_nodejs", version = "5.8.2")
Copy link
Contributor

Choose a reason for hiding this comment

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

Could some of these deps be dev dependencies instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably,
Any advice on how to identify which are ?

@alexeagle
Copy link
Contributor

alexeagle commented Mar 27, 2024

release versioning used by this repo is incompatible with how Go modules are versioned

as one option to consider (and I wonder if we are doing it right), https://github.com/aspect-build/aspect-cli/tags publishes two tags for each release, one that looks like a bazel module version and one that looks like a Go package version (prefixed with v)

@fmeum
Copy link
Contributor

fmeum commented Mar 27, 2024

@vladmos Would that be an option for buildtools?

@@ -4,3 +4,4 @@
.ijwb
*.iml
/.bazelrc.user
MODULE.bazel.lock
Copy link
Member

Choose a reason for hiding this comment

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

What's this file? Is it being created by your editor?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's the lockfile Bazel creates for MODULE.bazel. It is currently hard to share across machines, which is why it's commonly added to .gitignore.

Copy link
Member

Choose a reason for hiding this comment

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

bzlmod has a lockfile which afaiui the primary feature is it allows you to build offline. but it is pretty verbose, and i believe there are some cross-platform holes in it today, so it adds some maintenance burden to check in unless you really need it. ignoring it allows you the benefits locally w/o having to deal with updating it

Copy link
Member

@vladmos vladmos left a comment

Choose a reason for hiding this comment

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

Thanks! I bumped the versions again (for aspect_bazel_lib it was actually necessary to pass the CI tests) and updated the generated .go files (the diff was only in some version comments).

@vladmos vladmos merged commit 717ae2f into bazelbuild:main Jul 30, 2024
2 checks passed
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.

None yet

6 participants