diff mbox series

[v1] CI: Add jsonschema python module

Message ID 20230829143710.204875-1-andrejs.cainikovs@toradex.com
State Deferred
Delegated to: Tom Rini
Headers show
Series [v1] CI: Add jsonschema python module | expand

Commit Message

Andrejs Cainikovs Aug. 29, 2023, 2:37 p.m. UTC
Some TI boards utilizes `ti-board-config` via binman device
tree node, which when built via binman, triggers schema validation
for board specific yaml configuration files.

This change adds jsonschema python module to CI Docker image, which
allows these targets to be built by CI without errors.

Signed-off-by: Andrejs Cainikovs <andrejs.cainikovs@toradex.com>
---
 tools/docker/Dockerfile | 1 +
 1 file changed, 1 insertion(+)

Comments

Tom Rini Aug. 29, 2023, 3:04 p.m. UTC | #1
On Tue, Aug 29, 2023 at 04:37:10PM +0200, Andrejs Cainikovs wrote:

> Some TI boards utilizes `ti-board-config` via binman device
> tree node, which when built via binman, triggers schema validation
> for board specific yaml configuration files.
> 
> This change adds jsonschema python module to CI Docker image, which
> allows these targets to be built by CI without errors.
> 
> Signed-off-by: Andrejs Cainikovs <andrejs.cainikovs@toradex.com>
> ---
>  tools/docker/Dockerfile | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
> index 3d2b64a355f..2f3121ffcf6 100644
> --- a/tools/docker/Dockerfile
> +++ b/tools/docker/Dockerfile
> @@ -98,6 +98,7 @@ RUN apt-get update && apt-get install -y \
>  	python2.7 \
>  	python3 \
>  	python3-dev \
> +	python3-jsonschema \
>  	python3-pip \
>  	python3-pyelftools \
>  	python3-sphinx \

Interesting.  How exactly are you using these CI images? We have
tools/buildman/requirements.txt now to cover the newly-added modules in
CI, as we call pip on that.  But, it's not being used in the prepopulate
the pip cache stage in the Dockerfile.  And, I'm not immune to the idea
that we should instead be installing the distro package here.
Andrejs Cainikovs Aug. 29, 2023, 3:37 p.m. UTC | #2
On 29/08/2023 17:04, Tom Rini wrote:
> Interesting.  How exactly are you using these CI images? We have
> tools/buildman/requirements.txt now to cover the newly-added modules in
> CI, as we call pip on that.  But, it's not being used in the prepopulate
> the pip cache stage in the Dockerfile.  And, I'm not immune to the idea
> that we should instead be installing the distro package here.

I see in Dockerfile test/py/requirements.txt and
doc/sphinx/requirements.txt, but not tools/buildman/requirements.txt, so
whatever is mentioned there is not part of the image on CI runners.

Following is the part of *our own* .gitlab.yml file,
which uses a public Docker image
(docker.io/trini/u-boot-gitlab-ci-runner:jammy-20230624-20Jul2023, as of
now):

verdin-am62:
  stage: build
  script: |
    ...
    tools/buildman/buildman -o /tmp -P -E "verdin-am62" -M
    ...

And this is how it fails in our CI without this change:

Building current source for 3 boards (3 threads, 14 jobs per thread)
       arm:  +   verdin-am62_r5
+binman: Unknown entry type 'ti-board-config' in node
'/binman/board-cfg/ti-board-config' (expected etype/ti_board_config.py,
error 'No module named 'jsonschema''
+make[1]: *** [Makefile:1108: .binman_stamp] Error 1
+make: *** [Makefile:177: sub-make] Error 2
Andrejs Cainikovs Aug. 29, 2023, 9:03 p.m. UTC | #3
On 29/08/2023 17:37, Andrejs Cainikovs wrote:
> On 29/08/2023 17:04, Tom Rini wrote:
>> Interesting.  How exactly are you using these CI images? We have
>> tools/buildman/requirements.txt now to cover the newly-added modules in
>> CI, as we call pip on that.  But, it's not being used in the prepopulate
>> the pip cache stage in the Dockerfile.  And, I'm not immune to the idea
>> that we should instead be installing the distro package here.

Tom, I got the full picture right after I sent you my previous email.
This particular file, tools/buildman/requirements.txt in used in *CI*,
right.

Well, in this case my patch doesn't make much sense. Or it does.

I'd like to propose to install packages to runner image instead. I see
the reason why it might be convenient to have packages installed to
runner for particular jobs via .gitlab.yml - a bit less maintenance of
Docker image, which would need to be rebuilt every time there's new
dependency. However, if you think about pipeline execution times, on a
global scale, that's a waste. As an example, I see four occasions of:

pip install -r tools/buildman/requirements.txt

Twice in world build stage and twice in testsuites stage.

If we take another example, packages from py/requirements.txt are
installed 37 (if I counted correctly) times during entire testsuite
stage [2]. And I see you run them a lot [1].

Since you are maintainer of this, I will leave this up to you. But if
you're up for this proposal, I can do this change by myself. Of course,
this would imply rebuilding Docker image and pushing to registry from
your side, to keep stuff synchronized.

[1]: https://source.denx.de/u-boot/u-boot/-/pipelines/17556
[2]: https://source.denx.de/u-boot/u-boot/-/jobs/685270#L366

Regards,
Andrejs.

> 
> I see in Dockerfile test/py/requirements.txt and
> doc/sphinx/requirements.txt, but not tools/buildman/requirements.txt, so
> whatever is mentioned there is not part of the image on CI runners.
> 
> Following is the part of *our own* .gitlab.yml file,
> which uses a public Docker image
> (docker.io/trini/u-boot-gitlab-ci-runner:jammy-20230624-20Jul2023, as of
> now):
> 
> verdin-am62:
>   stage: build
>   script: |
>     ...
>     tools/buildman/buildman -o /tmp -P -E "verdin-am62" -M
>     ...
> 
> And this is how it fails in our CI without this change:
> 
> Building current source for 3 boards (3 threads, 14 jobs per thread)
>        arm:  +   verdin-am62_r5
> +binman: Unknown entry type 'ti-board-config' in node
> '/binman/board-cfg/ti-board-config' (expected etype/ti_board_config.py,
> error 'No module named 'jsonschema''
> +make[1]: *** [Makefile:1108: .binman_stamp] Error 1
> +make: *** [Makefile:177: sub-make] Error 2
>
Tom Rini Sept. 9, 2023, 11:20 p.m. UTC | #4
On Tue, Aug 29, 2023 at 05:37:35PM +0200, Andrejs Cainikovs wrote:
> On 29/08/2023 17:04, Tom Rini wrote:
> > Interesting.  How exactly are you using these CI images? We have
> > tools/buildman/requirements.txt now to cover the newly-added modules in
> > CI, as we call pip on that.  But, it's not being used in the prepopulate
> > the pip cache stage in the Dockerfile.  And, I'm not immune to the idea
> > that we should instead be installing the distro package here.
> 
> I see in Dockerfile test/py/requirements.txt and
> doc/sphinx/requirements.txt, but not tools/buildman/requirements.txt, so
> whatever is mentioned there is not part of the image on CI runners.
> 
> Following is the part of *our own* .gitlab.yml file,
> which uses a public Docker image
> (docker.io/trini/u-boot-gitlab-ci-runner:jammy-20230624-20Jul2023, as of
> now):
> 
> verdin-am62:
>   stage: build
>   script: |
>     ...
>     tools/buildman/buildman -o /tmp -P -E "verdin-am62" -M
>     ...
> 
> And this is how it fails in our CI without this change:
> 
> Building current source for 3 boards (3 threads, 14 jobs per thread)
>        arm:  +   verdin-am62_r5
> +binman: Unknown entry type 'ti-board-config' in node
> '/binman/board-cfg/ti-board-config' (expected etype/ti_board_config.py,
> error 'No module named 'jsonschema''
> +make[1]: *** [Makefile:1108: .binman_stamp] Error 1
> +make: *** [Makefile:177: sub-make] Error 2

Thinking on this more, yes, I think at least as it stands now we expect
pip / similar to be used by consumers of this container if needed for
the jsonschema module.
diff mbox series

Patch

diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
index 3d2b64a355f..2f3121ffcf6 100644
--- a/tools/docker/Dockerfile
+++ b/tools/docker/Dockerfile
@@ -98,6 +98,7 @@  RUN apt-get update && apt-get install -y \
 	python2.7 \
 	python3 \
 	python3-dev \
+	python3-jsonschema \
 	python3-pip \
 	python3-pyelftools \
 	python3-sphinx \