Re: [PATCH 09/45] CI: Install pyelftools for builds

From: Simon Glass <sjg@chromium.org>
To: Tom Rini <trini@konsulko.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>,
	AKASHI Takahiro <takahiro.akashi@linaro.org>,
	 Huang Jianan <jnhuang95@gmail.com>,
	Joel Stanley <joel@jms.id.au>, Rick Chen <rick@andestech.com>,
	U-Boot Mailing List <u-boot@lists.denx.de>
Subject: Re: [PATCH 09/45] CI: Install pyelftools for builds
Date: Fri, 30 Sep 2022 07:28:52 -0600	[thread overview]
Message-ID: <CAPnjgZ0WXTaVQFqKMma-KApoJG9WYN=RpVhCXh7A37qMWm++EQ@mail.gmail.com> (raw)
In-Reply-To: <20220930125624.GQ3044094@bill-the-cat>

Hi Tom,

On Fri, 30 Sept 2022 at 06:56, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Sep 29, 2022 at 05:55:54PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 29 Sept 2022 at 09:05, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Wed, Sep 28, 2022 at 04:20:41AM -0600, Simon Glass wrote:
> > > > Hi Heinrich,
> > > >
> > > > On Mon, 26 Sept 2022 at 00:35, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > > > >
> > > > > On 9/25/22 17:02, Simon Glass wrote:
> > > > > > Binman needs this module to build sandbox_vpl so install it in the
> > > > > > before_script and in the world build. The existing pip install is too
> > > > > > late for the buildman invocation.
> > > > > >
> > > > > > Add it to the docker file for the future.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > ---
> > > > > >
> > > > > >   .gitlab-ci.yml          | 2 ++
> > > > > >   tools/docker/Dockerfile | 1 +
> > > > > >   2 files changed, 3 insertions(+)
> > > > > >
> > > > > > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > > > > > index 8e94bf8d4e8..47e36edcc4a 100644
> > > > > > --- a/.gitlab-ci.yml
> > > > > > +++ b/.gitlab-ci.yml
> > > > > > @@ -34,6 +34,7 @@ stages:
> > > > > >       # If we've been asked to use clang only do one configuration.
> > > > > >       - export UBOOT_TRAVIS_BUILD_DIR=/tmp/${TEST_PY_BD}
> > > > > >       - echo BUILD_ENV ${BUILD_ENV}
> > > > > > +    - pip install pyelftools
> > > > >
> > > > > Why do we need pip install here? Is pyelftools used outside the Docker
> > > > > image to which you add pyelftools below?
> > > >
> > > > We don't need it if Tom wants to regen the docker image, but I'm not
> > > > sure how often he does that. So this change actually makes CI work, in
> > > > either case.
> > >
> > > I would re-generate the CI image as part of the series, and we should
> > > really do this separate and drop the pip install pyelftools we already
> > > have, too.
> >
> > OK, shall I resend this patch accordingly or will you just change it?
> > Or should I split into two patches, one to add to Dockerfile (I hate
> > the capital D!) and one to remove the pip install?
>
> In v2 of the series, doing all of this as patch #1 will make my life
> easier, how about that?

Yes OK.

Regards,
Simon