Skip to content

Conversation

@emosenkis
Copy link

  • 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
@Riksu9000 Riksu9000 added the maintenance Background work label Jan 10, 2023
@Riksu9000
Copy link
Contributor

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.

@BloodStainedCrow
Copy link
Contributor

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?

@FintasticMan
Copy link
Member

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.

@BloodStainedCrow
Copy link
Contributor

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:

  • Installing locales and formatting tools like clang-tidy in devcontainer
  • installing all required build components in docker (That should happen in .devcontainer anyway, see o.p.)
  • adding an additional user in devcontainer for vscode to connect to
  • in devcontainer the get scripts from build.sh are explicit instead of calling build.sh

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.
On the same note I would propose unifying the two build.sh scripts in /.devcontainer and /docker for that someone else will have to have a look at them to determine which is correct since they have weird differences like gcc-version where I do not know which is correct.

@BloodStainedCrow
Copy link
Contributor

If any currently active maintainers use docker for compilation I would assume everything used there is more accurate/up to date.

@FintasticMan
Copy link
Member

The main Docker image is up-to-date, and is used for the GitHub CI.

@BloodStainedCrow
Copy link
Contributor

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

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.

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

Labels

maintenance Background work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants