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 1.40.0 #84990

Closed
wants to merge 6 commits into from
Closed

grpc 1.40.0 #84990

wants to merge 6 commits into from

Conversation

bevanjkay
Copy link
Member

Created with brew bump-formula-pr.

@BrewTestBot BrewTestBot added the bump-formula-pr PR was created using `brew bump-formula-pr` label Sep 10, 2021
@cho-m cho-m added the revision bumps needed Reverse dependencies need to have their revision incremented in the same PR label Sep 10, 2021
@carlocab carlocab added CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. and removed revision bumps needed Reverse dependencies need to have their revision incremented in the same PR labels Sep 11, 2021
@carlocab
Copy link
Member

sysdig fails to build on Linux with

/home/linuxbrew/.linuxbrew/bin/ld: ../libsinsp/libsinsp.a(cri.grpc.pb.cc.o): undefined reference to symbol '_ZN4absl12lts_202103245MutexD1Ev'
/home/linuxbrew/.linuxbrew/bin/ld: /home/linuxbrew/.linuxbrew/lib/libabsl_synchronization.so.2103.0.1: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status
userspace/sysdig/CMakeFiles/csysdig.dir/build.make:136: recipe for target 'userspace/sysdig/csysdig' failed
make[2]: *** [userspace/sysdig/csysdig] Error 1

CC @Homebrew/linux

@carlocab carlocab added build failure CI fails while building the software linux Linux is specifically affected labels Sep 11, 2021
@github-actions
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. To keep this pull request open, add a help wanted or in progress label.

@github-actions github-actions bot added the stale No recent activity label Sep 14, 2021
@chenrui333 chenrui333 added in progress Stale bot should stay away and removed stale No recent activity labels Sep 15, 2021
@iMichka
Copy link
Member

iMichka commented Sep 16, 2021

Might have to do with the linker flags. Maybe the order? The missing symbol comes from abseil:

[ 99%] Linking CXX executable sysdig
cd /tmp/sysdig-20210915-64838-1rj5rj8/sysdig-0.27.1/build/userspace/sysdig && /home/linuxbrew/.linuxbrew/Cellar/cmake/3.21.2/bin/cmake -E cmake_link_script CMakeFiles/sysdig.dir/link.txt --verbose=1
/home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/shims/linux/super/g++-5  -Wall -ggdb   -std=c++0x -O3 -fno-strict-aliasing -DNDEBUG -rdynamic CMakeFiles/sysdig.dir/fields_info.cpp.o CMakeFiles/sysdig.dir/sysdig.cpp.o -o sysdig  ../libsinsp/libsinsp.a ../libscap/libscap.a -lelf -lz -lcurl -ljsoncpp -ltbb -lcares -lgrpc++_unsecure -lgrpc_unsecure -lprotobuf -lcares -lgrpc++_unsecure -lgrpc_unsecure -lprotobuf -ljq -Wl,-Bstatic -lb64 -Wl,-Bdynamic -lrt -lanl -lssl -lcrypto -Wl,-Bstatic -lluajit -Wl,-Bdynamic -ldl -lpthread
[100%] Linking CXX executable csysdig
cd /tmp/sysdig-20210915-64838-1rj5rj8/sysdig-0.27.1/build/userspace/sysdig && /home/linuxbrew/.linuxbrew/Cellar/cmake/3.21.2/bin/cmake -E cmake_link_script CMakeFiles/csysdig.dir/link.txt --verbose=1
/home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/shims/linux/super/g++-5  -Wall -ggdb   -std=c++0x -O3 -fno-strict-aliasing -DNDEBUG -rdynamic CMakeFiles/csysdig.dir/fields_info.cpp.o CMakeFiles/csysdig.dir/csysdig.cpp.o -o csysdig  ../libsinsp/libsinsp.a -lncurses -lform ../libscap/libscap.a -lelf -lz -lcurl -ljsoncpp -ltbb -lcares -lgrpc++_unsecure -lgrpc_unsecure -lprotobuf -lcares -lgrpc++_unsecure -lgrpc_unsecure -lprotobuf -ljq -Wl,-Bstatic -lb64 -Wl,-Bdynamic -lrt -lanl -lssl -lcrypto -Wl,-Bstatic -lluajit -Wl,-Bdynamic -ldl -lpthread
/home/linuxbrew/.linuxbrew/bin/ld: ../libsinsp/libsinsp.a(cri.grpc.pb.cc.o): undefined reference to symbol '_ZN4absl12lts_202103245MutexD1Ev'
/home/linuxbrew/.linuxbrew/bin/ld: /home/linuxbrew/.linuxbrew/lib/libabsl_synchronization.so.2103.0.1: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status

@carlocab
Copy link
Member

Let's see if building with gcc helps.

@carlocab
Copy link
Member

carlocab commented Oct 13, 2021

bear is an issue in nlohmann-json: nlohmann/json#3070, fix in progress at nlohmann/json#3073

apache-arrow probably needs to be built with LLVM 12.

@carlocab
Copy link
Member

Building sysdig with GCC-11 did not help:

/home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/shims/linux/super/g++-11  -Wall -ggdb   -std=c++0x -O3 -fno-strict-aliasing -DNDEBUG -rdynamic CMakeFiles/csysdig.dir/fields_info.cpp.o CMakeFiles/csysdig.dir/csysdig.cpp.o -o csysdig  ../libsinsp/libsinsp.a -lncurses -lform ../libscap/libscap.a -lelf -lz -lcurl -ljsoncpp -ltbb -lcares -lgrpc++_unsecure -lgrpc_unsecure -lprotobuf -lcares -lgrpc++_unsecure -lgrpc_unsecure -lprotobuf -ljq -Wl,-Bstatic -lb64 -Wl,-Bdynamic -lrt -lanl -lssl -lcrypto -Wl,-Bstatic -lluajit -Wl,-Bdynamic -ldl -lpthread 
/home/linuxbrew/.linuxbrew/bin/ld: ../libsinsp/libsinsp.a(cri.grpc.pb.cc.o): undefined reference to symbol '_ZN4absl12lts_202103245MutexD1Ev'
/home/linuxbrew/.linuxbrew/bin/ld: /home/linuxbrew/.linuxbrew/lib/libabsl_synchronization.so.2103.0.1: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status
userspace/sysdig/CMakeFiles/csysdig.dir/build.make:136: recipe for target 'userspace/sysdig/csysdig' failed
make[2]: *** [userspace/sysdig/csysdig] Error 1

@danielnachun
Copy link
Member

It looks like this is really a problem with abseil (and one we've seen before) where it can't find its own libraries. It's possibly related to the fact that the abseil libraries use $ORIGIN/../lib:/home/linuxbrew/.linuxbrew/Cellar/abseil/20210324.2/lib:/home/linuxbrew/.linuxbrew/lib as their RPATH, and probably need /home/linuxbrew/.linuxbrew/opt/abseil/lib as well.

@carlocab
Copy link
Member

It's possibly related to the fact that the abseil libraries use $ORIGIN/../lib:/home/linuxbrew/.linuxbrew/Cellar/abseil/20210324.2/lib:/home/linuxbrew/.linuxbrew/lib as their RPATH, and probably need /home/linuxbrew/.linuxbrew/opt/abseil/lib as well.

Why does it need the opt path too? Don't

/home/linuxbrew/.linuxbrew/opt/abseil/lib

and

/home/linuxbrew/.linuxbrew/Cellar/abseil/20210324.2/lib

point to the same place?

@carlocab
Copy link
Member

bear build failure should be fixed in #87353.

@carlocab
Copy link
Member

carlocab commented Oct 16, 2021

It looks like this is really a problem with abseil (and one we've seen before) where it can't find its own libraries. It's possibly related to the fact that the abseil libraries use $ORIGIN/../lib:/home/linuxbrew/.linuxbrew/Cellar/abseil/20210324.2/lib:/home/linuxbrew/.linuxbrew/lib as their RPATH, and probably need /home/linuxbrew/.linuxbrew/opt/abseil/lib as well.

Also, sysdig doesn't have an abseil dependency... Does it?

Edit: Oh, wait, missed the Linux-only deps.

@carlocab carlocab force-pushed the bump-grpc-1.40.0 branch 2 times, most recently from 14d650c to 28944bf Compare October 16, 2021 17:27
bevanjkay and others added 5 commits October 17, 2021 17:31
Also make `llvm` a test dependency on Mojave because of a `brew` bug.
Also, switch to LLVM 12, as this fails to build with LLVM 13.
@carlocab carlocab force-pushed the bump-grpc-1.40.0 branch 2 times, most recently from ba2799d to b0a2630 Compare October 17, 2021 09:36
@carlocab carlocab force-pushed the bump-grpc-1.40.0 branch 2 times, most recently from a9453b9 to f591c72 Compare October 17, 2021 13:57
Also, deparallelise the build on Linux and make dependencies explicit,
and compile using C++17 in order to use `abseil`.
@carlocab
Copy link
Member

sysdig has had 15 downloads on Linux in the past year. grpc has ~8,000 monthly installs on macOS. I don't think we can justify further delaying shipping an update to that many users to avoid breaking so few users. I'm going to merge this, possibly after making sysdig depends_on :macos.

Objections, @Homebrew/core?

@MikeMcQuaid
Copy link
Member

@carlocab :shipit:

@danielnachun
Copy link
Member

Can we just make sure to make a comment as to why it's disabled on Linux? Disabling a formula on Linux because it doesn't build is different from disabling it because it is fundamentally incompatible.

As an aside perhaps we will eventually need some separate DSL for "skip Linux because it doesn't build" versus "skip Linux because it is incompatible".

@cho-m
Copy link
Member

cho-m commented Oct 18, 2021

gRPC 1.41.0 has been available if we want to jump to that, or we can finish this and then open new PR: https://github.com/grpc/grpc/releases/tag/v1.41.0

Sysdig upstream issue: draios/sysdig#1778.

EDIT:

@danielnachun
Copy link
Member

Maybe try ENV.append "LDFLAGS", "-labsl_synchronization"? Sorry I should have suggested this earlier.

@cho-m
Copy link
Member

cho-m commented Oct 18, 2021

@danielnachun ArchLinux does something similar by patching CMake build scripts. Since they successfully updated to grpc 1.41.0 without breaking sysdig, we can borrow their fix and add comment for upstream issue (draios/sysdig#1778):

https://github.com/archlinux/svntogit-community/blob/packages/sysdig/trunk/0.27.1-grpc-absl-sync.patch

diff -rup sysdig-0.27.1/CMakeLists.txt sysdig-0.27.1-grpc-1.41/CMakeLists.txt
--- sysdig-0.27.1/CMakeLists.txt	2020-09-30 16:21:52.000000000 +0200
+++ sysdig-0.27.1-grpc-1.41/CMakeLists.txt	2021-10-08 23:46:51.058659662 +0200
@@ -552,6 +552,7 @@ if(NOT WIN32 AND NOT APPLE)
 			else()
 				message(FATAL_ERROR "Couldn't find system grpc")
 			endif()
+			find_library(ABSL_SYNC_LIB NAMES absl_synchronization)
 			find_program(GRPC_CPP_PLUGIN grpc_cpp_plugin)
 			if(NOT GRPC_CPP_PLUGIN)
 				message(FATAL_ERROR "System grpc_cpp_plugin not found")
diff -rup sysdig-0.27.1/userspace/libsinsp/CMakeLists.txt sysdig-0.27.1-grpc-1.41/userspace/libsinsp/CMakeLists.txt
--- sysdig-0.27.1/userspace/libsinsp/CMakeLists.txt	2020-09-30 16:21:52.000000000 +0200
+++ sysdig-0.27.1-grpc-1.41/userspace/libsinsp/CMakeLists.txt	2021-10-08 23:46:35.785353019 +0200
@@ -209,6 +209,7 @@ if(NOT WIN32)
 			target_link_libraries(sinsp
 				"${GRPCPP_LIB}"
 				"${GRPC_LIB}"
+				"${ABSL_SYNC_LIB}"
 				"${PROTOBUF_LIB}"
 				"${CARES_LIB}"
 				"${JQ_LIB}"

EDIT: It is also fine to merge this PR, and do follow-up PRs for fixing sysdig and updating to grpc 1.41.0

Copy link
Member

@carlocab carlocab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging this so that users can brew extract --version=1.40.0 grpc if they need to, plus there might be additional issues with 1.41.0.

@BrewTestBot
Copy link
Member

:shipit: @carlocab has triggered a merge.

@carlocab carlocab mentioned this pull request Oct 20, 2021
6 tasks
@github-actions github-actions bot added the outdated PR was locked due to age label Nov 18, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
build failure CI fails while building the software bump-formula-pr PR was created using `brew bump-formula-pr` CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. in progress Stale bot should stay away linux Linux is specifically affected outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants