Skip to content

Conversation

@fmeum
Copy link
Collaborator

@fmeum fmeum commented Oct 12, 2024

  • Gate module_ctx.extension_metadata(reproducible = True) usage behind a bazel_features check for compatibility with Bazel 6.
  • Use the "well-known" name com_google_protobuf instead of protobuf with WORKSPACE to avoid missing deps and duplication of protobuf targets.

@fmeum
Copy link
Collaborator Author

fmeum commented Oct 12, 2024

Context: bazel-contrib/toolchains_llvm#389

* Gate `module_ctx.extension_metadata(reproducible = True)` usage behind a `bazel_features` check for compatibility with Bazel 6.
* Use the "well-known" name `com_google_protobuf` instead of `protobuf` with WORKSPACE to avoid missing deps and duplication of protobuf targets.
bazel_dep(name = "bazel_skylib", version = "1.7.1")
bazel_dep(name = "platforms", version = "0.0.10")
bazel_dep(name = "protobuf", version = "27.0")
bazel_dep(name = "protobuf", version = "27.0", repo_name = "com_google_protobuf")
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is going backwards. We'd like to stop calling com_google_protobuf and use Protobuf only. In case of bzlmod, this should be possible. WORKSPACE files might be more problematic. It'd probably be better to just remove Protobuf dependency. It seems that keeping it creates more problems than needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, removing it would be better. Is that possible though?

I don't see much of a downside to using com_google_protobuf here for as long as the module supports WORKSPACE. The name is entirely internal to this module with Bzlmod and quite breaking when changed with WORKSPACE.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Removing it wouldn't break too many people. There are 2 levels. 1 to remove cc_proto_library. I only found 1 project on GitHub using it. 2 is to also remove cc_proto toolchain_type, without introducing an alias back to Protobuf (which I can't introduce until protobuf 29.0 is released)

@comius
Copy link
Collaborator

comius commented Nov 5, 2024

I think everything in this PR has now been merger.

@comius comius closed this Nov 5, 2024
@fmeum fmeum deleted the bazel-compat branch November 5, 2024 14:45
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.

2 participants