#30944 closed defect (fixed)

tox: Improve local-sudo-ubuntu-standard

Reported by: gh-tobiasdiez Owned by:
Priority: critical Milestone: sage-9.3
Component: porting Keywords:
Cc: mkoeppe, dimpase, gh-kliem Merged in:
Authors: Matthias Koeppe Reviewers: Jonathan Kliem
Report Upstream: N/A Work issues:
Branch: ab19133 (Commits, GitHub, GitLab) Commit: ab191339dfa275b3466f530a59268c756f5d1b05
Dependencies: #29124 Stopgaps:

Status badges

Description (last modified by mkoeppe)

As a follow up to #30923, we improve the local-sudo-... environments as follows:

  • Provide alternative environments local-root..., which assume that we are already root, removing the sudo requirement (which is not installed on github actions and not needed there)
  • Use configure --enable-build-as-root for local-root
  • Add DEBIAN_FRONTEND: noninteractive to the installation of packages via apt get (otherwise installation of tzdata blocks CI)
  • Make tox -e local-sudo-standard -- config.status only build config.status instead of also invoking make base-toolchain (which is run as the required first step of the Sage build system before something like make numpy can be run; see #30721).

Change History (44)

comment:1 follow-up: Changed 21 months ago by mkoeppe

The variant could be called local-root-ubuntu-standard, I guess

comment:2 Changed 21 months ago by mkoeppe

  • Branch set to u/mkoeppe/tox__improve_local_sudo_ubuntu_standard

comment:3 in reply to: ↑ description Changed 21 months ago by mkoeppe

  • Commit set to e9e5d47d5e936fe7f28bcb7a0e8ba5c0711deea8

Replying to gh-tobiasdiez:

  • Make tox -e local-sudo-standard -- config.status stop after configure (and don't run make)

Right, I forgot about the mess with TARGETS_PRE and TARGETS_OPTIONAL in tox.ini, which do not get the same defaults for local as they do for docker - I'll fix this


New commits:

30e624atox.ini: Add local-sudo
75ecd11tox.ini (local-sudo): Also use ...-bootstrap.txt
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
c8fbe0btox.ini (local-sudo): Ignore errors when IGNORE_MISSING_SYSTEM_PACKAGES=yes
ff34897tox.ini (local): Guess the package system if it is not provided as a factor
e9e5d47build/bin/sage-print-system-package-command (debian --yes): Use DEBIAN_FRONTEND=noninteractive

comment:4 Changed 21 months ago by mkoeppe

  • Dependencies set to #30923

comment:5 in reply to: ↑ 1 Changed 21 months ago by mkoeppe

Replying to mkoeppe:

The variant could be called local-root-ubuntu-standard, I guess

Adding this in #29124

comment:6 Changed 21 months ago by mkoeppe

  • Cc dimpase added; diampase removed

comment:7 Changed 21 months ago by mkoeppe

  • Dependencies changed from #30923 to #30923, #29124

comment:8 Changed 21 months ago by mkoeppe

Replying to gh-tobiasdiez in #29124:

I'm not sure about the naming of the local- environments. Why should I expect as a developer that local-sudo installs system packages? What about local-system-packages-ubuntu (for root) and local-sudo-system-packages-ubuntu (for sudo)?

All of these environments install system packages... The only one that does not is local-direct

comment:9 Changed 21 months ago by gh-tobiasdiez

Ah ok...than it makes sense.

comment:10 Changed 21 months ago by git

  • Commit changed from e9e5d47d5e936fe7f28bcb7a0e8ba5c0711deea8 to fd47bc39793be3d917f0ab1aac14ecce13970704

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

d318067build/pkgs/_prereq/spkg-configure.m4: New
85f8ce5src/doc/bootstrap: Remove special casing of build/pkgs/$SYSTEM.txt - _prereq is now just a standard package with spkg-configure.m4
0e97683tox.ini: Simplify, use sage-get-system-packages, sage-package list
da3f657tox.ini: Fixup - always include _bootstrap
8fde1dcbuild/bin/write-dockerfile.sh: Remove special casing of build/pkgs/$SYSTEM.txt
8d725b8src/doc/en/developer/portability_testing.rst: Update paths in documentation
bd7f66cMakefile (configure): Update dependency on moved files build/pkgs/*.txt
c18e935Merge branch 't/29124/script-packages-prereq-toolchain-bootstrap' into t/30944/tox__improve_local_sudo_ubuntu_standard
f18f849tox.ini: Add local-root
fd47bc3tox.ini (local-root): Pass --enable-build-as-root to configure

comment:11 Changed 21 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Description modified (diff)

comment:12 Changed 21 months ago by git

  • Commit changed from fd47bc39793be3d917f0ab1aac14ecce13970704 to d700ab293a857397e964334baeb8207d083daca9

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

d700ab2Merge tag '9.3.beta2' into t/30944/tox__improve_local_sudo_ubuntu_standard

comment:13 follow-up: Changed 21 months ago by mkoeppe

  • Dependencies changed from #30923, #29124 to #30923, #29124, #30721

To simplify the build rule for local, the next step is to do #30721 so we can eliminate the separate make base-toolchain step.

comment:14 Changed 21 months ago by git

  • Commit changed from d700ab293a857397e964334baeb8207d083daca9 to 9414be10c519113c5f77b99504447d8ead4074e5

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

5e9366dMerge tag '9.3.beta2' into t/30947/src_doc_bootstrap__simplify_by_using_new_options_of__sage__package_list_
6b9f3a3Merge branch 't/30947/src_doc_bootstrap__simplify_by_using_new_options_of__sage__package_list_' into t/29124/script-packages-prereq-toolchain-bootstrap
9414be1Merge branch 't/29124/script-packages-prereq-toolchain-bootstrap' into t/30944/tox__improve_local_sudo_ubuntu_standard

comment:15 Changed 21 months ago by git

  • Commit changed from 9414be10c519113c5f77b99504447d8ead4074e5 to e959873490b161ab53e0400d32612401e9db2d0c

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

e959873tox.ini (local): Do not build the toolchain when posargs = config.status or posargs = configure

comment:16 in reply to: ↑ 13 Changed 21 months ago by mkoeppe

  • Dependencies changed from #30923, #29124, #30721 to #30923, #29124
  • Status changed from new to needs_review

Replying to mkoeppe:

To simplify the build rule for local, the next step is to do #30721 so we can eliminate the separate make base-toolchain step.

Well, I'll work on that another time. For now I have implemented it in a different way, special casing on "config*" targets.

comment:17 follow-up: Changed 21 months ago by gh-tobiasdiez

Thanks! Looks good to me, as far as I understand the changes.

Just to make sure:

Make tox -e local-sudo-standard -- config.status stop after configure

Is fixed as well?

comment:18 in reply to: ↑ 17 ; follow-up: Changed 21 months ago by mkoeppe

Replying to gh-tobiasdiez:

Thanks! Looks good to me, as far as I understand the changes.

Just to make sure:

Make tox -e local-sudo-standard -- config.status stop after configure

Is fixed as well?

Yes. This is what I meant in the previous comment - "special casing on config* targets"

comment:19 in reply to: ↑ 18 Changed 21 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

Thanks! Looks good to me, as far as I understand the changes.

Just to make sure:

Make tox -e local-sudo-standard -- config.status stop after configure

Is fixed as well?

Yes. This is what I meant in the previous comment - "special casing on config* targets"

Ok, then is good! Me with my limited bash knowledge thought that the final make that builds the sage packages is still invoked even though -- config.status is specified.

comment:20 Changed 21 months ago by mkoeppe

It's only invoked on nonempty $TARGETS_OPTIONAL

comment:21 follow-up: Changed 21 months ago by gh-tobiasdiez

  • Status changed from needs_review to needs_work

Running tox -e local-sudo-ubuntu-standard -- SAGE_NUM_THREADS=16 build (in WLS) yields

Reading package lists... Done
E: Could not open lock file /var/lib/apt/lists/lock - open (13: Permission denied)
E: Unable to lock directory /var/lib/apt/lists/
W: Problem unlinking the file /var/cache/apt/pkgcache.bin - RemoveCaches (13: Permission denied)
W: Problem unlinking the file /var/cache/apt/srcpkgcache.bin - RemoveCaches (13: Permission denied)
ERROR: InvocationError for command /usr/bin/bash -c '$(build/bin/sage-print-system-package-command debian update) #' (exited with code 100)

Running the same command with sudo is successful, so I guess a --sudo is missing somewhere.

Moreover, ./bootstrap didn't run with following error:

/mnt/d/Programming/sage/src/bin/sage-env: line 133: cd: /mnt/d/Programming/Projects/sage: No such file or directory
Warning: overwriting SAGE_ROOT environment variable:
Old SAGE_ROOT=/mnt/d/Programming/Projects/sage
New SAGE_ROOT=
Error: You must set either the SAGE_LOCAL or SAGE_SCRIPTS_DIR environment variable to run this
Error setting environment variables by sourcing '/mnt/d/Programming/sage/src/bin/sage-env';
possibly contact sage-devel (see http://groups.google.com/group/sage-devel).

until I deleted src/bin/sage-env manually. And the root version sudo tox -e local-root-ubuntu-standard -- SAGE_NUM_THREADS=10 build yields

local-root-ubuntu-standard run-test: commands[2] | bash -c 'PACKAGES=$(build/bin/sage-get-system-packages debian $(PATH=build/bin:$PATH build/bin/sage-package list --has-file=spkg-configure.m4 :standard:) _bootstrap); $(build/bin/sage-print-system-package-command debian  --yes --no-install-recommends install $PACKAGES) || [ "$IGNORE_MISSING_SYSTEM_PACKAGES" = yes ] && echo "(ignoring errors)" '
/usr/bin/bash: DEBIAN_FRONTEND=noninteractive: command not found
ERROR: InvocationError for command /usr/bin/bash -c 'PACKAGES=$(build/bin/sage-get-system-packages debian $(PATH=build/bin:$PATH build/bin/sage-package list --has-file=spkg-configure.m4 :standard:) _bootstrap); $(build/bin/sage-print-system-package-command debian  --yes --no-install-recommends install $PACKAGES) || [ "$IGNORE_MISSING_SYSTEM_PACKAGES" = yes ] && echo "(ignoring errors)" ' (exited with code 1)
Last edited 21 months ago by gh-tobiasdiez (previous) (diff)

comment:22 Changed 21 months ago by git

  • Commit changed from e959873490b161ab53e0400d32612401e9db2d0c to 27368b1bc6e9ed5a117005674025747af78b4227

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

27368b1tox.ini (local-sudo): Run apt-get update with sudo

comment:23 in reply to: ↑ 21 Changed 21 months ago by mkoeppe

  • Dependencies changed from #30923, #29124 to #30923, #29124, #22731

Replying to gh-tobiasdiez:

Moreover, ./bootstrap didn't run with following error:

/mnt/d/Programming/sage/src/bin/sage-env: line 133: cd: /mnt/d/Programming/Projects/sage: No such file or directory
Warning: overwriting SAGE_ROOT environment variable:
Old SAGE_ROOT=/mnt/d/Programming/Projects/sage
New SAGE_ROOT=
Error: You must set either the SAGE_LOCAL or SAGE_SCRIPTS_DIR environment variable to run this
Error setting environment variables by sourcing '/mnt/d/Programming/sage/src/bin/sage-env';
possibly contact sage-devel (see http://groups.google.com/group/sage-devel).

until I deleted src/bin/sage-env manually.

This is fixed by #22731, I think. I'll merge it

comment:24 Changed 21 months ago by git

  • Commit changed from 27368b1bc6e9ed5a117005674025747af78b4227 to 7216c677975a520343df6ab9dad5fdf0fc09cfcc

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

8f88709src/bin/sage: Only source sage-env-config if it exists
ab2655esrc/bin/sage: Use python3 etc. from PATH instead of using SAGE_LOCAL
4577f37src/bin/sage-env: Make sage-env-config optional
c35c170sage.env: Add SAGE_VENV
c585d94src/sage: Replace some SAGE_LOCAL by SAGE_VENV
2fd195fsrc/bin/sage: Fix reference to trac ticket
f3b7a9csrc/sage/env.py: Add documentation
1fe77a8src/bin/sage-env: Do not set SAGE_SCRIPTS_DIR
38eebc3Merge tag '9.3.beta2' into t/22731/22731
7216c67Merge branch 'u/mkoeppe/22731' of git://trac.sagemath.org/sage into t/30944/tox__improve_local_sudo_ubuntu_standard

comment:25 Changed 21 months ago by git

  • Commit changed from 7216c677975a520343df6ab9dad5fdf0fc09cfcc to 1c2dfc62c731c586d569c3e9eae6cd806c25a960

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

1c2dfc6tox.ini (local-root, local-sudo): Run output of sage-print-system-package-command through eval

comment:26 Changed 21 months ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:27 Changed 21 months ago by git

  • Commit changed from 1c2dfc62c731c586d569c3e9eae6cd806c25a960 to 3c7e5c482e1c54ba140c3d134ebfc909bd943a3d

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

35241e7src/doc/en/developer/portability_testing.rst: Update paths in documentation
68baff1Makefile (configure): Update dependency on moved files build/pkgs/*.txt
fae4bd0bootstrap: Remove --enable-_recommended etc.
a9bd145m4/sage_spkg_collect.m4: Remove _recommended etc. from the configure package summary
93f5e32build/bin/sage-print-system-package-command (debian --yes): Use DEBIAN_FRONTEND=noninteractive
898758dtox.ini: Add local-root
559dd8etox.ini (local-root): Pass --enable-build-as-root to configure
a85f41ctox.ini (local): Do not build the toolchain when posargs = config.status or posargs = configure
1de912atox.ini (local-sudo): Run apt-get update with sudo
3c7e5c4tox.ini (local-root, local-sudo): Run output of sage-print-system-package-command through eval

comment:28 Changed 21 months ago by mkoeppe

  • Dependencies changed from #30923, #29124, #22731 to #29124

Rebased on top of 9.3.beta3 and rebased #29124

comment:29 Changed 21 months ago by git

  • Commit changed from 3c7e5c482e1c54ba140c3d134ebfc909bd943a3d to f7ff30c92e59308d84377411ea4edd9173c4cd75

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

184ae6dMerge tag '9.3.beta3' into t/30947/src_doc_bootstrap__simplify_by_using_new_options_of__sage__package_list_
084fbf6src/doc/bootstrap: Use ./sage
571cc49Merge branch 't/30947/src_doc_bootstrap__simplify_by_using_new_options_of__sage__package_list_' into t/29124/script-packages-prereq-toolchain-bootstrap
f7ff30cMerge branch 't/29124/script-packages-prereq-toolchain-bootstrap' into t/30944/tox__improve_local_sudo_ubuntu_standard

comment:30 Changed 20 months ago by git

  • Commit changed from f7ff30c92e59308d84377411ea4edd9173c4cd75 to e9ca2c1d848708c0a626bc412844043b005c985a

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

c529729Merge commit '3bb309944b7e8542b2ac88ed3c9d9a60e68644d7' of git://trac.sagemath.org/sage into t/29124/script-packages-prereq-toolchain-bootstrap
b73d6f0Merge tag '9.3.beta4' into t/29124/script-packages-prereq-toolchain-bootstrap
e9ca2c1Merge branch 't/29124/script-packages-prereq-toolchain-bootstrap' into t/30944/tox__improve_local_sudo_ubuntu_standard

comment:31 Changed 20 months ago by mkoeppe

Merged newest version of #29124. Still needs review

comment:32 Changed 20 months ago by mkoeppe

  • Cc gh-kliem added
  • Priority changed from major to critical

comment:33 Changed 20 months ago by git

  • Commit changed from e9ca2c1d848708c0a626bc412844043b005c985a to ab191339dfa275b3466f530a59268c756f5d1b05

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

3cf5ee4Merge commit 'a50ddf88975086b14a49895e371477df00fd57b5' of git://trac.sagemath.org/sage into t/30940/src_bin_sage_list_packages__make_it_work_if_sage_root_is_not_available
b4927e1sage.misc.package: Remove/adjust non-robust doctests
e5fe752Merge tag '9.3.beta4' into t/30940/src_bin_sage_list_packages__make_it_work_if_sage_root_is_not_available
78ff9d5src/sage/misc/package.py: Add one more # optional - build
a44042fMerge branch 't/30940/src_bin_sage_list_packages__make_it_work_if_sage_root_is_not_available' into t/29124/script-packages-prereq-toolchain-bootstrap
64bde5fMerge tag '9.3.beta5' into t/30940/src_bin_sage_list_packages__make_it_work_if_sage_root_is_not_available
e9a7572src/sage/misc/package.py: Improve source formatting
c7bcda9Merge branch 't/30940/src_bin_sage_list_packages__make_it_work_if_sage_root_is_not_available' into t/29124/script-packages-prereq-toolchain-bootstrap
9988c5fci-cygwin*.yml: Adjust to new script packages _bootstrap, _prereq
ab19133Merge branch 't/29124/script-packages-prereq-toolchain-bootstrap' into t/30944/tox__improve_local_sudo_ubuntu_standard

comment:34 Changed 20 months ago by mkoeppe

Merged newest #29124, needs review

comment:35 Changed 20 months ago by mkoeppe

Let's get this in please (dependency of #31064 and #31099)

comment:36 Changed 20 months ago by gh-kliem

I think #29124 isn't merged completely.

comment:37 Changed 20 months ago by gh-kliem

What does

+    local:             case "{posargs:}" in \
+    local:                 config*) ;; \
+    local:                 *)       make -k V=0 base-toolchain ;; \
+    local:             esac && \
-    local:                     make -k V=0 base-toolchain && \

do? (That is not the diff, but I think morally it is.)

comment:38 Changed 20 months ago by mkoeppe

It implements the last bullet point of the ticket description: When something like config.status is given as the target, make base-toolchain should not be run.

comment:39 Changed 20 months ago by gh-kliem

Thanks. But does it stop then or are the other things still run?:

+    local:             make -k V=0 SAGE_SPKG="sage-spkg -y -o" SAGE_CHECK=warn SAGE_CHECK_PACKAGES="!cython,!r,!python3,!nose,!gap,!cysignals,!linbox,!git,!ppl,!cmake,!networkx,!symengine_py" {env:TARGETS_PRE:} {posargs:build} && \
+    local:             ([ -z "{env:TARGETS_OPTIONAL:}" ] || make -k V=0 SAGE_SPKG="sage-spkg -y -o" SAGE_CHECK=warn SAGE_CHECK_PACKAGES="!cython,!r,!python3,!nose,!gap,!cysignals,!linbox,!git,!ppl,!cmake" {env:TARGETS_OPTIONAL:} || echo "(error ignored)" ) '

comment:40 Changed 20 months ago by mkoeppe

  • Description modified (diff)

I have reworded the ticket description to clarify it.

comment:41 Changed 20 months ago by mkoeppe

  • Description modified (diff)

comment:42 Changed 20 months ago by gh-kliem

  • Reviewers set to Jonathan Kliem
  • Status changed from needs_review to positive_review

LGTM.

comment:43 Changed 20 months ago by mkoeppe

Thank you!

comment:44 Changed 19 months ago by vbraun

  • Branch changed from u/mkoeppe/tox__improve_local_sudo_ubuntu_standard to ab191339dfa275b3466f530a59268c756f5d1b05
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.