sagemath/sage

sage.features.Feature.resolution: If SAGE_ROOT is available, recommend system packages

mkoeppe opened this issue · 42 comments

Currently, if a Feature is keyed to an spkg, a FeatureNotPresentError recommends to use sage -i to install the package.

When SAGE_ROOT (or sage_bootstrap, see #29039) is available, we can do better:

  1. We can recommend equivalent system packages for package systems available on the present system.

  2. Moreover, if SAGE_ROOT/bin/sage-spkg is not available, then we remove the recommendation to use sage -i. This is for the benefit of distribution packaging.

    (Distribution packages would typically not install sage-spkg, so this information is automatically suppressed -- no more patching is necessary.)

  3. Finally, for Python packages that have a requirements.txt (see #30024), we also show a pip invocation.

(1. and 3. depend on a few scripts from SAGE_ROOT/build/bin to be available in the PATH. Distribution packages could consider to install these scripts; perhaps, in some libexec-style directory. Also, SAGE_ROOT/build/pkgs/*/requirements.txt and SAGE_ROOT/build/pkgs/*/distros/ need to be made available at runtime.)

We also add the names of the detected package systems to the default optional tags in use by doctests.

CC: @kiwifb @jhpalmieri @slel @seblabbe

Component: build: configure

Author: Matthias Koeppe

Branch: 68a8e2a

Reviewer: Sébastien Labbé

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

Description changed:

--- 
+++ 
@@ -2,4 +2,7 @@
 
 When `SAGE_ROOT` (or `sage_bootstrap`) is available, we can do better: We can recommend equivalent system packages.
 
+Moreover, if `SAGE_ROOT/bin/sage-spkg` is not available, then we remove the recommendation to use `sage -i`. This is for the benefit of distribution packaging.
 
+
+

Author: Matthias Koeppe

New commits:

2a8e3b7Feature.resolution: If spkg is given, try to give system package information; only show 'sage -i' if sage-spkg is available

Commit: 2a8e3b7

comment:4

resolution should be lazier

Branch pushed to git repo; I updated commit sha1. New commits:

febf5e6Feature.resolution: Use lazy strings to delay system package lookup
081e736sage.features.package_systems: New, use it in Feature.resolution
36772a5Feature.resolution: Fix up
04fa02dbuild/bin/{sage-get-system-packages,sage-print-system-package-command}: Handle pip
3f66ab7build/bin/sage-print-system-package-command: Handle --prompt=PROMPT, fix error reporting
d2055acsage.features: Add features PackageSystem, SagePackageSystem, PipPackageSystem, use them in Feature.resolution

Changed commit from 2a8e3b7 to d2055ac

Branch pushed to git repo; I updated commit sha1. New commits:

fa10a90src/sage/features/__init__.py: Add doctests

Changed commit from d2055ac to fa10a90

Description changed:

--- 
+++ 
@@ -4,5 +4,6 @@
 
 Moreover, if `SAGE_ROOT/bin/sage-spkg` is not available, then we remove the recommendation to use `sage -i`. This is for the benefit of distribution packaging.
 
+Finally, for Python packages that have a `requirements.txt` (see #30024), we also show a `pip` invocation.
+ 
 
-
comment:8

I immediately thought that the doctests would fail in sage-on-gentoo and more generally in sage-on-distros. And I was right.

sage -t --long --warn-long 112.3 --random-seed=0 /usr/lib/python3.7/site-packages/sage/features/__init__.py
**********************************************************************
File "/usr/lib/python3.7/site-packages/sage/features/__init__.py", line 43, in sage.features
Failed example:
    Executable(name="random", executable="randomOochoz6x", spkg="random", url="http://rand.om").require()
Expected:
    Traceback (most recent call last):
    ...
    FeatureNotPresentError: random is not available.
    Executable 'randomOochoz6x' not found on PATH.
    ...try to run...sage -i random...
    Further installation instructions might be available at http://rand.om.
Got:
    <BLANKLINE>
    Traceback (most recent call last):
      File "/usr/lib/python3.7/site-packages/sage/doctest/forker.py", line 720, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/lib/python3.7/site-packages/sage/doctest/forker.py", line 1145, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.features[5]>", line 1, in <module>
        Executable(name="random", executable="randomOochoz6x", spkg="random", url="http://rand.om").require()
      File "/usr/lib/python3.7/site-packages/sage/features/__init__.py", line 192, in require
        raise FeatureNotPresentError(self, presence.reason, presence.resolution)
    sage.features.FeatureNotPresentError: random is not available.
    Executable 'randomOochoz6x' not found on PATH.
    No equivalent system packages for pip are known to Sage.
    Further installation instructions might be available at http://rand.om.
**********************************************************************
File "/usr/lib/python3.7/site-packages/sage/features/__init__.py", line 219, in sage.features.Feature.resolution
Failed example:
    Executable(name="CSDP", spkg="csdp", executable="theta", url="http://github.org/dimpase/csdp").resolution()
Expected:
    l'...To install CSDP...you can try to run...sage -i csdp...Further installation instructions might be available at http://github.org/dimpase/csdp.'
Got:
    l'No equivalent system packages for pip are known to Sage.\nFurther installation instructions might be available at http://github.org/dimpase/csdp.'
**********************************************************************
File "/usr/lib/python3.7/site-packages/sage/features/__init__.py", line 307, in sage.features.FeatureTestResult
Failed example:
    str(FeatureTestResult(package, True).resolution)
Expected:
    '...To install GAP package NOT_A_PACKAGE...you can try to run...sage -i no_package...'
Got:
    'No equivalent system packages for pip are known to Sage.'
**********************************************************************
File "/usr/lib/python3.7/site-packages/sage/features/__init__.py", line 309, in sage.features.FeatureTestResult
Failed example:
    str(FeatureTestResult(package, False).resolution)
Expected:
    '...To install GAP package NOT_A_PACKAGE...you can try to run...sage -i no_package...'
Got:
    'No equivalent system packages for pip are known to Sage.'
**********************************************************************
File "/usr/lib/python3.7/site-packages/sage/features/__init__.py", line 623, in sage.features.StaticFile
Failed example:
    StaticFile(name="no_such_file", filename="KaT1aihu", search_path=("/",), spkg="some_spkg", url="http://rand.om").require()
Expected:
    Traceback (most recent call last):
    ...
    FeatureNotPresentError: no_such_file is not available.
    'KaT1aihu' not found in any of ['/']...
    To install no_such_file...you can try to run...sage -i some_spkg...
    Further installation instructions might be available at http://rand.om.
Got:
    <BLANKLINE>
    Traceback (most recent call last):
      File "/usr/lib/python3.7/site-packages/sage/doctest/forker.py", line 720, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/lib/python3.7/site-packages/sage/doctest/forker.py", line 1145, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.features.StaticFile[1]>", line 1, in <module>
        StaticFile(name="no_such_file", filename="KaT1aihu", search_path=("/",), spkg="some_spkg", url="http://rand.om").require()
      File "/usr/lib/python3.7/site-packages/sage/features/__init__.py", line 192, in require
        raise FeatureNotPresentError(self, presence.reason, presence.resolution)
    sage.features.FeatureNotPresentError: no_such_file is not available.
    'KaT1aihu' not found in any of ['/']
    No equivalent system packages for pip are known to Sage.
    Further installation instructions might be available at http://rand.om.
**********************************************************************
File "/usr/lib/python3.7/site-packages/sage/features/__init__.py", line 673, in sage.features.StaticFile.absolute_path
Failed example:
    StaticFile(name="no_such_file", filename="KaT1aihu", search_path=(), spkg="some_spkg", url="http://rand.om").absolute_path()
Expected:
    Traceback (most recent call last):
    ...
    FeatureNotPresentError: no_such_file is not available.
    'KaT1aihu' not found in any of []...
    To install no_such_file...you can try to run...sage -i some_spkg...
    Further installation instructions might be available at http://rand.om.
Got:
    <BLANKLINE>
    Traceback (most recent call last):
      File "/usr/lib/python3.7/site-packages/sage/doctest/forker.py", line 720, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/lib/python3.7/site-packages/sage/doctest/forker.py", line 1145, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.features.StaticFile.absolute_path[2]>", line 1, in <module>
        StaticFile(name="no_such_file", filename="KaT1aihu", search_path=(), spkg="some_spkg", url="http://rand.om").absolute_path()
      File "/usr/lib/python3.7/site-packages/sage/features/__init__.py", line 687, in absolute_path
        resolution=self.resolution())
    sage.features.FeatureNotPresentError: no_such_file is not available.
    'KaT1aihu' not found in any of []
    No equivalent system packages for pip are known to Sage.
    Further installation instructions might be available at http://rand.om.
**********************************************************************
5 items had failures:
   1 of   7 in sage.features
   1 of   3 in sage.features.Feature.resolution
   2 of  11 in sage.features.FeatureTestResult
   1 of   3 in sage.features.StaticFile
   1 of   4 in sage.features.StaticFile.absolute_path
    [118 tests, 6 failures, 9.26 s]
----------------------------------------------------------------------
sage -t --long --warn-long 112.3 --random-seed=0 /usr/lib/python3.7/site-packages/sage/features/__init__.py  # 6 doctests failed

At least that may tell you if things are working as you expected them.

comment:9

Thanks for testing! This code is meant to work also on distributions without patching. I'll try to fix it.

Changed commit from fa10a90 to c01aa8f

Branch pushed to git repo; I updated commit sha1. New commits:

14e9395SagePackageSystem: Rename to sage_spkg (underscore) to make it an allowed optional tag
5c0da17src/sage/doctest/control.py: Add available package systems to optional tags
c01aa8fsrc/sage/features/__init__.py: Mark tests that check for 'sage -i' optional - sage_spkg

Description changed:

--- 
+++ 
@@ -1,9 +1,12 @@
 Currently, if a `Feature` is keyed to an `spkg`, a `FeatureNotPresentError` recommends to use `sage -i` to install the package.
 
-When `SAGE_ROOT` (or `sage_bootstrap`) is available, we can do better: We can recommend equivalent system packages.
+When `SAGE_ROOT` (or `sage_bootstrap`) is available, we can do better: 
 
-Moreover, if `SAGE_ROOT/bin/sage-spkg` is not available, then we remove the recommendation to use `sage -i`. This is for the benefit of distribution packaging.
+1. We can recommend equivalent system packages.
 
-Finally, for Python packages that have a `requirements.txt` (see #30024), we also show a `pip` invocation.
+2. Moreover, if `SAGE_ROOT/bin/sage-spkg` is not available, then we remove the recommendation to use `sage -i`. This is for the benefit of distribution packaging.
+
+3. Finally, for Python packages that have a `requirements.txt` (see #30024), we also show a `pip` invocation.
  
 
+We also add the names of the detected package systems to the default optional tags in use by doctests.

Description changed:

--- 
+++ 
@@ -2,11 +2,16 @@
 
 When `SAGE_ROOT` (or `sage_bootstrap`) is available, we can do better: 
 
-1. We can recommend equivalent system packages.
+1. We can recommend equivalent system packages for package systems available on the present system.
 
 2. Moreover, if `SAGE_ROOT/bin/sage-spkg` is not available, then we remove the recommendation to use `sage -i`. This is for the benefit of distribution packaging.
 
+   (Distribution packages would typically not install `sage-spkg`, so this information is automatically suppressed -- no more patching is necessary.)
+
 3. Finally, for Python packages that have a `requirements.txt` (see #30024), we also show a `pip` invocation.
  
+(1. and 3. depend on a few scripts from `SAGE_ROOT/build/bin` to be available in the `PATH`. Distribution packages could consider to install these scripts; perhaps, in some libexec-style directory.  Also, `SAGE_ROOT/build/pkgs/*/requirements.txt` and `SAGE_ROOT/build/pkgs/*/distros/` need to be made available at runtime.)
+
 
 We also add the names of the detected package systems to the default optional tags in use by doctests.
+

Description changed:

--- 
+++ 
@@ -1,6 +1,6 @@
 Currently, if a `Feature` is keyed to an `spkg`, a `FeatureNotPresentError` recommends to use `sage -i` to install the package.
 
-When `SAGE_ROOT` (or `sage_bootstrap`) is available, we can do better: 
+When `SAGE_ROOT` (or `sage_bootstrap`, see #29039) is available, we can do better: 
 
 1. We can recommend equivalent system packages for package systems available on the present system.
 
comment:15

With Ubuntu 18.04 + 9.2beta12 + #30053 (so that it uses sage's python 3.8 instead of system 3.6 which can't get the doc to build) + #30606, I obtain what's below.

I don't know if it is noise yet, so, I pasted it here instead: https://groups.google.com/g/sage-devel/c/_8oQ4T9WxD4/m/S0ywrlUNAwAJ

comment:16

Replying to @seblabbe:

With Ubuntu 18.04 + 9.2beta12 + #30053 (so that it uses sage's python 3.8 instead of system 3.6 which can't get the doc to build) + #30606, I obtain what's below.

I don't know if it is noise yet, so, I pasted it here instead: https://groups.google.com/g/sage-devel/c/_8oQ4T9WxD4/m/S0ywrlUNAwAJ

IMHO it's better to put "noise" here - you can edit later on - than on a list with a much larger readership.

Anyhow, changing Python needs a big rebuild (of everything depending on Python), so the error looks unsurprising.

comment:17

It was noise, sorry. Yes, you are right, I should have put it here, and erased it after...

comment:18

I get error building the documentation:

[dochtml] OSError: /home/slabbe/GitBox/sage/local/lib/python3.8/site-packages/sage/features/__init__.py:docstrin
g of sage.features.PackageSystem.spkg_installation_hint:9: WARNING: Block quote ends without a blank line; unexp
ected unindent.
comment:19

Many docstrings are starting with """ instead of r""". Maybe that's the issue?

comment:20

Yes it is. I pushed my current branch #30053 + #30606 rebased on top of #30053 + 1 commit fixing the docstring here: u/slabbe/30606 on which doc builds.

Branch pushed to git repo; I updated commit sha1. New commits:

5ef159c30606: fixing docstring """ -> r"""

Changed commit from c01aa8f to 5ef159c

comment:22

Thank you! I have cherry-picked your commit onto my branch.

comment:24

Patchbot shows the following doctest failing:

sage -t --long --random-seed=0 src/sage/doctest/control.py
**********************************************************************
File "src/sage/doctest/control.py", line 777, in sage.doctest.control.DocTestController.expand_files_into_sources
Failed example:
    sorted(DC.sources[0].options.optional)  # abs tol 1
Expected:
    ['guava', 'magma', 'py2']
Got:
    ['debian', 'guava', 'magma', 'pip', 'py3', 'sage_spkg']
**********************************************************************
File "src/sage/doctest/control.py", line 1009, in sage.doctest.control.DocTestController._optional_tags_string
Failed example:
    DC._optional_tags_string()
Expected:
    'openssl,sage'
Got:
    'debian,openssl,pip,sage,sage_spkg'
**********************************************************************
2 items had failures:
   1 of   8 in sage.doctest.control.DocTestController._optional_tags_string
   1 of  23 in sage.doctest.control.DocTestController.expand_files_into_sources
    [208 tests, 2 failures, 4.49 s]
----------------------------------------------------------------------
sage -t --long --random-seed=0 src/sage/doctest/control.py  # 2 doctests failed
----------------------------------------------------------------------

Also reported by the patchbot, EXAMPLE:: should always be plural EXAMPLES::.

Changed commit from 5ef159c to 68a8e2a

Branch pushed to git repo; I updated commit sha1. New commits:

d344adfMerge tag '9.2.beta13' into t/30606/sage_features_feature_resolution__if_sage_root_is_available__recommend_system_packages
a854438DocTestController.__init__: Only add package systems when 'optional' is used
68a8e2asrc/sage/features/__init__.py: Fix patchbot / relint warnings
comment:28

I did few tests, I get:

sage: from sage.features import package_systems
sage: package_systems()
[Feature('debian'), Feature('sage_spkg'), Feature('pip')]

So this looks good:

sage: from sage.features import PackageSystem
sage: PackageSystem('condaa').is_present()
FeatureTestResult('condaa', False)
sage: PackageSystem('conda').is_present()
FeatureTestResult('conda', False)
sage: PackageSystem('debian').is_present()
FeatureTestResult('debian', True)

It seems one needs to avoid PackageSystem('sage_spkg'):

sage: from sage.features import SagePackageSystem
sage: A = PackageSystem('sage_spkg')
sage: B = SagePackageSystem()
sage: A
Feature('sage_spkg')
sage: B
Feature('sage_spkg')
sage: A.is_present()        # expecting True
FeatureTestResult('sage_spkg', False)
sage: B.is_present()
FeatureTestResult('sage_spkg', True)

Similarly,

sage: PackageSystem('pip').is_present()   # I was expecting True here
FeatureTestResult('pip', False)

The following looks good:

sage: PackageSystem('pip').spkg_installation_hint('jupyterlab')
'To install jupyterlab using the pip package manager, you can try to run:\n!sage -pip install jupyterlab~=2.2.5'
sage: from sage.features import PipPackageSystem
sage: PipPackageSystem().is_present()
FeatureTestResult('pip', True)
sage: PipPackageSystem().spkg_installation_hint('jupyterlab')
'To install jupyterlab using the pip package manager, you can try to run:\n!sage -pip install jupyterlab~=2.2.5'
sage: SagePackageSystem().spkg_installation_hint('jupyterlab')
'To install jupyterlab using the Sage package manager, you can try to run:\n  !sage -i jupyterlab'

The following does not look good (or maybe it is?):

sage: PackageSystem('debian').spkg_installation_hint('jupyterlab')
'No equivalent system packages for debian are known to Sage.'
comment:29

Well, I understand the idea is to use SagePackageSystem() instead of PackageSystem('sage_spkg')
and PipPackageSystem() instead of PackageSystem('pip').

comment:30

Looks good. Doc builds ok. doctests failures are fixed.

Reviewer: Sébastien Labbé

comment:32

Replying to @seblabbe:

It seems one needs to avoid PackageSystem('sage_spkg'):

sage: from sage.features import SagePackageSystem
sage: A = PackageSystem('sage_spkg')
sage: B = SagePackageSystem()
sage: A
Feature('sage_spkg')
sage: B
Feature('sage_spkg')
sage: A.is_present()        # expecting True
FeatureTestResult('sage_spkg', False)
sage: B.is_present()
FeatureTestResult('sage_spkg', True)

Similarly,

sage: PackageSystem('pip').is_present()   # I was expecting True here
FeatureTestResult('pip', False)

I didn't implement delegation to the correct class by name -- note that also Feature does not do that even though the default repr seems to suggest unique representation behavior of this kind:

sage: from sage.features.bliss import Bliss                                                                                                                                                                               
sage: Bliss()                                                                                                                                                                                                             
Feature('sage.graphs.bliss')
sage: Bliss() is Bliss()                                                                                                                                                                                                  
True
sage: from sage.features import Feature                                                                                                                                                                                   
sage: Bliss() is Feature('sage.graphs.bliss')                                                                                                                                                                             
False
comment:33

Replying to @seblabbe:

The following does not look good (or maybe it is?):

sage: PackageSystem('debian').spkg_installation_hint('jupyterlab')
'No equivalent system packages for debian are known to Sage.'

It is correct because so far there is no system package information for this package -- build/pkgs/jupyterlab/distros does not exist.

(See #30124 - System package information, spkg-configure for Jupyter notebook package, rst2ipynb, and dependencies)

comment:34

Replying to @seblabbe:

I understand the idea is to use SagePackageSystem() instead of PackageSystem('sage_spkg')
and PipPackageSystem() instead of PackageSystem('pip').

That's right.

comment:35

Thanks for the review!

comment:36

I was wanting to have another go at this but I am flat out right now. I'll open follow up as needed if it gets merged as is.

slel commented

Changed commit from 68a8e2a to none

slel commented
comment:38

This breaks some tests when building with system Python 3.6.x, see

Follow-up ticket for that: #30740.