#30865 closed enhancement (fixed)
sage_bootstrap: Update/extend "sage package", fix "make download"
Reported by:  mkoeppe  Owned by:  

Priority:  major  Milestone:  sage9.3 
Component:  build: configure  Keywords:  
Cc:  slabbe, ghtobiasdiez, slelievre, jhpalmieri, vbraun  Merged in:  
Authors:  Matthias Koeppe  Reviewers:  Sébastien Labbé 
Report Upstream:  N/A  Work issues:  
Branch:  f33c363 (Commits, GitHub, GitLab)  Commit:  
Dependencies:  #30648  Stopgaps: 
Description (last modified by )
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/sagegetsystempackages
tox.ini
(forlocalhomebrew
,localconda
).github/workflows/ciwsl.yml
(using powershell)src/bin/sagedownloadupstream
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 nonnormal packages (i.e., script and pip packages, which do not havechecksums.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 hasfile=spkgconfigure.m4 :standard:
(needed forbootstrap
,src/doc/bootstrap
)sage package list hasfile=spkgconfigure.m4 hasfile=distros/debian.txt :standard: :optional:
(needed forbuild/bin/writedockerfile.sh
,tox.ini
)sage package list hasfile=SPKG.rst :all:
(needed forsrc/doc/bootstrap
)
 We fix and extend
sage package download
(follow up from #30648) so it supportssage package download :all:
(again  was broken)sage package download allowupstream :all:
(allows retrieving fromupstream_url
) and use it formake download
(#29896), removingsrc/bin/sagedownloadupstream
To run the testsuite of sage_bootstrap
, use
(cd build && tox)
Followup:
Change History (46)
comment:1 Changed 2 years ago by
comment:2 Changed 2 years ago by
Authors:  → Matthias Koeppe 

Description:  modified (diff) 
Summary:  sage_bootstrap: Implement system package tools → sage_bootstrap: Update/extend system package tools 
comment:3 Changed 2 years ago by
Branch:  → u/mkoeppe/sage_bootstrap__update_extend_system_package_tools 

comment:4 Changed 2 years ago by
Cc:  vbraun added 

Commit:  → c405d1026b35e181f114ebffb4b4452a0b22b75b 
Description:  modified (diff) 
comment:5 Changed 2 years ago by
Dependencies:  → #30648 

comment:6 Changed 2 years ago by
Description:  modified (diff) 

comment:7 Changed 2 years ago by
Description:  modified (diff) 

comment:8 Changed 2 years ago by
Commit:  c405d1026b35e181f114ebffb4b4452a0b22b75b → 3f9131304c9d0f93b76e6d85f1cbddb4be2e8c9c 

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 hasfile, multiple package classes (types)

4fbf753  src/doc/en/installation/source.rst: Remove outdated documentation on oldstyle packages

7082715  Makefile: Update documentation of 'make download'

3f91313  src/bin/sagedownloadupstream: Remove, use 'sage package download' instead

comment:9 Changed 2 years ago by
Commit:  3f9131304c9d0f93b76e6d85f1cbddb4be2e8c9c → f6b79bbc89bfe65a166dc0855ab62f93f90b7b88 

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 nonnormal package

f6b79bb  sage_bootstrap.app.download_cls: Only attempt to download tarballs for normal packages (with checksums.ini)

comment:10 Changed 2 years ago by
Description:  modified (diff) 

Status:  new → needs_review 
comment:11 Changed 2 years ago by
Description:  modified (diff) 

comment:12 Changed 2 years ago by
Commit:  f6b79bbc89bfe65a166dc0855ab62f93f90b7b88 → 4be654b22e7961c615b8103bad5c1f8832e18482 

Branch pushed to git repo; I updated commit sha1. New commits:
4be654b  sage_bootstrap.cmdline: Initialize has_files filter correctly

comment:14 Changed 2 years ago by
Description:  modified (diff) 

comment:15 Changed 2 years ago by
Summary:  sage_bootstrap: Update/extend system package tools → sage_bootstrap: Update/extend "sage package", fix "make download" 

comment:16 followup: 19 Changed 2 years ago by
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)
comment:17 Changed 2 years ago by
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))
comment:18 Changed 2 years ago by
I will try the branch later today, and come back with further comments if I have.
comment:19 followup: 23 Changed 2 years ago by
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.
comment:20 Changed 2 years ago by
Commit:  4be654b22e7961c615b8103bad5c1f8832e18482 → b762c3dfee999746ad0b8507460178ee6a63966b 

Branch pushed to git repo; I updated commit sha1. New commits:
b762c3d  sage_bootstrap.expand_class: Use .append, .extend instead of += for lists

comment:21 Changed 2 years ago by
Description:  modified (diff) 

comment:22 Changed 2 years ago by
Description:  modified (diff) 

comment:23 Changed 2 years ago by
comment:24 followups: 28 30 31 32 Changed 2 years ago by
Status:  needs_review → needs_work 

(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
hasfile=spkgconfigure.m4
, hasfile=distros/debian.txt
, hasfile=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/sagepackage", 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 nonnormal
in the file build/sage_bootstrap/tarball.py
?
I am currently waiting for the report of make ptestlong
.
comment:25 followup: 26 Changed 2 years ago by
The make ptestlong
finished with:
 sage t long randomseed=0 src/sage/interfaces/singular.py # Killed due to segmentation fault 
which is unrelated to this ticket.
comment:26 Changed 2 years ago by
Replying to slabbe:
The
make ptestlong
finished with: sage t long randomseed=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.
comment:27 Changed 2 years ago by
Reviewers:  → Sébastien Labbé 

comment:28 followup: 35 Changed 2 years ago by
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.
comment:29 Changed 2 years ago by
Commit:  b762c3dfee999746ad0b8507460178ee6a63966b → 51db2a9e3bbbe730846c00831991a3e7b8ff5b31 

Branch pushed to git repo; I updated commit sha1. New commits:
51db2a9  sage_bootstrap: Eliminate terminology 'package class' in help and error messages

comment:30 Changed 2 years ago by
Replying to slabbe:
I find it weird to call it
class
of package and usecls
(which is often use for representing a Python class. Maybenature
of the package? Anyway, this is out of topic.
I agree. I have now made a change to eliminate this (unexplained) terminology from userfacing messages. But I haven't changed the class name PackageClass
or argument names, to keep the diff small.
comment:31 Changed 2 years ago by
Replying to slabbe:
(4) Question: What is the meaning of the expression
nonnormal
in the filebuild/sage_bootstrap/tarball.py
?
This is referring to https://doc.sagemath.org/html/en/developer/packaging.html#packagesourcetypes
comment:32 Changed 2 years ago by
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.
comment:33 Changed 2 years ago by
Commit:  51db2a9e3bbbe730846c00831991a3e7b8ff5b31 → c0a0329d842ad8745789f8bf61bc456f0473e1fe 

Branch pushed to git repo; I updated commit sha1. New commits:
c0a0329  sage_bootstrap.cmdline: Expand description

comment:34 Changed 2 years ago by
Status:  needs_work → needs_review 

comment:35 followup: 37 Changed 2 years ago by
Replying to mkoeppe:
If it shows typos, then you probably ran
sage tox
, nottox
. 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.
comment:36 Changed 2 years ago by
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).]
comment:37 followup: 39 Changed 2 years ago by
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.
comment:38 Changed 2 years ago by
Commit:  c0a0329d842ad8745789f8bf61bc456f0473e1fe → f33c3635ba36440b260644c127dcc94e12c3ab40 

Branch pushed to git repo; I updated commit sha1. New commits:
f33c363  sage_bootstrap.package.Package.tarball_pattern: Simplify code

comment:39 Changed 2 years ago by
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 dotox 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 sdistmake: /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_bootstrap1.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/releaseprocess/#python2support pip 21.0 will remove support for this functionality.,pkgresources==0.0.0,sagebootstrap @ file:///home/slabbe/GitBox/sage/build/.tox/dist/sage_bootstrap1.0.zip py27 runtests: PYTHONHASHSEED='3230691582' py27 runtests: commands[0]  python2.7 m unittest discover ......
comment:40 Changed 2 years ago by
Actually, it did not get stuck, but I obtain three errors after few minutes:
$ tox GLOB sdistmake: /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_bootstrap1.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/releaseprocess/#python2support pip 21.0 will remove support for this functionality.,pkgresources==0.0.0,sagebootstrap @ file:///home/slabbe/GitBox/sage/build/.tox/dist/sage_bootstrap1.0.zip py27 runtests: PYTHONHASHSEED='3230691582' py27 runtests: commands[0]  python2.7 m unittest discover ......FF..............[xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx] [......................................................................] ......F........ERROR [packageall: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
.
comment:41 Changed 2 years ago by
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.
comment:42 Changed 2 years ago by
Status:  needs_review → positive_review 

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.)
comment:43 Changed 2 years ago by
Description:  modified (diff) 

comment:44 Changed 2 years ago by
Branch:  u/mkoeppe/sage_bootstrap__update_extend_system_package_tools → f33c3635ba36440b260644c127dcc94e12c3ab40 

Resolution:  → fixed 
Status:  positive_review → closed 
comment:45 followup: 46 Changed 2 years ago by
Commit:  f33c3635ba36440b260644c127dcc94e12c3ab40 

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?
Developers interested in improving the system package advice may want to work on this ticket