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

cppyy <=3.1.2 broken on macOS #78

Open
saraedum opened this issue May 28, 2024 · 17 comments
Open

cppyy <=3.1.2 broken on macOS #78

saraedum opened this issue May 28, 2024 · 17 comments
Labels
bug Something isn't working

Comments

@saraedum
Copy link
Member

saraedum commented May 28, 2024

Issue

We are seeing strange runtime compilation errors with cppyy on macOS in CI:

ERROR in cling::CIFactory::createCI(): cannot extract standard library include paths!
Invoking:
  LC_ALL=C clang-14  -O2 -DNDEBUG -xc++ -E -v /dev/null 2>&1 | sed -n -e '/^.*include/,${' -e '/^ \/.*++/p' -e '}'
Results was:
With exit code 0
In file included from input_line_3:38:
In file included from /Users/runner/miniconda3/envs/test/bin/../include/c++/v1/cassert:19:
In file included from /Users/runner/miniconda3/envs/test/bin/../include/c++/v1/__assert:14:
/Users/runner/miniconda3/envs/test/bin/../include/c++/v1/__verbose_abort:24:18: error: unknown type name '_LIBCPP_AVAILABILITY_VERBOSE_ABORT'
_LIBCPP_NORETURN _LIBCPP_AVAILABILITY_VERBOSE_ABORT _LIBCPP_OVERRIDABLE_FUNC_VIS _LIBCPP_ATTRIBUTE_FORMAT(__printf__, 1, 2)

or

/Users/runner/miniconda3/envs/test/bin/../include/c++/v1/__exception/exception.h:72:33: error: redefinition of 'exception'
class _LIBCPP_EXPORTED_FROM_ABI exception {
                                    ^
/Applications/Xcode_13.2.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/exception:99:29: note: previous definition is here
class _LIBCPP_EXCEPTION_ABI exception

These are related to an incompatibility in libcxx versions caused by the recent upgrade in conda-forge/libcxx-feedstock#131.

Workaround

Install a libcxx that is compatible with the local version of xcode. Typically, this means that you should downgrade to libcxx=16.

Environment info

macOS with xcode installed, e.g., macos-11 and macos-12 on the GitHub runners.

@saraedum saraedum added the bug Something isn't working label May 28, 2024
@wlav
Copy link

wlav commented May 28, 2024

Did the header guards change in a recent update of libcxx?

@saraedum
Copy link
Member Author

saraedum commented Jun 11, 2024

@wlav, yes, used to be a double underscore now it's one, i.e.,

-#ifndef __LIBCPP_TYPEINFO
+#ifndef _LIBCPP_TYPEINFO

@saraedum
Copy link
Member Author

saraedum commented Jun 11, 2024

I am not sure where this header search path comes from. Before running the tests in that CI run, the environment variables look good:

declare -x CFLAGS="-march=core2 -mtune=haswell -mssse3 -ftree-vectorize -fPIC -fstack-protector-strong -O2 -pipe -isystem /Users/runner/miniconda3/envs/test/include"
declare -x CPPFLAGS="-D_FORTIFY_SOURCE=2 -isystem /Users/runner/miniconda3/envs/test/include"
declare -x CXXFLAGS="-march=core2 -mtune=haswell -mssse3 -ftree-vectorize -fPIC -fstack-protector-strong -O2 -pipe -stdlib=libc++ -fvisibility-inlines-hidden -fmessage-length=0 -isystem /Users/runner/miniconda3/envs/test/include"

So, I guess something tries to be smart here and puts xcode on the search path (could be the test runner, or cppyy, or some macOS magic…) (I checked, it's not the test runner.)

@wlav
Copy link

wlav commented Jun 11, 2024

So, I guess something tries to be smart here

Yes, actually ... finding the header files on Mac has been a ton of trouble recently, so there's some code in cling's CIFactory.cpp that searches all the likely locations. The above flags are not considered.

CLING_OSX_SYSROOT is picked up from compilation time, MACOSX_DEPLOYMENT_TARGET from the environment (if provided); and if the two combined fail to find a valid path, xcrun --show-sdk-path is used.

@saraedum
Copy link
Member Author

I see. So currently that xcrun --show-sdk-path is probably what's used.
So when building cppyy-cling we should set CLING_OSX_SYSROOT maybe. Or just patch that logic away and use CONDA_PREFIX instead.

@wlav
Copy link

wlav commented Jun 11, 2024

CLING_OSX_SYSROOT is set by the logic in cling's CMakeLists.txt:

  if (CMAKE_OSX_SYSROOT)
    # CMAKE_OSX_SYSROOT hardcodes the concrete version of the sdk
    # (eg .../MacOSX11.1.sdk) which changes after every update of XCode. We use
    # the assumption that in the parent folder there is a symlink MacOSX.sdk
    # which points to the current active sdk. This change allows releases
    # to work when the users update their sdks. 
    # FIXME: That is a horrible hack and we should teach CIFactory to pick up
    # the SDK directory at runtime, just as we do for the include paths to C++.
    set (OSX_SYSROOT_DEFAULT_SDK ${CMAKE_OSX_SYSROOT})
    if (${OSX_SYSROOT_DEFAULT_SDK} MATCHES "MacOSX[.0-9]+\.sdk")
      get_filename_component(OSX_SYSROOT_DEFAULT_SDK ${OSX_SYSROOT_DEFAULT_SDK} DIRECTORY)
      set (OSX_SYSROOT_DEFAULT_SDK ${OSX_SYSROOT_DEFAULT_SDK}/MacOSX.sdk/)
    endif()
      
    file(APPEND ${CMAKE_CURRENT_BINARY_DIR}/cling-compiledata.h.in
      "
      #define CLING_OSX_SYSROOT \"${OSX_SYSROOT_DEFAULT_SDK}\"
    ")
  endif()

The above comes in from cling and that assumption was recently invalidated with the SDK having been moved to a significantly different location, so that's why I introduced xcrun --show-sdk-path as a patch in cppyy-backend. I'm happy to add further options, such as picking up CONDA_PREFIX, since I'm patching cling already. :)

@isuruf
Copy link
Member

isuruf commented Jun 11, 2024

You might need llvm/llvm-project@a3a2431

@wlav
Copy link

wlav commented Jun 11, 2024

The paths there are equally mostly hardwired, but yes. It's CLING_OSX_SYSROOT that effectively is equivalent to the first piece (Cling can not assume to be installed alongside the SDK, so it keeps the info from the build), and xcrun was introduced to capture that second path with a hope that that would cover all (future) cases.

@saraedum
Copy link
Member Author

@isuruf so do you think it might work out if we set CLING_OSX_SYSROOT to CONDA_PREFIX? Or are there any headers that need to be looked up in the actual macOS SDK path?

@saraedum
Copy link
Member Author

Or rather, what should -isysroot be in a conda environment? Is -isysroot $CONDA_PREFIX a sane setting? (I obviously have no understanding how this all works.)

@saraedum
Copy link
Member Author

Actually, import cppyy already says

 ERROR in cling::CIFactory::createCI(): cannot extract standard library include paths!
Invoking:
  LC_ALL=C clang-14  -O2 -DNDEBUG -xc++ -E -v /dev/null 2>&1 | sed -n -e '/^.*include/,${' -e '/^ \/.*++/p' -e '}'
Results was:
With exit code 0
In file included from input_line_3:38:
In file included from /Users/runner/miniconda3/envs/test/bin/../include/c++/v1/cassert:19:
In file included from /Users/runner/miniconda3/envs/test/bin/../include/c++/v1/__assert:14:
/Users/runner/miniconda3/envs/test/bin/../include/c++/v1/__verbose_abort:24:18: error: unknown type name '_LIBCPP_AVAILABILITY_VERBOSE_ABORT'
_LIBCPP_NORETURN _LIBCPP_AVAILABILITY_VERBOSE_ABORT _LIBCPP_OVERRIDABLE_FUNC_VIS _LIBCPP_ATTRIBUTE_FORMAT(__printf__, 1, 2)

@saraedum
Copy link
Member Author

saraedum commented Jun 11, 2024

To fix this for the instant, I think we want a pin on cppyy to require libcxx<17. Actually, this will the probably break things when xcode updates to llvm 17. I guess we should rather document that you need to install a libcxx that is compatible with your xcode for the old builds of cppyy.

@isuruf
Copy link
Member

isuruf commented Jun 11, 2024

Or rather, what should -isysroot be in a conda environment? Is -isysroot $CONDA_PREFIX a sane setting? (I obviously have no understanding how this all works.)

No, you need the actual macOS SDK for the C headers/runtime. For C++ headers/runtime we should use $CONDA_PREFIX.

@saraedum
Copy link
Member Author

@wlav if you have a patch that also tries $CONDA_PREFIX, I am happy to try it out and see if that helps.

@saraedum
Copy link
Member Author

saraedum commented Jun 12, 2024

Shouldn't @conda-forge/cling and @conda-forge/root have the same problem?

@saraedum
Copy link
Member Author

root has this in their meta.yaml, so they won't upgrade to libcxx 17 automatically.

    - {{ pin_compatible('libcxx', min_pin='x', max_pin='x') }}

@saraedum
Copy link
Member Author

cling doesn't have silicon builds so I cannot try this out.

@saraedum saraedum changed the title cppyy 3.1.2 is probably broken on macOS cppyy <=3.1.2 broken on macOS Jun 12, 2024
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
Development

No branches or pull requests

3 participants