[libvirt] [jenkins-ci PATCH v3 02/12] jobs: Remove archive_format from defaults

Andrea Bolognani posted 12 patches 7 years, 5 months ago
[libvirt] [jenkins-ci PATCH v3 02/12] jobs: Remove archive_format from defaults
Posted by Andrea Bolognani 7 years, 5 months ago
Instead of defining a default and overriding it on a
case-by-case basis, always define archive_format at the
project level.

This adds a bit of duplication, but it's consistent with
how we define other metadata for projects and it will
help us out later.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 jobs/defaults.yaml            | 1 -
 projects/libosinfo.yaml       | 1 +
 projects/libvirt-cim.yaml     | 1 +
 projects/libvirt-glib.yaml    | 1 +
 projects/libvirt-go-xml.yaml  | 1 +
 projects/libvirt-go.yaml      | 1 +
 projects/libvirt-perl.yaml    | 1 +
 projects/libvirt-python.yaml  | 1 +
 projects/libvirt-sandbox.yaml | 1 +
 projects/libvirt-tck.yaml     | 1 +
 projects/osinfo-db-tools.yaml | 1 +
 projects/virt-manager.yaml    | 1 +
 projects/virt-viewer.yaml     | 1 +
 13 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/jobs/defaults.yaml b/jobs/defaults.yaml
index bab5bc4..5927ce3 100644
--- a/jobs/defaults.yaml
+++ b/jobs/defaults.yaml
@@ -20,7 +20,6 @@
       - libvirt-fedora-rawhide
     mingw_machines:
       - libvirt-fedora-rawhide
-    archive_format: gz
     global_env: |
     local_env: |
     strip_buildrequires: |
diff --git a/projects/libosinfo.yaml b/projects/libosinfo.yaml
index 614eb63..55a4817 100644
--- a/projects/libosinfo.yaml
+++ b/projects/libosinfo.yaml
@@ -3,6 +3,7 @@
     name: libosinfo
     machines: '{all_machines}'
     title: libosinfo
+    archive_format: gz
     git_url: https://gitlab.com/libosinfo/libosinfo.git
     jobs:
       - autotools-build-job:
diff --git a/projects/libvirt-cim.yaml b/projects/libvirt-cim.yaml
index 027c31d..6d524df 100644
--- a/projects/libvirt-cim.yaml
+++ b/projects/libvirt-cim.yaml
@@ -3,6 +3,7 @@
     name: libvirt-cim
     machines: '{rpm_machines}'
     title: libvirt CIM
+    archive_format: gz
     git_url: https://github.com/libvirt/libvirt-cim.git
     jobs:
       - autotools-build-job:
diff --git a/projects/libvirt-glib.yaml b/projects/libvirt-glib.yaml
index dd2ba0c..993024a 100644
--- a/projects/libvirt-glib.yaml
+++ b/projects/libvirt-glib.yaml
@@ -3,6 +3,7 @@
     name: libvirt-glib
     machines: '{all_machines}'
     title: Libvirt GLib
+    archive_format: gz
     git_url: https://github.com/libvirt/libvirt-glib.git
     jobs:
       - autotools-build-job:
diff --git a/projects/libvirt-go-xml.yaml b/projects/libvirt-go-xml.yaml
index 4e44561..7e6e090 100644
--- a/projects/libvirt-go-xml.yaml
+++ b/projects/libvirt-go-xml.yaml
@@ -3,6 +3,7 @@
     name: libvirt-go-xml
     machines: '{all_machines}'
     title: Libvirt Go XML
+    archive_format: gz
     git_url: https://github.com/libvirt/libvirt-go-xml.git
     jobs:
       - go-build-job:
diff --git a/projects/libvirt-go.yaml b/projects/libvirt-go.yaml
index 7046dab..d90339a 100644
--- a/projects/libvirt-go.yaml
+++ b/projects/libvirt-go.yaml
@@ -3,6 +3,7 @@
     name: libvirt-go
     machines: '{all_machines}'
     title: Libvirt Go
+    archive_format: gz
     git_url: https://github.com/libvirt/libvirt-go.git
     jobs:
       - go-build-job:
diff --git a/projects/libvirt-perl.yaml b/projects/libvirt-perl.yaml
index 8426a67..dbb6caf 100644
--- a/projects/libvirt-perl.yaml
+++ b/projects/libvirt-perl.yaml
@@ -3,6 +3,7 @@
     name: libvirt-perl
     machines: '{all_machines}'
     title: Libvirt Perl
+    archive_format: gz
     git_url: https://github.com/libvirt/libvirt-perl.git
     jobs:
       - perl-modulebuild-build-job:
diff --git a/projects/libvirt-python.yaml b/projects/libvirt-python.yaml
index 69c1e17..05eea41 100644
--- a/projects/libvirt-python.yaml
+++ b/projects/libvirt-python.yaml
@@ -3,6 +3,7 @@
     name: libvirt-python
     machines: '{all_machines}'
     title: Libvirt Python
+    archive_format: gz
     git_url: https://github.com/libvirt/libvirt-python.git
     jobs:
       - python-distutils-build-job:
diff --git a/projects/libvirt-sandbox.yaml b/projects/libvirt-sandbox.yaml
index fc9203a..0831896 100644
--- a/projects/libvirt-sandbox.yaml
+++ b/projects/libvirt-sandbox.yaml
@@ -11,6 +11,7 @@
       - libvirt-fedora-28
       - libvirt-fedora-rawhide
     title: Libvirt Sandbox
+    archive_format: gz
     git_url: https://github.com/libvirt/libvirt-sandbox.git
     jobs:
       - autotools-build-job:
diff --git a/projects/libvirt-tck.yaml b/projects/libvirt-tck.yaml
index b98fd0a..3c8adfd 100644
--- a/projects/libvirt-tck.yaml
+++ b/projects/libvirt-tck.yaml
@@ -12,6 +12,7 @@
       - libvirt-freebsd-10
       - libvirt-freebsd-11
     title: Libvirt TCK
+    archive_format: gz
     git_url: https://github.com/libvirt/libvirt-tck.git
     jobs:
       - perl-modulebuild-build-job:
diff --git a/projects/osinfo-db-tools.yaml b/projects/osinfo-db-tools.yaml
index ccc960b..bcf9e0a 100644
--- a/projects/osinfo-db-tools.yaml
+++ b/projects/osinfo-db-tools.yaml
@@ -3,6 +3,7 @@
     name: osinfo-db-tools
     machines: '{all_machines}'
     title: osinfo database tools
+    archive_format: gz
     git_url: https://gitlab.com/libosinfo/osinfo-db-tools.git
     jobs:
       - autotools-build-job:
diff --git a/projects/virt-manager.yaml b/projects/virt-manager.yaml
index d7fb419..a89f90b 100644
--- a/projects/virt-manager.yaml
+++ b/projects/virt-manager.yaml
@@ -10,6 +10,7 @@
       - libvirt-freebsd-10
       - libvirt-freebsd-11
     title: Virtual Machine Manager
+    archive_format: gz
     git_url: https://github.com/virt-manager/virt-manager.git
     jobs:
       - python-distutils-build-job:
diff --git a/projects/virt-viewer.yaml b/projects/virt-viewer.yaml
index 57aa55f..884cf12 100644
--- a/projects/virt-viewer.yaml
+++ b/projects/virt-viewer.yaml
@@ -3,6 +3,7 @@
     name: virt-viewer
     machines: '{all_machines}'
     title: Virt Viewer
+    archive_format: gz
     git_url: https://pagure.io/virt-viewer.git
     jobs:
       - autotools-build-job:
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v3 02/12] jobs: Remove archive_format from defaults
Posted by Erik Skultety 7 years, 5 months ago
On Wed, Aug 22, 2018 at 11:44:17AM +0200, Andrea Bolognani wrote:
> Instead of defining a default and overriding it on a
> case-by-case basis, always define archive_format at the
> project level.
>
> This adds a bit of duplication, but it's consistent with
> how we define other metadata for projects and it will
> help us out later.

How is it going to help us later? So here you move something which is perfectly
fine to have in defaults and override it only when necessary and in the
following patch you're making a preparation changes for essentially the
opposite of this - moving autogen_args and similar to defaults (not that any
template needs to override these at the moment, but I'm getting confused by the
consistency you talk about).
I think this patch should be dropped.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v3 02/12] jobs: Remove archive_format from defaults
Posted by Erik Skultety 7 years, 5 months ago
On Wed, Aug 22, 2018 at 12:37:27PM +0200, Erik Skultety wrote:
> On Wed, Aug 22, 2018 at 11:44:17AM +0200, Andrea Bolognani wrote:
> > Instead of defining a default and overriding it on a
> > case-by-case basis, always define archive_format at the
> > project level.
> >
> > This adds a bit of duplication, but it's consistent with
> > how we define other metadata for projects and it will
> > help us out later.
>
> How is it going to help us later? So here you move something which is perfectly
> fine to have in defaults and override it only when necessary and in the
> following patch you're making a preparation changes for essentially the
> opposite of this - moving autogen_args and similar to defaults (not that any
> template needs to override these at the moment, but I'm getting confused by the

Actually there are templates overriding those ones, but the point stays.

Erik

> consistency you talk about).
> I think this patch should be dropped.
>
> Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v3 02/12] jobs: Remove archive_format from defaults
Posted by Andrea Bolognani 7 years, 5 months ago
On Wed, 2018-08-22 at 12:37 +0200, Erik Skultety wrote:
> On Wed, Aug 22, 2018 at 11:44:17AM +0200, Andrea Bolognani wrote:
> > Instead of defining a default and overriding it on a
> > case-by-case basis, always define archive_format at the
> > project level.
> > 
> > This adds a bit of duplication, but it's consistent with
> > how we define other metadata for projects and it will
> > help us out later.
> 
> How is it going to help us later?

Welp, I should definitely have expanded on those "will help us out
later" bits O:-)

> So here you move something which is perfectly
> fine to have in defaults and override it only when necessary and in the
> following patch you're making a preparation changes for essentially the
> opposite of this - moving autogen_args and similar to defaults (not that any
> template needs to override these at the moment, but I'm getting confused by the
> consistency you talk about).

The ultimate goal is to translate the JJB jobs definitions to
Ansible tasks while keeping the two as close as possible so that
further changes can be applied pretty much verbatim to both and
not make maintainance a nightmare.

Problem is, some JJB constructs are awfully difficult to translate
without adding extra boilerplate, so in those cases I opted for
moving to a different construct instead.

So for example autogen_args can be defined as a default because
every time we need to override it we can use

  - autotools-build-job:
      autogen_args: --enable-gtk-doc

which translates quite naturally to

  - include: '{{ playbook_base }}/jobs/autotools-build-job.yml'
    vars:
      autogen_args: --enable-gtk-doc

since variables passed to an included task are limited in scope
to the included task and don't affect any other module call, but
for archive_format we can't do the same because we want to
translate

  - project:
      name: libvirt-dbus
      archive_format: xz

to

  - set_fact:
      name: libvirt-dbus
      archive_format: xz

and set_fact affects the *global* state, which means that it will
overwrite the default every time it is called and subsequent tasks
might break depending on the order they're called in.

I guess I could create a task that takes care of stashing the
existing defaults before overriding them with the
project-appropriate ones, and another one that restores the
previous values after running all tasks for a project, but as I
said that's extra boilerplate that we can avoid thanks to this
patch.

(As an aside, I actually like having archive_format spelled out
 explicitly along with other project metadata such as name, title
 and git_url: it makes sense to me to have that kind of information
 all in one place. But the above is the actual reason why this
 patch is needed.)

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v3 02/12] jobs: Remove archive_format from defaults
Posted by Erik Skultety 7 years, 5 months ago
On Wed, Aug 22, 2018 at 01:12:58PM +0200, Andrea Bolognani wrote:
> On Wed, 2018-08-22 at 12:37 +0200, Erik Skultety wrote:
> > On Wed, Aug 22, 2018 at 11:44:17AM +0200, Andrea Bolognani wrote:
> > > Instead of defining a default and overriding it on a
> > > case-by-case basis, always define archive_format at the
> > > project level.
> > >
> > > This adds a bit of duplication, but it's consistent with
> > > how we define other metadata for projects and it will
> > > help us out later.
> >
> > How is it going to help us later?
>
> Welp, I should definitely have expanded on those "will help us out
> later" bits O:-)
>
> > So here you move something which is perfectly
> > fine to have in defaults and override it only when necessary and in the
> > following patch you're making a preparation changes for essentially the
> > opposite of this - moving autogen_args and similar to defaults (not that any
> > template needs to override these at the moment, but I'm getting confused by the
> > consistency you talk about).
>
> The ultimate goal is to translate the JJB jobs definitions to
> Ansible tasks while keeping the two as close as possible so that
> further changes can be applied pretty much verbatim to both and
> not make maintainance a nightmare.
>
> Problem is, some JJB constructs are awfully difficult to translate
> without adding extra boilerplate, so in those cases I opted for
> moving to a different construct instead.
>
> So for example autogen_args can be defined as a default because
> every time we need to override it we can use
>
>   - autotools-build-job:
>       autogen_args: --enable-gtk-doc
>
> which translates quite naturally to
>
>   - include: '{{ playbook_base }}/jobs/autotools-build-job.yml'
>     vars:
>       autogen_args: --enable-gtk-doc
>
> since variables passed to an included task are limited in scope
> to the included task and don't affect any other module call, but
> for archive_format we can't do the same because we want to
> translate
>
>   - project:
>       name: libvirt-dbus
>       archive_format: xz
>
> to
>
>   - set_fact:
>       name: libvirt-dbus
>       archive_format: xz
>
> and set_fact affects the *global* state, which means that it will
> overwrite the default every time it is called and subsequent tasks
> might break depending on the order they're called in.

Understood.

>
> I guess I could create a task that takes care of stashing the

Nah, that would just cause more confusion. I'd appreciate if you managed to
put a much condensed version of your explanation into the commit message for
future generations :).

Reviewed-by: Erik Skultety <eskultet@redhat.com>

> existing defaults before overriding them with the
> project-appropriate ones, and another one that restores the
> previous values after running all tasks for a project, but as I
> said that's extra boilerplate that we can avoid thanks to this
> patch.
>
> (As an aside, I actually like having archive_format spelled out
>  explicitly along with other project metadata such as name, title
>  and git_url: it makes sense to me to have that kind of information
>  all in one place. But the above is the actual reason why this
>  patch is needed.)
>
> --
> Andrea Bolognani / Red Hat / Virtualization
>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list