Add a version here.

done

Add a version here.

done

Add a version here.

done

Add a version here.

done

Brief warning: I don't think Bazel will treat http_archive repositories as bzlmod deps. I suspect in this case it's okay because rules_proto_grpc has a working WORKSPACE file, but I can't remember off the top of my head. Just something to keep an eye out for.

you're right, it is not considered a bazel_dep. I found one release from bcr, switched to use it

Looks like this is still an http archive, did you intend to replace this?

If rules_proto_grpc isn't working out-of-the-box with bzlmod, I think the only path to get things working is to manually enumerate the dependencies of rules_proto_grpc, pull it in as a http archive, and hope for the best. The one major drawback is that this instance of rules_proto_grpc becomes unique to nanopb, and any other references to rules_proto_grpc from other dependencies will produce a second source of truth.

I'm not sure if there's a way around this outside of requiring every project that uses nanopb to be hybrid WORKSPACE/blzmod, which isn't a satisfying result. I think you've done the best you can here.

I no longer work at Astranis, but we needed the BUILD file to use the google protobuf instead of a separate protobuf for python. Otherwise, you end up with duplicate modules defining the same messages and it won't work at runtime. Please revert this unless it's necessary

interestingly, I found there are two py_proto_library commonly used. One is from protobuf repo, the other one is from rules_python. For the one in protobuf, I have to build python proto separately from source instead of using built "descriptor_proto" target. But the one from rules_python can do it. I switched to use it and make it much cleaner here.

Yeah, the rules_python version is the future. The protobuf implementation was just a stop-gap and will eventually be removed.

Is the -rc2 version because the current release doesn't support bazel modules yet? I'd suggest a comment or holding off on merging the PR until released

no specific reason, just picked whatever latest https://registry.bazel.build/modules/rules_proto.
there is an older version 5.3.0-21.7 from bcr, too, which I think it implies bzlmod support is up, rc2 might refer to some new features of protobuf repo. but anyways I am not quite familiar with the naming convention for that repo, if you have a preferred version, I can modify it.

I would use the latest released versions that are not -rc (release candidate) or -alpha versions, please update those and confirm they still work

there is no stable version for rules_proto_grpc, I think the decision has to be made to use http_archive or alpha, any thoughts?

I normally commit this so that people that checkout the repo can reproduce the exact same dependencies as me, if necessary.

good caught, I did it simply because I rarely see a repo committing this, for example, https://github.com/protocolbuffers/protobuf, https://github.com/rules-proto-grpc/rules_proto_grpc, etc.

As far as I know this will be generated while doing bazel build, is there any reason people cannot reproduce the exact same deps without the lock file? I feel that in that case, the problem is in the MODULE.bazel file not because not committing the lock file.

It's often ignored because there's currently some usability issues with it: bazelbuild/bazel#20369

It's okay to check in (that's what it was designed for), but until Bazel changes the behavior there might be some annoying churn. Because the Bazel changes to nanopb are relatively infrequent, it's probably fine to check it in.

What I mean is that MODULE.bazel just specifies version numbers and only for direct dependencies, but the lock file will say exactly the hash of the code it pulled in and all the transitive dep versions as well.

fair enough, will commit it

Isn't the old name com_github_nanopb_nanopb?

mb, done

update the name

done

What I mean is that MODULE.bazel just specifies version numbers and only for direct dependencies, but the lock file will say exactly the hash of the code it pulled in and all the transitive dep versions as well.

Reposting your old comment since it got marked outdated:

interestingly, I found there are two py_proto_library commonly used. One is from protobuf repo, the other one is from rules_python. For the one in protobuf, I have to build python proto separately from source instead of using built "descriptor_proto" target. But the one from rules_python can do it. I switched to use it and make it much cleaner here.

I'm a little confused here, py_proto_library from com_google_protobuf will implicitly include descriptor_proto for you, you don't need to add it. The old code worked properly with bazel, can you post the error you were getting?

If you want to change this, @tobithiel can you confirm this still builds for Astranis?

ERROR: /home/sayter/inno_ws/nanopb/BUILD.bazel:93:17: ProtoCompile generator/proto/nanopb_pb2.py [for tool] failed: (Exit 1): protoc failed: error executing ProtoCompile command (from target //:nanopb_py_proto_genproto) bazel-out/k8-opt-exec-ST-13d3ddad9198/bin/external/protobuf~/protoc '--python_out=bazel-out/k8-opt-exec-ST-13d3ddad9198/bin/generator/proto' -Igenerator/proto generator/proto/nanopb.proto

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
google/protobuf/descriptor.proto: File not found.
nanopb.proto:9:1: Import "google/protobuf/descriptor.proto" was not found or had errors.
nanopb.proto:144:12: "google.protobuf.FieldDescriptorProto.Type" is not defined.
nanopb.proto:173:8: "google.protobuf.FileOptions" is not defined.
nanopb.proto:177:8: "google.protobuf.MessageOptions" is not defined.
nanopb.proto:181:8: "google.protobuf.EnumOptions" is not defined.
nanopb.proto:185:8: "google.protobuf.FieldOptions" is not defined.
ERROR: /home/sayter/inno_ws/nanopb/BUILD.bazel:93:17: ProtoCompile generator/proto/nanopb_pb2.py failed: (Exit 1): protoc failed: error executing ProtoCompile command (from target //:nanopb_py_proto_genproto) bazel-out/k8-opt-exec-ST-13d3ddad9198/bin/external/protobuf~/protoc '--python_out=bazel-out/k8-fastbuild/bin/generator/proto' -Igenerator/proto generator/proto/nanopb.proto

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
google/protobuf/descriptor.proto: File not found.
nanopb.proto:9:1: Import "google/protobuf/descriptor.proto" was not found or had errors.
nanopb.proto:144:12: "google.protobuf.FieldDescriptorProto.Type" is not defined.
nanopb.proto:173:8: "google.protobuf.FileOptions" is not defined.
nanopb.proto:177:8: "google.protobuf.MessageOptions" is not defined.
nanopb.proto:181:8: "google.protobuf.EnumOptions" is not defined.
nanopb.proto:185:8: "google.protobuf.FieldOptions" is not defined.

Since it couldn't fine descriptor.proto, I think it does not include descriptor_proto. The root cause should be version changed to protobuf module. To overcome it, two ways I have tried, 1) making another target for descriptor_proto_py, or 2) switch to py_proto_library from rules_proto.

Personally I like the 2)'s usage more, however, after testing it more. I found that I have to change

I tested more for the two and am realizing that my original attempt should be better. Since generator/proto/__init__.py needs to be changed accordingly because that py_proto_library doesn't have "include" attribute, and will raise an error as below

ERROR: /home/sayter/inno_ws/nanopb/tests/bazel_workspace_support/BUILD:9:24: Compiling protoc outputs for nanopb_plugin plugin on target @//:test_proto_nanopb_pb failed: (Exit 1): bash failed: error executing ProtoCompile command (from target //:test_proto_nanopb_pb) /bin/bash -c 'mkdir -p '\''bazel-out/k8-fastbuild/bin/_rpg_premerge_test_proto_nanopb_pb'\'' && bazel-out/k8-opt-exec-ST-13d3ddad9198/bin/external/com_google_protobuf_protoc_linux_x86_64/protoc.exe "$@"' ... (remaining 7 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
Traceback (most recent call last):
  File "/tmp/bazel-working-directory/__main__/bazel-out/k8-opt-exec-ST-13d3ddad9198/bin/external/nanopb/protoc-gen-nanopb.runfiles/__main__/../nanopb/generator/protoc-gen-nanopb.py", line 7, in <module>
    from nanopb_generator import *
  File "/home/sayter/inno_ws/nanopb/generator/nanopb_generator.py", line 67, in <module>
    nanopb_pb2 = proto.load_nanopb_pb2()
  File "/home/sayter/inno_ws/nanopb/generator/proto/__init__.py", line 86, in load_nanopb_pb2
    import nanopb_pb2 as nanopb_pb2_mod
ModuleNotFoundError: No module named 'nanopb_pb2'
--nanopb_plugin_out: protoc-gen-nanopb_plugin: Plugin failed with status code 1.
Use --verbose_failures to see the command lines of failed build steps.

Last, if there is a way to test it, I think it would be good to have it in the CI.

w.r.t. the python issue, it is exactly bazelbuild/rules_python#1543.

I tried to downgrade rules_python and it works now. I created another PR targeting this branch https://github.com/Sayter99/nanopb/pull/1/files ; feel free to take a look. In that branch I downgrade rules_proto_grpc too since it is the latest stable version.

tl;dr @mark64

1. changes - https://github.com/Sayter99/nanopb/pull/1/files
2. downgrade rules_python and rules_proto_grpc makes workspace working
3. tried to test different py_proto_library more but let's wait @tobithiel if there's a better way to test it

It would be great to eventually migrate to the latest version of rules_python and use their py_proto_library, but maybe that can be done in a separate change.

Re the ModuleNotFoundError: I would have hoped this can be fixed by setting the import_prefix and strip_import_prefix on the proto_library appropriately (so that no includes are necessary).

@tpudlik ty,

Re the ModuleNotFoundError: I would have hoped this can be fixed by setting the import_prefix and strip_import_prefix on the proto_library appropriately (so that no includes are necessary).

I have tried

proto_library(
    name = "nanopb_proto",
    srcs = [
        "generator/proto/nanopb.proto",
    ],
    strip_import_prefix = "generator/proto/",
    import_prefix = "generator/proto/",
    deps = ["@com_google_protobuf//:descriptor_proto"],
)

py_proto_library(
    name = "nanopb_py_proto",
    deps = [
        ":nanopb_proto",
    ],
)

but no luck. Any idea?

It would be great to eventually migrate to the latest version of rules_python and use their py_proto_library, but maybe that can be done in a separate change.

Makes great sense to me, however, I was not able to make it work when using rules_python > 0.26.0. I got the same error as bazelbuild/rules_python#1543

The following chain of repository dependencies lead to the missing definition.
 - @rules_python_internal
This could either mean you have to add the '@rules_python_internal' repository with a statement like `http_archive` in your WORKSPACE file (note that transitive dependencies are not added automatically), or move an existing definition earlier in your WORKSPACE file.
ERROR: Error computing the main repository mapping: cycles detected during computation of main repo mapping

It might come from deps of nanopb, I will look into it when I got some time.

I would use the latest released versions that are not -rc (release candidate) or -alpha versions, please update those and confirm they still work

The XXX comment should be replaced with something like this

Done

Looks like this is still an http archive, did you intend to replace this?

If rules_proto_grpc isn't working out-of-the-box with bzlmod, I think the only path to get things working is to manually enumerate the dependencies of rules_proto_grpc, pull it in as a http archive, and hope for the best. The one major drawback is that this instance of rules_proto_grpc becomes unique to nanopb, and any other references to rules_proto_grpc from other dependencies will produce a second source of truth.

I'm not sure if there's a way around this outside of requiring every project that uses nanopb to be hybrid WORKSPACE/blzmod, which isn't a satisfying result. I think you've done the best you can here.