Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#30923 closed enhancement (fixed)

tox.ini: Add environments local-sudo-ubuntu-standard, etc.

Reported by: Matthias Köppe Owned by:
Priority: major Milestone: sage-9.3
Component: porting Keywords:
Cc: Tobias Diez, Dima Pasechnik Merged in:
Authors: Matthias Koeppe Reviewers: Tobias Diez, Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: ff34897 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Matthias Köppe)

These environments would just run sudo apt-get ... etc. to install (but not remove!) packages, for use at the user's own risk.

To test (on an Ubuntu system):

    tox -e local-sudo-ubuntu-bionic-standard

or

    tox -e local-sudo-standard

Change History (33)

comment:1 Changed 2 years ago by Matthias Köppe

Authors: Matthias Koeppe

comment:2 Changed 2 years ago by Tobias Diez

Is it possible to put the sudo outside? I.e. sudo tox -r local-ubuntu-standard?

comment:3 Changed 2 years ago by Matthias Köppe

But we would only want to do the package installation as root, not the Sage build

comment:4 Changed 2 years ago by Matthias Köppe

Branch: u/mkoeppe/tox_ini__add_environments_local_sudo_ubuntu_standard__etc_

comment:5 Changed 2 years ago by Matthias Köppe

Commit: 30e624abbc52b5a5a060a4eca801e8fda303937f

Here's a first approximation.


New commits:

30e624atox.ini: Add local-sudo

comment:6 Changed 2 years ago by Matthias Köppe

Description: modified (diff)

comment:7 Changed 2 years ago by Tobias Diez

I think you forgot the confirmation -y after install: https://github.com/tobiasdiez/sage/commit/942149ee76a97628046042b7255a2d31630e7721

Moreover, I was wondering if one should also read the debian-bootstrap.txt files.

Finally, there are problems that some of the packages are not found. First, one should probably add sudo add-apt-repository ppa:deadsnakes/ppa since otherwise libpython3.7-dev is not found on ubuntu 20.04. Second, texlive-generic-extra doesn't exist on ubuntu 20.04. https://github.com/tobiasdiez/sage/runs/1404001507?check_suite_focus=true

Other than that, looks good to me! Thanks a lot.

comment:8 Changed 2 years ago by Matthias Köppe

Yes, I'll add bootstrap

I don't think we have libpython3.7-dev listed in the debian.txt files any more.

But yes, handling IGNORE_MISSING_SYSTEM_PACKAGES needs to be done.

For deadsnakes, there's #30217. This would not be the default, but a mix-in flavor.

comment:9 Changed 2 years ago by Tobias Diez

On a second thought, do you really thing it's the job of tox to install system packages? I would feel more comfortable to put the code into a shell script that you can run as build/install_system_packages ubuntu-2004, say. (Having such a script might be a good idea anyway).

comment:10 Changed 2 years ago by git

Commit: 30e624abbc52b5a5a060a4eca801e8fda303937f75ecd11e878e34a4c508d2f983b6d7527e689f94

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

75ecd11tox.ini (local-sudo): Also use ...-bootstrap.txt

comment:11 in reply to:  8 Changed 2 years ago by Tobias Diez

Replying to mkoeppe:

But yes, handling IGNORE_MISSING_SYSTEM_PACKAGES needs to be done.

and/or change texlive-generic-extra that exist on modern ubuntu systems as well.

comment:12 in reply to:  10 ; Changed 2 years ago by Tobias Diez

Replying to git:

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

75ecd11tox.ini (local-sudo): Also use ...-bootstrap.txt

Don't forget to change local-sudo-standard/maximal as well.

comment:13 in reply to:  9 ; Changed 2 years ago by Matthias Köppe

Replying to gh-tobiasdiez:

On a second thought, do you really thing it's the job of tox to install system packages? I would feel more comfortable to put the code into a shell script that you can run as build/install_system_packages ubuntu-2004, say. (Having such a script might be a good idea anyway).

It's working well with tox, in particular the environment factors give a nice compact way of expressing different configurations.

And I definitely do not want to add user-facing scripts to Sage that promises to install all necessary system packages. We deliberately only offer system package *advice* (at the end of ./configure). Already now users complain if that advice is not entirely accurate -- so we definitely do not want to make stronger promises! We simply do not have enough developers who could keep it up to date.

But there is a lot of room for refactoring what is in tox.ini into various helper scripts. I have described some possible directions in #30865 -- help is very welcome there.

comment:14 in reply to:  12 Changed 2 years ago by Matthias Köppe

Replying to gh-tobiasdiez:

Replying to git:

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

75ecd11tox.ini (local-sudo): Also use ...-bootstrap.txt

Don't forget to change local-sudo-standard/maximal as well.

No, that line is also executed for these environments - by the magic of tox's factor conditions.

comment:15 Changed 2 years ago by git

Commit: 75ecd11e878e34a4c508d2f983b6d7527e689f94050dcb8275b3a7e0db4c7ae3503428107bb5bf17

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

0dc79ebbuild/bin/sage-print-system-package-command: Handle --no-install-recommends, --yes for systems for which write-dockerfile.sh knows these flags
050dcb8tox.ini (local-sudo): Use --yes --no-install-recommends

comment:16 Changed 2 years ago by git

Commit: 050dcb8275b3a7e0db4c7ae3503428107bb5bf17c8fbe0beaec3a404d330aaebdf37186e9c6becec

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

c8fbe0btox.ini (local-sudo): Ignore errors when IGNORE_MISSING_SYSTEM_PACKAGES=yes

comment:17 Changed 2 years ago by Matthias Köppe

Description: modified (diff)
Status: newneeds_review

comment:18 Changed 2 years ago by git

Commit: c8fbe0beaec3a404d330aaebdf37186e9c6bececff34897cac35061afd348cc08a6559f95f406f14

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

ff34897tox.ini (local): Guess the package system if it is not provided as a factor

comment:19 Changed 2 years ago by Matthias Köppe

Description: modified (diff)

comment:20 in reply to:  13 Changed 2 years ago by Tobias Diez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

On a second thought, do you really thing it's the job of tox to install system packages? I would feel more comfortable to put the code into a shell script that you can run as build/install_system_packages ubuntu-2004, say. (Having such a script might be a good idea anyway).

It's working well with tox, in particular the environment factors give a nice compact way of expressing different configurations.

Sure, but it feels a bit like you are using tox for something it wasn't designed. Normally, in a tox setting, "environment" refers to a virtual python env, but for sage it is the complete system env including the os and system packages. I have the feeling a more general-purpose framework like invoke would be a better solution to manage these things.

And I definitely do not want to add user-facing scripts to Sage that promises to install all necessary system packages.

I didn't meant it as a user-facing script, but only for developers (i.e don't ship it).

comment:21 Changed 2 years ago by Tobias Diez

I've tested the current code in the wsl github workflow and it seems to work well: https://github.com/tobiasdiez/sage/actions/runs/365823571

comment:22 Changed 2 years ago by Matthias Köppe

Sounds like a positive review?

comment:23 Changed 2 years ago by Tobias Diez

Reviewers: Tobias Diez, ...

Yes, in principle. But I cannot really judge the changes in the bash script.

comment:24 Changed 2 years ago by Matthias Köppe

Cc: Dima Pasechnik added

comment:25 Changed 2 years ago by Dima Pasechnik

Reviewers: Tobias Diez, ...Tobias Diez, Dima Pasechnik
Status: needs_reviewpositive_review

ok

comment:26 Changed 2 years ago by Matthias Köppe

Thanks!

comment:27 Changed 2 years ago by Tobias Diez

Is it possible to call only the system package install code via tox (i.e. don't run rest of build and tests)?

That would be handy for #30371.

comment:28 in reply to:  27 Changed 2 years ago by Matthias Köppe

Replying to gh-tobiasdiez:

Is it possible to call only the system package install code via tox (i.e. don't run rest of build and tests)?

Yes, you can pass make targets to tox, e.g., tox -e local-sudo-standard -- configure will only run bootstrap, or tox -e local-sudo-standard -- config.status will stop after running ./configure

comment:29 Changed 2 years ago by Tobias Diez

Thanks! That kind of worked. I had to install sudo manually, since this is not installed by default on github actions and also not necessary. It would be nice if you could try to make it work without sudo.

local-sudo-standard run-test: commands[2] | bash -c 'PACKAGES=`sed "s/#.*//;" build/pkgs/$(build/bin/sage-guess-package-system)*.txt`; $(build/bin/sage-print-system-package-command $(build/bin/sage-guess-package-system) --sudo --yes --no-install-recommends install $PACKAGES) || [ "$IGNORE_MISSING_SYSTEM_PACKAGES" = yes ] && echo "(ignoring errors)" '
/usr/bin/bash: sudo: command not found

ERROR: InvocationError for command /usr/bin/bash -c 'PACKAGES=`sed "s/#.*//;" build/pkgs/$(build/bin/sage-guess-package-system)*.txt`; $(build/bin/sage-print-system-package-command $(build/bin/sage-guess-package-system) --sudo --yes --no-install-recommends install $PACKAGES) || [ "$IGNORE_MISSING_SYSTEM_PACKAGES" = yes ] && echo "(ignoring errors)" ' (exited with code 1)

Edit: Moreover, DEBIAN_FRONTEND: noninteractive was/is missing. And it fails with

configure: error: You cannot build Sage as root, switch to an unprivileged user.  (If building in a container, use --enable-build-as-root.)

Finally, tox -e local-sudo-standard -- config.status doesn't stop after contigure and continues with make, see https://github.com/tobiasdiez/sage/runs/1438029552?check_suite_focus=true

Since using this tox command for ci was one of the motivations but doesn't really work at the moment without extra modifications, I'll set the ticket back to needs work.

Last edited 2 years ago by Tobias Diez (previous) (diff)

comment:30 Changed 2 years ago by Tobias Diez

Status: positive_reviewneeds_work

comment:31 Changed 2 years ago by Volker Braun

Branch: u/mkoeppe/tox_ini__add_environments_local_sudo_ubuntu_standard__etc_ff34897cac35061afd348cc08a6559f95f406f14
Resolution: fixed
Status: needs_workclosed

comment:32 Changed 2 years ago by Volker Braun

Commit: ff34897cac35061afd348cc08a6559f95f406f14

Can you make a separate ticket for that? Once a ticket is in positive review I'll only unmerge it for critical bugs, not things that would also be good to have.

comment:33 Changed 2 years ago by Tobias Diez

Sure! It's now #30944. Sorry if I broke any (unwritten) laws; I wasn't sure about resetting it to "needs work".

Note: See TracTickets for help on using tickets.