Build p4runtime protobufs #328
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ffoulkes
requested review from
nupuruttarwar,
n-sandeep,
vsureshkumarp and
5abeel
as code owners
5abeel
reviewed
Nov 7, 2023
|
I changed the mechanics of the build so the protobufs build is done on demand, instead of being part of the regular build. To build just the protobufs: cmake --build build --target protobufsThis keeps the change from affecting developers who don't have Go (and a couple of additional packages) installed on their system. This includes the GitHub runners, which is how I discovered the problem. As you can see from the number of commits I made, this was a tricky problem to solve. 🙂 |
|
I've added a README with build instructions. |
5abeel
reviewed
Nov 9, 2023
5abeel
reviewed
Nov 9, 2023
5abeel
reviewed
Nov 11, 2023
Signed-off-by: Derek G Foster <[email protected]>
- Rename HOST_PROTOC to HOST_PROTOC_COMMAND. - Revise C++ gRPC file generation to limit itself to p4runtime.proto. None of the other protobuf files define services. Signed-off-by: Derek G Foster <[email protected]>
- Put all the cmake files in the same folder. Signed-off-by: Derek G Foster <[email protected]>
- Modularize by package (google, p4runtime) instead of by language (cpp, go, python). Create one target per (package, language) combination, and replace the tricksy path-manipulation logic with discrete lists of files. - Generate an empty __init__.py file in each Python directory, making the output Python modules. Signed-off-by: Derek G Foster <[email protected]>
Signed-off-by: Derek G Foster <[email protected]>
Signed-off-by: Derek G Foster <[email protected]>
Signed-off-by: Derek G Foster <[email protected]>
Signed-off-by: Derek G Foster <[email protected]>
Signed-off-by: Derek G Foster <[email protected]>
Signed-off-by: Derek G Foster <[email protected]>
- Move the include() commands after project(), to address an error raised by the GitHub workflow. - Add support for building a Python wheel. Currently under development. Controlled by the ENABLE_WHEEL option. - Go compiles are failing in the GitHub workflow. Placed them under control of the ENABLE_GO option, so they can be disabled until the issue is solved. Signed-off-by: Derek G Foster <[email protected]>
Signed-off-by: Derek G Foster <[email protected]>
- The GO_ENABLED option was disabled when building through the external project in the main listfile, despite having been set to TRUE when the option was defined. On inspection, it turned out that ALL the options were FALSE. cmake apparently does not apply the defaults when the build is done as an external project. Changed them to ordinary cache variables, and voila! All is right with the world. Pfui. - Modify the GitHub workflow to upload the protobuf tarballs as artifacts. - Modify .gitignore to ignore `build` and `install` directories anywhere in the tree, not just the root. - Rename the install directory from `proto` to `protobufs`. Signed-off-by: Derek G Foster <[email protected]>
- Try fixing the problem by moving `working-directory` after the `with` stanza. Signed-off-by: Derek G Foster <[email protected]>
- The previous fix didn't work. Try changing the path. Signed-off-by: Derek G Foster <[email protected]>
- Correct BYPRODUCT names in tarball custom targets. - Improve method used to generate __init__.py files. - Use COPY instead of INSTALL to copy collateral files to wheel directory. - Install Python packages under share/stratum/python. Signed-off-by: Derek G Foster <[email protected]>
- Set package version to 2023.11.0. - Update install requirements to track stratum-deps component versions. Signed-off-by: Derek G Foster <[email protected]>
Signed-off-by: Derek G Foster <[email protected]>
ffoulkes
added
major effort
Signed-off-by: Derek G Foster <[email protected]>
ffoulkes
changed the title
Signed-off-by: Derek G Foster <[email protected]>
Signed-off-by: Derek G Foster <[email protected]>
- Modify GitHub workflow to install just the Python dependencies needed to build the P4Runtime protobufs. - Remove invalid 'fcntl' module from requirements.txt file. Signed-off-by: Derek G Foster <[email protected]>
Signed-off-by: Derek G Foster <[email protected]>
Signed-off-by: Derek G Foster <[email protected]>
Signed-off-by: Derek G Foster <[email protected]>
|
The more I think about it, the more I wonder whether this should be part of networking-recipe at all. It's really an adjunct to The choices would seem to be:
In both cases, we would need to update ipu_sdk to build the wheel and install it in the RFS image. We would also have to consider how and whether to incorporate the wheel into networking-recipe build. Oy. |
- Extract logic to find the host Protobuf compiler and plugins from StratumDependencies.cmake and move it to a new HostProtoc find-module. StratumDependencies is now strictly for the target system. Signed-off-by: Derek G Foster <[email protected]>
Signed-off-by: Derek G Foster <[email protected]>
Signed-off-by: Derek G Foster <[email protected]>
5abeel
reviewed
Nov 15, 2023
requirements.txt
Outdated
| @@ -1,4 +1,6 @@ | |||
| p4runtime==1.3.0 | |||
| Jinja2>=3.0.0 | |||
| fcntl | |||
| select | |||
There was a problem hiding this comment.
select needs to be removed as per #345
Signed-off-by: Derek G Foster <[email protected]>
5abeel
reviewed
Nov 15, 2023
| @@ -0,0 +1,25 @@ | |||
| [metadata] | |||
| name = p4runtime | |||
There was a problem hiding this comment.
Should this be called p4runtime-dev or even ipdk-io-p4runtime-dev to differentiate from upstream p4runtime?
There was a problem hiding this comment.
We're currently differentiating our variant by using a distinctly different versioning scheme (YYYY-MM-N).
I think we want to use the same name as the existing module, so people don't have to change their programs to import something different.
Which is not to say that injecting ipdk-io into a name to distinguish it is a bad idea. I may steal it at the next appropriate opportunity... 😉
5abeel
approved these changes
Nov 15, 2023
5abeel
approved these changes
Nov 15, 2023
Signed-off-by: Derek G Foster <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A cmake project that compiles the
google rpcandp4runtimeprotobufs, generating serialization and gRPC service code for C++, Python, and Go. The outputs are:share/stratum/protobufsfolder.share/stratum/p4runtimefolder.This is in response to multiple requests that we supply a Python package, and one recent request to provide a Go package.