php namespace support for GPBMetadata

[cc5092]

#3162

TeBoring commented on 2 Jun
I don't think we need to change the namespace of GPBMetadata files. These files are transparent to users.

As you said, these GPBMetadata files are transparent to users. But I think it still makes sense to add namespace for GPBMetadata file.

Suppose I have 2 grpc go services and are developed by different teams. They define the protobuf file by "user.proto". this "user.proto" will generate a GPBMetadata\User.php file.

My laravel project needs to use both two grpc services they provided and for the psr-4 autoload, the composer.json file needs to be defined as follows:

"autoload": {
    "psr-4": {
        "App\\": "app/"
    },
    "classmap": [
        "app/FirstService/GPBMetadata/",
        "app/SecondService/GPBMetadata/"
    ]
}

Executing "composer dumpautoload" in the root directory of project, and we will get notice like this:

Warning: Ambiguous class resolution, "GPBMetadata\User" was found in both "$baseDir . 'app/FirstService/GPBMetadata/User.php" and "path_to_proj/app/SecondService/GPBMetadata/User.php", the first will be used.

So I think it makes sense to add GPBMetadata namespace support.

Looking forward to your reply.

ping @TeBoring

[TeBoring]

I see this is a problem. The current namespace of GPBMetatdata file is related to the place of proto file, e.g., foo/bar.proto will become GPBMetadata/Foo/Bar.php
One way to solve this is the way you mentioned.
The other way is that user could put their proto files in a unique place, e.g., first_service/user.proto and second_service/user.proto will become GPBMetadata/FirstService/User.php and GPBMetadata/SecondService/User.php.
As far as I see, both ways needs action from the user side. However, the second option has fewer maintaining cost, because user only need to put all proto files there. The first option will need to add the file option in every proto file.

[cc5092]

Hi
I can't do like your second option to generate GPBMetadata/FirstService/User.php.
Here is my code tree path.

current pwd is path_to_proj/proto/ and the compile command is

protoc --proto_path=first/ --php_out=. --grpc_out=. --plugin=protoc-gen-grpc=/usr/local/bin/grpc_php_plugin first/*.proto

The Pb class generate namespace successfully by option php_namespace in proto file. But the GPBMetadata still generate as GPBMetadata\User.php.

[TeBoring]

Is --proto_path=first/ required? If it is removed, you can get GPBMetadata\First\User.php

[TeBoring]

The import in your proto file may need to be changed accordingly.

[cc5092]

Ok, I tried second option, it works fine. thank you. @TeBoring

The remain problem is the namespace of GPBMetadata is not the same with the stub class.

For example. stub class namespace is "FirstService\User.php", but GPBMetadata is "GPBMetadata\FirstService\User.php". It's better if is "FirstService\GPBMetadata\User.php".

[cc5092]

And for psr-4 autoload, namespace like "FirstService\GPBMetadata\User.php" makes sense for create standalone library for other developer to import.

[fiboknacky]

+1 for this.
It'd be great if the GPBMetadata is generated based on the proto option for specifying namespaces.

[andydeng]

+1. This issue affects our project too. It will be great if we can choose GPBMetadata's namespace.

[atrauzzi]

Looking forward to seeing this resolved, would hate to get collisions in any projects that rely on multiple PB-based implementations.

The GPBNamespace without a prefix requires us to modify our autoloader with a very specific namespace which is inconvenient. If it followed the namespace of the classes themselves, I could have protoc output to a .gitignored directory.

[atrauzzi]

Separate note: It would also be nice if while all this was being done, GPBMetadata was switched to GpbMetadata so as to follow PSR conventions.

[TeBoring]

I will add a file option to add GPBMetadata namespace.

[atrauzzi]

Can you follow community+PSR conventions and make it GpbMetadata? Or at least let it be configurable to be PSR compliant?

[TeBoring]

Added in #4622

[atrauzzi]

🎉