`tox -e update_docker_platforms` #36954

mkoeppe · 2023-12-24T04:26:02Z

Furthering my hidden agenda of bringing portability tools to the masses.

We add .devcontainer/portability-*/devcontainer.json files for all platforms tested in our CI. This makes them selectable in GitHub Codespaces.
The files are generated by the new command tox -e update_docker_platforms, which also updates the workflow file .github/workflows/docker.yml.
The command also adds a table to the portability testing section in the developer's guide with links to GitHub Packages and friendly "Run in GitHub Codespaces" buttons.
Because 'tis the season of Spielzeugneid, we document how to run the portability testing in GitHub Codespaces via Docker-in-Docker, useful for those who cannot run Docker on their own computers. This uses a new devcontainer config "tox-docker-in-docker".
We make various other updates to the portability testing section in the developer's guide.

📝 Checklist

The title is concise, informative, and self-explanatory.
The description explains in detail what this PR is about.
I have linked a relevant issue or discussion.
I have created tests covering the changes.
I have updated the documentation accordingly.

⌛ Dependencies

Depends on Small update to Developer Guide section on portability testing #37030 (split out from here)

kwankyu · 2024-01-09T07:42:40Z

This looks better

.. list-table::
   :width: 200
   :widths: 50 20 50 50
   :header-rows: 1
   :stub-columns: 0

   * - Platform
     - System packages
     - Images
     -
   * - **ubuntu**-trusty-toolchain-gcc_9
     - minimal
     - |image-ubuntu-trusty-toolchain-gcc_9-minimal-with-system-packages| |image-ubuntu-trusty-toolchain-gcc_9-minimal-with-targets-pre| |image-ubuntu-trusty-toolchain-gcc_9-minimal-with-targets| |image-ubuntu-trusty-toolchain-gcc_9-minimal-with-targets-optional|
     - |codespace-ubuntu-trusty-toolchain-gcc_9-minimal|
   * -
     - standard
     - |image-ubuntu-trusty-toolchain-gcc_9-standard-with-system-packages| |image-ubuntu-trusty-toolchain-gcc_9-standard-with-targets-pre| |image-ubuntu-trusty-toolchain-gcc_9-standard-with-targets| |image-ubuntu-trusty-toolchain-gcc_9-standard-with-targets-optional|
     - |codespace-ubuntu-trusty-toolchain-gcc_9-standard|
   * -
     - maximal
     - |image-ubuntu-trusty-toolchain-gcc_9-maximal-with-system-packages| |image-ubuntu-trusty-toolchain-gcc_9-maximal-with-targets-pre|
     -

tobiasdiez

Please don't commit the auto-generated devcontainer files. The would show up at https://github.com/codespaces/new?hide_repo_select=true&ref=develop&repo=597660615&skip_quickstart=true, and a drop down box should not contain more than 10-20 options to be useful. Also, if I'm not mistaken, the docs already explain how to start a devcontainer for the portability tests.

kwankyu · 2024-01-09T11:22:21Z

Please don't commit the auto-generated devcontainer files. The would show up at https://github.com/codespaces/new?hide_repo_select=true&ref=develop&repo=597660615&skip_quickstart=true, and a drop down box should not contain more than 10-20 options to be useful.

I experimented on it. The dev container files appear sorted alphabetically by their filenames, and the new dev container files (that start with "portability-") appear to the bottom of the dropdown box. Hence there is no inconvenience incurred by this PR. The conda dev container (perhaps your favorite) is still placed at the top.

tobiasdiez · 2024-01-09T14:59:53Z

Thanks for testing, but I was already aware of the sorting and took this into account.

I would be fine with adding a few common configs, but not almost hundred.

mkoeppe · 2024-01-09T16:06:08Z

the docs already explain how to start a devcontainer for the portability tests.

That's right. This PR lowers the barrier further -- in particular by making these options available in the devcontainer dropdown menu on PRs.

I would be fine with adding a few common configs

There's no such thing, as I explained in detail in #36726.

