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.

Sign up for GitHub

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/sage/features/sagemath.py: Add feature SAGE_SRC #37650

Merged
merged 2 commits into from
May 2, 2024
Merged

src/sage/features/sagemath.py: Add feature SAGE_SRC #37650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7cd5a23
src/sage/features/sagemath.py: Add feature SAGE_SRC
mkoeppe Mar 22, 2024
576d8bf
src/sage/features/sagemath.py (SAGE_SRC): Check for sage-src-env-conf…
mkoeppe Apr 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion src/sage/features/sagemath.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,31 @@
from .join_feature import JoinFeature


class SAGE_SRC(StaticFile):
r"""
A :class:`~sage.features.Feature` which describes the presence of the
monolithic source tree of the Sage library.

"""
def __init__(self):
r"""
TESTS::

sage: from sage.features.sagemath import SAGE_SRC
sage: isinstance(SAGE_SRC(), SAGE_SRC)
True
"""
from sage.env import SAGE_SRC
# We check the file bin/sage-src-env-config.in, which by design is:
# - never installed,
# - not included in the sagemath-standard sdist,
# - included only in one modularized sdist, of pkgs/sage-conf_pypi,
# where it appears in a subdirectory (sage_root/src/bin/)
StaticFile.__init__(self, 'SAGE_SRC',
filename='bin/sage-src-env-config.in',
search_path=(SAGE_SRC,) if SAGE_SRC else ())


class sagemath_doc_html(StaticFile):
r"""
A :class:`~sage.features.Feature` which describes the presence of the documentation
Expand Down Expand Up @@ -1095,7 +1120,8 @@ def all_features():
sage: list(all_features())
[...Feature('sage.combinat'), ...]
"""
return [sagemath_doc_html(),
return [SAGE_SRC(),
sagemath_doc_html(),
sage__combinat(),
sage__geometry__polyhedron(),
sage__graphs(),
Expand Down
1 change: 1 addition & 0 deletions src/sage/misc/package_dir.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ def read_distribution(src_file):

EXAMPLES::

sage: # needs SAGE_SRC
sage: from sage.env import SAGE_SRC
sage: from sage.misc.package_dir import read_distribution
sage: read_distribution(os.path.join(SAGE_SRC, 'sage', 'graphs', 'graph_decompositions', 'tdlib.pyx'))
Copy link
Contributor

Choose a reason for hiding this comment

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

This line # needs tdlib as explained in #37645. You act as if SAGE_SRC indicates that you have the whole source tree, but in fact sagemath-standard sdist contains VERSION.txt afaict.

Copy link
Member

Choose a reason for hiding this comment

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

A fair point that I had not thought about. I was only thinking of the case where you test with an installation of sage that is not in source. In which case VERSION.txt is not installed and cannot be found.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in fact sagemath-standard sdist contains VERSION.txt afaict.

Very good point. And in fact all of the modularized distributions have VERSION.txt too.
Suggestions what we should test?

Copy link
Member

Choose a reason for hiding this comment

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

Presence of the bin or doc folders? It also has to be something that will be stable - for some time at least.

Copy link
Member

Choose a reason for hiding this comment

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

Producing a sdist right now to make sure I do not say something stupid.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not add a sentinel file in src/sage/features/ that is never included in any distribution? Using an arbitrary existing file is just asking for trouble when later that file is changed for an unrelated reason, etc.

OTOH, it feels weird that testing sage-setup needs SAGE_SRC. The read_distribution() function seems like a part of sage-setup not of sagelib, and why can't it be unit tested instead of testing with a .pyx from the sage source?

Copy link
Member

Choose a reason for hiding this comment

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

I totally get what you mean about the test, but it is meant to test "distribution", so stuff like sagemath-bliss or as in this case sagemath-tdlib. If you want to keep things in source tree, you have to provide the test suite with a "fake" mini distribution to test.

As for the file to test, it is arbitrary and I would have probably chosen pyproject.tml.m4 myself. But I think the file chosen is there for a long time which is a criterion.

Copy link
Contributor

Choose a reason for hiding this comment

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

What is wrong with a file src/sage/features/this-file-only-in-the-source.txt which IIUC it's never included in a sdist by default (https://setuptools.pypa.io/en/latest/userguide/miscellaneous.html)

The file can have a simple explanation of its purpose, being a text file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using an arbitrary existing file is just asking for trouble when later that file is changed for an unrelated reason, etc.

Well, this particular file I chose is not arbitrary but by its design has exactly the right properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The read_distribution() function seems like a part of sage-setup not of sagelib

No, it's in the right place. This module, like sage.features, is part of sagemath-environment.

Expand Down
7 changes: 5 additions & 2 deletions src/sage/misc/replace_dot_all.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@ def find_replacements(location, package_regex=None, verbose=False):

EXAMPLES::

sage: # needs SAGE_SRC
sage: from sage.misc.replace_dot_all import *
sage: location = os.path.join(sage.env.SAGE_SRC, 'sage/plot/arc.py')
sage: location = os.path.join(sage.env.SAGE_SRC, 'sage', 'plot', 'arc.py')
sage: find_replacements(location, package_regex='sage[.]plot[.]all', verbose=True)
[[..., ..., 'from sage.plot.graphics import Graphics']]
"""
Expand Down Expand Up @@ -295,8 +296,9 @@ def process_line(location, line, replacements, row_index, verbose=False):

Replacing the first line which needs a replacement in the source file with filepath ``src/sage/plot/arc.py``::

sage: # needs SAGE_SRC
sage: from sage.misc.replace_dot_all import *
sage: location = os.path.join(sage.env.SAGE_SRC, 'sage/plot/arc.py')
sage: location = os.path.join(sage.env.SAGE_SRC, 'sage', 'plot', 'arc.py')
sage: replacements = find_replacements(location, package_regex='sage[.]plot[.]all', verbose=True); replacements
[[477, 24, 'from sage.plot.graphics import Graphics']]
sage: with open(location, "r") as file:
Expand Down Expand Up @@ -401,6 +403,7 @@ def walkdir_replace_dot_all(dir, file_regex=r'.*[.](py|pyx|pxi)$', package_regex

EXAMPLES::

sage: # needs SAGE_SRC
sage: from sage.misc.replace_dot_all import *
sage: walkdir_replace_dot_all(os.path.join(sage.env.SAGE_SRC, 'sage')) # not tested
"""
Expand Down
2 changes: 2 additions & 0 deletions src/sage_setup/clean.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ def _find_stale_files(site_packages, python_packages, python_modules, ext_module
course. We check that when the doctest is being run, that is,
after installation, there are no stale files::

sage: # needs SAGE_SRC
sage: from sage.env import SAGE_SRC, SAGE_LIB, SAGE_ROOT
sage: from sage_setup.find import _cythonized_dir
sage: cythonized_dir = _cythonized_dir(SAGE_SRC)
Expand All @@ -98,6 +99,7 @@ def _find_stale_files(site_packages, python_packages, python_modules, ext_module

TODO: Also check extension modules::

sage: # needs SAGE_SRC
sage: stale_iter = _find_stale_files(SAGE_LIB, python_packages, python_modules, [], extra_files)
sage: from importlib.machinery import EXTENSION_SUFFIXES
sage: skip_extensions = tuple(EXTENSION_SUFFIXES)
Expand Down
1 change: 1 addition & 0 deletions src/sage_setup/find.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# sage.doctest: needs SAGE_SRC
"""
Recursive Directory Contents
"""
Expand Down
Loading