Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tox: Improve local-sudo-ubuntu-standard #30944

Closed
tobiasdiez opened this issue Nov 22, 2020 · 60 comments
Closed

tox: Improve local-sudo-ubuntu-standard #30944

tobiasdiez opened this issue Nov 22, 2020 · 60 comments

Comments

@tobiasdiez
Copy link
Contributor

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 build/make/Makefile.in: base-toolchain should be a dependency of every non-base, non-toolchain package #30721).

Depends on #29124

CC: @mkoeppe @dimpase @kliem

Component: porting

Author: Matthias Koeppe

Branch/Commit: ab19133

Reviewer: Jonathan Kliem

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

@tobiasdiez tobiasdiez added this to the sage-9.3 milestone Nov 22, 2020
@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 22, 2020

comment:1

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

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 22, 2020

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 22, 2020

Replying to @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

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 22, 2020

Commit: e9e5d47

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 23, 2020

Dependencies: #30923

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 24, 2020

comment:5

Replying to @mkoeppe:

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

Adding this in #29124

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 24, 2020

Changed dependencies from #30923 to #30923, #29124

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 24, 2020

comment:8

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

@tobiasdiez
Copy link
Contributor Author

comment:9

Ah ok...than it makes sense.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 25, 2020

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 25, 2020

Changed commit from e9e5d47 to fd47bc3

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 25, 2020

Author: Matthias Koeppe

@mkoeppe

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 25, 2020

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 25, 2020

Changed commit from fd47bc3 to d700ab2

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 25, 2020

comment:13

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

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 25, 2020

Changed dependencies from #30923, #29124 to #30923, #29124, #30721

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 25, 2020

Changed commit from d700ab2 to 9414be1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 25, 2020

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 26, 2020

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 26, 2020

Changed commit from 9414be1 to e959873

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 26, 2020

Changed dependencies from #30923, #29124, #30721 to #30923, #29124

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 26, 2020

comment:16

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.

@tobiasdiez
Copy link
Contributor Author

comment:17

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?

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 26, 2020

comment:18

Replying to @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"

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 6, 2020

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

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 6, 2020

comment:28

Rebased on top of 9.3.beta3 and rebased #29124

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 6, 2020

Changed dependencies from #30923, #29124, #22731 to #29124

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 10, 2020

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 10, 2020

Changed commit from 3c7e5c4 to f7ff30c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 16, 2020

Changed commit from f7ff30c to e9ca2c1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 16, 2020

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

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 16, 2020

comment:31

Merged newest version of #29124. Still needs review

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 29, 2020

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 29, 2020

Changed commit from e9ca2c1 to ab19133

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 29, 2020

comment:34

Merged newest #29124, needs review

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 6, 2021

comment:35

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

@kliem
Copy link
Contributor

kliem commented Jan 6, 2021

comment:36

I think #29124 isn't merged completely.

@kliem
Copy link
Contributor

kliem commented Jan 6, 2021

comment:37

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

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 6, 2021

comment:38

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.

@kliem
Copy link
Contributor

kliem commented Jan 6, 2021

comment:39

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

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 6, 2021

comment:40

I have reworded the ticket description to clarify it.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@kliem
Copy link
Contributor

kliem commented Jan 7, 2021

Reviewer: Jonathan Kliem

@kliem
Copy link
Contributor

kliem commented Jan 7, 2021

comment:42

LGTM.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 7, 2021

comment:43

Thank you!

@vbraun
Copy link
Member

vbraun commented Jan 24, 2021

Changed branch from u/mkoeppe/tox__improve_local_sudo_ubuntu_standard to ab19133

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants