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

[Andrea Bolognani]

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