Docker build fails -- no module 'cbor' when running tools/mcuboot/imgtool/boot_record.py. Importing 'cbor2' instead works.

[stoehraj]

Verification

• [X] I searched for similar bug reports and found none was relevant.

What happened?

Docker build fails, with "no module named 'cbor'"

What should happen instead?

Docker build succeeds.

Reproduction steps

1. From root directory, build infinitime-build docker image:

   docker image build -t infinitime-build ./docker

   When building the image, it looks at mcuboot/requirements.txt and downloads cbor2 to fulfill the requirement of cbor>=1.0.0 with this output:

   Collecting cbor2 (from -r /opt/mcuboot/scripts/requirements.txt (line 4))
   Downloading https://files.pythonhosted.org/packages/49/74/2b9f9b76e2831f54d70e177a4e602e72cb86441632d1bcc15be1f04a73f6/cbor2-5.4.2.post1.tar.gz (85kB)
   

2. Alternatively, pull latest pre-built image:

   docker pull pfeerick/infinitime-build

3. Run the docker build:

   docker run --rm -it -v $(pwd):/sources infinitime-build

   or

   docker run --rm -it -v $(pwd):/sources pfeerick/infinitime-build

4. The build fails with this output:

   Traceback (most recent call last):
     File "../../tools/mcuboot/imgtool.py", line 17, in <module>
       from imgtool import main
     File "/sources/tools/mcuboot/imgtool/main.py", line 23, in <module>
       from imgtool import image, imgtool_version
     File "/sources/tools/mcuboot/imgtool/image.py", line 22, in <module>
       from .boot_record import create_sw_component_data
     File "/sources/tools/mcuboot/imgtool/boot_record.py", line 17, in <module>
       import cbor
   ModuleNotFoundError: No module named 'cbor'
   

More details?

Modifying tools/mcuboot/imgtool/boot_record.py to use import cbor2 rather than import cbor fixes the issue and allows the build to finish successfully.

I'm not sure where exactly this should be fixed (or if this impacts non-Docker builds) so I documented it here for somebody more familiar with the build system.

Version

develop

Companion app

N/A

[JF002]

Thanks for the report! Is this an issue in the docker file, or an issue in imgtool from MCUBoot that imports cbor while the requirement file installs cbor2 ?

[AlexXZero]

I found that same issue on my PC using docker container. I have fixed it locally just installing cbor packet into the docker container:

Then I'm able to build FW using command /opt/build.sh. But this fix is working only while container is logged in, i.e. I need to install cbor packet each time when I run docker container.

I'm not sure if there is a proper solution, but I suggest to add pip3 install cbor into Dockerfile, then rebuild the docker image.

~### Update Looks like adding RUN pip3 install cbor to the Docker file doesn't fix the issue. I tested it using local docker image and I can't see original error any more, but I get another error:~

[ 92%] Building CXX object src/CMakeFiles/pinetime-mcuboot-app.dir/components/heartrate/Biquad.cpp.o
Traceback (most recent call last):
  File "../../tools/mcuboot/imgtool.py", line 20, in <module>
    main.imgtool()
  File "/usr/local/lib/python3.6/dist-packages/click/core.py", line 1128, in __call__
    return self.main(*args, **kwargs)
  File "/usr/local/lib/python3.6/dist-packages/click/core.py", line 1053, in main
    rv = self.invoke(ctx)
  File "/usr/local/lib/python3.6/dist-packages/click/core.py", line 1659, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/usr/local/lib/python3.6/dist-packages/click/core.py", line 1395, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/local/lib/python3.6/dist-packages/click/core.py", line 754, in invoke
    return __callback(*args, **kwargs)
  File "/sources/tools/mcuboot/imgtool/main.py", line 309, in sign
    img.save(outfile, hex_addr)
  File "/sources/tools/mcuboot/imgtool/image.py", line 226, in save
    with open(path, 'wb') as f:
PermissionError: [Errno 13] Permission denied: 'pinetime-mcuboot-recovery-image-1.8.0.bin'
[ 92%] Building CXX object src/CMakeFiles/pinetime-mcuboot-app.dir/components/heartrate/Ptagc.cpp.o
src/CMakeFiles/pinetime-mcuboot-recovery.dir/build.make:1580: recipe for target 'src/pinetime-mcuboot-recovery-1.8.0.out' failed
make[2]: *** [src/pinetime-mcuboot-recovery-1.8.0.out] Error 1
make[2]: *** Deleting file 'src/pinetime-mcuboot-recovery-1.8.0.out'
CMakeFiles/Makefile2:254: recipe for target 'src/CMakeFiles/pinetime-mcuboot-recovery.dir/all' failed
make[1]: *** [src/CMakeFiles/pinetime-mcuboot-recovery.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....

Update 2

Ignore the previous update. It was my fault cased by building previous FW image using root user (I needed it to be able to install cbor before building the image). I proved that both fixes are working using clean repo.

So we have two options to fix the issue:

1. Update docker image to install cbor packet into it.

diff --git a/docker/Dockerfile b/docker/Dockerfile
index 0a4f69f..3a1df95 100644
--- a/docker/Dockerfile
+++ b/docker/Dockerfile
@@ -26,6 +26,7 @@ RUN apt-get update -qq \
 
 RUN pip3 install adafruit-nrfutil
 RUN pip3 install -Iv cryptography==3.3
+RUN pip3 install cbor
 
 # build.sh knows how to compile
 COPY build.sh /opt/

2. Modifying tools/mcuboot/imgtool/boot_record.py to use import cbor2 rather than import cbor

diff --git a/tools/mcuboot/imgtool/boot_record.py b/tools/mcuboot/imgtool/boot_record.py
index 4112b22..255c639 100644
--- a/tools/mcuboot/imgtool/boot_record.py
+++ b/tools/mcuboot/imgtool/boot_record.py
@@ -14,7 +14,7 @@
 # limitations under the License.
 
 from enum import Enum
-import cbor
+import cbor2
 
 
 class SwComponent(int, Enum):

[AlexXZero]

I guess initially cbor packet was installed to docker image as dependence of adafruit-nrfutil, then it was also used by imgtool. Then dependency for adafruit-nrfutil was updated and now it requires cbor2 packet. (Note: even if packet name looks similar, but actually it is another packet which API is different).

It looks like the cbor2 API still includes the required cbor.dumps function for imgtool, but we can't be 100% sure it's compatible with imgtool's expected behavior. So I suggest to install both packages (cbor2 and cbor) in docker container. (Note: we don't need to select the cbor2 package manually, as it will be installed as a dependency).

Alternative option: copy tools/mcuboot/requirements.txt into docker container, then use it to install every required for imgtool packets (to avoid similar errors in the future).

[JF002]

Thanks for this analysis :+1: ! So I guess the less intrusive solution (the one that implies the less changes to mcuboot files) is simply to install cbor in the container?

[stoehraj]

@AlexXZero -- thanks for all the additional effort here. I don't think installing cbor in the container is the cleanest solution here, though.

Looks like in the build.sh script for the docker image, it gets the requirements.txt file directly from the mcu-tools/mcuboot repository:

GetMcuBoot() {
  git clone https://github.com/mcu-tools/mcuboot.git "$TOOLS_DIR/mcuboot"
  pip3 install -r "$TOOLS_DIR/mcuboot/scripts/requirements.txt"
}

https://github.com/InfiniTimeOrg/InfiniTime/blob/4f649a85448fc79b55202d9db0e8f4c8828975e3/docker/build.sh#L45

Perhaps it would be better to have this point towards Infinitime's requirements.txt file (https://github.com/InfiniTimeOrg/InfiniTime/blob/develop/tools/mcuboot/requirements.txt) instead? This would ensure that the docker image is always in-line with what Infinitime is using rather than just the default branch from mcu-tools/mcuboot.

[stoehraj]

Or, instead, we could point to the specific requirements.txt from mcutools for the version Infinitime is using: https://github.com/mcu-tools/mcuboot/blob/71b8f981df4f4e65df1757c6ba14ce8d2b7e7696/scripts/requirements.txt

[stoehraj]

@AlexXZero, @JF002 -- This is what that might look like in docker/build.sh:

 GetMcuBoot() {
-  git clone https://github.com/mcu-tools/mcuboot.git "$TOOLS_DIR/mcuboot"
-  pip3 install -r "$TOOLS_DIR/mcuboot/scripts/requirements.txt"
+  mkdir -p "$TOOLS_DIR/mcuboot"
+  wget -q https://raw.githubusercontent.com/mcu-tools/mcuboot/71b8f981df4f4e65df1757c6ba14ce8d2b7e7696/scripts/requirements.txt \
+    -O "$TOOLS_DIR/mcuboot/requirements.txt"
+  pip3 install -r "$TOOLS_DIR/mcuboot/requirements.txt"
 }

Just tried this locally and it built the image and the resulting container built Infinitime just fine with no other changes. Also, it has the advantage of not cloning the entire mcu-tools/mcuboot repository just to get that requirements.txt file so it's a bit faster to build the image as well!

I can get a PR up with these changes if desired.

[AlexXZero]

@stoehraj Thank you for your feedback, I really like your suggestion. For me it also sounds more proper than just adding a cbor packet into Docker container. I also noticed that whole tools/mcuboot folder is come from MCUBoot repo. Should we use git module here some how, instead of copy of the original folder? (Probably it should be another ticket with this improvement).

@JF002 I found the following solution for adding a specific folder:

1. Create a submodules directory.
2. Add the submodule in this directory.
3. Create a symlink to the specific directory inside the submodule.

This way you have default Git submodule behaviour and in your project you only use a subset of the whole submodule.

Any thoughts about this?

[stoehraj]

Personally, I think if there's no intention of moving off of the current version of mcuboot then it wouldn't be worth the effort to add it as a submodule.

[JF002]

Mhm the integration of MCUBoot in the project seems a bit messy... It's not the first time an update in the MCUBoot repository breaks the build or the docker container of InfiniTime. That's OK, MCUBoot is evolving, they change their libraries, dependencies,... But I honestly thought we had already fixed the version of MCUBoot we were pulling to avoid those breaks.

There is very little point on trying to use the latest version of MCUBoot, as the bootloader is unlikely to be upgraded any time soon.

A bit of context: InfiniTime source code does not use MCUBoot at all. The only thing that is more or less related to MCUBoot is the 0x8000 offset that is applied when linking the binary intended to be run by the bootloader (pinetime-mcuboot-app).

However, the build system uses a tool ('imgtool.py`) to generate an image that is bootable by MCUBoot. And this tool come from the MCUBoot repo, of course.

If you look at src/CMakeLists.txt (https://github.com/InfiniTimeOrg/InfiniTime/blob/develop/src/CMakeLists.txt#L948), you'll notice that it uses 'imgtool.pyfrom the foldertools/mcubootfrom the source code, and **not** the one that was cloned into/opt/` by the docker build script.

IIRC, at the beginning, the mcuboot image generation was not implemented in CMake, so the docker container had to install mcuboot to generate the image. This is not the case anymore : the whole mcuboot repo is copied into the InfiniTime project (in src/tools/mcuboot), and we use the script from that folder (we could probably use a git submodule that points to a specific version of mcuboot, instead of copying the whole repo in the source code).

If I'm not wrong, we should be able to remove the download of MCUBoot in the container, and just use the file requirements.txt from the source code, as @stoehraj suggested.

[emosenkis]

This is still failing for me at HEAD (commit 7103f9d8063431b778df83244de5862d963e1ce8) with the same cbor import failure, fixable by simply running pip install cbor. Regardless of which fix is the best, it would be great to fix this so it's not adding friction to new contributors.

[JF002]

@emosenkis Which docker image do you use? We updated the docker image from pfeerick/infinitime-build to infinitime/infinitime-build a while ago. I use this image locally and it's also the one used by our CI on Github, so it should just work.

[emosenkis]

I used the one picked up automatically by VSCode from .devcontainer - I sent a PR to address the issues I ran into