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

[package] abseil/*: consider pinning the ABI at install time #7249

Closed
coryan opened this issue Sep 11, 2021 · 6 comments · Fixed by #7443
Closed

[package] abseil/*: consider pinning the ABI at install time #7249

coryan opened this issue Sep 11, 2021 · 6 comments · Fixed by #7443
Labels
bug Something isn't working

Comments

@coryan
Copy link
Contributor

coryan commented Sep 11, 2021

Package and Environment Details (include every applicable attribute)

  • Package Name/Version: abseil/all versions
  • Operating System+version: any
  • Compiler+version: any

Steps to reproduce (Include if Applicable)

Try to use Abseil/* with C++17 enabled. A good example can be found in #5766 .

By default, Abseil assumes that the whole application (including Abseil) is compiled with the same compiler and flags. A good discussion can be found in abseil/abseil-cpp#696. If Abseil is compiled with C++11 and used from an application compiled in C++17 this does not work, as the ABI (and arguably the API) changes depending on the C++ flags in effect when the Abseil headers are used.

However, it is possible to "pin" Abseil's API to whatever was used when it was compiled and installed.

Specifically one would need to set the pre-processor macros in this file to 0 if compiled with C++ < 17, and to 2 if compiled with C++ >= 17:

https://github.com/abseil/abseil-cpp/blob/master/absl/base/options.h

We use this to compile Abseil with C++11 and then use it from both C++11 and C++17:

https://github.com/googleapis/google-cloud-cpp/blob/7ee1d91d4fb6915dc50617c01ebc04777e183ebd/ci/cloudbuild/dockerfiles/fedora-34.Dockerfile#L60

@coryan coryan added the bug Something isn't working label Sep 11, 2021
@coryan
Copy link
Contributor Author

coryan commented Sep 11, 2021

/FYI: @devjgm

@coryan
Copy link
Contributor Author

coryan commented Sep 23, 2021

I will be sending a PR for this.

@coryan
Copy link
Contributor Author

coryan commented Sep 27, 2021

@uilianries I can try to fix this bug, but first I would like to check if this is a bad idea.

My proposal would be to pin the ABI of the Abseil package to the ABI used when it was compiled. One does this by setting these defines:

https://github.com/abseil/abseil-cpp/blob/1ce4ceca2b2931bc4d7e470228c2dbb2f3dfea0f/absl/base/options.h#L130
https://github.com/abseil/abseil-cpp/blob/1ce4ceca2b2931bc4d7e470228c2dbb2f3dfea0f/absl/base/options.h#L157
https://github.com/abseil/abseil-cpp/blob/1ce4ceca2b2931bc4d7e470228c2dbb2f3dfea0f/absl/base/options.h#L183

The values must be 0 if Abseil was compiled with C++ < 17. The values must be 1 if Abseil was compiled with C++ >= 17, and then one can only use the installed library with C++ >= 17.

It seems to me one could change the options.h header in the package() step. Does that sound feasible?

@uilianries
Copy link
Member

@coryan Thank you for checking a possible workaround. Indeed you suggestion seems be enough for our case. Instead of copying a new file, is it possible only replacing that value in file before building, like a patch?

@coryan
Copy link
Contributor Author

coryan commented Sep 27, 2021

Yes, I believe that is possible. I will try to create a PR based on this discussion, will ping you once I submit it and you can tell me what I did wrong 😀

@uilianries
Copy link
Member

Thank you so much! I'm present to all PRs (notified by email), but feel free to ping me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants