Implement py_proto_library

[comius]

py_proto_library was tested manually on Bazel repository using Bazel 6.0.0 with and without using bzlmod.

Extend README.md to mention also py_proto_library.

With bzlmod py_proto_library just works. Users without bzlmod, need to import rules_proto and protobuf repositories into their WORKSPACE file.

py_proto_library is compatible with Bazel >=5.4.0 and >=6.0.0.

[comius]

cc @rickeylev@haberman

[meteorcloudy]

LGTM, from dependencies management perspective.

[aaliddell]

FWIW transitive proto compilation with an aspect is probably not the right method to use any longer. Both rules_proto and rules_proto_grpc have dropped aspect based compilation in favour of more explicit direct compilation of the named protos only within a target. This is because transitive compilation is both artificially limiting in terms of passable options and more importantly leads to multiple definitions of the same proto message if depended on via differing trees (a serious problem in C++, a nuisance in Python). See here for further details: https://rules-proto-grpc.com/en/latest/transitivity.html

[comius]

FWIW transitive proto compilation with an aspect is probably not the right method to use any longer. Both rules_proto and rules_proto_grpc have dropped aspect based compilation in favour of more explicit direct compilation of the named protos only within a target. This is because transitive compilation is both artificially limiting in terms of passable options and more importantly leads to multiple definitions of the same proto message if depended on via differing trees (a serious problem in C++, a nuisance in Python). See here for further details: https://rules-proto-grpc.com/en/latest/transitivity.html

Compiling only directly is a good choice for gRPC generated libraries.

For non-service proto libraries, it's better to use lang_proto_library with an aspect. It removes redundancies - having to add py_proto_library next to every proto_library in the transitive closure. Aspects take care that you're 1:1 mapping from proto dependency graph to python graph - even if lang_proto_library is duplicated. If there are problems with proto dependency graph, they are not going to be solved by either direct or aspect based solution. Aspect based solution will probably fail, whereas no-aspect/direct solution will potentially hide problems.

[rickeylev]

https://rules-proto-grpc.com/en/latest/transitivity.html says

In hindsight, [recursive aspect] was not the correct behaviour and led to many bugs, since you may end up creating a library that contains compiled proto files from a third party, where you should instead be depending on a proper library for that third party’s protos.

Is there a place to further discuss this (this PR is probably not the best venue)? This situation sounds similar to some of the problems we have with SWIG (and C bindings in general) within Google, and pushing the codegen burden elsewhere has its own set of problems. @aaliddell

[aaliddell]

Is there a place to further discuss this (this PR is probably not the best venue)? This situation sounds similar to some of the problems we have with SWIG (and C bindings in general) within Google, and pushing the codegen burden elsewhere has its own set of problems. @aaliddell

You can email me or put it in a new discussion here; not sure I’ll be much help though with SWIG. Or put it on the bazel-discuss mailing list if you want a wider audience. Or slack. Too many options 😵‍💫

Compiling only directly is a good choice for gRPC generated libraries.

For non-service proto libraries, it's better to use lang_proto_library with an aspect. It removes redundancies - having to add py_proto_library next to every proto_library in the transitive closure. Aspects take care that your 1:1 mapping from proto dependency graph to python graph - even if lang_proto_library is duplicated. If there are problems with proto dependency graph, they are not going to be solved by either direct or aspect based solution. Aspect based solution will probably fail, whereas no-aspect/direct solution will potentially hide problems.

Ok, I thought I’d check your weren’t just blindly copying some of the other language rulesets’ behaviour without knowing the implications; seems like you’re on top of it 👍

[rickeylev]

per chat with Ivo: PR is not yet ready to merge; he's going to refactor things into a single bzlmod module file because deps for modules are fetched lazily (and thus no proto dep will be brought in unless necessary)

[tpudlik]

I just ran into an interesting bug in the unofficial py_proto_library from https://github.com/protocolbuffers/protobuf/: it doesn't respect tags = ["manual"]. No tags are forwarded to the code generation rule, so the code generation runs unconditionally. So I thought I'd confirm: this "official" implementation doesn't suffer from this bug, does it?

[jiawen]

I just discovered this comment in com_google_protobuf//:protobuf.bzl.py_proto_library(). It suggests that Bazel 5.3 will include py_proto_library() internally.

Is this the implementation? But only available when using bzlmod and/or Bazel 6?

[hrfuller]

Still wrapping my head around bzlmod stuff. Is the idea that users would have to depend on the py_proto_module seperately from rules_python in their MODULE.bazel file in order to use the py_proto_library rule; or would any dependency on rules_python include the ability to use py_proto_library?

[comius]

I just discovered this comment in com_google_protobuf//:protobuf.bzl.py_proto_library(). It suggests that Bazel 5.3 will include py_proto_library() internally.

Is this the implementation? But only available when using bzlmod and/or Bazel 6?

Yes, this is the implementation. It will work with Bazel 5.3 and Bazel 6. And also without bzlmod.

Still wrapping my head around bzlmod stuff. Is the idea that users would have to depend on the py_proto_module seperately from rules_python in their MODULE.bazel file in order to use the py_proto_library rule; or would any dependency on rules_python include the ability to use py_proto_library?

I fixed the PR, so that the users only need to depend on rules_python. And proto repositories are fetched only if you use py_proto_library,

[comius]

@rickeylev:

per chat with Ivo: PR is not yet ready to merge; he's going to refactor things into a single bzlmod module file because deps for modules are fetched lazily (and thus no proto dep will be brought in unless necessary)

The PR is now ready.

[alexeagle]

This forgot to update the docs/ folder so that the new stardoc comments are published for users to find.

[comius]

py_proto_library was tested manually on Bazel repository using Bazel 6.0.0 with and without using bzlmod.

Extend README.md to mention also py_proto_library.

With bzlmod py_proto_library just works. Users without bzlmod, need to import rules_proto and protobuf repositories into their WORKSPACE file.

py_proto_library is compatible with Bazel >=5.4.0 and >=6.0.0.

[comius]

cc @rickeylev@haberman

[meteorcloudy]

LGTM, from dependencies management perspective.

[aaliddell]

FWIW transitive proto compilation with an aspect is probably not the right method to use any longer. Both rules_proto and rules_proto_grpc have dropped aspect based compilation in favour of more explicit direct compilation of the named protos only within a target. This is because transitive compilation is both artificially limiting in terms of passable options and more importantly leads to multiple definitions of the same proto message if depended on via differing trees (a serious problem in C++, a nuisance in Python). See here for further details: https://rules-proto-grpc.com/en/latest/transitivity.html

[comius]

FWIW transitive proto compilation with an aspect is probably not the right method to use any longer. Both rules_proto and rules_proto_grpc have dropped aspect based compilation in favour of more explicit direct compilation of the named protos only within a target. This is because transitive compilation is both artificially limiting in terms of passable options and more importantly leads to multiple definitions of the same proto message if depended on via differing trees (a serious problem in C++, a nuisance in Python). See here for further details: https://rules-proto-grpc.com/en/latest/transitivity.html

Compiling only directly is a good choice for gRPC generated libraries.

For non-service proto libraries, it's better to use lang_proto_library with an aspect. It removes redundancies - having to add py_proto_library next to every proto_library in the transitive closure. Aspects take care that you're 1:1 mapping from proto dependency graph to python graph - even if lang_proto_library is duplicated. If there are problems with proto dependency graph, they are not going to be solved by either direct or aspect based solution. Aspect based solution will probably fail, whereas no-aspect/direct solution will potentially hide problems.

[rickeylev]

https://rules-proto-grpc.com/en/latest/transitivity.html says

In hindsight, [recursive aspect] was not the correct behaviour and led to many bugs, since you may end up creating a library that contains compiled proto files from a third party, where you should instead be depending on a proper library for that third party’s protos.

Is there a place to further discuss this (this PR is probably not the best venue)? This situation sounds similar to some of the problems we have with SWIG (and C bindings in general) within Google, and pushing the codegen burden elsewhere has its own set of problems. @aaliddell

[aaliddell]

Is there a place to further discuss this (this PR is probably not the best venue)? This situation sounds similar to some of the problems we have with SWIG (and C bindings in general) within Google, and pushing the codegen burden elsewhere has its own set of problems. @aaliddell

You can email me or put it in a new discussion here; not sure I’ll be much help though with SWIG. Or put it on the bazel-discuss mailing list if you want a wider audience. Or slack. Too many options 😵‍💫

Compiling only directly is a good choice for gRPC generated libraries.

For non-service proto libraries, it's better to use lang_proto_library with an aspect. It removes redundancies - having to add py_proto_library next to every proto_library in the transitive closure. Aspects take care that your 1:1 mapping from proto dependency graph to python graph - even if lang_proto_library is duplicated. If there are problems with proto dependency graph, they are not going to be solved by either direct or aspect based solution. Aspect based solution will probably fail, whereas no-aspect/direct solution will potentially hide problems.

Ok, I thought I’d check your weren’t just blindly copying some of the other language rulesets’ behaviour without knowing the implications; seems like you’re on top of it 👍

[rickeylev]

per chat with Ivo: PR is not yet ready to merge; he's going to refactor things into a single bzlmod module file because deps for modules are fetched lazily (and thus no proto dep will be brought in unless necessary)

[tpudlik]

I just ran into an interesting bug in the unofficial py_proto_library from https://github.com/protocolbuffers/protobuf/: it doesn't respect tags = ["manual"]. No tags are forwarded to the code generation rule, so the code generation runs unconditionally. So I thought I'd confirm: this "official" implementation doesn't suffer from this bug, does it?

[jiawen]

I just discovered this comment in com_google_protobuf//:protobuf.bzl.py_proto_library(). It suggests that Bazel 5.3 will include py_proto_library() internally.

Is this the implementation? But only available when using bzlmod and/or Bazel 6?

[hrfuller]

Still wrapping my head around bzlmod stuff. Is the idea that users would have to depend on the py_proto_module seperately from rules_python in their MODULE.bazel file in order to use the py_proto_library rule; or would any dependency on rules_python include the ability to use py_proto_library?

[comius]

I just discovered this comment in com_google_protobuf//:protobuf.bzl.py_proto_library(). It suggests that Bazel 5.3 will include py_proto_library() internally.

Is this the implementation? But only available when using bzlmod and/or Bazel 6?

Yes, this is the implementation. It will work with Bazel 5.3 and Bazel 6. And also without bzlmod.

Still wrapping my head around bzlmod stuff. Is the idea that users would have to depend on the py_proto_module seperately from rules_python in their MODULE.bazel file in order to use the py_proto_library rule; or would any dependency on rules_python include the ability to use py_proto_library?

I fixed the PR, so that the users only need to depend on rules_python. And proto repositories are fetched only if you use py_proto_library,

[comius]

@rickeylev:

per chat with Ivo: PR is not yet ready to merge; he's going to refactor things into a single bzlmod module file because deps for modules are fetched lazily (and thus no proto dep will be brought in unless necessary)

The PR is now ready.

[alexeagle]

This forgot to update the docs/ folder so that the new stardoc comments are published for users to find.

[comius]

py_proto_library was tested manually on Bazel repository using Bazel 6.0.0 with and without using bzlmod.

Extend README.md to mention also py_proto_library.

With bzlmod py_proto_library just works. Users without bzlmod, need to import rules_proto and protobuf repositories into their WORKSPACE file.

py_proto_library is compatible with Bazel >=5.4.0 and >=6.0.0.

[comius]

cc @rickeylev@haberman

[meteorcloudy]

LGTM, from dependencies management perspective.

[aaliddell]

FWIW transitive proto compilation with an aspect is probably not the right method to use any longer. Both rules_proto and rules_proto_grpc have dropped aspect based compilation in favour of more explicit direct compilation of the named protos only within a target. This is because transitive compilation is both artificially limiting in terms of passable options and more importantly leads to multiple definitions of the same proto message if depended on via differing trees (a serious problem in C++, a nuisance in Python). See here for further details: https://rules-proto-grpc.com/en/latest/transitivity.html

[comius]

FWIW transitive proto compilation with an aspect is probably not the right method to use any longer. Both rules_proto and rules_proto_grpc have dropped aspect based compilation in favour of more explicit direct compilation of the named protos only within a target. This is because transitive compilation is both artificially limiting in terms of passable options and more importantly leads to multiple definitions of the same proto message if depended on via differing trees (a serious problem in C++, a nuisance in Python). See here for further details: https://rules-proto-grpc.com/en/latest/transitivity.html

Compiling only directly is a good choice for gRPC generated libraries.

For non-service proto libraries, it's better to use lang_proto_library with an aspect. It removes redundancies - having to add py_proto_library next to every proto_library in the transitive closure. Aspects take care that you're 1:1 mapping from proto dependency graph to python graph - even if lang_proto_library is duplicated. If there are problems with proto dependency graph, they are not going to be solved by either direct or aspect based solution. Aspect based solution will probably fail, whereas no-aspect/direct solution will potentially hide problems.

[rickeylev]

https://rules-proto-grpc.com/en/latest/transitivity.html says

In hindsight, [recursive aspect] was not the correct behaviour and led to many bugs, since you may end up creating a library that contains compiled proto files from a third party, where you should instead be depending on a proper library for that third party’s protos.

Is there a place to further discuss this (this PR is probably not the best venue)? This situation sounds similar to some of the problems we have with SWIG (and C bindings in general) within Google, and pushing the codegen burden elsewhere has its own set of problems. @aaliddell

[aaliddell]

Is there a place to further discuss this (this PR is probably not the best venue)? This situation sounds similar to some of the problems we have with SWIG (and C bindings in general) within Google, and pushing the codegen burden elsewhere has its own set of problems. @aaliddell

You can email me or put it in a new discussion here; not sure I’ll be much help though with SWIG. Or put it on the bazel-discuss mailing list if you want a wider audience. Or slack. Too many options 😵‍💫

Compiling only directly is a good choice for gRPC generated libraries.

For non-service proto libraries, it's better to use lang_proto_library with an aspect. It removes redundancies - having to add py_proto_library next to every proto_library in the transitive closure. Aspects take care that your 1:1 mapping from proto dependency graph to python graph - even if lang_proto_library is duplicated. If there are problems with proto dependency graph, they are not going to be solved by either direct or aspect based solution. Aspect based solution will probably fail, whereas no-aspect/direct solution will potentially hide problems.

Ok, I thought I’d check your weren’t just blindly copying some of the other language rulesets’ behaviour without knowing the implications; seems like you’re on top of it 👍

[rickeylev]

per chat with Ivo: PR is not yet ready to merge; he's going to refactor things into a single bzlmod module file because deps for modules are fetched lazily (and thus no proto dep will be brought in unless necessary)

[tpudlik]

I just ran into an interesting bug in the unofficial py_proto_library from https://github.com/protocolbuffers/protobuf/: it doesn't respect tags = ["manual"]. No tags are forwarded to the code generation rule, so the code generation runs unconditionally. So I thought I'd confirm: this "official" implementation doesn't suffer from this bug, does it?

[jiawen]

I just discovered this comment in com_google_protobuf//:protobuf.bzl.py_proto_library(). It suggests that Bazel 5.3 will include py_proto_library() internally.

Is this the implementation? But only available when using bzlmod and/or Bazel 6?

[hrfuller]

Still wrapping my head around bzlmod stuff. Is the idea that users would have to depend on the py_proto_module seperately from rules_python in their MODULE.bazel file in order to use the py_proto_library rule; or would any dependency on rules_python include the ability to use py_proto_library?

[comius]

I just discovered this comment in com_google_protobuf//:protobuf.bzl.py_proto_library(). It suggests that Bazel 5.3 will include py_proto_library() internally.

Is this the implementation? But only available when using bzlmod and/or Bazel 6?

Yes, this is the implementation. It will work with Bazel 5.3 and Bazel 6. And also without bzlmod.

Still wrapping my head around bzlmod stuff. Is the idea that users would have to depend on the py_proto_module seperately from rules_python in their MODULE.bazel file in order to use the py_proto_library rule; or would any dependency on rules_python include the ability to use py_proto_library?

I fixed the PR, so that the users only need to depend on rules_python. And proto repositories are fetched only if you use py_proto_library,

[comius]

@rickeylev:

per chat with Ivo: PR is not yet ready to merge; he's going to refactor things into a single bzlmod module file because deps for modules are fetched lazily (and thus no proto dep will be brought in unless necessary)

The PR is now ready.

[alexeagle]

This forgot to update the docs/ folder so that the new stardoc comments are published for users to find.

[comius]

py_proto_library was tested manually on Bazel repository using Bazel 6.0.0 with and without using bzlmod.

Extend README.md to mention also py_proto_library.

With bzlmod py_proto_library just works. Users without bzlmod, need to import rules_proto and protobuf repositories into their WORKSPACE file.

py_proto_library is compatible with Bazel >=5.4.0 and >=6.0.0.

[comius]

cc @rickeylev@haberman

[meteorcloudy]

LGTM, from dependencies management perspective.

[aaliddell]

FWIW transitive proto compilation with an aspect is probably not the right method to use any longer. Both rules_proto and rules_proto_grpc have dropped aspect based compilation in favour of more explicit direct compilation of the named protos only within a target. This is because transitive compilation is both artificially limiting in terms of passable options and more importantly leads to multiple definitions of the same proto message if depended on via differing trees (a serious problem in C++, a nuisance in Python). See here for further details: https://rules-proto-grpc.com/en/latest/transitivity.html

[comius]

FWIW transitive proto compilation with an aspect is probably not the right method to use any longer. Both rules_proto and rules_proto_grpc have dropped aspect based compilation in favour of more explicit direct compilation of the named protos only within a target. This is because transitive compilation is both artificially limiting in terms of passable options and more importantly leads to multiple definitions of the same proto message if depended on via differing trees (a serious problem in C++, a nuisance in Python). See here for further details: https://rules-proto-grpc.com/en/latest/transitivity.html

Compiling only directly is a good choice for gRPC generated libraries.

For non-service proto libraries, it's better to use lang_proto_library with an aspect. It removes redundancies - having to add py_proto_library next to every proto_library in the transitive closure. Aspects take care that you're 1:1 mapping from proto dependency graph to python graph - even if lang_proto_library is duplicated. If there are problems with proto dependency graph, they are not going to be solved by either direct or aspect based solution. Aspect based solution will probably fail, whereas no-aspect/direct solution will potentially hide problems.

[rickeylev]

https://rules-proto-grpc.com/en/latest/transitivity.html says

In hindsight, [recursive aspect] was not the correct behaviour and led to many bugs, since you may end up creating a library that contains compiled proto files from a third party, where you should instead be depending on a proper library for that third party’s protos.

Is there a place to further discuss this (this PR is probably not the best venue)? This situation sounds similar to some of the problems we have with SWIG (and C bindings in general) within Google, and pushing the codegen burden elsewhere has its own set of problems. @aaliddell

[aaliddell]

Is there a place to further discuss this (this PR is probably not the best venue)? This situation sounds similar to some of the problems we have with SWIG (and C bindings in general) within Google, and pushing the codegen burden elsewhere has its own set of problems. @aaliddell

You can email me or put it in a new discussion here; not sure I’ll be much help though with SWIG. Or put it on the bazel-discuss mailing list if you want a wider audience. Or slack. Too many options 😵‍💫

Compiling only directly is a good choice for gRPC generated libraries.

For non-service proto libraries, it's better to use lang_proto_library with an aspect. It removes redundancies - having to add py_proto_library next to every proto_library in the transitive closure. Aspects take care that your 1:1 mapping from proto dependency graph to python graph - even if lang_proto_library is duplicated. If there are problems with proto dependency graph, they are not going to be solved by either direct or aspect based solution. Aspect based solution will probably fail, whereas no-aspect/direct solution will potentially hide problems.

Ok, I thought I’d check your weren’t just blindly copying some of the other language rulesets’ behaviour without knowing the implications; seems like you’re on top of it 👍

[rickeylev]

per chat with Ivo: PR is not yet ready to merge; he's going to refactor things into a single bzlmod module file because deps for modules are fetched lazily (and thus no proto dep will be brought in unless necessary)

[tpudlik]

I just ran into an interesting bug in the unofficial py_proto_library from https://github.com/protocolbuffers/protobuf/: it doesn't respect tags = ["manual"]. No tags are forwarded to the code generation rule, so the code generation runs unconditionally. So I thought I'd confirm: this "official" implementation doesn't suffer from this bug, does it?

[jiawen]

I just discovered this comment in com_google_protobuf//:protobuf.bzl.py_proto_library(). It suggests that Bazel 5.3 will include py_proto_library() internally.

Is this the implementation? But only available when using bzlmod and/or Bazel 6?

[hrfuller]

Still wrapping my head around bzlmod stuff. Is the idea that users would have to depend on the py_proto_module seperately from rules_python in their MODULE.bazel file in order to use the py_proto_library rule; or would any dependency on rules_python include the ability to use py_proto_library?

[comius]

I just discovered this comment in com_google_protobuf//:protobuf.bzl.py_proto_library(). It suggests that Bazel 5.3 will include py_proto_library() internally.

Is this the implementation? But only available when using bzlmod and/or Bazel 6?

Yes, this is the implementation. It will work with Bazel 5.3 and Bazel 6. And also without bzlmod.

Still wrapping my head around bzlmod stuff. Is the idea that users would have to depend on the py_proto_module seperately from rules_python in their MODULE.bazel file in order to use the py_proto_library rule; or would any dependency on rules_python include the ability to use py_proto_library?

I fixed the PR, so that the users only need to depend on rules_python. And proto repositories are fetched only if you use py_proto_library,

[comius]

@rickeylev:

per chat with Ivo: PR is not yet ready to merge; he's going to refactor things into a single bzlmod module file because deps for modules are fetched lazily (and thus no proto dep will be brought in unless necessary)

The PR is now ready.

[alexeagle]

This forgot to update the docs/ folder so that the new stardoc comments are published for users to find.

