Re: [libvirt-glib PATCH] gitlab: introduce CI jobs testing git master & distro libvirt

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2020-06-11 at 17:42 +0100, Daniel P. Berrangé wrote:
>  ci/libvirt-centos-7.Dockerfile                |  88 +++++++
>  ci/libvirt-centos-8.Dockerfile                |  64 +++++
>  ci/libvirt-centos-stream.Dockerfile           |  58 +++++
>  ci/libvirt-debian-10.Dockerfile               |  58 +++++
>  ci/libvirt-debian-9.Dockerfile                |  61 +++++
>  ci/libvirt-debian-sid.Dockerfile              |  58 +++++
>  ci/libvirt-fedora-31.Dockerfile               |  55 +++++
>  ci/libvirt-fedora-32.Dockerfile               |  55 +++++
>  ...rt-fedora-rawhide-cross-mingw32.Dockerfile | 133 ++++++++++
>  ...rt-fedora-rawhide-cross-mingw64.Dockerfile | 133 ++++++++++
>  ci/libvirt-fedora-rawhide.Dockerfile          |  56 +++++
>  ci/libvirt-opensuse-151.Dockerfile            |  57 +++++
>  ci/libvirt-ubuntu-1804.Dockerfile             |  61 +++++
>  ci/libvirt-ubuntu-2004.Dockerfile             |  58 +++++
>  ci/refresh                                    |  36 +++

Please put all the Dockerfiles in ci/containers, as is already the
case for libvirt: the extra directory will ensure things remain tidy
even after we roll out support for Cirrus CI to all projects.

You're also missing the usual README.rst explaining how the
Dockerfiles are generated.

[...]
>  stages:
>    - prebuild
> +  - containers
> +  - builds
> +  - docs

The 'docs' stage is not used anywhere.

> +.script_variables: &script_variables |
> +  export MAKEFLAGS="-j$(getconf _NPROCESSORS_ONLN)"
> +  export VROOT="$SCRATCH_DIR/vroot"
> +  export CCACHE_BASEDIR="$(pwd)"
> +  export CCACHE_DIR="$CCACHE_BASEDIR/ccache"
> +  export CCACHE_MAXSIZE="500M"
> +  export PATH="$CCACHE_WRAPPERSDIR:$VROOT/bin:$PATH"
> +  export SCRATCH_DIR="/tmp/scratch"
> +  export PKG_CONFIG_PATH="$VROOT/lib/pkgconfig"

You need to define $SCRATCH_DIR before $VROOT, otherwise the latter
will get the wrong value.

I also just realized that the way we set $CCACHE_BASEDIR might not
work for the libvirt build that we perform as a prerequisite, and
since we use the same paths across builds anyway there doesn't seem
to be a point in setting it. So I suggest we have

  export SCRATCH_DIR="/tmp/scratch"
  export VROOT="$SCRATCH_DIR/vroot"
  export CCACHE_DIR="$SCRATCH_DIR/ccache"

> +.git_native_build_job_template: &git_native_build_job_definition
> +  image: $CI_REGISTRY_IMAGE/ci-$NAME:latest
> +  stage: builds
> +  before_script:
> +    - *script_variables
> +    - export LD_LIBRARY_PATH="$VROOT/lib"

You could set $LD_LIBRARY_PATH in script_variables without causing
any issue for the dist-build jobs.

> +.dist_native_build_job_template: &dist_native_build_job_definition
> +  image: $CI_REGISTRY_IMAGE/ci-$NAME:latest
> +  stage: builds
> +  before_script:
> +    - *script_variables

This job is missing GitLab CI cache support, as is the one above.

> +  script:
> +    - mkdir build
> +    - cd build
> +    - ../autogen.sh --prefix="$VROOT"
> +    - $MAKE install
> +    - $MAKE dist

Do we want distcheck here, or is the combination of dist plus
building the RPM package (which effectively runs check inside the
generated tarball) good enough?

> +x64-centos-7-container:
> +  <<: *container_job_definition
> +  variables:
> +    NAME: centos-7
> +
> +x64-centos-8-container:
> +  <<: *container_job_definition
> +  variables:
> +    NAME: centos-8

You have a Dockerfile for CentOS Stream, but you're not building the
corresponding container...

> +x64-centos-8-git-build:
> +  <<: *git_native_build_job_definition
> +  variables:
> +    NAME: centos-8
> +
> +x64-centos-7-dist-build:
> +  <<: *dist_native_build_job_definition
> +  variables:
> +    NAME: centos-7
> +
> +x64-centos-8-dist-build:
> +  <<: *dist_native_build_job_definition
> +  variables:
> +    NAME: centos-8

... or running the corresponding build.

-- 
Andrea Bolognani / Red Hat / Virtualization




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux