Skip to content

Run vtproto multiple times on different files of the same package #118

Closed

Description

Context

Currently, it is not possible to run vtproto multiple times for the same package on different files because it would generate multiple times the same helpers, so the code would not compile.

As an example, let's take the pool package of this repository with the following proto files: pool.proto, pool_with_slice_reuse.proto, and pool_with_oneof.proto. As long as we run vtproto once with all those files, it works fine. However, if we run vtproto three times (once for each file), then the generated code doesn't compile because there are three declarations of the same functions / variables:

  • encodeVarint
  • sov
  • soz
  • skip
  • ErrInvalidLength
  • ErrIntOverflow
  • ErrUnexpectedEndOfGroup

I believe this is an issue because this behavior contrasts with the standard protobuf compiler, causing difficulties to onboard automation that assumes the ability to run the compiler for one file at a time. As a user, I'm currently facing this challenge and I would love to fix this issue to improve compatibility with automation and ensure a seamless transition for new users as well. 😃

Possible solutions

I can think of at least three possible solutions to tackle this issue. Before working on an implementation, I would like to gauge interest in the different solutions. If there is a preferred one, I can provide a Pull Request. My recommended one is solution A. There may also be other ways that I haven't think about.

A. Add helpers to vtprotobuf as a package and use them from the generated code (recommended)

We could expose public helpers (e.g. EncodeVarint) as part of the vtprotobuf package, as is currently done for vtprotobuf/types/known/... for supporting well-known types. The generated _vtproto.pb.go files of users would then have a dependency on a vtprotobuf/protohelpers package (for example) to use these helpers.

I think the dependency is fine but, if this is not acceptable for some users, we could add an option to generate today's no-dependency helpers instead. There is an existing --go-vtproto_opt=wkt=false option to avoid depending on vtprotobuf. We could consolidate both options under something like --go-vtproto_opt=vtproto_dep=false, which would disable both support for well-known types and usage of global helpers.

B. Generate a different file containing all helpers for the package

Besides files generated from source proto files, when running vtproto, we could generate an additional file defining the helpers, that we could name vtproto_helpers.go for instance. This way, when running vtproto once for each file, that file would be regenerated and left unchanged.

With this, there is no need for a dependency to vtprotobuf. However, one potential issue that I see is that we would lose the 1:1 mapping between source proto files and generated files, which may not be friendly to automation either. Indeed, the regular proto compiler generates one file for each source file and each generated file corresponds to one source file: there is an exact 1:1 mapping. This would no longer be the case for vtproto with this implementation.

C. Generate different helpers for each file

We could generate the same helpers with different names for different files. For instance (still taking the pool package of this repository), pool.proto would generate encodeVarint_pool, sov_pool, soz_pool, etc. and pool_with_slice_reuse.proto would generate encodeVarint_pool_with_slice_reuse, sov_pool_with_slice_reuse, soz_pool_with_slice_reuse.

I see two issues with this approach, though:

  • the generated code is more verbose and less readable;
  • some helpers are public so they are exposed to users. Not only would a public ErrUnexpectedEndOfGroup_pool_with_slice_reuse be less readable, it would also be confusing to have the same public error defined in several places under different names.
    • note that these public errors are currently generated for each package, so they are already defined in several places (with the same name). Defining them only once in vtprotobuf might be an additional benefit of solution A.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions