DEPS: Bump meson-python, lower pin Cython #62086
Conversation
mroeschke
added
Build
|
pandas/_libs/tslibs/ccalendar.cp313-win_amd64.pyd.p/pandas/_libs/tslibs/ccalendar.pyx.c(9749): error C2220: the following warning is treated as an error |
|
Is it just me or is the meson system harder to debug than the old system? |
Yeah there's a little more indirection than before (meson-python -> meson vs just setuptools), but I understand the "standardization" of meson/cmake is better than the wild-west of using setuptools |
| fi | ||
| pip install $EDITABLE . --no-build-isolation -v --no-deps --config-settings=setup-args="-Werror" --config-settings=setup-args="-Wno-error=unused-parameter" |
There was a problem hiding this comment.
@WillAyd any advice on how to pass -Werror -Wno-error=unused-parameter though meson(-python) during the build step? I was trying to follow https://mesonbuild.com/meson-python/how-to-guides/meson-args.html
There was a problem hiding this comment.
You probably want to pass it as -Ccompile-args="-Wno-error=unused-parameter
The -C is short for --config-settings in relatively more modern versions of pip, so that cuts down on the verbosity. Also, the argument you are providing is something that the compiler will process, hence the change of setup-args to compile-args
However, I'd suggest just putting this directly in the Meson configuration rather than controlling via the pip frontend:
cc = meson.get_compiler('c')
add_project_arguments(
cc.get_supported_arguments(['-Wno-error=unused-parameter']),
language: 'c',
)That will suppress it for end users who probably don't care about these warnings anyway, while also working cross-platform (i.e. it will be ignored with an MSVC toolchain)
|
I don't have a strong point of view on bumping Meson, although I think it would be good to keep the lower bound as is until a new feature is required. (I definitely agree with un-pinning) If we do need the 1.6 bump, you will also want to add that change to the |
mroeschke
changed the title
| run: | | ||
| if [[ ${{ inputs.editable }} == "true" ]]; then | ||
| EDITABLE="-e" | ||
| else | ||
| EDITABLE="" | ||
| fi | ||
| pip install $EDITABLE . --no-build-isolation -v --no-deps --config-settings=setup-args="-Werror" --config-settings=setup-args="-Wno-error=unused-parameter" | ||
| pip install $EDITABLE . --no-build-isolation -v --no-deps -Ccompile-args="-Werror" |
There was a problem hiding this comment.
This actually should stay as setup-args not compile-args. Although it will work in either case on Unix platforms, -Werror is a setup argument in Meson as well. Passing it as a setup argument is portable across platforms, whereas this same command will fail with MSVC compilers
Co-authored-by: William Ayd <[email protected]>
Co-authored-by: William Ayd <[email protected]>
This reverts commit 930715b.
There was a problem hiding this comment.
@mroeschke you un-bumped meson on Will's request, but should the same then be done for meson-python. Or is there a good reason to still bump that one? (I don't really have an opinion here, just asking)
| - meson>=1.2.1,<2 | ||
| - meson-python>=0.17.1,<1 |
There was a problem hiding this comment.
Do we want to keep the pin here since this is the build with minimum versions?
|
mesonbuild/meson-python#716 suggested to use the jinja2 FileSystemLoader instead of PackageLoader, and it seems that @WillAyd tried that in #60681. @WillAyd do you remember if that worked? |
Just testing it out directly, seems to work fine -> #62123 |
Since we recently adopted building with Cython 3 a while back, we don't want to allow users to build pandas with a Cython version lower than that