Bump Bazel dependency versions to enable using latest Bazel #3979
Conversation
|
@smolkaj You might want to take a look once the CI checks pass. |
|
Nice! Thanks for figuring this out. |
| commit = "0d40261b67283999bf0f03bd6b40b5374c7aebd0", | ||
| shallow_since = "1611340571 -0800", | ||
| # Newest commit on main branch as of April 11, 2023. | ||
| commit = "90553b90a12ead5c19700e7fef21164dea5b6d22", |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Just to clarify -- are you saying p4c uses different versions of P4Runtime in the cbuild vs bazel build? Or something else?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Sure, we can try that in the future. Good to know the complications.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
I have no issues with merging this.
The context is in #3796. I did some further debugging and found the compatibility issues come from dependencies. So updating dependencies solves the issue without having to constrain the Bazel version.