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

libcxx run export conflict with sysroot installation of libcxx with old version of clangdev #188

Open
1 task done
SylvainCorlay opened this issue Oct 24, 2022 · 12 comments
Labels

Comments

@SylvainCorlay
Copy link
Member

SylvainCorlay commented Oct 24, 2022

Solution to issue cannot be found in the documentation.

  • I checked the documentation.

Issue

Some packages such as cling and xeus-cling have a runtime dependency on non-latest versions of clangdev (such as clangdev 5 or clangdev 9). Recent builds of clangdev 5 and 9 are compiled with more recent versions of clang, which have a strong run export on libcxx. As a consequence, recent builds of clangdev 5 and clangdev 9 have a runtime dependency on libcxx, which is also installed in the SDK version shipped with XCode.

Having libcxx in both the conda prefix and the sysroot poses some issues for old versions of clang, because libcxx uses the include_next statement to import files with same name in the hierarchy of include search directories, like here. The duplication causes the include_next statement of the conda prefix to go to the sysroot's libcxx instead of the sysroot base directory.

This issue was resolved in recent versions of clang by https://reviews.llvm.org/D89001 ("Don't look into for C++ headers if they are found alongside the toolchain").

A possible solution would be to remove the search for libcxx in a patch for recent builds of clangdev 5 and other impacted versions, since they ship libcxx as a dependency anyways.

cc @isuruf @chrisburr FYI

Installed packages

N/A

Environment info

N/A
@JohanMabille
Copy link

JohanMabille commented Oct 25, 2022

After further investigations, it looks like the patch of clangdev 5 is required but not sufficient. We also need to patch recent versions of libcxx which use C++20 only features without preprocessor guards.

For instance, this function is problematic as it relies on __builtin_is_constant_evaluated which is a C++2a feature according to https://reviews.llvm.org/D55500

@SylvainCorlay
Copy link
Member Author

clang 9 and therefore cling 0.9 are also impacted.

@AngryMaciek
Copy link

I came here from jupyter-xeus/xeus-cling#403
Is there any progress on fixing this issue?

@h-vetinari
Copy link
Member

Well, it sounds to me like the solution is simply to build clang 5/9 with old compilers, so that you don't get the run-export on new libcxx?

@AngryMaciek
Copy link

AngryMaciek commented Jun 2, 2023

Thats a workaround, not a clean solution; How about:

A possible solution would be to remove the search for libcxx in a patch for recent builds of clangdev 5 and other impacted versions, since they ship libcxx as a dependency anyways.

Plus also: #188 (comment)

@h-vetinari
Copy link
Member

h-vetinari commented Jun 2, 2023

Thats a workaround, not a clean solution

How so? New compilers need newer support libraries, including libcxx. If you need older support libs, use older compilers. I mean, it might work, but why risk that?

Plus also: #188 (comment)

That's kind of my point -- we're not going to do:

We also need to patch recent versions of libcxx which use C++20 only features without preprocessor guards.

That's completely outside the bounds of what's tested upstream, and puts the burden on us to ensure our newly-patched frankenstein actually doesn't break in creative ways. That's hardly a cleaner solution than using an old compiler to build an ancient compiler.

@AngryMaciek
Copy link

AngryMaciek commented Jun 2, 2023

Fair enough, I come from xeus-cling thread mentioned above; in there @SylvainCorlay wrote he is "working on a clean fix" (jupyter-xeus/xeus-cling#403 (comment)), so I am checking up.

@h-vetinari
Copy link
Member

h-vetinari commented Jun 2, 2023

I mean, if there are patches that are only very light and obviously™ correct, then we can consider it (and I may be overruled by other maintainers, not saying I have any sort of final word). But I don't see the case to be honest. I'm happy to help keep the old compilers working, but that to me includes a period-appropriate dependency setup (incl. compilers used to build). Clang 5.0 is over 5 years old, which is an eternity in this space.

I don't know the constraints of xeus, but moving beyond such old clang versions is hopefully on the roadmap already.

@h-vetinari
Copy link
Member

Xref root-project/cling#475

@sifatraquib
Copy link

any update?

@h-vetinari
Copy link
Member

Someone needs to come up with a PR (to libcxx, to make it possible for use with clang 5). Depending on how invasive it is, we'll see if this is achievable.

Recent builds of clangdev 5 and 9 are compiled with more recent versions of clang, which have a strong run export on libcxx

Or you just try to build clangdev 5/9 here with older compilers, to avoid the problematic run-export in the first place.

I don't know the constraints w.r.t. cling well enough to do this myself, but I'll help with review etc. once something's there.

@h-vetinari
Copy link
Member

Hey all

With conda-forge/libcxx-testing-feedstock#3 resp. conda-forge/libcxx-feedstock#127 we now have a way to test our libcxx against older clang builds (currently going back to 11, as that was what our lower bound for testing was before that PR).

That means libcxx 16 in conda-forge currently "supports"1 at least clang 11-16, which is already 3 versions more than upstream policy (14, 15 & 16).

To get an idea of the degree of the breakage for older clangs, I opened conda-forge/libcxx-testing-feedstock#5. It looks like clang 10 still passes, but from clang 9 on down, we have the problem with is_trivially_constructible.

Footnotes

  1. doesn't spectacularly blow up

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

No branches or pull requests

5 participants