Add support for custom "protoc" path and scan using "which" on Linux #2844

Merged
merged 1 commit into from Feb 13, 2017

Projects

None yet

2 participants

@davidebeatrici
Member

On Linux the protobuf compiler binary's name is usually protoc, except when using a MinGW one to cross-compile for Windows.
The binary in the MXE Debian package is called x86_64-w64-mingw32.static-protoc, x86_64-w64-mingw32.shared-protoc, i686-w64-mingw32.static-protoc or i686-w64-mingw32.shared-protoc, depending on the chosen target.

@mkrautz
Member
mkrautz commented Feb 13, 2017

@davidebeatrici Try adding

MULTIARCH_TRIPLE = $$system($${QMAKE_CC} -print-multiarch 2>/dev/null)
message($$MULTIARCH_TRIPLE)

to protobuf.pri and see if that gives you "x86_64-w64-mingw32".
If so, then I would suggest you use that hint for finding the binary.

Also, I believe the search order should be:

  • $$MULTIARCH_TRIPLE.static-protoc
  • $$MULTIARCH_TRIPLE-protoc
  • protoc
@mkrautz
Member
mkrautz commented Feb 13, 2017

Also, please call the file protoc.pri instead of protobuf.pri

@mkrautz

Please address my comments :-)

@mkrautz
Member
mkrautz commented Feb 13, 2017

Also, if you add the MULTIARCH_TRIPLE stuff, you don't need to special-case win32-g++ -- you should be able to do use that trick for all platforms. Unsupported compilers will simply give you an empty MULTIARCH_TRIPLE, in which case the .pri file should fall back to simply using "protoc".

src/murmur/murmur_grpc/murmur_grpc.pro
@@ -28,7 +29,7 @@ grpch.CONFIG *= no_link explicit_dependencies target_predeps
grpc.output = ${QMAKE_FILE_BASE}.grpc.pb.cc
grpc.depends = ${QMAKE_FILE_BASE}.pb.h
-grpc.commands = protoc --grpc_out=. --plugin=protoc-gen-grpc=$$system(which grpc_cpp_plugin) -I. -I.. ${QMAKE_FILE_NAME}
+grpc.commands = $${PROTOC} --grpc_out=. --plugin=$$PROTOC-gen-grpc=$$system(which grpc_cpp_plugin) -I. -I.. ${QMAKE_FILE_NAME}
@mkrautz
mkrautz Feb 13, 2017 Member

Abort! Search and replace gone wrong! --plugin=$$PROTOC-gen-grpc= does not seem like a valid parameter name :-)

+ }
+ }
+ }
+}
@mkrautz
mkrautz Feb 13, 2017 Member

Still missing a newline at the end of the file.

protoc.pri
+ message("The 'which' command is not available on the system. Unable to search for protoc installations. Assuming 'protoc'... If this is not correct, please point the MUMBLE_PROTOC environment variable at a working protoc binary.")
+ PROTOC=protoc
+ } else {
+ MULTIARCH_TRIPLE = $$system($${QMAKE_CC} -print-multiarch 2>/dev/null)
@mkrautz
mkrautz Feb 13, 2017 edited Member

Please wrap the following three test cases in

!isEmpty(MULTIARCH_TRIPLE) {
...
}

there is no reason to check them, if MULTIARCH_TRIPLE is not set.

src/mumble_proto/mumble_proto.pro
@@ -4,6 +4,7 @@
# Mumble source tree or at <https://www.mumble.info/LICENSE>.
include(../../compiler.pri)
+include(../../protobuf.pri)
@mkrautz
mkrautz Feb 13, 2017 Member

include(../../protoc.pri)

src/murmur/murmur.pro
@@ -4,6 +4,7 @@
# Mumble source tree or at <https://www.mumble.info/LICENSE>.
include(../mumble.pri)
+include(../../protobuf.pri)
@mkrautz
mkrautz Feb 13, 2017 Member

include(../../protoc.pri)

src/murmur/murmur_grpc/murmur_grpc.pro
@@ -4,6 +4,7 @@
# Mumble source tree or at <https://www.mumble.info/LICENSE>.
include(../../../compiler.pri)
+include(../../../protobuf.pri)
@mkrautz
mkrautz Feb 13, 2017 Member

include(../../../protoc.pri)

@mkrautz

Please fix the issues I've highlighted. Thanks!

protoc.pri
+ message("The 'which' command is not available on the system. Unable to search for protoc installations. Assuming 'protoc'... If this is not correct, please point the MUMBLE_PROTOC environment variable at a working protoc binary.")
+ PROTOC=protoc
+ } else {
+ MULTIARCH_TRIPLE = $$system($${QMAKE_CC} -print-multiarch 2>/dev/null)
@mkrautz
mkrautz Feb 13, 2017 Member

For MinGW, you should use

MULTIARCH_TRIPLE = $$system($${QMAKE_CC} -dumpmachine 2>/dev/null)
@davidebeatrici davidebeatrici Add support for custom "protoc" path and scan using "which" on Linux
c0b6750
@mkrautz mkrautz merged commit e0b384b into mumble-voip:master Feb 13, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment