Re: [PATCH v6 24/25] gitlab: add python linters to CI

[Cleber Rosa <crosa@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2193 bytes --]

On Wed, May 12, 2021 at 07:12:40PM -0400, John Snow wrote:
> Add python3.6 to the fedora container image: we need it to run the
> linters against that explicit version to make sure we don't break our
> minimum version promise.
> 
> Add pipenv so that we can fetch precise versions of pip packages we need
> to guarantee test reproducability.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  .gitlab-ci.yml                         | 12 ++++++++++++
>  tests/docker/dockerfiles/fedora.docker |  2 ++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index dcb6317aace..a371c0c7163 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -779,6 +779,18 @@ check-patch:
>      GIT_DEPTH: 1000
>    allow_failure: true
>  
> +
> +check-python:
> +  stage: test
> +  image: $CI_REGISTRY_IMAGE/qemu/fedora:latest
> +  script:
> +    - cd python
> +    - make venv-check
> +  variables:
> +    GIT_DEPTH: 1000
> +  needs:
> +    job: amd64-fedora-container
> +
>  check-dco:
>    stage: build
>    image: $CI_REGISTRY_IMAGE/qemu/centos8:latest
> diff --git a/tests/docker/dockerfiles/fedora.docker b/tests/docker/dockerfiles/fedora.docker
> index 915fdc1845e..6908d69ac37 100644
> --- a/tests/docker/dockerfiles/fedora.docker
> +++ b/tests/docker/dockerfiles/fedora.docker
> @@ -84,6 +84,7 @@ ENV PACKAGES \
>      numactl-devel \
>      perl \
>      perl-Test-Harness \
> +    pipenv \
>      pixman-devel \
>      python3 \
>      python3-PyYAML \
> @@ -93,6 +94,7 @@ ENV PACKAGES \
>      python3-pip \
>      python3-sphinx \
>      python3-virtualenv \
> +    python3.6 \

I personally would prefer having a different container image for this
job.  Because it would:

1. Be super simple (FROM fedora:33 / dnf -y install python3.6 pipenv)
2. Not carry all this unnecessary baggage
3. Not risk building QEMU with Python 3.6 (suppose the ./configure
   probe changes unintentionally)

But, AFAICT there is no precedent in requiring new images for
different types of checks.  So, unless someone else complains loudly,
I'm OK with this.

Reviewed-by: Cleber Rosa <crosa@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]