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

MAINT: vendor Tempita in scipy/_build_utils #20572

Merged
merged 2 commits into from Apr 29, 2024

Conversation

rgommers
Copy link
Member

@rgommers rgommers commented Apr 24, 2024

  • Addresses a part of BUG: generate_f2py mod is called by the wrong interpreter #20535.
  • Purpose: make build more robust, stop relying on import Cython but only on invoking the cython executable
  • Context for purpose: to make cross compilation better, we want to stop invoking the Python interpreter we're building for. Hence, we should avoid having to use/import things at build time outside of Python's stdlib. We're almost there, only a few places left where we invoke custom_target(..., command: [py3, ...]) in meson.build files. Running Tempita is one of them.

This is an unmodified copy from Cython/Tempita/ as of commit 023d4af35 in Cython's master branch (21 April 2024). Tempita hasn't been maintained independently on PyPI for 10+ years; we've used the Cython version which is semi-public (undocumented) for a while, with the note that we should vendor it if that ever fails. It now failed once (see #20535), and conceptually that makes sense - this is the only place where we don't invoke the cython executable but actually do import Cython. That can fail in a distro setup where Cython is installed for a different Python interpreter than the active one.

License

Opening as a draft PR to clarify the licensing situation of this code first:

  • The original Tempita was MIT-licensed, see https://pypi.org/project/Tempita/ (a single declaration, no full copy of the MIT license text in the sdist)
  • That code was vendored in Cython in 2011, without making a note of the license: https://github.com/cython/cython/commits/master/Cython/Tempita
  • Cython itself is Apache 2.0 licensed (not compatible with SciPy's BSD-3 license). However, there have been no substantial changes to the Tempita code since 2011 it looks like, but rather commits are minor maintenance/style change (running 2to3, pyupgrade, etc.). So I think we should be able still consider this code as MIT-licensed - but that requires confirmation from @scoder I think, as Cython lead dev and author of most of the commits that touched this code.
    • @scoder are you fine with including that Cython/Tempita/ code under the original MIT license, or do you now consider it mixed MIT/Apache licensed?
    • Note that NumPy also vendored a copy until not long ago, and that does have an MIT license file: https://github.com/numpy/numpy/tree/maintenance/1.26.x/tools/npy_tempita. Using that would be the alternative. It is less well maintained than the Cython code, so we'd need to polish it a bit.

TODO:

  • Clarify license situation
  • Update LICENSES_bundled.txt

@rgommers rgommers added Build issues Issues with building from source, including different choices of architecture, compilers and OS maintenance Items related to regular maintenance tasks Cython Issues with the internal Cython code base Meson Items related to the introduction of Meson as the new build system for SciPy labels Apr 24, 2024
@scoder
Copy link

scoder commented Apr 24, 2024

IANAL, but I looked through the changes and IMHO none of them would hand any non-trivial chunk of code over to other authors than in the original copy of Tempita that we included and possibly me. Almost all change were trivial adaptations or removals, many of them machine generated.

I hereby grant you the right to use my changes under the original code's MIT license, or the 3-clause BSD license that SciPy currently uses.

@rgommers
Copy link
Member Author

Thanks @scoder, much appreciated!

@rgommers
Copy link
Member Author

I knew the lint CI job was going to fail; python dev.py lint calls the tools/lint.py --diff-against which seems to not honor the exclude files in lint.toml. I'm not planning to reformat this code, so plan to just ignore the failure (it goes away by itself post-merge, at least until someone touches the file again). @lucascolley is this a known issue with the linter perhaps?

@lucascolley
Copy link
Member

lucascolley commented Apr 24, 2024

is this a known issue with the linter perhaps?

Right, I agree with your diagnosis. I can't think of an obvious refactor. It seems we would have to extract the exclude field from lint.toml in lint.py to perform set difference on the list of files. I'm happy to just let this fail.

Alternatively, maybe ruff check --exclude=x.py x.py y.py should only lint y.py.

Copy link
Member

@lucascolley lucascolley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

never mind, here's the one-line (okay, two-line) fix!

tools/lint.toml Show resolved Hide resolved
rgommers and others added 2 commits April 25, 2024 14:19
This is an unmodified copy from `Cython/Tempita/` as of commit
`023d4af35` in Cython's master branch (21 April 2024).
Tempita hasn't been maintained independently on PyPI for 10+ years;
we've used the Cython version which is semi-public (undocumented)
for a while, with the note that we should vendor it if that ever
fails. It now failed once (see scipy#20535), and conceptually that
makes sense - this is the only place where we don't invoke the
`cython` executable but actually do `import Cython`. That can fail
in a distro setup where Cython is installed for a different Python
interpreter than the active one.

The license file was taken over from the `maintenance/1.26.x` branch
in the `numpy` repo - which came from the original upstream repo
for Tempita (now disappeared) as discussed in numpy#8096.
See scipy#20572 for more clarification about the MIT license that
this code is under.
Co-authored-by: Lucas Colley <lucas.colley8@gmail.com>
@rgommers rgommers added this to the 1.14.0 milestone Apr 25, 2024
@rgommers rgommers marked this pull request as ready for review April 25, 2024 12:24
@rgommers rgommers requested a review from larsoner as a code owner April 25, 2024 12:24
Copy link
Member

@lucascolley lucascolley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@rgommers
Copy link
Member Author

Thanks for the review @lucascolley. Going to hit the green button here, so I can work on the next step.

@rgommers rgommers merged commit d561d9e into scipy:main Apr 29, 2024
30 checks passed
@rgommers rgommers deleted the vendor-tempita branch April 29, 2024 03:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build issues Issues with building from source, including different choices of architecture, compilers and OS Cython Issues with the internal Cython code base maintenance Items related to regular maintenance tasks Meson Items related to the introduction of Meson as the new build system for SciPy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants