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

grpc fails to build #4508

Closed
Lartza opened this issue Sep 27, 2020 · 12 comments · Fixed by #4509
Closed

grpc fails to build #4508

Lartza opened this issue Sep 27, 2020 · 12 comments · Fixed by #4509
Labels
build Everything related to compiling/building the code

Comments

@Lartza
Copy link
Contributor

Lartza commented Sep 27, 2020

Describe the bug
Building with grpc fails with linking errors

[ 97%] Linking CXX executable ../../murmurd
/usr/bin/ld: CMakeFiles/murmur.dir/MurmurGRPCImpl.cpp.o: undefined reference to symbol 'gpr_log'
/usr/bin/ld: /usr/lib/libgpr.so.10: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status
make[2]: *** [src/murmur/CMakeFiles/murmur.dir/build.make:505: murmurd] Error 1
make[2]: Leaving directory '/home/lartza/mumble-build'
make[1]: *** [CMakeFiles/Makefile2:239: src/murmur/CMakeFiles/murmur.dir/all] Error 2
make[1]: Leaving directory '/home/lartza/mumble-build'
make: *** [Makefile:149: all] Error 2
make: Leaving directory '/home/lartza/mumble-build'

Steps to Reproduce
Steps to reproduce the behavior:

  1. git clone https://github.com/mumble-voip/mumble.git
  2. cmake -B mumble-build -S mumble -Dclient:BOOL='OFF' -Dgrpc:BOOL='ON'
  3. make -C mumble-build

Expected behavior
Murmur builds successfully

Additional context
Arch Linux, grpc 1.30.2

@Krzmbrzl
Copy link
Member

I can't reproduce this on my Linux machine (KDE Neon 5.19.4 - based on Ubuntu 20.04).

I am building against libgrpc++-dev 1.16.1-1ubuntu5, which I strongly assume to be grpc 1.16.1

However according to gRPC's README we have to link against it in a different way... 👀

@Krzmbrzl
Copy link
Member

The way to link against gRPC seems to have changed with v1.27: grpc/grpc@48fa247#diff-d4f971a0f0d8b1c128417b6a51c7f6cc

The commit message indicates though that this behavior should have been used before as well but only since 1.28 it seems to be a problem 🤔

@Krzmbrzl Krzmbrzl added the build Everything related to compiling/building the code label Sep 28, 2020
Krzmbrzl added a commit to Krzmbrzl/mumble that referenced this issue Sep 28, 2020
There seem to be issues with newer versions of gRPC with the way we used
to link against it. Therefore this commit adapts the link behavior to
how it is suggested in the gRPC's README.

Fixes mumble-voip#4508
@Krzmbrzl
Copy link
Member

@Lartza could you try to compile using the changes from #4509 please?

Krzmbrzl added a commit to Krzmbrzl/mumble that referenced this issue Sep 28, 2020
There seem to be issues with newer versions of gRPC with the way we used
to link against it. Therefore this commit adapts the link behavior to
how it is suggested in the gRPC's README.

Fixes mumble-voip#4508
@radioactiveman
Copy link
Contributor

@Lartza: Please try the patch below, I assume it will fix this issue.

diff --git a/src/murmur/CMakeLists.txt b/src/murmur/CMakeLists.txt
index 4401bbb5a..babf967d8 100644
--- a/src/murmur/CMakeLists.txt
+++ b/src/murmur/CMakeLists.txt
@@ -190,7 +190,7 @@ if(dbus AND NOT WIN32 AND NOT APPLE)
 endif()
 
 if(grpc)
-	find_pkg(GRPC REQUIRED)
+	find_pkg(gRPC REQUIRED)
 
 	protobuf_generate(LANGUAGE cpp TARGET murmur PROTOS ${GRPC_FILE} OUT_VAR BUILT_GRPC_FILES)
 

@Lartza
Copy link
Contributor Author

Lartza commented Oct 15, 2020

@Lartza: Please try the patch below, I assume it will fix this issue.

diff --git a/src/murmur/CMakeLists.txt b/src/murmur/CMakeLists.txt
index 4401bbb5a..babf967d8 100644
--- a/src/murmur/CMakeLists.txt
+++ b/src/murmur/CMakeLists.txt
@@ -190,7 +190,7 @@ if(dbus AND NOT WIN32 AND NOT APPLE)
 endif()
 
 if(grpc)
-	find_pkg(GRPC REQUIRED)
+	find_pkg(gRPC REQUIRED)
 
 	protobuf_generate(LANGUAGE cpp TARGET murmur PROTOS ${GRPC_FILE} OUT_VAR BUILT_GRPC_FILES)
 

For me at least this does indeed fix the issue.

@Krzmbrzl
Copy link
Member

That's very interesting. But I guess the solution then is to simply pass multiple name variants to find_pkg in order to cover all cases 👍

@Lartza
Copy link
Contributor Author

Lartza commented Oct 15, 2020

I did notice with GRPC it says "GRPC found" and gRPC it says "gRPC found | Version: 1.30.2"

@Krzmbrzl
Copy link
Member

On my system if I use GRPC, it emits the same message as you encountered but if I use gRPC it can't find it 👀

Maybe this is due to the different versions.

@Lartza do you happen to have another version of gRPC installed on your system?

@Lartza
Copy link
Contributor Author

Lartza commented Oct 15, 2020

Don't believe I do, "grpc" is the only distro package installed and this is everything relevant mlocate finds https://gist.github.com/Lartza/d59c1c360898a03c7e9f4f7a1b769436

@radioactiveman
Copy link
Contributor

radioactiveman commented Oct 15, 2020

According to the gRPC Hello world example "gRPC" should be the correct spelling, for both versions.

1.16.1: https://github.com/grpc/grpc/blob/v1.16.1/examples/cpp/helloworld/CMakeLists.txt#L73
1.30.2: https://github.com/grpc/grpc/blob/v1.30.2/examples/cpp/helloworld/CMakeLists.txt#L116

@Krzmbrzl: Please check if the patch below works on your system. Feel free to add it to #4509 if you give credits in the commit message.

diff --git a/src/murmur/CMakeLists.txt b/src/murmur/CMakeLists.txt
index 4401bbb5..139e46eb 100644
--- a/src/murmur/CMakeLists.txt
+++ b/src/murmur/CMakeLists.txt
@@ -190,7 +190,7 @@ if(dbus AND NOT WIN32 AND NOT APPLE)
 endif()
 
 if(grpc)
-	find_pkg(GRPC REQUIRED)
+	find_pkg("gRPC;GRPC" REQUIRED)
 
 	protobuf_generate(LANGUAGE cpp TARGET murmur PROTOS ${GRPC_FILE} OUT_VAR BUILT_GRPC_FILES)
 
@@ -228,6 +228,11 @@ if(grpc)
 		VERBATIM
 	)
 
+	# Disable warnings for the generated source files
+	foreach(CURRENT_FILE IN LISTS GRPC_GENERATED_FILES)
+		set_source_files_properties("${CURRENT_FILE}" PROPERTIES COMPILE_FLAGS "-w")
+	endforeach()
+
 	add_custom_target(generate-grpc-files
 		DEPENDS
 			"MurmurRPC.proto.Wrapper.cpp"

@davidebeatrici
Copy link
Member

https://github.com/mumble-voip/mumble/blob/master/cmake/FindModules/FindGRPC.cmake was added because some packages currently don't provide the CMake module.

Debian: https://packages.debian.org/bullseye/amd64/libgrpc-dev/filelist
Ubuntu: https://packages.ubuntu.com/groovy/amd64/libgrpc-dev/filelist

However, the module we have in the repository appears to cause issues on certain distributions.

@radioactiveman I confirm your patch works on Debian.

@Krzmbrzl
Copy link
Member

@radioactiveman yep this also works on my system - thank you very much!

https://github.com/mumble-voip/mumble/blob/master/cmake/FindModules/FindGRPC.cmake was added because some packages currently don't provide the CMake module.

Ah yes - that was why we use GRPC. I found that out at some point but forgot again 🤦

I will update the respective PR accordingly 👍

Krzmbrzl added a commit to Krzmbrzl/mumble that referenced this issue Oct 16, 2020
There seem to be issues with newer versions of gRPC with the way we used
to link against it. Therefore this commit adapts the link behavior to
how it is suggested in the gRPC's README.

Fixes mumble-voip#4508
Krzmbrzl added a commit to Krzmbrzl/mumble that referenced this issue Oct 17, 2020
There seem to be issues with newer versions of gRPC with the way we used
to link against it. Therefore this commit adapts the link behavior to
how it is suggested in the gRPC's README.

Fixes mumble-voip#4508
Krzmbrzl added a commit to Krzmbrzl/mumble that referenced this issue Oct 24, 2020
We are using a custom FindGRPC.cmake script in order to locate the gRPC
targets as not all distribution variants of gRPC include the respective
cmake support. Those that do however will use "gRPC" (note the lowercase
"g") as the target's name and therefore our current way of searching for
it via find_pkg won't find them.

The solution is to first look for "gRPC" and only if that fails, check
for "GRPC". This way the official target "gRPC" should be found and
preferred if present.

Fixes mumble-voip#4508

Co-Authored-By: Thomas Lange <thomas-lange2@gmx.de>
Krzmbrzl added a commit that referenced this issue Oct 24, 2020
We are using a custom FindGRPC.cmake script in order to locate the gRPC
targets as not all distribution variants of gRPC include the respective
cmake support. Those that do however will use "gRPC" (note the lowercase
"g") as the target's name and therefore our current way of searching for
it via find_pkg won't find them.

The solution is to first look for "gRPC" and only if that fails, check
for "GRPC". This way the official target "gRPC" should be found and
preferred if present.

Fixes #4508
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Everything related to compiling/building the code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants