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

envconfig: read CYTHON from the environment and use it if set #12674

Merged
merged 1 commit into from
Jan 1, 2024

Conversation

erikbs
Copy link
Contributor

@erikbs erikbs commented Dec 27, 2023

Add support for specifying Cython compiler using the CYTHON environment variable. If not set, proceed with the names hard coded for Cython. Inspired by issue #1645, which ended up with this solution being implemented for VALAC (bc30ad6).

There is already a line in the existing code that reads the CYTHON variable, but only in test code.

Motivation: MacPorts uses versioned names (e.g., cython-3.12) and so does not expose cython or cython3 by default (there is a command to create symlinks to cython/cython3). Its NumPy package is currently built by patching NumPy’s bundled version of Meson to use the solution I am proposing here, and that is also where I got the idea from.

Note: I read that these environment variables are deprecated and that environment variables in Meson generally should be “avoided whenever possible”, but as far as I could see there is no new solution in place yet?

Edit:
I should add that cython-3.12 itself is a symlink to the cython binary, but it resides in a folder that is not on PATH. This is deliberate, so that different Python versions can live side by side.

Add support for specifying Cython compiler using the CYTHON environment
variable. If not set, proceed with the names hard coded for Cython.
@erikbs erikbs requested a review from jpakkane as a code owner December 27, 2023 18:30
eli-schwartz added a commit that referenced this pull request Dec 28, 2023
…s note

In commit 58c2aeb, an asterisk used to
indicate "see note at bottom" was doubled up into an "italicize this
paragraph", which didn't contextually make any sense.

See: #12674 (comment)
@eli-schwartz
Copy link
Member

Note: I read that these environment variables are deprecated

The docs have a formatting typo. The issue is that some environment variables were deprecated in favor of other environment variables. The environment variables listed in that table under "note" should not be used, in favor of exclusively using the variables listed in the second or third column.

@eli-schwartz eli-schwartz merged commit 0e1cba6 into mesonbuild:master Jan 1, 2024
31 checks passed
@eli-schwartz
Copy link
Member

Motivation: MacPorts uses versioned names (e.g., cython-3.12) and so does not expose cython or cython3 by default (there is a command to create symlinks to cython/cython3).

I'd just like to point out for the record that you should not be doing this.

The cython program does not care what version of python it internally implements, it still produces transpiled C/C++ files the same. There's no good reason to name it according to the version of python that was used as an underlying framework -- the current two names we check are a filthy hack to work around the fact that Debian is both very common, and very broken -- they renamed cython to cython3 which is incorrect, and it caused a lot of people and projects tremendous pain.

gerioldman pushed a commit to gerioldman/meson that referenced this pull request Jul 2, 2024
…s note

In commit 58c2aeb, an asterisk used to
indicate "see note at bottom" was doubled up into an "italicize this
paragraph", which didn't contextually make any sense.

See: mesonbuild#12674 (comment)
@erikbs erikbs deleted the feature/cython-env-var branch December 29, 2024 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants