Wireshark-dev: Re: [Wireshark-dev] Petri-Dish circumvention breaks daily-build

From: Pascal Quantin <pascal.quantin@xxxxxxxxx>
Date: Thu, 10 Sep 2015 08:42:29 +0200


Le 10 sept. 2015 8:35 AM, "Roland Knall" <rknall@xxxxxxxxx> a écrit :
>
> Hi Pascal
>
> That was nothing against you or the way this patch was handled. We use Gerrit here ourselves together with Jenkins, and the scenario you describe actually happened here as well. Therefore it was decided, that no patch can be submitted without Jenkins not running at least once, but it can be overruled if necessary.
>
> Also, in this case it was an honest mistake but it struck a nerve with me, because I am battling with this for months now and it is just a little bit frustrating from my point of view.
>
> But in any case, no broken feelings here on my end, and I apologize for my harsh tone in the last e-mail. Just want to raise awareness on the issue.

Don't worry, I did not take it personally and I can understand your frustration.
For your information making the Petri Dish step mandatory is something we discussed during last Sharkfest so it's something planned but not ready yet (I'm not in charge of the infrastructure so I'm not sure what are the missing steps; some false positives in the pre commit scripts and Petri Dish git checkout errors probably count here).

Pascal.
>
> regards,
> Roland
>
> On Thu, Sep 10, 2015 at 8:30 AM, Pascal Quantin <pascal.quantin@xxxxxxxxx> wrote:
>>
>>
>> Le 10 sept. 2015 8:00 AM, "Roland Knall" <rknall@xxxxxxxxx> a écrit :
>> >
>> > Hi
>> >
>> > In our company we have our own Wireshark tools and plugins, which use the main wireshark repository. To ensure that those do not break (and if they break we can fix them in due time), we synchronize with the Wireshark repo every night and build it with our own tools on Windows and Linux.
>> >
>> > And nearly every other week or so, the build breaks because of patches introducing issues, which would have been caught by Petri-Dish. The latest being a NFS patch (#10429), where a variable is being used uninitialized. There where many patches to NFS, and the top-most one only was being sent to petri-dish, but not rebased first, which meant it was not checking the code of it's predecessors but only the one of itself. 
>> >
>> > Now if I submit code, it correctly and rightly so must pass the Petri-Dish step to ensure cross-plattform compatibility for building at least. I have nothing against that, in fact, I really prefer it, becuase every now and then you can overlook something.
>> >
>> > But all (I checked the last 7) the breaks in our build-system happened, because a core-developer submitted a patch without petri-dish. I do not think it is fair, that non-core-developers are being sent through an extra step and core's can circumvent this step. In my opinion it should be a necessity for everyone.
>> >
>> > regards,
>> > Roland
>> >
>> >
>>
>> Hi Roland,
>>
>> When I sent the NFS serie to Petri Dish I took what I thought was the latest NFS patch of the 14 patchs queue. So Petri Dish should have tested all of them. Looks like I got confused at some point (I do not find Gerrit UI really intuitive for long series) as the error appeared despite my attempt and I'm sorry about that. Note that those patches were not from a non core developer, so I do not agree with your statement that this is a core vs non core extra step.
>> I agree with you that we should improve in this area. But we are far better than 2 years ago where the tree was broken almost every day. And we try to fix failure as soon as possible, but it can get delayed due to real life.
>> Moreover for now Petri Dish just runs a x86 Linux and Windows bots. It means that some errors seen on other platforms will not be caught.
>>
>> Pascal.
>>