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

Add devcontainer.json for development with VS Code in a Docker container #33671

Closed
mkoeppe opened this issue Apr 10, 2022 · 409 comments
Closed

Add devcontainer.json for development with VS Code in a Docker container #33671

mkoeppe opened this issue Apr 10, 2022 · 409 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 10, 2022

https://code.visualstudio.com/docs/remote/containers

We add two new sections in the developer's guide:
https://47f8b984ef964a5aa34147393fbdc32e0dde88ad--sagemath-tobias.netlify.app/developer/portability_testing.html#using-our-pre-built-docker-images-published-on-ghcr-io

We also set up devcontainer configurations for the CoCalc and computop/sage Docker images, as well as downstream distribution packaging of Sage.

Tested devcontainer.json of

  • portability-ubuntu-jammy-standard: builds well; runs well
  • develop-docker-computop: builds well; runs well
  • downstream-docker-cocalc: builds well; runs well (except machines with the issue Do not require AVX when building with SAGE_FAT_BINARY #32434)
  • downstream-docker-computop: builds well; runs well
  • downstream-archlinux-latest: builds well; runs well
  • downstream-conda-forge-latest: builds well; runs well

Follow-ups:

Depends on #33873
Depends on #34352

CC: @tobiasdiez @dimpase @williamstein @culler @saraedum @kwankyu

Component: user interface

Author: Tobias Diez, Matthias Koeppe, Kwankyu Lee

Branch/Commit: 4affef2

Reviewer: Kwankyu Lee, Matthias Koeppe

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

@mkoeppe mkoeppe added this to the sage-9.6 milestone Apr 10, 2022
@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Apr 17, 2022
@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 11, 2022

Dependencies: #29536

@tobiasdiez
Copy link
Contributor

Author: Tobias Diez

@tobiasdiez
Copy link
Contributor

Commit: 85a7100

@tobiasdiez
Copy link
Contributor

Changed dependencies from #29536 to none

@tobiasdiez
Copy link
Contributor

Branch: public/build/devcontainer

@tobiasdiez
Copy link
Contributor

comment:5

A basic devcontainer definition is now added for ubuntu systems.


New commits:

85a7100Add devcontainer for ubuntu

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 14, 2022

Changed commit from 85a7100 to 11110ae

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 14, 2022

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

11110aeAdd bootstrap

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 14, 2022

Changed commit from 11110ae to c5295e3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 14, 2022

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

c5295e3enable-build-as-root

@tobiasdiez

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 14, 2022

comment:9

Thanks! That's a beginning. Why do you not use the full build (-with-targets)?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 14, 2022

Changed commit from c5295e3 to 872c0c8

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 14, 2022

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

58e112abootstrap-conda: Use script package _develop instead of hardcoded list of dev tools; remove unused RECOMMENDED
21e06c5bootstrap-conda: Write an additional comment
4290b11Add pytest-xdist package
a365828build/pkgs/pytest/dependencies: Add missing dep
a2e396bbuild/pkgs/pytest_xdist/dependencies: Add missing dep
2ec0061Merge #33825
5857902build/pkgs/_develop/dependencies: Add pytest, pytest_xdist
2368adcbootstrap-conda: Write an additional comment (fixup)
0df31a9bootstrap-conda: Rewrite using fewer redirects
872c0c8.devcontainer: Generalize to other distros

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 14, 2022

Changed author from Tobias Diez to Tobias Diez, Matthias Koeppe

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 14, 2022

Dependencies: #33825

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 14, 2022

Changed dependencies from #33825 to #33851

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 14, 2022

Changed commit from 872c0c8 to 96bb5ad

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 14, 2022

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

8d7f7abMerge #33851
96bb5ad.devcontainer: Generalize to other distros

@tobiasdiez
Copy link
Contributor

comment:14

Replying to @mkoeppe:

Thanks! That's a beginning. Why do you not use the full build (-with-targets)?

I didn't saw the point in that. In contrast to gitpod you don't know which version of sage the developer will use in the devcontainer, especially since devcontainer are cached by vscode until they are manually rebuild.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 14, 2022

comment:15

You also don't know that on gitpod, as I've explained in #33113 comment:18

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 22, 2022

comment:294

OK

@tobiasdiez
Copy link
Contributor

comment:295

Sorry Matthias, but the original version of the code was working well with codespaces. I'm happy you further worked on it, but it's not fair that you introduce issues with codespaces and then say "I don't care, lets fix them in a follow up ticket". And this is not for the sake of codespaces, but as I said above microsoft treats the codespaces as the defacto standard; so if its not working there then vscode will likely break in the future as well (and it will not work with other providers, such as devcontainer cli and gitpod). In fact, the vscode team works on using the same cli as codespaces, see microsoft/vscode-remote-release#6859 and microsoft/vscode-remote-release#7033.

So what about deleting the portability dockerfile completely, and simply specifying the image in the devcontainer? Since the dockerfile is simply forwarding to the github image anyway, this should suffice and be quicker. Moreover, can we use a different "target" that doesn't include the sage source code?

Finally, the names of the "post" scripts need to be changed and the docs changed to reflect the recent changes of "onCreateCommand" etc.

@kwankyu
Copy link
Collaborator

kwankyu commented Aug 22, 2022

comment:296

Replying to @tobiasdiez:

... as I said above microsoft treats the codespaces as the defacto standard; so if its not working there then vscode will likely break in the future as well (and it will not work with other providers, such as devcontainer cli and gitpod). In fact, the vscode team works on using the same cli as codespaces,

Would you comment on: #34403 comment:3 ?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 22, 2022

comment:297

Replying to @tobiasdiez:

Sorry Matthias, but the original version of the code was working well with codespaces. I'm happy you further worked on it, but it's not fair that you introduce issues with codespaces and then say "I don't care, lets fix them in a follow up ticket".

I've set the scope of the ticket to be actionable. Work on codespaces is not actionable for me. You want to support them, you do the work on the follow-up ticket.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 22, 2022

Changed commit from d5610b1 to 179406f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 22, 2022

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

179406fsrc/doc/en/developer/portability_testing.rst: Update references to devcontainer ...Command

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 22, 2022

comment:299

Replying to @tobiasdiez:

So what about deleting the portability dockerfile completely

It's there as preparation for the following improvement (which is waiting for #29536):

  • always pull FROM the build stage -with-system-packages
  • then COPY the Sage SAGE_LOCAL (but not the old Sage source tree) from the requested build stage such as the default -with-targets
    This creates a smaller version of the container image.

@mkoeppe

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 22, 2022

Changed commit from 179406f to 4affef2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 22, 2022

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

4affef2.devcontainer/onCreate.sh, portability-updateContent.sh: Rename from post_create.sh, portability-post_start.sh

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 22, 2022

comment:302

Replying to @tobiasdiez:

the names of the "post" scripts need to be changed and the docs changed to reflect the recent changes of "onCreateCommand" etc.

Thanks for catching this. Done now.

@kwankyu
Copy link
Collaborator

kwankyu commented Aug 24, 2022

comment:304

Simple renaming. Positive again.

@vbraun
Copy link
Member

vbraun commented Aug 30, 2022

Changed branch from public/build/devcontainer to 4affef2

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