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

[John Snow <jsnow@redhat.com>]

On 5/25/21 3:55 PM, Cleber Rosa wrote:
> 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>
> 

Hm....

I'll try. You're right that it would make the pipeline a lot simpler 
because it'd have less cruft.

but thank you for the R-B in the meantime ;)

--js