Subject: Re: clang in jenkins

From: Daniel van Vugt <[email-masked]>
Date: Tue, 26 Mar 2013 09:46:57 +0800

Martin,

Scanning the code with Clang raises errors that GCC can't find yet (but 
may be able to in future). They're almost always valid errors too; 
things we definitely want to fix.

In the case of eventually running code built with clang; yes it is 
important for quality. Not for releases, but my experience with multiple 
projects has been that if code crashes with clang then it definitely 
needs fixing somewhere.

In the past I've had the luxury (or misfortune) of having to support 
half a dozen compilers simultaneously (and a dozen unique platforms). 
And I always found that adding yet another compiler/platform revealed 
interesting errors and bugs that the existing compilers did not (even in 
supposedly mature/stable code).

Yes, code quality is definitely "delivering on what we promised". And 
clang is a very easy way for us to raise the quality bar a little 
higher. If it's not in CI then we will have the pain of periodically 
someone scanning the code with clang and logging bugs for things that 
could easily have been detected automatically, and sooner.

- Daniel

On 22/03/13 23:55, Martin Mrazik wrote:
> FYI
>
> Let me know if you there is a broader agreement on clang.
> If you care there are couple of options we can do --
> 1. run clang build daily
> 2. trigger clang build on trunk everytime there is a change in trunk
> 3. keep clang in ci (but I think alan_g would call that a distraction)
> but remove from autolanding
> ...
>
> M.
>
>
> <mmrazik> btw. there is a clang build enabled in addition
> <alan_g> are we supporting clang?
> <mmrazik> alan_g: is the question "do we care about clang errors" or
> "can we compile lp:mir with clang"?
> <mmrazik> the later is true
> <mmrazik> duflu asked me to have clang job in jenkins earlier today
> <mmrazik> this is what the jenkins job is doing: cmake ..
> -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DMIR_DISABLE_INPUT=ON
> <alan_g> mmrazik: the question is "does spending time on clang help
> deliver what we promised"
> <mmrazik> right
> <mmrazik> alan_g: can't answer
> <mmrazik> shall I drop an e-mail to mir-devel ?
> <mmrazik> I thought you guys care given you approved the MP
> <alan_g> AFAIK clang isn't a supported platform (just a nice to have).
> <kdub> alan_g, +1
> <kdub> alf_, re-review of android-display-factory? :)
> <alf_> kdub: on it
> <mmrazik> kdub, alan_g: so you are -1 on clang blocking the ci/autolanding
> <mmrazik> what if I keep it for ci and remove for autolanding? I.e. you
> will be able to merge code even if clang fails
> <alan_g> mmrazik: yep
> <kdub> mmrazik, yes, at least until we figure get our gcc arm compiles
> to a nice point
> <mmrazik> so should I remove clang completely?
> <alan_g> mmrazik: g++ arm is a supported platform
> <mmrazik> shall I translate g++ arm as building natively on arm (like we
> do in ppa)?
> <alan_g> mmrazik: clang shouldn't distract you or us from what we are
> trying to deliver
> * mmrazik is removing the clang job
>

---

From: Robert Ancell <[email-masked]>
Date: Tue, 26 Mar 2013 15:04:57 +1300

I'm also -1 on requiring clang for landing - we shouldn't block on clang
generating errors. GCC/Ubuntu are our platforms and anything else at the
time is a bonus.

I do vote a +1 for option 3 below. If Jenkins could say "approved but
has clang errors" then that would be a useful indicator of future problems.

--Robert

On 26/03/13 14:46, Daniel van Vugt wrote:
> Martin,
>
> Scanning the code with Clang raises errors that GCC can't find yet
> (but may be able to in future). They're almost always valid errors
> too; things we definitely want to fix.
>
> important for quality. Not for releases, but my experience with
> multiple projects has been that if code crashes with clang then it
> definitely needs fixing somewhere.
>
> interesting errors and bugs that the existing compilers did not (even
> in supposedly mature/stable code).
>

---

From: Daniel van Vugt <[email-masked]>
Date: Tue, 26 Mar 2013 10:08:18 +0800

If you commit code that can't build with clang, that's instantly a bug 
that needs to be logged and fixed later. I would prefer we found out 
about such errors immediately and did not allow them to land in the 
first place.

I guess option 3 does allow for this. As a human can assess the error 
from clang and decide "Needs fixing".

---

From: Martin Mrazik <[email-masked]>
Date: Tue, 26 Mar 2013 08:19:10 +0100

Hi,

> could easily have been detected automatically, and sooner.

Just a short comment given the e-mail is directed to me. I personally
agree, but it is you (i.e. mir-team) who needs to tell me if this is
what you want or not. I don't have enough information to assess this and
it looks like you are not on the same page on how useful clang is.

If you give me a clear "yes, we want this" I'm happy to (re)enable it.
All the work in jenkins has been already done.

Thanks,

Martin
-- 
Martin Mrazik | Product Strategy Quality Lead
Key fingerprint:
0373 C5E5 6A04 39FF 9D06 31B8 B296 2F5D 35FF D83B

---

From: Daniel van Vugt <[email-masked]>
Date: Tue, 26 Mar 2013 15:26:35 +0800

You'd be hard-pressed to find a good argument against automated static 
analysis. It's always useful to improve code quality from more angles.

I believe Alan's concern was that we were devoting resources to the 
effort that we couldn't spare... ?

- Daniel

---

From: Martin Mrazik <[email-masked]>
Date: Tue, 26 Mar 2013 09:15:06 +0100

> has clang errors" then that would be a useful indicator of future problems.

[...]

>>> 3. keep clang in ci but remove from autolanding

Just a small clarification -- this means that CI would still vote "Needs
Fixing" and you would see why (clang build failing). If this vote is
ignored and the global state of MP is changed to Approved, jenkins would
change its vote to Approved and merge the code during autolanding.

But I tend to agree with Daniel -- this is just static analysis, why
would you ignore it? Are you concerned that clang is going to produce
false errors?

M.

---

From: Alan Griffiths <[email-masked]>
Date: Tue, 26 Mar 2013 09:24:34 +0000

> effort that we couldn't spare... ?

I would have no objection to having clang in CI if all that was
happening is that it detected new errors. But there is more work needed
than that - and it is the timing of that work that concerns me. I think
after Daniel's work on clang support that the code we've written is
pretty close to being clean under clang.

However, the same is not true of the input stack we've forked from
android - and I don't think we should be trying to clean up that code
until we have it under automated tests. (FWIW There's other clean-up to
that code we've also postponed - adding clang to that exercise is
probably better timing.)

So, while long term I think clang support would be a good thing and I'd
keep it in the backlog, I don't think it something we should be spending
time on in the current iteration. (I'd rather we stabilised our arm
support.)

---

From: Daniel van Vugt <[email-masked]>
Date: Tue, 26 Mar 2013 17:30:48 +0800

Part of my original request to Martin last week was to ignore the input 
stack:

   cmake .. -DCMAKE_C_COMPILER=clang \
            -DCMAKE_CXX_COMPILER=clang++ \
            -DMIR_DISABLE_INPUT=ON

Yes, more work is needed on input to make it clang-friendly (bug 
#1157613). But it's not a blocking clang-CI support. We can get most of 
the way there right now with -DMIR_DISABLE_INPUT=ON.

---

From: Martin Mrazik <[email-masked]>
Date: Tue, 26 Mar 2013 11:01:25 +0100

>             -DCMAKE_CXX_COMPILER=clang++ \

FYI -- the clang job I have (and which was enabled for a while last
Friday) is doing exactly this.

Thanks,

Martin

---

From: Daniel van Vugt <[email-masked]>
Date: Tue, 26 Mar 2013 19:05:22 +0800

OK, we all just had a hangout.

Alan's concern was that Martin was also asked to do some work with 
respect to Mir-ARM CI/autobuilding or something. That is definitely 
higher priority than clang. However we also agreed that if clang CI can 
be made to work soon (and I don't see why not) then it should be 
finished first.

---

From: Martin Mrazik <[email-masked]>
Date: Tue, 26 Mar 2013 13:29:49 +0100

> be made to work soon (and I don't see why not) then it should be 

The clang job is re-enabled (just the "cmake .. -D*" stuff that was
mentioned in this thread earlier).

Regarding mir-arm work -- the only thing that is on my list is to run
the tests on a real phone. Truth is I'm struggling there a bit with
networking and I'm waiting for automatic image validation on the phone
(done by Sergio). I'll bump the priority of this task.

Is there something else you would like to see? Something like building
with a real gcc on arm (i.e. not cross-compiling like right now)?

Ideally Thomi (CCed) should be your primary point of contact anytime you
want stuff like this to happen. I agree we are a bit bad in tracking
this so I'm suggesting the following:

1. lets track all quality and mir related tasks in the following
blueprint: https://blueprints.launchpad.net/ubuntu/+spec/mir-quality-1304

2. lets have a (public) hangout early next week to identify any pending
items and lets create a backlog of work-items for April. All of that
ideally in a NZ-friendly time (btw when are you switching from DST in NZ?).

Thanks,

Martin

---

From: Thomi Richards <[email-masked]>
Date: Wed, 27 Mar 2013 07:51:40 +1300

Hi,

> ideally in a NZ-friendly time (btw when are you switching from DST in NZ?).

Sounds good - we switch from DST on the first Sunday of April: 
http://www.dia.govt.nz/Daylight-Saving

Cheers,

-- 
Thomi Richards