-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Improve devcontainer #1497
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
base: main
Are you sure you want to change the base?
Improve devcontainer #1497
Conversation
emosenkis
commented
Dec 28, 2022
- Installs lv_font_conv and cbors
- Installs sudo and adds user to sudo group in case they need to make changes later
- Installs lv_font_conv and cbors - Installs sudo and adds user to sudo group in case they need to make changes later
|
I don't think the currently active maintainers use the devcontainer, so you'll have to be more descriptive about the purpose of these changes, so we can properly review them. |
|
Currently when developing InfiniTime inside the devcontainer, neither lv_font_conv nor npm(to install lv_font_conv manually) is installed. This results in being unable to compile. With this change both npm and lv_font_conv are installed automatically, and compilation succeeds. Question: Might it be more reasonable to only install npm and handle installing lv_font_conv via a vscode task. Just like installing the git submodules is handled? @emosenkis What is cbors for? |
|
From reading the docs, I think it should be possible to adapt the devcontainer to use the main Dockerfile, which will reduce code duplication and maintenance cost. I don't use VS Code myself though, so I don't think I can help too much with that. If someone who is more familiar with devcontainers can shed some light, that would be appreciated. |
|
You are totally correct. We currently seem to have two almost identical copies of Dockerfile and build.sh script in /.devcontainer and /docker. The differences seem to be:
None of these should be a problem to unify these two Dockerfiles. I would propose to add a argument so that the parts only necessary for devcontainer are only installed when needed. |
|
If any currently active maintainers use docker for compilation I would assume everything used there is more accurate/up to date. |
|
The main Docker image is up-to-date, and is used for the GitHub CI. |
|
Perfect! So we only need to conditionally add the required packages for the devcontainer and create another user. |
| RUN pip3 install -r ./mcuboot/scripts/requirements.txt | ||
|
|
||
| RUN npm install -g lv_font_conv | ||
| RUN pip install cbor # Upstream mcuboot has moved on to cbor2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pip3 for consistency with above.