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.

Sign up for GitHub

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

Bump Bazel dependency versions to enable using latest Bazel #3979

Merged
merged 1 commit into from
Apr 12, 2023
Merged

Bump Bazel dependency versions to enable using latest Bazel #3979

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .bazelversion
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +0,0 @@
5.4.0
24 changes: 12 additions & 12 deletions bazel/p4c_deps.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ filegroup(
if not native.existing_rule("com_github_nelhage_rules_boost"):
git_repository(
name = "com_github_nelhage_rules_boost",
# Newest commit on main branch as of May 3, 2021.
commit = "2598b37ce68226fab465c0f0e10988af872b6dc9",
# Newest commit on main branch as of April 11, 2023.
commit = "ded8ba4bcdadb50a2fb2f363b1501eb775d13aac",
remote = "https://github.com/nelhage/rules_boost",
shallow_since = "1611019749 -0800",
shallow_since = "1680804650 -0700",
)
if not native.existing_rule("com_github_p4lang_p4runtime"):
# Cannot currently use local_repository due to Bazel limitation,
Expand All @@ -43,9 +43,9 @@ filegroup(
git_repository(
name = "com_github_p4lang_p4runtime",
remote = "https://github.com/p4lang/p4runtime",
# Newest commit on main branch as of Jan 22, 2021.
commit = "0d40261b67283999bf0f03bd6b40b5374c7aebd0",
shallow_since = "1611340571 -0800",
# Newest commit on main branch as of April 11, 2023.
commit = "90553b90a12ead5c19700e7fef21164dea5b6d22",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This P4Runtime version is now very different from the one the compiler actually uses.
https://github.com/p4lang/p4runtime/tree/7a322f35f0c80bf20bc7fcc96f9d1ab77e5fd07a
So the open-source framework will not test that version.

Copy link
Member

Choose a reason for hiding this comment

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

Just to clarify -- are you saying p4c uses different versions of P4Runtime in the cbuild vs bazel build? Or something else?

Copy link
Collaborator

@fruffy fruffy Apr 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, if the version in the p4c_deps file is what is used to generate the protobuf C++ files etc. But it looks like this has been the case for a while already.

Copy link
Member Author

Choose a reason for hiding this comment

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

For the bazel build alone, I guess the major user is Google. And internally at Google, we do build p4c together with the latest version of its dependencies including p4runtime. So I think using the latest version here makes sense.

But I do worry there could be potential issues, maybe uncaught yet, that stems from this version difference.

Also ping @lzhzero for reference.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can try to update the P4Runtime version for the compiler, too. But I think there might be some complications because of the protobuf version.

Copy link
Member Author

@qobilidop qobilidop Apr 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Sure, we can try that in the future. Good to know the complications.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for raising that, Fabian.

Thus far we haven't been super careful about keeping these versions aligned with the cbuild, which is the source of truth. Agreed that there is a danger that this could cause issues, but thus far we haven't seen any. So personally, I am okay with winging things for now -- we can refine our approach once we hit an issue. Wdyt?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah I have no problem with that. Let me know when you want this merged.

The only reason why the open-source P4C version is not updated is because of the Protobuf dependency. It seems, to get a newer version of Protobuf, we need to build it from scratch. That adds an unacceptable overhead. I have not found a good alternative to the Ubuntu packaged version.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, yes, Google likes building its libraries from scratch, so there may not be a good alternative.

We can go ahead and merge this from my side. @qobilidop ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have no issues with merging this.

shallow_since = "1680213111 -0700",
# strip_prefix is broken; we use patch_cmds as a workaround,
# see https://github.com/bazelbuild/bazel/issues/10062.
# strip_prefix = "proto",
Expand All @@ -64,14 +64,14 @@ filegroup(
# ideally be kept in sync with the submodule test/frameworks/gtest.
http_archive(
name = "com_google_googletest",
urls = ["https://github.com/google/googletest/archive/release-1.10.0.tar.gz"],
strip_prefix = "googletest-release-1.10.0",
sha256 = "9dc9157a9a1551ec7a7e43daea9a694a0bb5fb8bec81235d8a1e6ef64c716dcb",
urls = ["https://github.com/google/googletest/archive/refs/tags/v1.13.0.tar.gz"],
strip_prefix = "googletest-1.13.0",
sha256 = "ad7fdba11ea011c1d925b3289cf4af2c66a352e18d4c7264392fead75e919363",
)
if not native.existing_rule("com_google_protobuf"):
http_archive(
name = "com_google_protobuf",
url = "https://github.com/protocolbuffers/protobuf/releases/download/v3.13.0/protobuf-all-3.13.0.tar.gz",
strip_prefix = "protobuf-3.13.0",
sha256 = "465fd9367992a9b9c4fba34a549773735da200903678b81b25f367982e8df376",
url = "https://github.com/protocolbuffers/protobuf/releases/download/v21.10/protobuf-all-21.10.tar.gz",
strip_prefix = "protobuf-21.10",
sha256 = "6fc9b6efc18acb2fd5fb3bcf981572539c3432600042b662a162c1226b362426",
)