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

Migrate to cc_proto_library from com_google_protobuf #1226

Merged
merged 1 commit into from
Jan 16, 2025

Conversation

kgreenek
Copy link
Contributor

Closes #1199

This is a more minimal change to simply migrate away from rules_cc's cc_proto_library without updating all the external deps.

This will prepare for the upcoming 0.1.X release of rules_cc where cc_proto_library is removed.

@kgreenek
Copy link
Contributor Author

Follow up to #1210

@rodaine rodaine merged commit 19960f4 into bufbuild:main Jan 16, 2025
5 checks passed
@rodaine
Copy link
Member

rodaine commented Jan 16, 2025

Thanks!

@ryanli
Copy link

ryanli commented Jan 21, 2025

Thanks for the fix @kgreenek! Can a new release be cut with this fix? This is blocking the update to rules_cc==0.1.0 for us.

@kgreenek
Copy link
Contributor Author

@ryanli FYI The rules_cc authors actually yanked 0.1.0. It's recommended to use 0.0.17, which is newer than 0.1.0 and still has the cc_proto_library defined for now. All the packages in the BCR that depended on rules_cc==0.1.0 were updated to use rules_cc==0.0.17 instead.

@kgreenek kgreenek deleted the kgk/cc_proto_library-migration branch January 21, 2025 16:55
@kgreenek
Copy link
Contributor Author

That being said, +1 for a new release with this change. That would unblock everything that depends on this when rules_cc does eventually remove the cc_proto_library macro

@ryanli
Copy link

ryanli commented Jan 22, 2025

It was added back to main to bazelbuild/rules_cc@28cf2e8 but I don't see 0.1.0 yanked in BCR. But yeah, the point still stands.

@kgreenek
Copy link
Contributor Author

They did yank it, but it turns out yanking the highest version number causes problems downstream, because there is essentially no way to get a valid version when you do that besides using single_version_override.

The did a post-mortem and added a CI check to disallow yanking the highest version of anything in BCR.

Here's the PR where they unyanked it:
bazelbuild/bazel-central-registry#3603

Version 0.0.14 and 0.1.0 should be avoided.

@nicksnyder
Copy link
Member

protoc-gen-validate 1.2.1 has been released with this change.

@kgreenek
Copy link
Contributor Author

@nicksnyder Would you be willing to push the latest version to the BCR?

It's a pretty straight-forward process if you haven't done it before. I'm happy to do it, but want to encourage folks in the community to publish to the BCR whenever possible.

Looks like somebody already created a MODULE.bazel file too for 1.0.4 (see version 1.0.4.bcr.2).

See:
https://github.com/bazelbuild/bazel-central-registry/tree/main/modules/protoc-gen-validate
https://registry.bazel.build/modules/protoc-gen-validate

@nicksnyder
Copy link
Member

We haven't pushed to the BCR before. Abstractly happy to do it but would want it either automated in CI (if possible) or a documented process anyone could follow. Would you be willing to contribute a PR that does one of those (add CI or add a README)?

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.

Problems with 'validate_proto' not declared in package 'validate'
4 participants