sage_bootstrap: Update/extend "sage -package", fix "make download"

[mkoeppe]

(from #29146, #30861)

There are several places where the spkg (build/pkgs/*/) and system package information (build/pkgs/*/distros/*.txt) is parsed:

• bootstrap
• src/doc/bootstrap
• build/bin/write_dockerfile.sh
• build/bin/sage-get-system-packages
• tox.ini (for local-homebrew, local-conda)
• .github/workflows/ci-wsl.yml (using powershell)
• src/bin/sage-download-upstream
• m4/sage_spkg_*.m4

The purpose of this ticket is to prepare refactoring of this code by extending the sage_bootstrap package.

• We update sage_bootstrap.package so that it understands non-normal packages (i.e., script and pip packages, which do not have checksums.ini)
• We change sage -package list so it supports the following invocations:
  • sage -package list :standard: :optional: (several "classes" (types), not just one)
  • sage -package list --has-file=spkg-configure.m4 :standard: (needed for bootstrap, src/doc/bootstrap)
  • sage -package list --has-file=spkg-configure.m4 --has-file=distros/debian.txt :standard: :optional: (needed for build/bin/write-dockerfile.sh, tox.ini)
  • sage -package list --has-file=SPKG.rst :all: (needed for src/doc/bootstrap)
• We fix and extend sage -package download (follow up from #30648) so it supports
  • sage -package download :all: (again - was broken)
  • sage -package download --allow-upstream :all: (allows retrieving from upstream_url) and use it for make download (#29896), removing src/bin/sage-download-upstream

To run the testsuite of sage_bootstrap, use

  (cd build && tox)

Follow-up:

• The actual refactoring of the above scripts (#30947, #30951, #30968, ...)
• Wishlist item: sage -package list --not-source=pip :optional: :experimental: (would simplify bootstrap)

[mkoeppe]

Developers interested in improving the system package advice may want to work on this ticket

[mkoeppe]

New commits:

​2265048	sage_bootstrap.package: Handle non-normal packages (which have no checksums.ini etc.)
​c405d10	sage_bootstrap.package.Package._init_type: Do not accept 'script' or 'pip' any more

[git]

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

​e463513	30648: download_cls -> download
​0b343d1	Merge commit 'e4635137337490ce452945728c8667f3f51b5b40' of git://trac.sagemath.org/sage into t/30865/sage_bootstrap__update_extend_system_package_tools
​5fc8441	sage_bootstrap.app.Application.download_cls: Pass allow_upstream to download
​794a15c	sage_bootstrap.cndline: Back to using download_cls
​6369bb4	sage -package list: Handle --has-file, multiple package classes (types)
​4fbf753	src/doc/en/installation/source.rst: Remove outdated documentation on old-style packages
​7082715	Makefile: Update documentation of 'make download'
​3f91313	src/bin/sage-download-upstream: Remove, use 'sage --package download' instead

[git]

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

​69f7ea8	sage_bootstrap.PackageClass: Accept several arguments, simplify use
​40d6604	sage_bootstrap.package.Package.has_file: New, use it
​6475263	sage_bootstrap.PackageClass: Handle filter has_files, simplify use
​769f635	sage_bootstrap.tarball: Raise an error if attempting to download a tarball of a non-normal package
​f6b79bb	sage_bootstrap.app.download_cls: Only attempt to download tarballs for normal packages (with checksums.ini)

[git]

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

​4be654b

sage_bootstrap.cmdline: Initialize has_files filter correctly

[mkoeppe]

Ready for review

[slabbe]

The current branch is doing this:

-    def download_cls(self, package_name_or_class):
+    def download_cls(self, package_name_or_class, allow_upstream=False):

So should the change I made in #30648 be reverted then?

(I recall that the problem fixed in #30648 was that download_cls method had no argument allow_upstream where download method did, so I changed download_cls to download. But at the time, I was wondering if that was the good fix. Because another fix could have been to add the allow_upstream argument to the download_cls method which you are doing in this branch)

[slabbe]

Doing += on lists creates a new list in memory as opposed to append or extend:

sage: L = list(range(10))                                                       
sage: K = list('abcd')                                                          
sage: id(L)                                                                     
140556349258880
sage: id(L+K)                                                                   
140556349147776
sage: L.extend(K)                                                               
sage: id(L)                                                                     
140556349258880

So using += to append elements to a list uses a quadratic amount of memory until the garbage collector gets called.

So I would suggest the following change:

- self.names += [package_name_or_class]
+ self.names.append(package_name_or_class)

as well as (5 times):

- self.names += [pkg.name for pkg in Package.all() if predicate(pkg)]
+ self.names.extend(pkg.name for pkg in Package.all() if predicate(pkg))

[slabbe]

I will try the branch later today, and come back with further comments if I have.

[mkoeppe]

Replying to slabbe:

The current branch is doing this:

-    def download_cls(self, package_name_or_class):
+    def download_cls(self, package_name_or_class, allow_upstream=False):

So should the change I made in #30648 be reverted then?

Yes, in fact I have already reverted it as part of the branch on this ticket.

[git]

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

​b762c3d

sage_bootstrap.expand_class: Use .append, .extend instead of += for lists

[slabbe]

Replying to mkoeppe:

Yes, in fact I have already reverted it as part of the branch on this ticket.

Ok, yes, I agree. Since #30648 is not in sage yet, this is why I don't see it in the diff.

[slabbe]

(cd build && tox)

I did that. It finishes with a big list of typos and says that there were errors while running the tests. When I scroll back, I can't see the errors, because the typos takes all of the lines in the finite history of the terminal. Are the output of tox logged somewhere?

I find it weird to call it class of package and use cls (which is often use for representing a Python class. Maybe nature of the package? Anyway, this is out of topic.

(0) I confirm that sage --package list works with inputs such as --has-file=spkg-configure.m4, --has-file=distros/debian.txt, --has-file=SPKG.rst. Also sage --package download works. Also make download works.

Sorry, I still have few more comments listed below.

(1) When I read the documentation of sage --package, I don't know what Sage Bootstrap Library means. I suggest to add a sentence in the file sage_bootstrap.cmdline.py to explain it. Something as follows, but please replace the verb deal with any more precise verb:

 description = \
 """
 Sage Bootstrap Library
+
+It deals with all of the packages in SageMath.
 """

(2) I don't like the following error message:

$ sage --package list :standardd:
Traceback (most recent call last):
  File "/home/slabbe/GitBox/sage/build/bin/sage-package", line 42, in <module>
    run()
  File "/home/slabbe/GitBox/sage/build/bin/../sage_bootstrap/cmdline.py", line 306, in run
    app.list_cls(*args.package_class, has_files=args.has_files)
  File "/home/slabbe/GitBox/sage/build/bin/../sage_bootstrap/app.py", line 69, in list_cls
    pc = PackageClass(*package_classes, **filters)
  File "/home/slabbe/GitBox/sage/build/bin/../sage_bootstrap/expand_class.py", line 48, in __init__
    raise ValueError('Package name cannot start with ":", got %s', package_name_or_class)
ValueError: ('Package name cannot start with ":", got %s', ':standardd:')

It tells me the input cannot start with ":", but this is false, as :standard: is accepted.

Thus, I would suggest the following change in sage_bootstrap/expand_class.py:

             else:
-                if package_name_or_class.startswith(':'):
-                    raise ValueError('Package name cannot start with ":", got %s', package_name_or_class)
-                if package_name_or_class.endswith(':'):
-                    raise ValueError('Package name cannot end with ":", got %s', package_name_or_class)
+                if package_name_or_class.startswith(':') or package_name_or_class.endswith(':'):
+                    raise ValueError('Valid package classes are :all:, :standard:, :optional:, :experimental: or :huge:, got %s', package_name_or_class)
                 self.names.append(package_name_or_class)

Question: do you confirm that the following is a desirable feature?

$ sage --package list pari
pari

(3) Finally, in build/sage_bootstrap/package.py, I suggest:

-        if pattern:
-            return self.tarball_pattern.replace('VERSION', self.version)
+        if pattern:
+            return pattern.replace('VERSION', self.version)

(4) Question: What is the meaning of the expression non-normal in the file build/sage_bootstrap/tarball.py ?

I am currently waiting for the report of make ptestlong.

[slabbe]

The make ptestlong finished with:

----------------------------------------------------------------------
sage -t --long --random-seed=0 src/sage/interfaces/singular.py  # Killed due to segmentation fault
----------------------------------------------------------------------

which is unrelated to this ticket.

[slelievre]

Replying to slabbe:

The make ptestlong finished with:

----------------------------------------------------------------------
sage -t --long --random-seed=0 src/sage/interfaces/singular.py  # Killed due to segmentation fault
----------------------------------------------------------------------

which is unrelated to this ticket.

Indeed that is different, and tracked at #30945.

[mkoeppe]

Replying to slabbe:

(cd build && tox)

I did that. It finishes with a big list of typos

If it shows typos, then you probably ran sage -tox, not tox. If you don't have tox in your global environment, you can use (cd build && sage -sh -c tox).

Are the output of tox logged somewhere?

Yes, it creates a subdirectory .tox, where you'll find logs.

[git]

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

​51db2a9

sage_bootstrap: Eliminate terminology 'package class' in help and error messages

[mkoeppe]

Replying to slabbe:

I find it weird to call it class of package and use cls (which is often use for representing a Python class. Maybe nature of the package? Anyway, this is out of topic.

I agree. I have now made a change to eliminate this (unexplained) terminology from user-facing messages. But I haven't changed the class name PackageClass or argument names, to keep the diff small.

[mkoeppe]

Replying to slabbe:

(4) Question: What is the meaning of the expression non-normal in the file build/sage_bootstrap/tarball.py ?

This is referring to ​https://doc.sagemath.org/html/en/developer/packaging.html#package-source-types

[mkoeppe]

Replying to slabbe:

I would suggest the following change in sage_bootstrap/expand_class.py:

             else:
-                if package_name_or_class.startswith(':'):
-                    raise ValueError('Package name cannot start with ":", got %s', package_name_or_class)
-                if package_name_or_class.endswith(':'):
-                    raise ValueError('Package name cannot end with ":", got %s', package_name_or_class)
+                if package_name_or_class.startswith(':') or package_name_or_class.endswith(':'):
+                    raise ValueError('Valid package classes are :all:, :standard:, :optional:, :experimental: or :huge:, got %s', package_name_or_class)
                 self.names.append(package_name_or_class)

I have made a similar change in the above commit.

Question: do you confirm that the following is a desirable feature?

$ sage --package list pari
pari

Yes, this is a feature, and the above commit documents it.

[git]

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

​c0a0329

sage_bootstrap.cmdline: Expand description

[slabbe]

Replying to mkoeppe:

If it shows typos, then you probably ran sage -tox, not tox. If you don't have tox in your global environment, you can use (cd build && sage -sh -c tox).

Well, I first did sudo apt install tox on a Ubuntu 18.04 but, then I got problems because that tox is using python2. So, your guess is right, that was after the run of sage -tox.

Are the output of tox logged somewhere?

Yes, it creates a subdirectory .tox, where you'll find logs.

Great. Thank you.

[slabbe]

Thanks for the answers and commits, I will check this tomorrow morning Bordeaux time.

[I think my comment (3) is still not fixed (well, it is just a small detail).]

[mkoeppe]

Replying to slabbe:

I first did sudo apt install tox on a Ubuntu 18.04 but, then I got problems because that tox is using python2.

That should actually work too. sage_bootstrap is designed to work on a wide range of Python versions including 2.x. tox provisions virtual environments to test the package with each available Python version. To run it with a specific Python version, you can do tox -e py37 etc. It does not matter on what version of Python tox itself runs.

[git]

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

​f33c363

sage_bootstrap.package.Package.tarball_pattern: Simplify code

[slabbe]

Replying to mkoeppe:

Replying to slabbe:

I first did sudo apt install tox on a Ubuntu 18.04 but, then I got problems because that tox is using python2.

That should actually work too. sage_bootstrap is designed to work on a wide range of Python versions including 2.x. tox provisions virtual environments to test the package with each available Python version. To run it with a specific Python version, you can do tox -e py37 etc. It does not matter on what version of Python tox itself runs.

Ok, I see. But after cd build, the tox command prints few lines and seems to get stuck:

$ cd build
$ tox
GLOB sdist-make: /home/slabbe/GitBox/sage/build/setup.py
py26 create: /home/slabbe/GitBox/sage/build/.tox/py26
ERROR: InterpreterNotFound: python2.6
py27 create: /home/slabbe/GitBox/sage/build/.tox/py27
py27 inst: /home/slabbe/GitBox/sage/build/.tox/dist/sage_bootstrap-1.0.zip
py27 installed: DEPRECATION: Python 2.7 reached the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 is no longer maintained. pip 21.0 will drop support for Python 2.7 in January 2021. More details about Python 2 support in pip can be found at https://pip.pypa.io/en/latest/development/release-process/#python-2-support pip 21.0 will remove support for this functionality.,pkg-resources==0.0.0,sage-bootstrap @ file:///home/slabbe/GitBox/sage/build/.tox/dist/sage_bootstrap-1.0.zip
py27 runtests: PYTHONHASHSEED='3230691582'
py27 runtests: commands[0] | python2.7 -m unittest discover
......

[slabbe]

Actually, it did not get stuck, but I obtain three errors after few minutes:

$ tox
GLOB sdist-make: /home/slabbe/GitBox/sage/build/setup.py
py26 create: /home/slabbe/GitBox/sage/build/.tox/py26
ERROR: InterpreterNotFound: python2.6
py27 create: /home/slabbe/GitBox/sage/build/.tox/py27
py27 inst: /home/slabbe/GitBox/sage/build/.tox/dist/sage_bootstrap-1.0.zip
py27 installed: DEPRECATION: Python 2.7 reached the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 is no longer maintained. pip 21.0 will drop support for Python 2.7 in January 2021. More details about Python 2 support in pip can be found at https://pip.pypa.io/en/latest/development/release-process/#python-2-support pip 21.0 will remove support for this functionality.,pkg-resources==0.0.0,sage-bootstrap @ file:///home/slabbe/GitBox/sage/build/.tox/dist/sage_bootstrap-1.0.zip
py27 runtests: PYTHONHASHSEED='3230691582'
py27 runtests: commands[0] | python2.7 -m unittest discover
......FF..............[xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx]
[......................................................................]
......F........ERROR [package|all:249]: Failed to open pari
..
======================================================================
FAIL: test_error (test.test_download.DownloadTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/slabbe/GitBox/sage/build/test/test_download.py", line 49, in test_error
    self.assertIsNotFoundError(log.messages())
  File "/home/slabbe/GitBox/sage/build/test/test_download.py", line 64, in assertIsNotFoundError
    "[Errno 404] Not Found: '//files.sagemath.org/sage_bootstrap/this_url_does_not_exist'"))
AssertionError: False is not true

======================================================================
FAIL: test_ignore_errors (test.test_download.DownloadTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/slabbe/GitBox/sage/build/test/test_download.py", line 57, in test_ignore_errors
    self.assertIsNotFoundError(log.messages())
  File "/home/slabbe/GitBox/sage/build/test/test_download.py", line 64, in assertIsNotFoundError
    "[Errno 404] Not Found: '//files.sagemath.org/sage_bootstrap/this_url_does_not_exist'"))
AssertionError: False is not true

======================================================================
FAIL: test_checksum (test.test_tarball.TarballTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/slabbe/GitBox/sage/build/test/test_tarball.py", line 59, in test_checksum
    '[......................................................................]\n')
AssertionError: '[xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx]\n[......................................................................]\n' != '[......................................................................]\n'

----------------------------------------------------------------------
Ran 39 tests in 228.835s

FAILED (failures=3)

After that it seems tox does the same with other version of Python it finds on the system. With python 3.6, it returns two errors instead of three (the test_checksum does not fail).

Let me now check whether this is related to the current ticket or not by rerunning it after a git checkout develop.

[slabbe]

I also get errors with 9.3.beta1, but not exactly the same. test_ignore_errors and test_error both fails as with the branch. But test_download fails only on 9.3.beta1 whereas test_checksum fails only with the current branch.

======================================================================
FAIL: test_error (test.test_download.DownloadTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/slabbe/GitBox/sage/build/test/test_download.py", line 49, in test_error
    self.assertIsNotFoundError(log.messages())
  File "/home/slabbe/GitBox/sage/build/test/test_download.py", line 63, in assertIsNotFoundError
    self.assertTrue(messages[0][1].endswith(
AssertionError: False is not true

======================================================================
FAIL: test_ignore_errors (test.test_download.DownloadTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/slabbe/GitBox/sage/build/test/test_download.py", line 57, in test_ignore_errors
    self.assertIsNotFoundError(log.messages())
  File "/home/slabbe/GitBox/sage/build/test/test_download.py", line 63, in assertIsNotFoundError
    self.assertTrue(messages[0][1].endswith(
AssertionError: False is not true

======================================================================
FAIL: test_download (test.test_package_cmdline.SagePackageTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/slabbe/GitBox/sage/build/test/test_package_cmdline.py", line 115, in test_download
    self.assertTrue(stderr.startswith('Using cached file'))
AssertionError: False is not true

----------------------------------------------------------------------
Ran 39 tests in 134.424s

FAILED (failures=3)

I don't know how much this is related to the current ticket. From my point of view, I give a positive review to the commits done up to now.

Matthias, I let you change the status to positive review if you think the tox issues are unrelated or if they can be dealt in another ticket.

[mkoeppe]

Thank you!

(I also get the additional test_checksum failure on this branch. This test only passes on a release tag; this is not specific to this ticket. The other failures, also not specific to this ticket, should be investigated on a separate ticket.)

[slelievre]

I tried make download and it failed to download scipoptsuite.

Which is expected. Indeed, reading from the last scipoptsuite ticket:

• #24662: Upgrade scipoptsuite to 5.0.1

Upstream archive: <snip> (DO NOT put on sage servers -- we cannot redistribute this archive)

How do we deal with that?

[mkoeppe]

Replying to slelievre:

(DO NOT put on sage servers -- we cannot redistribute this archive)

How do we deal with that?

Let's discuss this on #31034