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 v18.1.8 #155

Merged
merged 18 commits into from
Jul 10, 2024
Merged

libcxx v18.1.8 #155

merged 18 commits into from
Jul 10, 2024

Conversation

h-vetinari
Copy link
Member

@h-vetinari h-vetinari commented Jun 20, 2024

Cleaned-up version of #144, based on #154 (will be rebased once that's in)


Closes #136
Closes #144
Closes #156

@conda-forge-webservices
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

h-vetinari commented Jun 22, 2024

@isuruf, I'd appreciate your input on how to handle libunwind here. At first I thought this might be related to the warning

[1136/1183] Linking CXX shared library lib/libc++.1.0.dylib
x86_64-apple-darwin13.4: warning: argument unused during compilation: '--unwindlib=none' [-Wunused-command-line-argument]

but as it turns out, this is just an artefact of an old CMake work-around (injecting flags to both compiler & linker, instead of only the linker), but backporting that PR makes no difference to the fact that basic tests here fail on both linux & unix without libunwind.so, but pass if we package it.

I don't know if this situation is similar to libcxxabi, but I have to assume it was intentional not to ship libunwind so far? At least the tests (including the downstream ones) pass if it gets packaged, so it doesn't seem to have the same problems as libcxxabi?

If we do ship it, putting it in the libcxx output (as I'm doing here provisionally) is wrong from a layering perspective, as libcxxabi also depends on libunwind (so we'd probably want a separate output?):

WARNING (libcxxabi,lib/libc++abi.so.1.0): lib/libunwind.so.1 not found in any packages
WARNING (libcxxabi,lib/libc++abi.so.1.0): /lib64/libpthread.so.0 not found in packages, sysroot(s) nor the missing_dso_whitelist.
.. is this binary repackaging?
WARNING (libcxxabi,lib/libc++abi.so.1.0): /lib64/libc.so.6 not found in packages, sysroot(s) nor the missing_dso_whitelist.
.. is this binary repackaging?
WARNING (libcxxabi,lib/libc++abi.so.1.0): /lib64/ld-linux-x86-64.so.2 not found in packages, sysroot(s) nor the missing_dso_whitelist.
.. is this binary repackaging?

The fact that the sysroot isn't being found on linux is a separate question/regression, which I'm currently working around (cd27eb9). Feedback on this (and the other points in the OP) is certainly welcome, but the libunwind stuff is the most important to me.

@h-vetinari
Copy link
Member Author

It's also possible that the dependence on libunwind (despite the --unwindlib=none flag) is a regression upstream, but that's hard for me to tell, and in any case, the 18.x series saw it's last release already...

@h-vetinari h-vetinari marked this pull request as ready for review June 24, 2024 00:09
clang 15 isn't supported by libcxx 18 anymore either, but seems to work in
limited testing; OTOH, clang 14 blows up on simple header inclusion already
@h-vetinari
Copy link
Member Author

@isuruf, I'd appreciate your input on how to handle libunwind here.

If you have a moment...

@isuruf
Copy link
Member

isuruf commented Jun 30, 2024

Yeah, we definitely don't want to ship our own libunwind in macOS. Not sure about Linux

@h-vetinari
Copy link
Member Author

Yeah, we definitely don't want to ship our own libunwind in macOS.

Is there a way to test what might go wrong if we did ship it? The current libcxx-testing doesn't fail when we do. The rest should have been fixed by a3a0b87, i.e. doing the same as for libcxxabi. For linux I'm now following that approach (i.e. doing the same as for libcxxabi), and turning it into an output.

@h-vetinari
Copy link
Member Author

@isuruf, could you please review this PR when you have a moment?

@h-vetinari
Copy link
Member Author

@isuruf, I consider this ready for merging now. Please let me know if you have any remaining comments.

@isuruf isuruf merged commit f2935e0 into conda-forge:main Jul 10, 2024
7 checks passed
@h-vetinari h-vetinari deleted the 18 branch July 10, 2024 05:29
@h-vetinari
Copy link
Member Author

Thank you!

@h-vetinari
Copy link
Member Author

Hm, just seeing this on main now:

  "errors": [
    "output linux-64/libunwind-18.1.8-ha4b6fd6_0.conda not allowed for conda-forge/libcxx-feedstock"
  ],

I wasn't aware of https://github.com/conda-forge/libunwind-feedstock, which repackages https://github.com/libunwind/libunwind. I'm not sure if it's possible to build libcxx against the existing libunwind (though they aim at doing the same thing).

Or, if we end up allowing two implementations, we'd probably need a mutex, which sounds a bit like overkill for this?

Any thoughts @conda-forge/libunwind?

@h-vetinari
Copy link
Member Author

OK, I believe I fixed this in #160

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.

Enable _LIBCPP_HARDENING_MODE_FAST?
2 participants