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

src/setup.py: Don't cythonize everything in src/ #32672

Closed
mkoeppe opened this issue Oct 12, 2021 · 12 comments
Closed

src/setup.py: Don't cythonize everything in src/ #32672

mkoeppe opened this issue Oct 12, 2021 · 12 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 12, 2021

In the version of setup.py used by the editable install:

 extensions = cythonize(
        ["**/*.pyx"],

at least it should restrict itself to sage/**/*.pyx
so that it does not go cythonizing in random directories that the user may have in this directory... such as a venv...

Depends on #32673

CC: @tobiasdiez @kliem

Component: build

Author: Matthias Koeppe

Branch/Commit: ff2a352

Reviewer: Jonathan Kliem

Issue created by migration from https://trac.sagemath.org/ticket/32672

@mkoeppe mkoeppe added this to the sage-9.5 milestone Oct 12, 2021
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 12, 2021

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 12, 2021

Author: Matthias Koeppe

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 12, 2021

Commit: 669ea2c

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 12, 2021

New commits:

669ea2csrc/setup.py: Restrict cythonize to sage/**

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 12, 2021

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

17acb7bDo not need Cython for sdist or egg_info
ff2a352src/setup.py: Restrict cythonize to sage/**

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 12, 2021

Changed commit from 669ea2c to ff2a352

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 12, 2021

Dependencies: #32673

@mkoeppe

This comment has been minimized.

@kliem
Copy link
Contributor

kliem commented Oct 13, 2021

Reviewer: Jonathan Kliem

@kliem
Copy link
Contributor

kliem commented Oct 13, 2021

comment:7

LGTM.

Btw, this is how I verified how it works. cythonize calls extended_iglob

sage: _ = !cd $SAGE_SRC
sage: from Cython.Build.Dependencies import extended_iglob
sage: a = list(extended_iglob("sage/**/*.pyx"))
sage: b = list(extended_iglob("**/*.pyx"))
sage: list(t for t in b if not t in a)
['foo.pyx']
sage: list(t for t in a if not t in b)
[]

In my case I placed foo.pyx in SAGE_SRC.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 13, 2021

comment:8

Thank you!

@vbraun
Copy link
Member

vbraun commented Oct 19, 2021

@vbraun vbraun closed this as completed in 9cbde3c Oct 19, 2021
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants