Opened 9 months ago

Closed 7 months ago

#33873 closed enhancement (fixed)

Refactor system package scripts

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.7
Component: scripts Keywords:
Cc: dimpase Merged in:
Authors: Matthias Koeppe Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: d75da7e (Commits, GitHub, GitLab) Commit: d75da7ef800c271d60429c3e5f721bd5f729ca28
Dependencies: Stopgaps:

Status badges

Description (last modified by mkoeppe)

We extend the scripts sage-get-system-packages and sage-print-system-package-command:

  • When system=auto, they now invoke sage-guess-package-system.
  • sage-print-system-package-command gets a new option --spkg; then it invokes sage-get-system-packages.

This simplifies some uses.

Change History (16)

comment:1 Changed 9 months ago by mkoeppe

Branch: u/mkoeppe/refactor_system_package_scripts

comment:2 Changed 9 months ago by mkoeppe

Authors: Matthias Koeppe
Commit: d75da7ef800c271d60429c3e5f721bd5f729ca28

New commits:

ab5a4f1build/bin/sage-get-system-packages: If system=auto, use sage-guess-package-system
798f4d3build/bin/sage-print-system-package-command: If system=auto, use sage-guess-package-system
ef249ccbuild/bin/sage-print-system-package-command: New option --spkg
d75da7etox.ini: Simplify using new options of sage-print-system-package-command

comment:3 Changed 9 months ago by mkoeppe

Status: newneeds_review

comment:4 Changed 8 months ago by mkoeppe

Description: modified (diff)

comment:5 Changed 7 months ago by mkoeppe

Cc: dimpase added

comment:6 Changed 7 months ago by dimpase

how can one test that?

comment:7 Changed 7 months ago by mkoeppe

For example, on a Linux machine using tox -e local-sudo-standard -- config.status

comment:8 Changed 7 months ago by mkoeppe

#33819 also makes (limited) use of it

comment:9 in reply to:  7 Changed 7 months ago by dimpase

Replying to mkoeppe:

For example, on a Linux machine using tox -e local-sudo-standard -- config.status

how about something without sudo?

comment:10 Changed 7 months ago by dimpase

Reviewers: Dima Pasechnik
Status: needs_reviewpositive_review

I tried local-standard instead, it worked.

comment:11 Changed 7 months ago by mkoeppe

Thanks!

comment:12 in reply to:  10 ; Changed 7 months ago by mkoeppe

Replying to dimpase:

I tried local-standard instead, it worked.

I think this does not actually invoke the package scripts at all, but you can do tox -e local-root-standard and wait for the error from not being root

comment:13 Changed 7 months ago by dimpase

why is sudo/root needed at all for these tests? Is it meant to allow a system-wide installation of (parts of) Sage?

comment:14 in reply to:  12 Changed 7 months ago by dimpase

Replying to mkoeppe:

Replying to dimpase:

I tried local-standard instead, it worked.

I think this does not actually invoke the package scripts at all, but you can do tox -e local-root-standard and wait for the error from not being root

the error is coming very soon:

$ tox -e local-root-standard -- config.status
local-root-standard create: /home/scratch/scratch2/dimpase/sage/sagetrac-mirror/.tox/local-root-standard
local-root-standard run-test-pre: PYTHONHASHSEED='263570377'
local-root-standard run-test: commands[0] | bash -c 'if [ ! -d /home/scratch/scratch2/dimpase/sage/sagetrac-mirror/.tox/local-root-standard/Library/Caches ]; then mkdir -p /home/scratch/scratch2/dimpase/sage/sagetrac-mirror/.tox/Caches && mkdir -p /home/scratch/scratch2/dimpase/sage/sagetrac-mirror/.tox/local-root-standard/Library && ln -sf /home/scratch/scratch2/dimpase/sage/sagetrac-mirror/.tox/Caches /home/scratch/scratch2/dimpase/sage/sagetrac-mirror/.tox/local-root-standard/Library/; fi'
local-root-standard run-test: commands[1] | bash -c 'case "" in 1|y*|Y*);; *) eval $(build/bin/sage-print-system-package-command auto  update) ;; esac'
local-root-standard run-test: commands[2] | bash -c 'case "" in 1|y*|Y*);; *) export PATH="build/bin:$PATH" && eval $(sage-print-system-package-command auto  --yes --no-install-recommends --spkg install $(sage-package list --has-file=spkg-configure.m4 :standard:) _bootstrap    ) || [ "$IGNORE_MISSING_SYSTEM_PACKAGES" = yes ] && echo "(ignoring errors)" ;; esac'
Error: This command has to be run with superuser privileges (under the root user on most systems).
ERROR: InvocationError for command /usr/local/bin/bash -c 'case "" in 1|y*|Y*);; *) export PATH="build/bin:$PATH" && eval $(sage-print-system-package-command auto  --yes --no-install-recommends --spkg install $(sage-package list --has-file=spkg-configure.m4 :standard:) _bootstrap    ) || [ "$IGNORE_MISSING_SYSTEM_PACKAGES" = yes ] && echo "(ignoring errors)" ;; esac' (exited with code 1)
...
Error: This command has to be run with superuser privileges (under the root user on most systems).

comment:15 in reply to:  13 Changed 7 months ago by mkoeppe

Replying to dimpase:

why is sudo/root needed at all for these tests? Is it meant to allow a system-wide installation of (parts of) Sage?

These tox environments are primarily for use on containers to install system packages. It's not intended to be user-facing.

One use is in .github/workflows/ci-wsl.yml

comment:16 Changed 7 months ago by vbraun

Branch: u/mkoeppe/refactor_system_package_scriptsd75da7ef800c271d60429c3e5f721bd5f729ca28
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.