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

BLD: improvements to meson.build files #54949

Merged
merged 2 commits into from
Sep 8, 2023

Conversation

rgommers
Copy link
Contributor

@rgommers rgommers commented Sep 2, 2023

  • closes #xxxx (Replace xxxx with the GitHub issue number)
  • Tests added and passed if fixing a bug or adding a new feature
  • All code checks passed.
  • Added type annotations to new arguments/methods/functions.
  • Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

The second commit is a bug fix: run generate_version.py with a shebang, not 'python'. The way this was before can result in build failures. It assumed that python is a working Python 3.x interpreter, and that is not always true. See for example this bug report for the exact same thing in NumPy, where python isn't working for Sage: numpy/numpy#24514

Meson guarantees that .py scripts with a shebang on the top line will be run with a Python interpreter (if there's none on the PATH, it can use the one Meson itself is run with). Hence this is the most robust way of using run_command on a .py script.

The first commit is cleanups to make meson.build files more idiomatic:

  • Use pure: false only in a single place. This is recommended for robustness, this way you can't forget it in a subdirectory and end up with a subtly broken package only on niche Linux distros that split purelib and platlib directories.
  • Use py.install_sources with a list input rather than in a foreach loop.
  • Remove the werror comment: it's never a good idea to enable -Werror by default in the build config of a library, that can easily break builds. This should be done in one or more CI jobs instead.

- Use `pure: false` only in a single place. This is recommended for
  robustness, this way you can't forget it in a subdirectory and end up
  with a subtly broken package only on niche Linux distros that split
  purelib and platlib directories.
- Use `py.install_sources` with a list input rather than in a foreach
  loop.
- Remove the `werror` comment: it's never a good idea to enable
  `-Werror` by default in the build config of a library, that can easily
  break builds. This should be done in one or more CI jobs instead.
The way this was before can result in build failures. It assumed that
`python` is a working Python 3.x interpreter, and that is not always
true. See for example this bug report for the exact same thing in
NumPy, where `python` isn't working for Sage:
numpy/numpy#24514

Meson guarantees that .py scripts with a shebang on the top line will
be run with a Python interpreter (if there's none on the PATH, it can
use the one Meson itself is run with). Hence this is the most robust
way of using `run_command` on a .py script.
@WillAyd
Copy link
Member

WillAyd commented Sep 2, 2023

Lgtm. @lithomas1

@lithomas1 lithomas1 added the Build Library building on various platforms label Sep 2, 2023
@lithomas1 lithomas1 added this to the 2.1.1 milestone Sep 3, 2023
@lithomas1 lithomas1 merged commit 3334832 into pandas-dev:main Sep 8, 2023
70 of 71 checks passed
@lithomas1
Copy link
Member

thanks @rgommers.

meeseeksmachine pushed a commit to meeseeksmachine/pandas that referenced this pull request Sep 8, 2023
@rgommers rgommers deleted the meson-tweaks branch September 8, 2023 14:47
mroeschke pushed a commit that referenced this pull request Sep 8, 2023
…files) (#55065)

Backport PR #54949: BLD: improvements to meson.build files

Co-authored-by: Ralf Gommers <[email protected]>
mroeschke pushed a commit to mroeschke/pandas that referenced this pull request Sep 11, 2023
* BLD: some changes to make meson.build more idiomatic

- Use `pure: false` only in a single place. This is recommended for
  robustness, this way you can't forget it in a subdirectory and end up
  with a subtly broken package only on niche Linux distros that split
  purelib and platlib directories.
- Use `py.install_sources` with a list input rather than in a foreach
  loop.
- Remove the `werror` comment: it's never a good idea to enable
  `-Werror` by default in the build config of a library, that can easily
  break builds. This should be done in one or more CI jobs instead.

* BLD: run `generate_version.py` with a shebang, not 'python'

The way this was before can result in build failures. It assumed that
`python` is a working Python 3.x interpreter, and that is not always
true. See for example this bug report for the exact same thing in
NumPy, where `python` isn't working for Sage:
numpy/numpy#24514

Meson guarantees that .py scripts with a shebang on the top line will
be run with a Python interpreter (if there's none on the PATH, it can
use the one Meson itself is run with). Hence this is the most robust
way of using `run_command` on a .py script.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Library building on various platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants