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: debundle abseil #11758

Merged
merged 3 commits into from
Jun 4, 2022
Merged

grpc: debundle abseil #11758

merged 3 commits into from
Jun 4, 2022

Conversation

kmilos
Copy link
Contributor

@kmilos kmilos commented Jun 1, 2022

Resolves #11562

@kmilos kmilos marked this pull request as draft June 1, 2022 08:28
@kmilos
Copy link
Contributor Author

kmilos commented Jun 1, 2022

Converting to draft, as there are indeed linking errors to abseil-cpp:

FAILED: libgpr.dll libgpr.dll.a 
  cmd.exe /C "cd . && D:\M\msys64\ucrt64\bin\g++.exe -march=x86-64 -mtune=generic -O2 -pipe -DSTRSAFE_NO_DEPRECATE  -O3 -DNDEBUG  -pipe -Wl,--disable-dynamicbase,--default-image-base-low -shared -o libgpr.dll -Wl,--out-implib,libgpr.dll.a -Wl,--major-image-version,23,--minor-image-version,0 @CMakeFiles\gpr.rsp  && cd ."
  D:/M/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/12.1.0/../../../../x86_64-w64-mingw32/bin/ld.exe: CMakeFiles/gpr.dir/src/core/lib/gprpp/host_port.cc.obj:host_port.cc:(.text+0xb0): undefined reference to `absl::lts_20211102::string_view::rfind(char, unsigned long long) const'
  D:/M/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/12.1.0/../../../../x86_64-w64-mingw32/bin/ld.exe: CMakeFiles/gpr.dir/src/core/lib/gprpp/host_port.cc.obj:host_port.cc:(.text+0x150): undefined reference to `absl::lts_20211102::string_view::find(char, unsigned long long) const'
  D:/M/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/12.1.0/../../../../x86_64-w64-mingw32/bin/ld.exe: CMakeFiles/gpr.dir/src/core/lib/gprpp/host_port.cc.obj:host_port.cc:(.text+0x198): undefined reference to `absl::lts_20211102::string_view::find(char, unsigned long long) const'
  D:/M/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/12.1.0/../../../../x86_64-w64-mingw32/bin/ld.exe: CMakeFiles/gpr.dir/src/core/lib/gprpp/host_port.cc.obj:host_port.cc:(.text+0x1f9): undefined reference to `absl::lts_20211102::string_view::find(char, unsigned long long) const'
  D:/M/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/12.1.0/../../../../x86_64-w64-mingw32/bin/ld.exe: CMakeFiles/gpr.dir/src/core/lib/gprpp/host_port.cc.obj:host_port.cc:(.text+0x279): undefined reference to `absl::lts_20211102::string_view::find(char, unsigned long long) const'
  D:/M/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/12.1.0/../../../../x86_64-w64-mingw32/bin/ld.exe: CMakeFiles/gpr.dir/src/core/lib/gprpp/host_port.cc.obj:host_port.cc:(.text+0x338): undefined reference to `absl::lts_20211102::string_view::find(char, unsigned long long) const'
  D:/M/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/12.1.0/../../../../x86_64-w64-mingw32/bin/ld.exe: CMakeFiles/gpr.dir/src/core/lib/gprpp/host_port.cc.obj:host_port.cc:(.text+0x4b8): more undefined references to `absl::lts_20211102::string_view::find(char, unsigned long long) const' follow

and many more...

@kmilos
Copy link
Contributor Author

kmilos commented Jun 1, 2022

These are indeed not exported from ucrt64/bin/libabsl_strings.dll...

Maybe because we built abseil-cpp w/ C++17 and std versions are used instead? Here's a hint from /ucrt64/include/absl/options.h

// ABSL_OPTION_USE_STD_STRING_VIEW
//
// This option controls whether absl::string_view is implemented as an alias to
// std::string_view, or as an independent implementation.
//
// A value of 0 means to use Abseil's implementation.  This requires only C++11
// support, and is expected to work on every toolchain we support.
//
// A value of 1 means to use an alias to std::string_view.  This requires that
// all code using Abseil is built in C++17 mode or later.
//
// A value of 2 means to detect the C++ version being used to compile Abseil,
// and use an alias only if a working std::string_view is available.  This
// option is useful when you are building your program from source.  It should
// not be used otherwise -- for example, if you are distributing Abseil in a
// binary package manager -- since in mode 2, absl::string_view will name a
// different type, with a different mangled name and binary layout, depending on
// the compiler flags passed by the end user.  For more info, see
// https://abseil.io/about/design/dropin-types.
//
// User code should not inspect this macro.  To check in the preprocessor if
// absl::string_view is a typedef of std::string_view, use the feature macro
// ABSL_USES_STD_STRING_VIEW.

#define ABSL_OPTION_USE_STD_STRING_VIEW 2

The other linking errors are probably related...

@kmilos kmilos marked this pull request as ready for review June 1, 2022 10:34
@coryan
Copy link

coryan commented Jun 1, 2022

FWIW, abseil/abseil-cpp#696 may be a good read as it suggests possible workarounds. Note that while -DABSL_PROPAGATE_CXX_STD=ON sounds promising, it only requires C++11. Since this PR always compiles Abseil with C++17, that suggests include/absl/options.h should be changed with something like:

sed -i 's/^#define ABSL_OPTION_USE_\(.*\) 2/#define ABSL_OPTION_USE_\1 1/' "absl/base/options.h"

@kmilos
Copy link
Contributor Author

kmilos commented Jun 1, 2022

@coryan
Copy link

coryan commented Jun 1, 2022

@coryan I have actually read it. AFAICT the conclusion was that you'd need to build any clients of Abseil w/ the same configuration without fiddling w/ the source.

That works, it just fails with fairly incomprehensible error messages when folks do not follow the rules. Consider an application developer using these packages, and trying to use it with code compiled with -std=c++11. Their code will compile, and then fail to link. With the suggested change the code will fail to compile.

In other words, I am not saying that your current PR is wrong, it is a big improvement over the current state. I am saying it could provide less confusing errors when people do the "Wrong Thing":tm:

@kmilos
Copy link
Contributor Author

kmilos commented Jun 1, 2022

Ok, let's make it somewhat *-proof then, hope this looks good now.

Copy link

@coryan coryan left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

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

Successfully merging this pull request may close these issues.

Debundle abseil-cpp from gprc
3 participants