Protobuf: generated c++ include guards are not unique for different message types

[a4z]

I have 2 projects, each with proto files with the same name.
They define the same message name, but use different packages

a/query.proto

syntax = "proto3";
option optimize_for = LITE_RUNTIME;
package a;
message Query {;  
  string what = 1;
}

b/query.proto

syntax = "proto3";
option optimize_for = LITE_RUNTIME;
package b;
message Query {;  
  string what = 1;
}

the generated header go to different directories,

and in theory it should be possible to use

#inlcude<a/query.pb.h>
#inlcude<b/query.pb.h>


a::Query aquery;
b::Query bquery;

since these are clearly different types from different headers.

but in practice this does not work because the include guards in the generated header files are equal.

#ifndef PROTOBUF_query_2eproto__INCLUDED
#define PROTOBUF_query_2eproto__INCLUDED

It would be nice to put the package name in the include guard so that the upper described scenario does work without workaround like renaming proto files, since this is not always possible.
Thank you!

[thomasvl]

If you pass a/query.proto when invoking protoc, I believe the "A" should end up in the include guard also.

[a4z]

Thanks for the hint @thomasvl
It is true what you write, but for me in best case a workaround and sometimes not even practicable
for example, if the proto files are processed and generated via CMake or come with a 3rd party library

would the include guard be generated like that
namespace_namespace_typename
than everything would just work. And the namespace names are also generated and not thrown away.

I think a place in the file location that defined the include guard is not the best solution , this means, depending on the location of the file, generated output looks different and might one time work, an other time, as described above, not.

[ghost]

I just ran into this problem myself. I had to rename some messages to get everything to compile again. Since I work with CMake the tip posted by @thomasvl doesn't work either (the messages all have to be in the same folder for CMake to process them correctly).

This should be easily fixable. I'd send in a pull request but that is impossible to do for google because I don't have the google account linked to my github email address so that'd be a waste of time.

[a4z]

do you need a google account for submitting a pull request here on github @martijnotto ?

[ghost]

Yes. If you don't they won't even look at it.

[a4z]

aha, I have a google account, could it work if I fork this, you fork my fork, I pull you changes and sent them here?
edit:
just checked, the fix is indeed not difficult, I can do this but it has to wait until since it is summer now

[xfxyjwf]

@martijnotto Not sure if you get that impression from other Google projects, but we do accept pull requests from anyone as long as they have signed Google CLA.

For this particular issue, as Thomas mentioned, you need to pass different proto files using different file paths to protoc, or the generated code won't work. Changing the include guard doesn't solve the problem because we use the file path as unique ID in multiple places.

As for CMake, we use CMake for our own tests and our .proto files live completely outside of the cmake directory and they spread across multiple directories. There is no such restriction that messages must be in the same folder for CMake to work.

[a4z]

@xfxyjwf of course changing the include path that gets written would solve the problem,
if this would effect anything else and you use 'unique ids' that special, the bug is actually much more serious. (imaging a unbelievable head shaking from me ;-)

[ghost]

@xfxyjwf Signing the CLA only works if the email address for the commits is the same as that of the google account used. Since I use unique addresses for services I have no google account linked to my github address and thus it doesn't work. I have tried in the past and gave up.

Anyways, to fix this the unique identifier should be changed so that it is a combination of the filename and the package it uses. It makes no sense to only use the filename as the include guard. An even easier solution would be to just use #pragma once, it seems all common compilers now support this. Is there any rule prohibiting you from simply doing that?

About CMake, the usual way to do this is to use 'protobuf_generate_cpp', of which the documentation says the following:

"NOTE: The PROTOBUF_GENERATE_CPP macro & add_executable() or add_library() calls only work properly within the same directory."

[xfxyjwf]

It's by design that different proto files must use a different import path. Imagining someone tries to import your query.proto:

syntax = "proto2";
import "query.proto";

Protoc has to know which query.proto you are trying to import. If you make two query.proto appear exactly the same to protoc, protoc won't be able to tell which one is the correct one.

This semantics is built into the proto langauge and implementation. Changing the include path to avoid the immediate symbol conflict only leads to more serious problems down the path (where you will find it's impossible to reuse these protos or simply can't build anything in a different language).

For CMake, @martijnotto Have you tried this:

protobuf_generate_cpp(PROTO_A_SRCS PROTO_A_HDRS a/query.proto)
protobuf_generate_cpp(PROTO_B_SRCS PROTO_B_HDRS b/query.proto)

[a4z]

Thanks for explaining the reasons @xfxyjwf , however, imho these are total different problems.
One is not generating include guards respecting the package name specified withing the proto file itself, let us see this in isolation.

The other problem, you describe, is a different issue and I can see no relation, we should not talk about this here, but just to show why I think it is something different:
what if you #include foo.h in your code and you have multiple include paths where each contain a foo.h, how can the compiler find the right one? This is not an issue. On the compiler it is -I, for protoc it is -IPATH, --proto_path=PATH.
Referring to your example protoc should pic up the first "query.proto" it finds.
Of course the user has to do/specify it correct, either by using the correct search path or by fully qualifying the import path.

However, what is the relation to generated include guards that contain the specified packages form within the proto file itself ? There is none!
That

package PackageName;
message Query {;  
  string what = 1;
}

should generate #ifndef PROTOBUF_PackageName_query_2eproto__INCLUDED , because that is what is specified within the protofile, there is no relation to what you describe.
I can see no breaking change in simply adding the user specified package name into the include guard.

Referring your question to cmake, @martijnotto has given you the information form the cmake hellp
https://cmake.org/cmake/help/v3.1/module/FindProtobuf.html

Note The PROTOBUF_GENERATE_CPP macro and add_executable() or add_library() calls only work properly within the same directory.

[xfxyjwf]

1. Using the same import path for multiple .proto files is an unsupported use case.
2. You tried to use .protos in this unsupported way and ran into macro conflicts.
3. You are proposing a change to workaround the macro conflicts so you can use .protos in an unsupported way.
4. We do not intend to allow using identical import path for multiple .proto files. Therefore we will not accept any change that makes protobuf more permissive on this.

[a4z]

Thanks, understood that its easier to change the cmake find protobuf than you to accept namespace_class include guard instead of just class include guard as all sane code does it. Except protoc generated code but this is of course a feature, not a bug ;-)