[comius]

py_proto_library was tested manually on Bazel repository using Bazel 6.0.0 with and without using bzlmod.

Extend README.md to mention also py_proto_library.

With bzlmod py_proto_library just works. Users without bzlmod, need to import rules_proto and protobuf repositories into their WORKSPACE file.

py_proto_library is compatible with Bazel >=5.4.0 and >=6.0.0.

[comius]

cc @rickeylev@haberman

[meteorcloudy]

LGTM, from dependencies management perspective.

[aaliddell]

FWIW transitive proto compilation with an aspect is probably not the right method to use any longer. Both rules_proto and rules_proto_grpc have dropped aspect based compilation in favour of more explicit direct compilation of the named protos only within a target. This is because transitive compilation is both artificially limiting in terms of passable options and more importantly leads to multiple definitions of the same proto message if depended on via differing trees (a serious problem in C++, a nuisance in Python). See here for further details: https://rules-proto-grpc.com/en/latest/transitivity.html

[comius]

FWIW transitive proto compilation with an aspect is probably not the right method to use any longer. Both rules_proto and rules_proto_grpc have dropped aspect based compilation in favour of more explicit direct compilation of the named protos only within a target. This is because transitive compilation is both artificially limiting in terms of passable options and more importantly leads to multiple definitions of the same proto message if depended on via differing trees (a serious problem in C++, a nuisance in Python). See here for further details: https://rules-proto-grpc.com/en/latest/transitivity.html

Compiling only directly is a good choice for gRPC generated libraries.

For non-service proto libraries, it's better to use lang_proto_library with an aspect. It removes redundancies - having to add py_proto_library next to every proto_library in the transitive closure. Aspects take care that you're 1:1 mapping from proto dependency graph to python graph - even if lang_proto_library is duplicated. If there are problems with proto dependency graph, they are not going to be solved by either direct or aspect based solution. Aspect based solution will probably fail, whereas no-aspect/direct solution will potentially hide problems.

[rickeylev]

https://rules-proto-grpc.com/en/latest/transitivity.html says

In hindsight, [recursive aspect] was not the correct behaviour and led to many bugs, since you may end up creating a library that contains compiled proto files from a third party, where you should instead be depending on a proper library for that third party’s protos.

Is there a place to further discuss this (this PR is probably not the best venue)? This situation sounds similar to some of the problems we have with SWIG (and C bindings in general) within Google, and pushing the codegen burden elsewhere has its own set of problems. @aaliddell

[aaliddell]

Is there a place to further discuss this (this PR is probably not the best venue)? This situation sounds similar to some of the problems we have with SWIG (and C bindings in general) within Google, and pushing the codegen burden elsewhere has its own set of problems. @aaliddell

You can email me or put it in a new discussion here; not sure I’ll be much help though with SWIG. Or put it on the bazel-discuss mailing list if you want a wider audience. Or slack. Too many options 😵‍💫

Compiling only directly is a good choice for gRPC generated libraries.

For non-service proto libraries, it's better to use lang_proto_library with an aspect. It removes redundancies - having to add py_proto_library next to every proto_library in the transitive closure. Aspects take care that your 1:1 mapping from proto dependency graph to python graph - even if lang_proto_library is duplicated. If there are problems with proto dependency graph, they are not going to be solved by either direct or aspect based solution. Aspect based solution will probably fail, whereas no-aspect/direct solution will potentially hide problems.

Ok, I thought I’d check your weren’t just blindly copying some of the other language rulesets’ behaviour without knowing the implications; seems like you’re on top of it 👍

[rickeylev]

per chat with Ivo: PR is not yet ready to merge; he's going to refactor things into a single bzlmod module file because deps for modules are fetched lazily (and thus no proto dep will be brought in unless necessary)

[tpudlik]

I just ran into an interesting bug in the unofficial py_proto_library from https://github.com/protocolbuffers/protobuf/: it doesn't respect tags = ["manual"]. No tags are forwarded to the code generation rule, so the code generation runs unconditionally. So I thought I'd confirm: this "official" implementation doesn't suffer from this bug, does it?

[jiawen]

I just discovered this comment in com_google_protobuf//:protobuf.bzl.py_proto_library(). It suggests that Bazel 5.3 will include py_proto_library() internally.

Is this the implementation? But only available when using bzlmod and/or Bazel 6?

[hrfuller]

Still wrapping my head around bzlmod stuff. Is the idea that users would have to depend on the py_proto_module seperately from rules_python in their MODULE.bazel file in order to use the py_proto_library rule; or would any dependency on rules_python include the ability to use py_proto_library?

[comius]

I just discovered this comment in com_google_protobuf//:protobuf.bzl.py_proto_library(). It suggests that Bazel 5.3 will include py_proto_library() internally.

Is this the implementation? But only available when using bzlmod and/or Bazel 6?

Yes, this is the implementation. It will work with Bazel 5.3 and Bazel 6. And also without bzlmod.

Still wrapping my head around bzlmod stuff. Is the idea that users would have to depend on the py_proto_module seperately from rules_python in their MODULE.bazel file in order to use the py_proto_library rule; or would any dependency on rules_python include the ability to use py_proto_library?

I fixed the PR, so that the users only need to depend on rules_python. And proto repositories are fetched only if you use py_proto_library,

[comius]

@rickeylev:

per chat with Ivo: PR is not yet ready to merge; he's going to refactor things into a single bzlmod module file because deps for modules are fetched lazily (and thus no proto dep will be brought in unless necessary)

The PR is now ready.

[alexeagle]

This forgot to update the docs/ folder so that the new stardoc comments are published for users to find.

[comius]

py_proto_library was tested manually on Bazel repository using Bazel 6.0.0 with and without using bzlmod.

Extend README.md to mention also py_proto_library.

With bzlmod py_proto_library just works. Users without bzlmod, need to import rules_proto and protobuf repositories into their WORKSPACE file.

py_proto_library is compatible with Bazel >=5.4.0 and >=6.0.0.

[comius]

cc @rickeylev@haberman

[meteorcloudy]

LGTM, from dependencies management perspective.

[aaliddell]

FWIW transitive proto compilation with an aspect is probably not the right method to use any longer. Both rules_proto and rules_proto_grpc have dropped aspect based compilation in favour of more explicit direct compilation of the named protos only within a target. This is because transitive compilation is both artificially limiting in terms of passable options and more importantly leads to multiple definitions of the same proto message if depended on via differing trees (a serious problem in C++, a nuisance in Python). See here for further details: https://rules-proto-grpc.com/en/latest/transitivity.html

[comius]

FWIW transitive proto compilation with an aspect is probably not the right method to use any longer. Both rules_proto and rules_proto_grpc have dropped aspect based compilation in favour of more explicit direct compilation of the named protos only within a target. This is because transitive compilation is both artificially limiting in terms of passable options and more importantly leads to multiple definitions of the same proto message if depended on via differing trees (a serious problem in C++, a nuisance in Python). See here for further details: https://rules-proto-grpc.com/en/latest/transitivity.html

Compiling only directly is a good choice for gRPC generated libraries.

For non-service proto libraries, it's better to use lang_proto_library with an aspect. It removes redundancies - having to add py_proto_library next to every proto_library in the transitive closure. Aspects take care that you're 1:1 mapping from proto dependency graph to python graph - even if lang_proto_library is duplicated. If there are problems with proto dependency graph, they are not going to be solved by either direct or aspect based solution. Aspect based solution will probably fail, whereas no-aspect/direct solution will potentially hide problems.

[rickeylev]

https://rules-proto-grpc.com/en/latest/transitivity.html says

In hindsight, [recursive aspect] was not the correct behaviour and led to many bugs, since you may end up creating a library that contains compiled proto files from a third party, where you should instead be depending on a proper library for that third party’s protos.

Is there a place to further discuss this (this PR is probably not the best venue)? This situation sounds similar to some of the problems we have with SWIG (and C bindings in general) within Google, and pushing the codegen burden elsewhere has its own set of problems. @aaliddell

[aaliddell]

Is there a place to further discuss this (this PR is probably not the best venue)? This situation sounds similar to some of the problems we have with SWIG (and C bindings in general) within Google, and pushing the codegen burden elsewhere has its own set of problems. @aaliddell

You can email me or put it in a new discussion here; not sure I’ll be much help though with SWIG. Or put it on the bazel-discuss mailing list if you want a wider audience. Or slack. Too many options 😵‍💫

Compiling only directly is a good choice for gRPC generated libraries.

For non-service proto libraries, it's better to use lang_proto_library with an aspect. It removes redundancies - having to add py_proto_library next to every proto_library in the transitive closure. Aspects take care that your 1:1 mapping from proto dependency graph to python graph - even if lang_proto_library is duplicated. If there are problems with proto dependency graph, they are not going to be solved by either direct or aspect based solution. Aspect based solution will probably fail, whereas no-aspect/direct solution will potentially hide problems.

Ok, I thought I’d check your weren’t just blindly copying some of the other language rulesets’ behaviour without knowing the implications; seems like you’re on top of it 👍

[rickeylev]

per chat with Ivo: PR is not yet ready to merge; he's going to refactor things into a single bzlmod module file because deps for modules are fetched lazily (and thus no proto dep will be brought in unless necessary)

[tpudlik]

I just ran into an interesting bug in the unofficial py_proto_library from https://github.com/protocolbuffers/protobuf/: it doesn't respect tags = ["manual"]. No tags are forwarded to the code generation rule, so the code generation runs unconditionally. So I thought I'd confirm: this "official" implementation doesn't suffer from this bug, does it?

[jiawen]

I just discovered this comment in com_google_protobuf//:protobuf.bzl.py_proto_library(). It suggests that Bazel 5.3 will include py_proto_library() internally.

Is this the implementation? But only available when using bzlmod and/or Bazel 6?

[hrfuller]

Still wrapping my head around bzlmod stuff. Is the idea that users would have to depend on the py_proto_module seperately from rules_python in their MODULE.bazel file in order to use the py_proto_library rule; or would any dependency on rules_python include the ability to use py_proto_library?

[comius]

I just discovered this comment in com_google_protobuf//:protobuf.bzl.py_proto_library(). It suggests that Bazel 5.3 will include py_proto_library() internally.

Is this the implementation? But only available when using bzlmod and/or Bazel 6?

Yes, this is the implementation. It will work with Bazel 5.3 and Bazel 6. And also without bzlmod.

Still wrapping my head around bzlmod stuff. Is the idea that users would have to depend on the py_proto_module seperately from rules_python in their MODULE.bazel file in order to use the py_proto_library rule; or would any dependency on rules_python include the ability to use py_proto_library?

I fixed the PR, so that the users only need to depend on rules_python. And proto repositories are fetched only if you use py_proto_library,

[comius]

@rickeylev:

per chat with Ivo: PR is not yet ready to merge; he's going to refactor things into a single bzlmod module file because deps for modules are fetched lazily (and thus no proto dep will be brought in unless necessary)

The PR is now ready.

[alexeagle]

This forgot to update the docs/ folder so that the new stardoc comments are published for users to find.

[comius]

py_proto_library was tested manually on Bazel repository using Bazel 6.0.0 with and without using bzlmod.

Extend README.md to mention also py_proto_library.

With bzlmod py_proto_library just works. Users without bzlmod, need to import rules_proto and protobuf repositories into their WORKSPACE file.

py_proto_library is compatible with Bazel >=5.4.0 and >=6.0.0.

[comius]

cc @rickeylev@haberman

[meteorcloudy]

LGTM, from dependencies management perspective.

[aaliddell]

FWIW transitive proto compilation with an aspect is probably not the right method to use any longer. Both rules_proto and rules_proto_grpc have dropped aspect based compilation in favour of more explicit direct compilation of the named protos only within a target. This is because transitive compilation is both artificially limiting in terms of passable options and more importantly leads to multiple definitions of the same proto message if depended on via differing trees (a serious problem in C++, a nuisance in Python). See here for further details: https://rules-proto-grpc.com/en/latest/transitivity.html

[comius]

FWIW transitive proto compilation with an aspect is probably not the right method to use any longer. Both rules_proto and rules_proto_grpc have dropped aspect based compilation in favour of more explicit direct compilation of the named protos only within a target. This is because transitive compilation is both artificially limiting in terms of passable options and more importantly leads to multiple definitions of the same proto message if depended on via differing trees (a serious problem in C++, a nuisance in Python). See here for further details: https://rules-proto-grpc.com/en/latest/transitivity.html

Compiling only directly is a good choice for gRPC generated libraries.

For non-service proto libraries, it's better to use lang_proto_library with an aspect. It removes redundancies - having to add py_proto_library next to every proto_library in the transitive closure. Aspects take care that you're 1:1 mapping from proto dependency graph to python graph - even if lang_proto_library is duplicated. If there are problems with proto dependency graph, they are not going to be solved by either direct or aspect based solution. Aspect based solution will probably fail, whereas no-aspect/direct solution will potentially hide problems.

[rickeylev]

https://rules-proto-grpc.com/en/latest/transitivity.html says

In hindsight, [recursive aspect] was not the correct behaviour and led to many bugs, since you may end up creating a library that contains compiled proto files from a third party, where you should instead be depending on a proper library for that third party’s protos.

Is there a place to further discuss this (this PR is probably not the best venue)? This situation sounds similar to some of the problems we have with SWIG (and C bindings in general) within Google, and pushing the codegen burden elsewhere has its own set of problems. @aaliddell

[aaliddell]

Is there a place to further discuss this (this PR is probably not the best venue)? This situation sounds similar to some of the problems we have with SWIG (and C bindings in general) within Google, and pushing the codegen burden elsewhere has its own set of problems. @aaliddell

You can email me or put it in a new discussion here; not sure I’ll be much help though with SWIG. Or put it on the bazel-discuss mailing list if you want a wider audience. Or slack. Too many options 😵‍💫

Compiling only directly is a good choice for gRPC generated libraries.

For non-service proto libraries, it's better to use lang_proto_library with an aspect. It removes redundancies - having to add py_proto_library next to every proto_library in the transitive closure. Aspects take care that your 1:1 mapping from proto dependency graph to python graph - even if lang_proto_library is duplicated. If there are problems with proto dependency graph, they are not going to be solved by either direct or aspect based solution. Aspect based solution will probably fail, whereas no-aspect/direct solution will potentially hide problems.

Ok, I thought I’d check your weren’t just blindly copying some of the other language rulesets’ behaviour without knowing the implications; seems like you’re on top of it 👍

[rickeylev]

per chat with Ivo: PR is not yet ready to merge; he's going to refactor things into a single bzlmod module file because deps for modules are fetched lazily (and thus no proto dep will be brought in unless necessary)

[tpudlik]

I just ran into an interesting bug in the unofficial py_proto_library from https://github.com/protocolbuffers/protobuf/: it doesn't respect tags = ["manual"]. No tags are forwarded to the code generation rule, so the code generation runs unconditionally. So I thought I'd confirm: this "official" implementation doesn't suffer from this bug, does it?

[jiawen]

I just discovered this comment in com_google_protobuf//:protobuf.bzl.py_proto_library(). It suggests that Bazel 5.3 will include py_proto_library() internally.

Is this the implementation? But only available when using bzlmod and/or Bazel 6?

[hrfuller]

Still wrapping my head around bzlmod stuff. Is the idea that users would have to depend on the py_proto_module seperately from rules_python in their MODULE.bazel file in order to use the py_proto_library rule; or would any dependency on rules_python include the ability to use py_proto_library?

[comius]

I just discovered this comment in com_google_protobuf//:protobuf.bzl.py_proto_library(). It suggests that Bazel 5.3 will include py_proto_library() internally.

Is this the implementation? But only available when using bzlmod and/or Bazel 6?

Yes, this is the implementation. It will work with Bazel 5.3 and Bazel 6. And also without bzlmod.

Still wrapping my head around bzlmod stuff. Is the idea that users would have to depend on the py_proto_module seperately from rules_python in their MODULE.bazel file in order to use the py_proto_library rule; or would any dependency on rules_python include the ability to use py_proto_library?

I fixed the PR, so that the users only need to depend on rules_python. And proto repositories are fetched only if you use py_proto_library,

[comius]

@rickeylev:

per chat with Ivo: PR is not yet ready to merge; he's going to refactor things into a single bzlmod module file because deps for modules are fetched lazily (and thus no proto dep will be brought in unless necessary)

The PR is now ready.

[alexeagle]

This forgot to update the docs/ folder so that the new stardoc comments are published for users to find.

[comius]

py_proto_library was tested manually on Bazel repository using Bazel 6.0.0 with and without using bzlmod.

Extend README.md to mention also py_proto_library.

With bzlmod py_proto_library just works. Users without bzlmod, need to import rules_proto and protobuf repositories into their WORKSPACE file.

py_proto_library is compatible with Bazel >=5.4.0 and >=6.0.0.

[comius]

cc @rickeylev@haberman

[meteorcloudy]

LGTM, from dependencies management perspective.

[aaliddell]

FWIW transitive proto compilation with an aspect is probably not the right method to use any longer. Both rules_proto and rules_proto_grpc have dropped aspect based compilation in favour of more explicit direct compilation of the named protos only within a target. This is because transitive compilation is both artificially limiting in terms of passable options and more importantly leads to multiple definitions of the same proto message if depended on via differing trees (a serious problem in C++, a nuisance in Python). See here for further details: https://rules-proto-grpc.com/en/latest/transitivity.html

[comius]

FWIW transitive proto compilation with an aspect is probably not the right method to use any longer. Both rules_proto and rules_proto_grpc have dropped aspect based compilation in favour of more explicit direct compilation of the named protos only within a target. This is because transitive compilation is both artificially limiting in terms of passable options and more importantly leads to multiple definitions of the same proto message if depended on via differing trees (a serious problem in C++, a nuisance in Python). See here for further details: https://rules-proto-grpc.com/en/latest/transitivity.html

Compiling only directly is a good choice for gRPC generated libraries.

For non-service proto libraries, it's better to use lang_proto_library with an aspect. It removes redundancies - having to add py_proto_library next to every proto_library in the transitive closure. Aspects take care that you're 1:1 mapping from proto dependency graph to python graph - even if lang_proto_library is duplicated. If there are problems with proto dependency graph, they are not going to be solved by either direct or aspect based solution. Aspect based solution will probably fail, whereas no-aspect/direct solution will potentially hide problems.

[rickeylev]

https://rules-proto-grpc.com/en/latest/transitivity.html says

In hindsight, [recursive aspect] was not the correct behaviour and led to many bugs, since you may end up creating a library that contains compiled proto files from a third party, where you should instead be depending on a proper library for that third party’s protos.

Is there a place to further discuss this (this PR is probably not the best venue)? This situation sounds similar to some of the problems we have with SWIG (and C bindings in general) within Google, and pushing the codegen burden elsewhere has its own set of problems. @aaliddell

[aaliddell]

Is there a place to further discuss this (this PR is probably not the best venue)? This situation sounds similar to some of the problems we have with SWIG (and C bindings in general) within Google, and pushing the codegen burden elsewhere has its own set of problems. @aaliddell

You can email me or put it in a new discussion here; not sure I’ll be much help though with SWIG. Or put it on the bazel-discuss mailing list if you want a wider audience. Or slack. Too many options 😵‍💫

Compiling only directly is a good choice for gRPC generated libraries.

For non-service proto libraries, it's better to use lang_proto_library with an aspect. It removes redundancies - having to add py_proto_library next to every proto_library in the transitive closure. Aspects take care that your 1:1 mapping from proto dependency graph to python graph - even if lang_proto_library is duplicated. If there are problems with proto dependency graph, they are not going to be solved by either direct or aspect based solution. Aspect based solution will probably fail, whereas no-aspect/direct solution will potentially hide problems.

Ok, I thought I’d check your weren’t just blindly copying some of the other language rulesets’ behaviour without knowing the implications; seems like you’re on top of it 👍

[rickeylev]

per chat with Ivo: PR is not yet ready to merge; he's going to refactor things into a single bzlmod module file because deps for modules are fetched lazily (and thus no proto dep will be brought in unless necessary)

[tpudlik]

I just ran into an interesting bug in the unofficial py_proto_library from https://github.com/protocolbuffers/protobuf/: it doesn't respect tags = ["manual"]. No tags are forwarded to the code generation rule, so the code generation runs unconditionally. So I thought I'd confirm: this "official" implementation doesn't suffer from this bug, does it?

[jiawen]

I just discovered this comment in com_google_protobuf//:protobuf.bzl.py_proto_library(). It suggests that Bazel 5.3 will include py_proto_library() internally.

Is this the implementation? But only available when using bzlmod and/or Bazel 6?

[hrfuller]

Still wrapping my head around bzlmod stuff. Is the idea that users would have to depend on the py_proto_module seperately from rules_python in their MODULE.bazel file in order to use the py_proto_library rule; or would any dependency on rules_python include the ability to use py_proto_library?

[comius]

I just discovered this comment in com_google_protobuf//:protobuf.bzl.py_proto_library(). It suggests that Bazel 5.3 will include py_proto_library() internally.

Is this the implementation? But only available when using bzlmod and/or Bazel 6?

Yes, this is the implementation. It will work with Bazel 5.3 and Bazel 6. And also without bzlmod.

Still wrapping my head around bzlmod stuff. Is the idea that users would have to depend on the py_proto_module seperately from rules_python in their MODULE.bazel file in order to use the py_proto_library rule; or would any dependency on rules_python include the ability to use py_proto_library?

I fixed the PR, so that the users only need to depend on rules_python. And proto repositories are fetched only if you use py_proto_library,

[comius]

@rickeylev:

per chat with Ivo: PR is not yet ready to merge; he's going to refactor things into a single bzlmod module file because deps for modules are fetched lazily (and thus no proto dep will be brought in unless necessary)

The PR is now ready.

[alexeagle]

This forgot to update the docs/ folder so that the new stardoc comments are published for users to find.