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

Update bazel and deps #1111

Merged
merged 6 commits into from
Nov 26, 2024
Merged

Update bazel and deps #1111

merged 6 commits into from
Nov 26, 2024

Conversation

keith
Copy link
Contributor

@keith keith commented May 24, 2024

This modernizes the bazel setup to support 7.x, still using the WORKSPACE

@keith keith force-pushed the ks/update-bazel-and-deps branch from 261e913 to 984c7b5 Compare May 24, 2024 23:37

# gazelle:repository_macro dependencies.bzl%go_third_party
go_third_party()
pinned_maven_install()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is still a bit of a mess since you cannot do some of these loads in the same bzl file where you pull in the dep, I think we'd have to create another file if we wanted to eliminate all the custom things done here.

name = "pgv_pip_deps",
requirements = "@com_envoyproxy_protoc_gen_validate//python:requirements.in",
requirements_lock = "@com_envoyproxy_protoc_gen_validate//python:requirements.txt",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the old rule was removed the new one requires a lockfile

go_third_party()

maven_install(
artifacts = PROTOBUF_MAVEN_ARTIFACTS,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is required for protobuf java code now

@@ -238,4 +238,5 @@ java_proto_gen_validate = rule(
"srcjar": "lib%{name}-src.jar",
},
implementation = _java_proto_gen_validate_impl,
toolchains = ["@bazel_tools//tools/jdk:toolchain_type"],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -35,9 +35,5 @@ cc_test(

py_proto_library(
name = "bar_py_proto",
srcs = ["bar.proto"],
deps = [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

py_proto_library from rules_python now works how all the others do

@@ -9,7 +9,7 @@ public final class RequiredValidation {
private RequiredValidation() {
}

public static void required(String field, GeneratedMessageV3 value) throws ValidationException {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

"requirements.in",
"setup.cfg",
])

compile_pip_requirements(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this also creates a test to make sure the lockfile is updated, so the new lockfile is slightly easier to manage with that

@keith keith mentioned this pull request May 25, 2024
This modernizes the bazel setup to support 7.x, still using the
WORKSPACE
@keith
Copy link
Contributor Author

keith commented Nov 26, 2024

@rodaine could you take a look at this one? I just rebased but looks like you need to approve the CI run

@keith
Copy link
Contributor Author

keith commented Nov 26, 2024

thanks! pushed a fix for that failure, hopefully that's the only one!

@keith
Copy link
Contributor Author

keith commented Nov 26, 2024

green 🎉

Copy link
Member

@rodaine rodaine left a comment

Choose a reason for hiding this comment

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

Thank you so much, @keith!

@rodaine rodaine merged commit bbd0e25 into bufbuild:main Nov 26, 2024
4 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.

2 participants