Re: [PATCH libvirt-dbus v2 2/3] gitlab: introduce CI jobs testing git master & distro libvirt

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

 



On Wed, 2020-06-24 at 17:26 +0100, Daniel P. Berrangé wrote:
> The dbus build needs to validate one axis
> 
>   - A variety of libvirt versions
> 
> We test a variety of libvirt versions by running a build against the
> distro provided libvirt packages. All that is then missing is a build
> against the latest libvirt git master, which only needs to be run on
> a single distro, for which CentOS 8 is picked as a stable long life
> base.

This...

> +++ b/.gitlab-ci.yml
> +x64-centos-8-git-build:
> +  <<: *git_native_build_job_definition
> +  variables:
> +    NAME: centos-8
> +
> +x64-centos-stream-git-build:
> +  <<: *git_native_build_job_definition
> +  variables:
> +    NAME: centos-stream

... contradicts this...

> +++ b/ci/containers/refresh
> +for host in $HOSTS
> +do
> +    if test "$host" = "libvirt-centos-8" || test "$host" = "libvirt-centos-stream"
> +    then
> +        $LCITOOL dockerfile $host libvirt+minimal,libvirt-glib,libvirt-dbus > $host.Dockerfile

... and this.

What's the rationale for building libvirt and libvirt-glib from git
on CentOS Stream in addition to CentOS 8?

> +    if test "$host" = "libvirt-debian-9" || test "$host" = "libvirt-ubuntu-1804"
> +    then
> +	sed -i -e 's/libvirt-dev/libvirt-dev libvirt-daemon/' $host.Dockerfile

This line is not indented correctly.

Additionally, please add a comment explaining why this hack is needed
in the first place, something along the lines of

  Before Debian version 4.10.0-2, some of the runtime files needed by
  libvirt were mistakenly shipped in the libvirt-daemon package

One more nitpick: the conditional would look nicer and be easier to
tweak later as

  if test "$host" = "libvirt-debian-9" ||
     test "$host" = "libvirt-ubuntu-1804"
  then

Same applies above.

-- 
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