On 09/19/2014 07:43 PM, Lukas Slebodnik wrote:
On (19/09/14 16:15), Jakub Hrozek wrote:
> On Fri, Sep 19, 2014 at 04:15:25PM +0300, Nikolai Kondrashov wrote:
>> Hi Lukas,
>> On 09/19/2014 08:51 AM, Lukas Slebodnik wrote:
>>> The mock config with name default is usually symbolic link
>>> to the configuration file of local architecture. The side effect
>>> of this patch is that we will not try to rebuild on old architectures
>>> src.rpm for new architectures(fedora). It caused issues with mock tmpfs
>> There is one other issue. This way we won't run RHEL6 mock builds locally
>> normally, as, AFAIK, people usually don't work on RHEL6.
It was a purpose. I wrote it few time. The purpose of mock build is to test
in clean enviroment. In anoter words, it will test spec file whether there are
all dependencies and whether configure script detect all build dependencies.
Because with installed minimal build dependencies and wrong spec file;
process should stop at configure time and not at build with compile time error.
Sure. I just wanted to make sure that regular local RHEL6 building wouldn't be
missed, as you asked me to add RHEL6 mock build in particular.
>> Still, we would have our CI servers.
If Michal's proposal with "--test=teat1,test2..." will be implemented than
user can specify mock build also for epel6. With current version it is
difficult to fullfill CI server requirements and user requirements.
In git push instances, it does not make sense to run mock twice. User/developer
can want this. On the other hand, user/developer can want skipp all test and
execute just code coverage, but it is not possible at the moment.
Michal's proposal is more suitable for developers usecase.
I would prefer to modify current version to fit to the "CI servers" use-case.
There is still possibility to change script locally and use epel-6-x86_64
instead of default config. I know, it is not flexible, but better than nothing.
Yes. We can work on this in the future.
>>> + stage mock-build mock_privileged_deps
>>> + --resultdir ci-mock-result \
>> Would it be better to perhaps name the stage just "mock"?
>> Otherwise, the patch looks and works fine to me.
>> Thank you, Lukas.
>> There is still the problem of Clang running on all hosts. I think it will be
>> better to run it just on one. OTOH, developers might run CI on different
>> distros and Clang might produce different issues there. I see that on Fedora
>> Rawhide at least. Anyway, we can live with it for now and solve it later.
I agree that Clang static analysis is executed on all hosts, but on the other
hand, a result of this test is ignored. It does not make sense to execute them
on CI servers. We save 10 minutes on each server. IIRC, there is a ticket to
compare/print count of clang warnings. If we have this it will make sense
to run this test on servers. Otherwise it shouldn't be executed.
It is ignored for now, and if we care for performance more than the ability to
check up on Clang results manually (which I guess we are), then we can disable
it with a separate patch.
I will be checking if we can implement suppressions on Clang quickly,
similarly to Valgrind. If we can, I will do them next, so we can benefit from
static analyzer. In that case the matter of (not) running Clang on all hosts
will arise again.
> Should we apply this patch atop your CI: run distro-independent
One more time, division into distro-(in)?dependent test is wrong.
It will need to be changed anyway if we want push his patch.
If we don't want to remove(comment out) clang static analysis completely.
I will accept temporary new option "--developers-only" (or better name).
This option will enable tests which will not be executed on "CI servers".
It will be still better than current "distro-(in)?dependent" version.
It will perhaps be simpler to simply disable it (comment out) for now.