Fix some compilation issues and bump to 2.5.0rc1 #973
Conversation
Cantera doesn't use this part of Sundials, and bundling all of these files into the cantera library leads to linker errors when compiling with MinGW and on macOS, due to redefinition of symbols like 'F2C_CVODE_matrix'.
interfaces/cython/SConscript
Outdated
| # Only applies to Python 3.8. The field that is deprecated in Python 3.8 | ||
| # and causes the warnings to appear will be removed in Python 3.9 so no | ||
| # further warnings should be issued. | ||
| if localenv['HAS_CLANG'] and localenv['python_version'] == parse_version('3.8'): |
There was a problem hiding this comment.
If we're using the python_version from the original env here, it will be set based on the value of python_cmd (or the Python running SCons if python_cmd isn't given). In that case, do we need to set py_version on Line 39 of this file? They should be exactly the same version.
There was a problem hiding this comment.
Ah, I missed that we were reading the Python version in this SConscript file. I agree that they should be exactly the same. For the sake of making this a more minimal patch, I'll update this to use the local py_version variable.
There was a problem hiding this comment.
Sounds good. Just make sure to wrap it in parse_version(). That line got missed when we did that change earlier, so now it's reporting the deprecation warnings for me on my Mac again 😄
There was a problem hiding this comment.
Are you saying that the current main branch is giving you warnings? Because that would be consistent with what I saw while testing this on Ubuntu, where py_version is the string 3.8 and whatever parse_version returns returns False for comparisons to string values.
My fix (at the moment) is to just compare the string values, since it's just the X.Y version string, but we could wrap both of them in parse_version if there's a reason to.
There was a problem hiding this comment.
Yes, that exactly what I mean. I don't see why not use parse_version(), that's what it's there for 😁
speth
force-pushed
the
fix-mingw-sundials
branch
from
e6244e4
to
97c5a3a
Compare
The deprecation warnings previously observed only on macOS are also emitted when using Clang 10.0 on Linux. Also, despite supposedly being resolved in Cython 0.29.14, I still get these warnings in Cython 0.29.21.
Compiling demo_ftnlib.cpp as a shared object (with -fPIC) fixes an error about being unable to relocate some pthread-related symbol.
speth
force-pushed
the
fix-mingw-sundials
branch
from
97c5a3a
to
5098d78
Compare
Changes proposed in this pull request
Checklist
scons build&scons test) and unit tests address code coverage