[libvirt] [jenkins-ci PATCH 0/2] fix libosinfo rpm build

Pavel Hrdina posted 2 patches 7 years, 6 months ago
Failed in applying to current master (apply log)
jobs/autotools.yaml      | 5 ++++-
jobs/defaults.yaml       | 2 +-
jobs/perl-makemaker.yaml | 3 +++
projects/libosinfo.yaml  | 2 +-
projects/libvirt.yaml    | 2 +-
5 files changed, 10 insertions(+), 4 deletions(-)
[libvirt] [jenkins-ci PATCH 0/2] fix libosinfo rpm build
Posted by Pavel Hrdina 7 years, 6 months ago
Pavel Hrdina (2):
  jobs: rename check_env into job_env
  jobs: use job_env in all job templates

 jobs/autotools.yaml      | 5 ++++-
 jobs/defaults.yaml       | 2 +-
 jobs/perl-makemaker.yaml | 3 +++
 projects/libosinfo.yaml  | 2 +-
 projects/libvirt.yaml    | 2 +-
 5 files changed, 10 insertions(+), 4 deletions(-)

-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 0/2] fix libosinfo rpm build
Posted by Andrea Bolognani 7 years, 6 months ago
On Thu, 2017-11-02 at 13:45 +0100, Pavel Hrdina wrote:
> Pavel Hrdina (2):
>   jobs: rename check_env into job_env
>   jobs: use job_env in all job templates
> 
>  jobs/autotools.yaml      | 5 ++++-
>  jobs/defaults.yaml       | 2 +-
>  jobs/perl-makemaker.yaml | 3 +++
>  projects/libosinfo.yaml  | 2 +-
>  projects/libvirt.yaml    | 2 +-
>  5 files changed, 10 insertions(+), 4 deletions(-)

I'm not sure this approach is the best one.

I like the idea of having a place where we can stick common
environment variables - for us that mostly means paths, and we
should definitely move guest configuration out of its definition
in Jenkins and into this repository, as we prototyped yesterday.

But here you're still keeping that info local. Moreover, you're
moving the VIR_TEST_* variables from check_env to job_env, which
means they end up being defined even during regular 'make'. I seem
to recall Dan speaking up against that earlier.

I think we should have:

  global_env - as the name implies, global; for $VIRT_PREFIX and all
               other paths that affect one or more build jobs, like
               $OSINFO_SYSTEM_DIR and $MAKE
  build_env  - local; for variables that affect the build step of a
               single job (can't think of any right now)
  test_env   - local; for variables that affect the test suite of a
               single job, like $VIR_TEST_DEBUG

For projects that inherit from anything but generic-*-job,
global_env will be included automatically; projects that don't use
any of the known build system will have to take care of that
themselves, but then again they already have to do that for make_env.

Moreover, we should be able to convert libvirt-cim to use
autotools-*-job by tweaking its autogen.sh script, which would leave
osinfo-db as the only user of generic-*-job.

-- 
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 0/2] fix libosinfo rpm build
Posted by Pavel Hrdina 7 years, 6 months ago
On Thu, Nov 02, 2017 at 04:32:12PM +0100, Andrea Bolognani wrote:
> On Thu, 2017-11-02 at 13:45 +0100, Pavel Hrdina wrote:
> > Pavel Hrdina (2):
> >   jobs: rename check_env into job_env
> >   jobs: use job_env in all job templates
> > 
> >  jobs/autotools.yaml      | 5 ++++-
> >  jobs/defaults.yaml       | 2 +-
> >  jobs/perl-makemaker.yaml | 3 +++
> >  projects/libosinfo.yaml  | 2 +-
> >  projects/libvirt.yaml    | 2 +-
> >  5 files changed, 10 insertions(+), 4 deletions(-)
> 
> I'm not sure this approach is the best one.
> 
> I like the idea of having a place where we can stick common
> environment variables - for us that mostly means paths, and we
> should definitely move guest configuration out of its definition
> in Jenkins and into this repository, as we prototyped yesterday.

This is a fix of the commit that broke it, I'm not trying to move all
environment variables out of Jenkins.

> But here you're still keeping that info local. Moreover, you're
> moving the VIR_TEST_* variables from check_env to job_env, which
> means they end up being defined even during regular 'make'. I seem
> to recall Dan speaking up against that earlier.

This is not true, the VIR_TEST_* variables are defined only for
"autotools-check-job", see <projects/libvirt.yaml>.

> 
> I think we should have:
> 
>   global_env - as the name implies, global; for $VIRT_PREFIX and all
>                other paths that affect one or more build jobs, like
>                $OSINFO_SYSTEM_DIR and $MAKE

I'm not sure whether there is some different way how to do it but I
would rather use different approach than using these defines since
you can easily overwrite them and forget to include it.

>   build_env  - local; for variables that affect the build step of a
>                single job (can't think of any right now)
>   test_env   - local; for variables that affect the test suite of a
>                single job, like $VIR_TEST_DEBUG

I thought about splitting it into {build,test} but we don't need to have
it that way and it feels like an overkill.

> For projects that inherit from anything but generic-*-job,
> global_env will be included automatically; projects that don't use
> any of the known build system will have to take care of that
> themselves, but then again they already have to do that for make_env.

And this is exactly why I don't want to do it this way, the "global_env"
needs to be present always, like it is now with all the environment
variables configured in Jenkins.

> Moreover, we should be able to convert libvirt-cim to use
> autotools-*-job by tweaking its autogen.sh script, which would leave
> osinfo-db as the only user of generic-*-job.

+1

Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 0/2] fix libosinfo rpm build
Posted by Daniel P. Berrange 7 years, 6 months ago
On Thu, Nov 02, 2017 at 04:49:38PM +0100, Pavel Hrdina wrote:
> On Thu, Nov 02, 2017 at 04:32:12PM +0100, Andrea Bolognani wrote:
> > Moreover, we should be able to convert libvirt-cim to use
> > autotools-*-job by tweaking its autogen.sh script, which would leave
> > osinfo-db as the only user of generic-*-job.
> 
> +1

The only reason libvirt-cim didn't use the autotools job was that its
setup script was called autoconfiscate insteadof autogen.sh. I see that
Andrea has fixed that crazyness now so we can use the autotools job
easily enough i expect :-)


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 0/2] fix libosinfo rpm build
Posted by Andrea Bolognani 7 years, 6 months ago
On Thu, 2017-11-02 at 15:56 +0000, Daniel P. Berrange wrote:
> On Thu, Nov 02, 2017 at 04:49:38PM +0100, Pavel Hrdina wrote:
> > On Thu, Nov 02, 2017 at 04:32:12PM +0100, Andrea Bolognani wrote:
> > > Moreover, we should be able to convert libvirt-cim to use
> > > autotools-*-job by tweaking its autogen.sh script, which would leave
> > > osinfo-db as the only user of generic-*-job.
> > 
> > +1
> 
> The only reason libvirt-cim didn't use the autotools job was that its
> setup script was called autoconfiscate insteadof autogen.sh. I see that
> Andrea has fixed that crazyness now so we can use the autotools job
> easily enough i expect :-)

There's one more difference still: libvirt-cim's autogen.sh doesn't
automatically call configure, which autotools-*-job expect. Easily
fixed, though.

-- 
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 0/2] fix libosinfo rpm build
Posted by Andrea Bolognani 7 years, 6 months ago
On Thu, 2017-11-02 at 16:49 +0100, Pavel Hrdina wrote:
> On Thu, Nov 02, 2017 at 04:32:12PM +0100, Andrea Bolognani wrote:
> > I'm not sure this approach is the best one.
> > 
> > I like the idea of having a place where we can stick common
> > environment variables - for us that mostly means paths, and we
> > should definitely move guest configuration out of its definition
> > in Jenkins and into this repository, as we prototyped yesterday.
> 
> This is a fix of the commit that broke it, I'm not trying to move all
> environment variables out of Jenkins.

I understand that, and I didn't expect you to of course, but I was
considering your changes with that frame of reference and it looked
to me like you were changing some things that would have to be
changed again for that to happen, so I thought it would be better
to go in that direction right away.

> > But here you're still keeping that info local. Moreover, you're
> > moving the VIR_TEST_* variables from check_env to job_env, which
> > means they end up being defined even during regular 'make'. I seem
> > to recall Dan speaking up against that earlier.
> 
> This is not true, the VIR_TEST_* variables are defined only for
> "autotools-check-job", see <projects/libvirt.yaml>.

You're right, sorry for the noise.

> > I think we should have:
> > 
> >   global_env - as the name implies, global; for $VIRT_PREFIX and all
> >                other paths that affect one or more build jobs, like
> >                $OSINFO_SYSTEM_DIR and $MAKE
> 
> I'm not sure whether there is some different way how to do it but I
> would rather use different approach than using these defines since
> you can easily overwrite them and forget to include it.

See below.

> >   build_env  - local; for variables that affect the build step of a
> >                single job (can't think of any right now)
> >   test_env   - local; for variables that affect the test suite of a
> >                single job, like $VIR_TEST_DEBUG
> 
> I thought about splitting it into {build,test} but we don't need to have
> it that way and it feels like an overkill.

After looking into it better, I agree that splitting them doesn't
make much sense at all. How about local_env instead of job_env, to
complement global_env defined above? Assuming we don't discard my
proposal entirely, that is :)

> > For projects that inherit from anything but generic-*-job,
> > global_env will be included automatically; projects that don't use
> > any of the known build system will have to take care of that
> > themselves, but then again they already have to do that for make_env.
> 
> And this is exactly why I don't want to do it this way, the "global_env"
> needs to be present always, like it is now with all the environment
> variables configured in Jenkins.

Is there any way we can have that guarantee without asking users
of generic-*-job (which shouldn't be many at all, ideally just the
one after we've fixed libvirt-cim) to include it themselves?

I'm really not an expert in jenkins-job-builder but since we're
setting the environment using export in the 'command' section of
the job definition, I don't see much of a way around it.

So while I agree with your concern about forgetting to include
global_env when defining new jobs, I think that it would still be
a better approach than keeping the environment variables in the
per-worker Jenkins configuration, where they are not tracked and
basically entirely invisible to anyone but the CI administrators.

-- 
Andrea Bolognani / Red Hat / Virtualization

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