-
Notifications
You must be signed in to change notification settings - Fork 118
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
Python wheels impose vague and undocumented restrictions on the use of C++ features #1963
Comments
This came up again in #1929, where an intermediate commit had a non- |
The latest manifestation of this in #2027 involves a class with an struct foo {
std::string m_name;
short m_number;
}; which cannot safely be passed between code compiled as part of the wheel ( |
Note that in BlueBrain/libsonata#273 we seem to have come across another, this time |
A lot of time has passed since this issue was originally opened, so I thought I'd provide some updates:
The (supported) successor to
For reference, here is a shell oneliner to get the value of printf '#include <iostream>\nint main(){std::cout<<_GLIBCXX_USE_CXX11_ABI;}' | g++ -x c++ - && ./a.out This prints 0 on @nrnhines are you aware of any other users that would be interested in running NEURON 9 via Python wheels (I specifically mention wheels since the recommended way to install NEURON on a cluster is using Spack or similar because wheels are unable to take advantage of almost any hardware-specific features that clusters may offer) on systems older than the above? UPDATE: the upgrading of the C++ standard will probably need to wait a bit since GCC is not forwards compatible, but in #3306 I managed to get NEURON wheels which shouldn't experience any dual ABI issues. |
Overview
While working on #1922 I started seeing linker errors:
in the CI jobs that test the Linux Python wheels (which are also built as part of the CI). The relevant change in that PR was that I changed the return type of that function from
char*
tostd::string
.In general, replacing C-style strings and manual memory management with standard types such as
std::string
is a Good Thing ™️, so it's unfortunate that this causes problems with use cases that we consider important (pip install neuron
on Linux...). If supporting these use cases forbids the use of certain C++ features or types in certain places, this should be made explicit and documented.Detailed description
The problem is that the Python wheels are built using an image based on
manylinux2014
that is based on CentOS7, and that this only supports using a pre-C++11 ABI forstd::string
(link, link, ...). This means that the old ABI is used for code inlibnrniv.so
, which is shipped inside the wheels.The typical workflow of running
nrnivmodl
afterpip install neuron
on a "user" machine builds C++ sources using the compiler toolchain that is installed on that "user" machine, which must support at least C++17 and will default to using a C++11-compatiblestd::string
implementation. This causes link errors if any code compiled on the user machine tries to refer to symbols insidelibnrniv.so
that usestd::string
(for example, a function that returnsstd::string
).Trying to force the use of the old ABI on user machines inside
nrnivmodl
would probably be fragile, and would not avoid issues with linking to other software on the user machine that uses the new ABI. Dropping support formanylinux2014
would(?) cause issues withpip install neuron
using vanilla Python 3.7.x.These issues can be avoided by only using a restricted subset of C++ in functions/variables that may be called from code compiled on the user machine, but there is no(?) well-defined list of these functions/variables or enforcement mechanism beyond hoping that the wheel-based CI pipelines catch any infractions.
Further information
Some other links that may be of interest:
The text was updated successfully, but these errors were encountered: