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

mkoeppe · 2020-11-04T22:01:05Z

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 sage --package download is broken #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 (fix 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 (src/doc/bootstrap: Simplify by using new options of "sage -package list" #30947, tox.ini, build/bin/write-dockerfile.sh: Simplify by using new options of "sage -package list" #30951, bootstrap: Simplify by using new options of "sage -package list" #30968, ...)
Wishlist item: sage -package list --not-source=pip :optional: :experimental: (would simplify bootstrap)

Depends on #30648

CC: @seblabbe @tobiasdiez @slel @jhpalmieri @vbraun

Component: build: configure

Author: Matthias Koeppe

Branch: f33c363

Reviewer: Sébastien Labbé

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

The text was updated successfully, but these errors were encountered:

mkoeppe · 2020-11-05T18:27:05Z

comment:1

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

mkoeppe · 2020-11-19T17:58:45Z

Author: Matthias Koeppe

mkoeppe · 2020-11-19T18:04:45Z

Branch: u/mkoeppe/sage_bootstrap__update_extend_system_package_tools

mkoeppe · 2020-11-19T18:45:39Z

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`

mkoeppe · 2020-11-19T18:45:39Z

Commit: c405d10

mkoeppe · 2020-11-19T18:56:52Z

Dependencies: #30648

sagetrac-git · 2020-11-19T20:12:02Z

Changed commit from c405d10 to 3f91313

sagetrac-git · 2020-11-19T20:12:02Z

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`

sagetrac-git · 2020-11-19T20:42:24Z

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)`

sagetrac-git · 2020-11-19T20:42:24Z

Changed commit from 3f91313 to f6b79bb

sagetrac-git · 2020-11-19T20:54:34Z

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

`4be654b`	`sage_bootstrap.cmdline: Initialize has_files filter correctly`

sagetrac-git · 2020-11-19T20:54:34Z

Changed commit from f6b79bb to 4be654b

mkoeppe · 2020-11-19T20:56:56Z

comment:13

Ready for review

seblabbe · 2020-11-22T09:18:49Z

comment:16

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)

seblabbe · 2020-11-22T09:49:33Z

comment:17

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))

seblabbe · 2020-11-23T14:41:59Z

Reviewer: Sébastien Labbé

mkoeppe · 2020-11-23T18:29:23Z

comment:28

Replying to @seblabbe:

(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.

sagetrac-git · 2020-11-23T19:01:53Z

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

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

sagetrac-git · 2020-11-23T19:01:53Z

Changed commit from b762c3d to 51db2a9

mkoeppe · 2020-11-23T19:04:00Z

comment:30

Replying to @seblabbe:

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 · 2020-11-23T19:05:33Z

comment:31

Replying to @seblabbe:

(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 · 2020-11-23T19:07:56Z

comment:32

Replying to @seblabbe:

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.

sagetrac-git · 2020-11-23T19:19:36Z

Changed commit from 51db2a9 to c0a0329

sagetrac-git · 2020-11-23T19:19:36Z

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

`c0a0329`	`sage_bootstrap.cmdline: Expand description`

seblabbe · 2020-11-23T21:48:16Z

comment:35

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.

seblabbe · 2020-11-23T21:53:19Z

comment:36

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 · 2020-11-23T23:25:48Z

comment:37

Replying to @seblabbe:

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.

sagetrac-git · 2020-11-23T23:28:20Z

Changed commit from c0a0329 to f33c363

sagetrac-git · 2020-11-23T23:28:20Z

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

`f33c363`	`sage_bootstrap.package.Package.tarball_pattern: Simplify code`

seblabbe · 2020-11-24T08:03:34Z

comment:39

Replying to @mkoeppe:

Replying to @seblabbe:

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
......

seblabbe · 2020-11-24T08:11:02Z

comment:40

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.

seblabbe · 2020-11-24T08:32:01Z

comment:41

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 · 2020-11-24T17:28:29Z

comment:42

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.)

vbraun · 2020-11-29T11:57:50Z

Changed branch from u/mkoeppe/sage_bootstrap__update_extend_system_package_tools to f33c363

slel · 2020-12-10T02:18:07Z

comment:45

I tried make download and it failed to download scipoptsuite.

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

Upgrade scipoptsuite to 5.0.1 #24662: Upgrade scipoptsuite to 5.0.1

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

How do we deal with that?

slel · 2020-12-10T02:18:07Z

Changed commit from f33c363 to none

mkoeppe · 2020-12-10T03:20:46Z

comment:46

Replying to @slel:

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

How do we deal with that?

Let's discuss this on #31034

mkoeppe added this to the sage-9.3 milestone Nov 4, 2020

mkoeppe added c: build: configure labels Nov 4, 2020

This comment has been minimized.

Sign in to view

mkoeppe changed the title ~~sage_bootstrap: Implement system package tools~~ sage_bootstrap: Update/extend system package tools Nov 19, 2020

This comment has been minimized.

Sign in to view

mkoeppe added the s: needs review label Nov 19, 2020

This comment has been minimized.

Sign in to view

mkoeppe changed the title ~~sage_bootstrap: Update/extend system package tools~~ sage_bootstrap: Update/extend "sage -package", fix "make download" Nov 19, 2020

mkoeppe added s: needs review and removed s: needs work labels Nov 23, 2020

mkoeppe added s: positive review and removed s: needs review labels Nov 24, 2020

This comment has been minimized.

Sign in to view

vbraun removed the s: positive review label Nov 29, 2020

vbraun closed this as completed in d47fefd Nov 29, 2020

dimpase mentioned this issue Dec 29, 2020

fix make download #29896

Closed

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

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

Comments

mkoeppe commented Nov 4, 2020

mkoeppe commented Nov 5, 2020

mkoeppe commented Nov 19, 2020

This comment has been minimized.

mkoeppe commented Nov 19, 2020

mkoeppe commented Nov 19, 2020

mkoeppe commented Nov 19, 2020

This comment has been minimized.

mkoeppe commented Nov 19, 2020

This comment has been minimized.

This comment has been minimized.

sagetrac-git mannequin commented Nov 19, 2020

sagetrac-git mannequin commented Nov 19, 2020

sagetrac-git mannequin commented Nov 19, 2020

sagetrac-git mannequin commented Nov 19, 2020

This comment has been minimized.

This comment has been minimized.

sagetrac-git mannequin commented Nov 19, 2020

sagetrac-git mannequin commented Nov 19, 2020

mkoeppe commented Nov 19, 2020

This comment has been minimized.

This comment has been minimized.

seblabbe commented Nov 22, 2020

seblabbe commented Nov 22, 2020

seblabbe commented Nov 23, 2020

mkoeppe commented Nov 23, 2020

sagetrac-git mannequin commented Nov 23, 2020

sagetrac-git mannequin commented Nov 23, 2020

mkoeppe commented Nov 23, 2020

mkoeppe commented Nov 23, 2020

mkoeppe commented Nov 23, 2020

sagetrac-git mannequin commented Nov 23, 2020

sagetrac-git mannequin commented Nov 23, 2020

seblabbe commented Nov 23, 2020

seblabbe commented Nov 23, 2020

mkoeppe commented Nov 23, 2020

sagetrac-git mannequin commented Nov 23, 2020

sagetrac-git mannequin commented Nov 23, 2020

seblabbe commented Nov 24, 2020

seblabbe commented Nov 24, 2020

seblabbe commented Nov 24, 2020

mkoeppe commented Nov 24, 2020

This comment has been minimized.

vbraun commented Nov 29, 2020

slel commented Dec 10, 2020

slel commented Dec 10, 2020

mkoeppe commented Dec 10, 2020