SteamRE / SteamKit

SteamKit2 is a .NET library designed to interoperate with Valve's Steam network. It aims to provide a simple, yet extensible, interface to perform various actions on the network.
GNU Lesser General Public License v2.1
2.63k stars 497 forks source link

ProtoBuf dumper: add format specification into file #1365

Open Christian-Stieber opened 5 months ago

Christian-Stieber commented 5 months ago

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 commented 3 months ago

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:

https://github.com/SteamRE/SteamKit/blob/c7ec81a00c3ab28ba396058beca6509881d701f5/Resources/ProtobufDumper/ProtobufDumper/ProtobufDumper.cs#L292-L297