mkoeppe · 2024-01-09T16:23:29Z

This looks better

.. list-table::
   :width: 200
   :widths: 50 20 50 50
   :header-rows: 1
   :stub-columns: 0

Not sure about this. Have you tried this in a narrow window?

kwankyu · 2024-01-10T03:42:48Z

Not sure about this. Have you tried this in a narrow window?

I just did. In the narrowest window (perhaps corresponding to the vertical mode of hand-held devices), the PR version looks okay. In this case, the font is too small in my version. Except this extreme case, my version shows always the four images stacked nicely and codespaces button separately on the right, which look tidy overall (even in the extreme case).

src/doc/en/developer/portability_testing.rst

kwankyu · 2024-01-10T04:23:03Z

Instead of tox -e update_docker_platforms, how about

tox -e update_devcontainer_files

as this is what is done? Just a mild suggestion.

tobiasdiez · 2024-01-10T04:27:35Z

I would be fine with adding a few common configs

There's no such thing, as I explained in detail in #36726.

There is, and for that system we already added a devcontainer file.

tobiasdiez · 2024-01-10T04:29:43Z

And please move the script out of tox. There doesn't seem anything specific in there that needs tox/an isolated environment. Perhaps we should introduce a new tools directory as a common place for development tools.

mkoeppe · 2024-01-10T04:42:46Z

I would be fine with adding a few common configs

There's no such thing, as I explained in detail in #36726.

There is, and for that system we already added a devcontainer file.

Tobias, stop writing for an audience. You and I both know that you have no involvement whatsoever in maintaining the Sage distribution and the associated portability work. Such declarations from you about this effort just have no value in this discussion.

mkoeppe · 2024-01-10T04:45:23Z

And please move the script out of tox.

Nope, that's where it belongs, it's the central place where the different system configurations are defined.

mkoeppe · 2024-01-10T04:49:54Z

Instead of tox -e update_docker_platforms, how about
tox -e update_devcontainer_files
as this is what is done? Just a mild suggestion.

That's one thing that it does, but it also updates the file .github/workflows/docker.yml, which defines the CI Linux workflow.

kwankyu · 2024-01-10T05:01:50Z

That's one thing that it does, but it also updates the file .github/workflows/docker.yml, which defines the CI Linux workflow.

I see. Then this

commands =
    bash -c 'DEFAULT_SYSTEM_FACTORS=$(echo \
                     ubuntu-{trusty-toolchain-gcc_9,xenial-toolchain-gcc_9,bionic-gcc_8,focal,jammy,lunar,mantic} \
                     debian-{buster-gcc_spkg,bullseye,bookworm,trixie,sid} \
                     linuxmint-{20.1,20.2,20.3,21,21.1,21.2} \
                     fedora-{30,31,32,33,34,35,36,37,38,39} \
                     centos-7-devtoolset-gcc_11 \
                     centos-stream-{8,9}-python3.9 \
                     almalinux-{8-python3.9,9-python3.11} \
                     gentoo-python{3.10,3.11} \
                     archlinux-latest \
                     opensuse-{15.3-gcc_11-python3.9,15.4-gcc_11-python3.10,15.5-gcc_11-python3.11} \
                     opensuse-tumbleweed{-python3.10,} \
                     conda-forge-python3.11 \
                     ubuntu-bionic-gcc_8-i386 \
                     debian-bullseye-i386); \

is responsible for defining all linux platforms that our CI machine works on and thus sage officially supports. Right?

Then it seems that this important list is buried deep in tox.ini. Would it be possible to pull this definition out of the quoted string to a top-level visible place (such as setenv= or something)? Even the color codes COLOR_... are more visible now.

mkoeppe · 2024-01-10T05:03:21Z

Would it be possible to pull this definition out of the quoted string to a top-level visible place (such as setenv= or something)?

That's a good idea. I'll work on that

mkoeppe · 2024-01-10T05:11:45Z

But first here's another layout variant....

kwankyu · 2024-01-10T05:12:47Z

OK for me. Thanks.

tobiasdiez · 2024-01-10T05:36:16Z

I would be fine with adding a few common configs

There's no such thing, as I explained in detail in #36726.

There is, and for that system we already added a devcontainer file.

Tobias, stop writing for an audience. You and I both know that you have no involvement whatsoever in maintaining the Sage distribution and the associated portability work. Such declarations from you about this effort just have no value in this discussion.

Please stop these ad hominem attacks against reviewers. Moreover, it is surely against our code of conduct to try to limit the discussion of certain topics to a subgroup of the community (and even more so if a single person decides who is in that subgroup, or if it is based on who contributed to a certain part of the project before).

(Finally, I did actually have quite a few commits changing things in the sage-the-distro folders build and pkgs.)

mkoeppe · 2024-01-10T05:51:32Z

Would it be possible to pull this definition out of the quoted string to a top-level visible place (such as setenv= or something)?

That's a good idea. I'll work on that

Done in e663e2d

kwankyu · 2024-01-10T06:56:25Z

Looking at the rendered table with 3 columns, it seems that the codespaces button occupies too much space, and we still have a room for the column for "system packages". Thus I still prefer the 4 column format as suggested above. But I won't insist, as this seems to be a personal preference.

By the way, here

and here

there are broken links. Could they be fixed?

mkoeppe · 2024-01-11T04:06:30Z

Looking at the rendered table with 3 columns, it seems that the codespaces button occupies too much space

I've adjusted the column width and another detail, please take a look.

mkoeppe · 2024-01-11T04:16:39Z

there are broken links. Could they be fixed?

The one for conda was systematic, I'm suppressing this row now. The other one must have been some query rate limit in the badge generation service; this should go away if you clear the cache

kwankyu · 2024-01-11T04:28:23Z

Looking at the rendered table with 3 columns, it seems that the codespaces button occupies too much space

I've adjusted the column width and another detail, please take a look.

Looks good to me. Thanks.

kwankyu · 2024-01-11T04:30:19Z

there are broken links. Could they be fixed?

The one for conda was systematic, I'm suppressing this row now. The other one must have been some query rate limit in the badge generation service; this should go away if you clear the cache

I see no broken links now. Thanks.

kwankyu

LGTM.

…ustomization

… docker-in-docker

… github-cli as dev container features

github-actions · 2024-01-22T05:57:56Z

Documentation preview for this PR (built with commit aa7dab3; changes) is ready! 🎉

kwankyu · 2024-01-22T20:52:02Z

Enough lingering here.

mkoeppe · 2024-01-22T20:53:22Z

Thanks!

sagemathgh-36954: `tox -e update_docker_platforms`   Furthering my hidden agenda of bringing portability tools to the masses. - We add `.devcontainer/portability-*/devcontainer.json` files for all platforms tested in our CI. This makes them selectable in GitHub Codespaces. - The files are generated by the new command `tox -e update_docker_platforms`, which also updates the workflow file `.github/workflows/docker.yml`. - The command also [adds a table to the portability testing section in the developer's guide](https://deploy-preview-36954-- sagemath.netlify.app/html/en/developer/portability_testing#using-our- pre-built-docker-images-published-on-ghcr-io) with links to GitHub Packages and friendly "Run in GitHub Codespaces" buttons. - Because 'tis the season of Spielzeugneid, [we document how to run the portability testing in GitHub Codespaces via Docker-in- Docker](https://deploy-preview-36954-- sagemath.netlify.app/html/en/developer/portability_testing#testing-sage- on-a-different-platform-using-docker), useful for those who cannot run Docker on their own computers. This uses a new devcontainer config "tox- docker-in-docker". - We make various other updates to the portability testing section in the developer's guide.    ### 📝 Checklist     - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies  - Depends on sagemath#37030 (split out from here)  URL: sagemath#36954 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee, Matthias Köppe, Tobias Diez

tobiasdiez suggested changes Jan 9, 2024

View reviewed changes

kwankyu reviewed Jan 10, 2024

View reviewed changes

src/doc/en/developer/portability_testing.rst Outdated Show resolved Hide resolved

tobiasdiez added s: needs work and removed s: needs review labels Jan 10, 2024

kwankyu approved these changes Jan 11, 2024

View reviewed changes

kwankyu added s: positive review and removed s: needs review labels Jan 11, 2024

Matthias Koeppe and others added 19 commits January 21, 2024 16:50

Variant

e170dc6

Cleaner variant

54e32b4

Include table only in HTML doc

dcec5ab

Better colors

6303842

.devcontainer/tox-docker-in-docker: New

4767193

.devcontainer/tox-docker-in-docker: Use a smaller image

c4fbed5

src/doc/en/developer/portability_testing.rst: Fix

59e332f

.devcontainer/tox-docker-in-docker: Use a smaller image (fixup)

1614c85

.devcontainer/tox-docker-in-docker/devcontainer.json: Add openFiles c…

af27fbe

…ustomization

src/doc/en/developer/portability_testing.rst: Document running tox in…

dc68401

… docker-in-docker

.devcontainer/tox-docker-in-docker/devcontainer.json: Install tox and…

033911f

… github-cli as dev container features

Distinguish minimal and standard by colors

c742a17

Table cosmetics

5614a9c

add -maximal to table

1bd75c0

Try a different 3-column layout

cc12dcc

tox.ini: Make master platform list more visible, add comments

35a7cc4

Tune 3-column layout

582d1fa

Suppress table row for conda-minimal; no images available

ef155e4

src/doc/en/developer/portability_testing.rst: Use .. NOTE::

aa7dab3

kwankyu added s: positive review and removed s: needs review labels Jan 22, 2024

vbraun merged commit 1baddee into sagemath:develop Feb 2, 2024

github-actions bot removed the s: positive review label Feb 2, 2024

Uh oh!

tox -e update_docker_platforms #36954

tox -e update_docker_platforms #36954

Uh oh!

Conversation

mkoeppe commented Dec 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Checklist

⌛ Dependencies

Uh oh!

kwankyu commented Jan 9, 2024

Uh oh!

tobiasdiez left a comment

Choose a reason for hiding this comment

Uh oh!

kwankyu commented Jan 9, 2024

Uh oh!

tobiasdiez commented Jan 9, 2024

Uh oh!

mkoeppe commented Jan 9, 2024

Uh oh!

mkoeppe commented Jan 9, 2024

Uh oh!

kwankyu commented Jan 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

kwankyu commented Jan 10, 2024

Uh oh!

tobiasdiez commented Jan 10, 2024

Uh oh!

tobiasdiez commented Jan 10, 2024

Uh oh!

mkoeppe commented Jan 10, 2024

Uh oh!

mkoeppe commented Jan 10, 2024

Uh oh!

mkoeppe commented Jan 10, 2024

Uh oh!

kwankyu commented Jan 10, 2024

Uh oh!

mkoeppe commented Jan 10, 2024

Uh oh!

mkoeppe commented Jan 10, 2024

Uh oh!

kwankyu commented Jan 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tobiasdiez commented Jan 10, 2024

Uh oh!

mkoeppe commented Jan 10, 2024

Uh oh!

kwankyu commented Jan 10, 2024

Uh oh!

mkoeppe commented Jan 11, 2024

Uh oh!

mkoeppe commented Jan 11, 2024

Uh oh!

kwankyu commented Jan 11, 2024

Uh oh!

kwankyu commented Jan 11, 2024

Uh oh!

kwankyu left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 22, 2024

Uh oh!

kwankyu commented Jan 22, 2024

Uh oh!

mkoeppe commented Jan 22, 2024

Uh oh!

Uh oh!

`tox -e update_docker_platforms` #36954

`tox -e update_docker_platforms` #36954

mkoeppe commented Dec 24, 2023 •

edited

Loading

kwankyu commented Jan 10, 2024 •

edited

Loading

kwankyu commented Jan 10, 2024 •

edited

Loading