Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ProtoBuf dumper: add format specification into file #1365

Open
Christian-Stieber opened this issue Jun 12, 2024 · 1 comment
Open

ProtoBuf dumper: add format specification into file #1365

Christian-Stieber opened this issue Jun 12, 2024 · 1 comment

Comments

@Christian-Stieber
Copy link

What problem is this feature trying to solve?

Google protobuf compiler complains about the protobuf files not specifying their syntax-version, and there's no way to disable that warning. Thus builds always output stuff like

[libprotobuf WARNING C:\Users\Christian\Source\Repos\Christians-Steam-Bot\out\build\windows-x64-debug\vcpkg_installed\vcpkg\blds\protobuf\src\v3.21.12-fdb7676342.clean\src\google\protobuf\compiler\parser.cc:646] No syntax specified for the proto file: steammessages_client_objects.proto. Please use 'syntax = "proto2";' or 'syntax = "proto3";' to specify a syntax version. (Defaulted to proto2 syntax.)

This, of course, happens for every protobuf file that's used in a build.

How would you like it to be solved?

It would be nice if the protobuf dumper would just make these warnings go away by putting the suggested syntax=proto2 into each file.

This is assuming it's not causing problems with other protobuf parsers -- I'm actually somewhat concerned because of that, since it feels like a trivial change to make, but it hasn't been done...

Have you considered any alternative solutions

Sure. I have this workaround for the Linux builds:

stieber@gatekeeper:/Desktop/Source/Repos/Christians-Steam-Bot/Christians-Steam-Framework/steamdatabase $ cat protoc.sh
#! /bin/bash

/usr/bin/protoc "$@" 2>&1 | grep -v "libprotobuf WARNING .* No syntax specified for the proto file: " | grep -v "warning: Import .*\.proto is unused\."
exit 0

and hacked the CMakeLists accordingly to replace the detected protoc with the script.

But, in addition to this not working on Windows (haven't looked into options there), it really feels like a super-dirty workaround for a warning that should be fixed "at the source".

Additional Information

No response

@xPaw
Copy link
Member

xPaw commented Aug 31, 2024

It's not done because Valve's protobufs don't specify it, so it is not set in the file descriptor. There is code to dump the syntax version if it's specified:

if ( !string.IsNullOrEmpty( proto.syntax ) )
{
AppendHeadingSpace( sb, ref marker );
sb.AppendLine( $"syntax = {Util.ToLiteral( proto.syntax )};" );
marker = true;
}

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

No branches or pull requests

2 participants