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 -e update_docker_platforms #36954

Merged
merged 24 commits into from
Feb 2, 2024

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented Dec 24, 2023

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

@mkoeppe mkoeppe self-assigned this Dec 24, 2023
@mkoeppe mkoeppe force-pushed the devcontainer-portability-generate branch 8 times, most recently from ba2be1c to 615799f Compare December 24, 2023 08:06
@mkoeppe mkoeppe marked this pull request as ready for review December 24, 2023 08:22
@mkoeppe mkoeppe force-pushed the devcontainer-portability-generate branch 2 times, most recently from 228c3a0 to ce59d0f Compare December 25, 2023 05:54
@mkoeppe mkoeppe force-pushed the devcontainer-portability-generate branch 3 times, most recently from e4c2d42 to 61a4ac0 Compare December 28, 2023 08:18
@kwankyu
Copy link
Collaborator

kwankyu commented Jan 9, 2024

This looks better

Screen Shot 2024-01-09 at 4 40 34 PM

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

Copy link
Contributor

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Collaborator

kwankyu commented Jan 9, 2024

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.

Screenshot 2024-01-09 at 8 14 19 PM

@tobiasdiez
Copy link
Contributor

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
Copy link
Contributor Author

mkoeppe commented Jan 9, 2024

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
Copy link
Contributor Author

mkoeppe commented Jan 9, 2024

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
Copy link
Collaborator

kwankyu commented Jan 10, 2024

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

Screen Shot 2024-01-10 at 12 47 40 PM

@kwankyu
Copy link
Collaborator

kwankyu commented Jan 10, 2024

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
Copy link
Contributor

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
Copy link
Contributor

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 mkoeppe force-pushed the devcontainer-portability-generate branch from 390c949 to aa7dab3 Compare January 22, 2024 00:50
Copy link

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

@kwankyu
Copy link
Collaborator

kwankyu commented Jan 22, 2024

Enough lingering here.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 22, 2024

Thanks!

vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 24, 2024
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

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.

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [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

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->
- Depends on sagemath#37030 (split out from here)

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36954
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Matthias Köppe, Tobias Diez
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 27, 2024
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

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.

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [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

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->
- Depends on sagemath#37030 (split out from here)

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36954
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Matthias Köppe, Tobias Diez
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 29, 2024
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

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.

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [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

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->
- Depends on sagemath#37030 (split out from here)

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36954
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Matthias Köppe, Tobias Diez
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 30, 2024
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

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.

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [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

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->
- Depends on sagemath#37030 (split out from here)

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36954
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Matthias Köppe, Tobias Diez
@vbraun vbraun merged commit 1baddee into sagemath:develop Feb 2, 2024
18 of 19 checks passed
@mkoeppe mkoeppe deleted the devcontainer-portability-generate branch February 2, 2024 22:05
@mkoeppe mkoeppe added this to the sage-10.3 milestone Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants