Skip to content

Conversation

@madsodgaard
Copy link

Allows the developer to supply a custom directory that will be used as the base of the protofiles.

{
    "invocations": [
        {
            "protoFiles": [
                "file1.proto",
                "file2.proto",
            ],
            "visibility": "public",
            "protoPath": "proto_repo_submodule/protos"
        }
    ]
}

This is useful if you are storing protofiles inside a Git repo and adding that as a submodule to other repos. Giving you a directory structure like:

MySwiftApp/
  - Sources
    - MyAppTarget
      - proto_repo_submodule
        - protos
          - file1.proto
          - file2.proto

and any proto files will be importing other proto files relative to that "protos" directory:

such as in file1.proto

import "file2.proto"

@FranzBusch
Copy link
Member

Thanks for opening this PR. Just for my understanding this is a convenience and you could achieve the same with the current options by just passing the full path in the protoFiles option right?

@madsodgaard
Copy link
Author

@FranzBusch Unfortunately not. The issue is when the protofiles use import. Any import statements will look for files relative to the proto-path passed to protoc. So If I pass:

{
    "invocations": [
        {
            "protoFiles": [
                "proto_repo_submodule/protos/file1.proto",
                "proto_repo_submodule/protos/file2.proto",
            ],
            "visibility": "public"
        }
    ]
}

and file1.proto, does import "file2.proto2 (which is the correct thing to do), that won't work. Because we just pass MyLibrary/Sources as the protopath.

Copy link
Member

@FranzBusch FranzBusch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM! @glbrntt can you check if that makes sense for the gRPC plugin as well?

/// Whether import statements should be preceded with visibility.
var useAccessLevelOnImports: Bool?
/// Overrides the path to look for protobuf files
var protoPath: String?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should likely be an array rather than a String as you can specify -I/--proto_path multiple times to protoc.

Given the example in the PR description it's not hard to imagine multiple git submodules being added.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea, I changed it to array 👍

@madsodgaard madsodgaard requested a review from glbrntt December 3, 2025 10:05
@madsodgaard
Copy link
Author

@glbrntt @FranzBusch In terms of using protoPaths as an array, this has some implications for specifying the input files to SwiftPM.

Consider:

MySwiftApp/
  - Sources
    - MyAppTarget
      - proto_repo_submodule1
        - protos
          - file1.proto
          - file2.proto
      - proto_repo_submodule2
        - protos
          - file3.proto
          - file4.proto

using

{
    "invocations": [
        {
            "protoFiles": [
                "file1.proto",
                "file2.proto",
                "file3.proto",
                "file4.proto"
            ],
            "visibility": "public",
            "protoPaths": ["proto_repo_submodule1/protos", "proto_repo_submodule2/protos"]
        }
    ]
}

we need to pass the .proto files as inputs of the SwiftPM plugin. Should we just pass all possible combinations of file paths, so?

  • proto_repo_submodule1/protos/file1.proto
  • proto_repo_submodule2/protos/file1.proto
  • etc...

Alternatively, we should just provide the protoPath as a String (so a single directory), and the files in protoFiles are relative to that. And if you have a different submodule/folder structure, that would be a separate invocation?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants