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

Add static builds per cxx_version #35

Merged
merged 5 commits into from
Aug 9, 2022
Merged

Conversation

h-vetinari
Copy link
Member

@h-vetinari h-vetinari commented May 25, 2022

Differently from almost any other library, abseil's ABI depends on the C++ standard version used to compile it, and abseil specifically only supports compiling all dependencies with a given standard version.

This is obviously infeasible for an ecosystem like conda-forge, where different packages have different needs (some cannot yet be compiled with e.g. C++17, some already require that).

This makes the general case - a dynamic library - infeasible, because that dynamic library cannot coexist in the same environment with separate ABIs. Since abseil 20211102, we are on C++17 for the dynamic builds, and some libraries (like tensorflow) had issues with this. An experiment with switching windows builds to a higher C++ standard also failed, as e.g. arrow still needs C++11.

In order to cater to different needs (while keeping dynamic libs as a default on unix), this PR add static libraries for all available C++ standards. This way, a project can specify the C++ ABI it needs from abseil. This can even be done without opting out of the migrator, e.g. by:

requirements:
  host:
    - libabseil_static             # unpinned to catch migrator
    - libabseil_static =*=_cxx11   # but pin build string to e.g. C++11

This is compatible with the CFEP 18 caveat for exceptions

Building static archives (in separated packages) is still useful for some non-common workflows. These don't represent the majority of uses of our packages but can profit a lot from our using our existing infrastructure.

Abseil is such an exception (even though we will continue to prefer the dynamic libs for those who can live with C++17).

Closes #29

FAQ

Edit: if you've come here to check out what happened or how to use it, please note the following:

  • If your package uses abseil, please update the name of the package in your meta.yaml, after the migrator for abseil 20220623.0 comes past.
  • The general rule/guidance of using shared libs remains; only fall back to using the static builds if absolutely necessary.
  • This ABI of libabseil depends on the version of the C++ standard used to compile it, and crucially, this needs to match the C++ standard versions used to compile consuming packages (e.g. a consuming feedstock).
  • If such a feedstock is not yet ready to compile with C++17 (the version used to compile the shared libabseil builds), you can fall back to a static builds for the right C++ version, e.g. libabseil-static =*=cxx14* for C++14. Don't forget to add a second unpinned line of libabseil-static so future migrators will update the pin nevertheless.
    • Also, if you use the static builds, be aware that you need to bundle the abseil license (Apache 2) into the feedstock.
  • Be careful that using something like CMake's target_link_libraries(<target> PUBLIC absl::something) might need to be patched out for packages that only use abseil internally, as the static library has no run-export and will therefore not be present in the final environment.
  • If a package has a runtime-dependence on abseil (for example because abseil types are part of the API of said package), you should not use static builds, but rather upgrade to C++17 and use the shared builds.
  • If you have questions, please reach out to @conda-forge/abseil-cpp.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@h-vetinari
Copy link
Member Author

@wolfv
I realise what I'm trying to do here is quite a hack, but it's the only way I can see for building the same output with different build-strings.

The problem is that I want to select one of these as a wrapper package, but while the initial solve works out until

BUILD START: ['libabseil_dynamic-20211102.0-cxx17_h48a1fff_2.tar.bz2', 'libabseil-20211102.0-h93e1e8c_2.tar.bz2', 'abseil-cpp-20211102.0-ha770c72_2.tar.bz2']

it then immediately fails to find one of those packages.

Reloading output folder: /home/conda/feedstock_root/build_artifacts
Mamba failed to solve:
 - libabseil 20211102.0.* h93e1e8c_2

with channels:

The reported errors are:
- Encountered problems while solving:
-   - nothing provides requested libabseil 20211102.0.* h93e1e8c_2

Any insight into where mamba is failing here?

@h-vetinari
Copy link
Member Author

h-vetinari commented Jul 26, 2022

@conda-forge/core, I'd like to have some more eyes on this to have people agree to this exception to CFEP-18:

More details in #29, but in short:

  • the ABI of abseil depends on the C++ standard version used to compile (and upstream only supports the scenario where an entire stack is compiled against the same version).
  • we cannot enforce a uniform C++ standard across conda-forge; some packages still need C++11, some already need C++17.
  • it's therefore impossible to do everything with a shared lib, because we cannot have the same version of abseil several times (per respective C++ std ABI) as a runtime dependence.

This PR proposes to :

  • stay with shared libs on unix, compiled with C++17 (upstream doesn't support the full library on windows as a shared build).
  • keep using the shared lib for all feedstocks capable of using C++17.
  • provide an escape hatch in the form of static libs that feedstocks stuck on older C++ standards can use to compile against.

I'm also using the chance of such an overhaul to align this feedstock with the discussion from conda-forge/conda-forge.github.io#1073. Happy to take that out if people are opposed to that.

@h-vetinari
Copy link
Member Author

To underscore the need for this: current gRPC versions need C++14 and fail in CI when hacked to use C++11. OTOH arrow still needs C++11 on windows. This provides a way out for feedstocks to progress at their respective pace.

To demonstrate my point, I've pushed the current state of this PR into the abseil_dev label (modulo a rename _dynamic -> _shared that's missing in the dev branch) and it passes the gRPC CI in conda-forge/grpc-cpp-feedstock#196.

Copy link

@ngam ngam left a comment

Choose a reason for hiding this comment

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

Vote of confidence, not that I know anything about Windows, but I'd like to see this go through!

recipe/meta.yaml Outdated Show resolved Hide resolved
recipe/meta.yaml Outdated Show resolved Hide resolved
@h-vetinari
Copy link
Member Author

h-vetinari commented Aug 4, 2022

TL;DR: Planning to merge this next Monday EOD AOE

@conda-forge/core
This was discussed in the dev meeting last week (minutes). I had suggested to leave 1-2 weeks for comment, and now a bit over one week has passed.

I've got the reinstated migrator ready to go as well (at least I think so; happy for review).

Please comment if you have any remaining questions or objections.

PS. One thought I kept thinking about was that despite fully shared builds not being supported upstream, it would still be good to keep building a shared(-as-much-as-possible) build on windows. I've added that in the last two commits (not counting the rerender) the changes in meta.yaml are now squashed into 796c60c; though 01dd753 is pretty much identical to before.

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.

Compile (static) abseil for different C++ versions?
5 participants