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

Jump to bottom

Provide Bazel BUILD files for protos #4

Closed
bocon13 opened this issue Jul 18, 2018 · 5 comments
Closed

Provide Bazel BUILD files for protos #4

bocon13 opened this issue Jul 18, 2018 · 5 comments
Assignees
@antoninbas

Comments

@bocon13
Copy link
Member

bocon13 commented Jul 18, 2018

Add support for building protos with Bazel so that this repository can be included in other projects as a remote workspace.
@antoninbas antoninbas self-assigned this Jul 26, 2018
@antoninbas
Copy link
Member

So actually, this is harder / more annoying than it looks. If I want to use a recent Bazel version (say 0.13), I cannot use the Protobuf & gRPC versions that I want to use (resp. 3.2.0 and 1.3.2) because that Protobuf version has a BUILD file that is no longer a correct Bazel BUILD file.

So if I don't see how I can satisfy all of these:
a) recent Bazel
b) older versions of Protobuf & gRPC
c) pre-defined rules (Bazel built-in proto_library or better yet these rules: https://github.com/pubref/rules_protobuf)

I could probably give up on c), but then I would need to invoke protoc manually (i.e. define my own rules) and that doesn't sound ideal.

@bocon13 do you have thoughts on this? Things seem to work with the latest Protobuf version, but that's not really what I'd like to use.

@bocon13
Copy link
Member Author

bocon13 commented Jul 26, 2018 •
edited
Loading

I've been looking into this, and there are some bugs/problems with native proto_library and cc_proto_library when used with the proto_sources_root attribute. The work around seems to be to use a genrule to remap the proto file to the appropriate path in the generated sources directory (as we do in Stratum), then use the proto_library rule. So, as it stands, requirement (c) is not great in the short term.

It seems like protobuf has a rule in 3.2.0 that could be used in the short term: https://github.com/google/protobuf/blob/v3.2.0/protobuf.bzl#L149

Are you saying that this file doesn't work with latest Bazel? I haven't tried this yet, but I can try in the next few days. We could always fix it and host it in this repo as an externally injected BUILD file.

@bocon13
Copy link
Member Author

bocon13 commented Jul 26, 2018

In the longer term, I'd like to move to native rules for proto_library, cc_proto_library and something based on native rules for gRPC. This would make it easier to support multiple languages.

@antoninbas
Copy link
Member

The problem is that all the Bazel files in Protobuf 3.2.0 (whether it's BUILD files or protobuf.bzl) are not compliant with recent versions of Bazel (such as the one we use for Stratum).
For example, for protobuf.bzl:

ERROR: /home/antonin/Documents/p4lang/PI/proto/p4runtime/proto/BUILD:8:1: Traceback (most recent call last):
	File "/home/antonin/Documents/p4lang/PI/proto/p4runtime/proto/BUILD", line 8
		cc_proto_library(name = "p4types", deps = [], srcs = ..."])
	File "/home/antonin/.cache/bazel/_bazel_antonin/6af8fe00661db4e7432e035b48767a7b/external/com_google_protobuf/protobuf.bzl", line 229, in cc_proto_library
		cc_libs += [default_runtime]
trying to mutate a frozen object

For the BUILD file, this line is not correct and should read :protobuf: https://github.com/google/protobuf/blob/3.2.x/BUILD#L34

I think I'm just going to fork Protobuf 3.2.0 and patch the BUILD files if it's not too much work.

antoninbas added a commit that referenced this issue Jul 30, 2018
@antoninbas
Attempt at adding Bazel rules for protots
5b939e5 
An attempt at #4

We import rules to invoke protoc from pubref/rules_protobuf. The
alternatives would have been 1) try to use native Bazel rules only and
rules from the gRPC reprository, or 2) invoke protoc directly in the
BUILD file.
1) is an issue because of multiple issues with the native & gRPC rules.
2) may actually be the best solution, in terms of usability and
compatibility with different versions of protobuf, gRPC and Bazel.

See BAZEL_NOTES.md for a more comprehensive discussion.
@antoninbas antoninbas mentioned this issue Jul 30, 2018
Merged
antoninbas added a commit that referenced this issue Jul 31, 2018
@antoninbas
Attempt at adding Bazel rules for protos
8358774 
An attempt at #4

We import rules to invoke protoc from pubref/rules_protobuf. The
alternatives would have been 1) try to use native Bazel rules only and
rules from the gRPC reprository, or 2) invoke protoc directly in the
BUILD file.
1) is an issue because of multiple issues with the native & gRPC rules.
2) may actually be the best solution, in terms of usability and
compatibility with different versions of protobuf, gRPC and Bazel.

See BAZEL_NOTES.md for a more comprehensive discussion.
antoninbas added a commit that referenced this issue Aug 2, 2018
@antoninbas
Attempt at adding Bazel rules for protos
6c98eee 
An attempt at #4

We import rules to invoke protoc from pubref/rules_protobuf. The
alternatives would have been 1) try to use native Bazel rules only and
rules from the gRPC reprository, or 2) invoke protoc directly in the
BUILD file.
1) is an issue because of multiple issues with the native & gRPC rules.
2) may actually be the best solution, in terms of usability and
compatibility with different versions of protobuf, gRPC and Bazel.

See BAZEL_NOTES.md for a more comprehensive discussion.
antoninbas added a commit that referenced this issue Aug 3, 2018
@antoninbas
Attempt at adding Bazel rules for protos (#20)
63ebbf8 
* Attempt at adding Bazel rules for protos

An attempt at #4

We import rules to invoke protoc from pubref/rules_protobuf. The
alternatives would have been 1) try to use native Bazel rules only and
rules from the gRPC reprository, or 2) invoke protoc directly in the
BUILD file.
1) is an issue because of multiple issues with the native & gRPC rules.
2) may actually be the best solution, in terms of usability and
compatibility with different versions of protobuf, gRPC and Bazel.

See BAZEL_NOTES.md for a more comprehensive discussion.

* Use fully-qualified path for external build_file

To enable use of deps.bzl by third-party consummers.
@antoninbas
Copy link
Member

Closing this for now as initial support was added. For improvements (support for other languages, use native rules instead of pubref rules, CI for rules), new issues can be opened.

chrispsommers added a commit that referenced this issue Feb 22, 2024
@saynb @antoninbas @chrispsommers
Generic Table First draft (#435)
cc1e04e 
* Initial changes for generic tables

* second changes

* 3rd changes

* 4th changes

* 5th changes

* Adding p4types and p4info changes for GenericData (#4)

Adding another enum alongside P4IDs for generic tables as well

* Adding varbits and a readme writeup (#5)

* Adding varbit and a writeup

* Adding proto changes

* Adding genericTable for direct resources. Adding P4datatypespec to genericdatatype in p4types

* * Adding more container types
** list
** bag
** set
** ordered_set

* Moving GenericTable spec update to one big section to keep it
organized

* * Add Table categories introduction

* * Adding 3-level property details (Table, Entry, field)
* Adding more text on the data types in the spec
* Adding Details on Operations in the Table categories

* * Adding Read RPC details in table categories
* Adding a table to show valid combinations of table properties
* Adding default entry rules

* Expanding combinations

* review comments

* * Explain more on indexed tables. How are duplicates handled
* Add regular tables in the truth table
* Calculation table can be volatile
* Add column to lay out some examples for the table categories
* Mention that the entry properties do not go in p4info
* Flow chart to show INSERT decision flow. Added PNG file
* Add in float more verbiage regarding volatile
* Renamed generic_set to generic_unordered_set
* Removed bag and ordered_set
* Add p4type for string with min/max size

* Update Go dependencies (#438)

* Fixes #439 (#440)

Change CI  workflow to skip publishing if PR spawned by dependabot

* * Adding categories and properties in p4info
* Adding example changes for above in GenericTables.md
* Add default values to p4info.

* * Removed GenericTable category from the p4info
* Added more text about generic table type
* Corrected typos and reference links
* Replaced png file with svg file
* Removed is_const_table since table properties are present

* Adding space for dummy commit

* Correcting typo

* Adding dev branches to workflows (#443)

* Correcting linter errors

* Generating go and py files

* Adding text regarding direct resources as GenericTables in TableEntry

* some review comment changes

* Andy review comments

* remove whitespace

* * Correcting GenericTable.md according to the p4info
* Removing repeated file dfrom Param and Match top level

* * Removing float from p4info and spec
* Adding default value text to spec
* Adding UnorderedSet to runtime

* * Adding section to P4DataTypeSpec to indicate adding to
GenericDataTypeSpec for any new additions
* Removing P4DataTypeSpec from GenericDataTypeSpec
* Adding default value to bool in GenericData

* Moving GenericTable.md to docs

* Correcting spec for removal of p4_type in generic_type. Adding generated code

* * Shortening the field names in `GenericTable` to remove "genric_table_"
prefixes
* Correcting the text for type_name

* * Making default_value as bytes
* Adding "scope" to the union example

* Adding default string value

* review comments

* linter error

* Review comments

Signed-off-by: Sayan Bandyopadhyay <sayan.bandyopadhyay@intel.com>

* Correcting whitespace errors

Signed-off-by: Sayan Bandyopadhyay <sayan.bandyopadhyay@intel.com>

* Updating generated code

Signed-off-by: Sayan Bandyopadhyay <sayan.bandyopadhyay@intel.com>

---------

Signed-off-by: Sayan Bandyopadhyay <sayan.bandyopadhyay@intel.com>
Signed-off-by: Bandyopadhyay, Sayan <sayan.bandyopadhyay@intel.com>
Co-authored-by: Antonin Bas <antonin.bas@gmail.com>
Co-authored-by: Chris Sommers <31145757+chrispsommers@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@antoninbas antoninbas

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bocon13 @antoninbas