[prev in list] [next in list] [prev in thread] [next in thread]
List: wireshark-dev
Subject: Re: [Wireshark-dev] Petri-Dish circumvention breaks daily-build
From: Alexis La Goutte <alexis.lagoutte () gmail ! com>
Date: 2015-09-10 7:55:59
Message-ID: CAHzrgZ=6c3Y6pzhyBG7Gd5nwFM=i3X3WdHei1vhBDt+1BEa=Cw () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Hi Roland,
Like Pascal say, we talk on last Sharkfest about Automated build/Gerrit and
Infrastructure Stuff..
And it is complicated, need to make some change on actually infrastructure
but missing time...
I think, we will see after Wireshark 2.0 to make some change on
infrastructure.
(the idea is to have a regular build (actually trunk buildbot) before
merging but can be limited the number of commit by day...
Regards,
On Thu, Sep 10, 2015 at 8:42 AM, Pascal Quantin <pascal.quantin@gmail.com>
wrote:
>
> Le 10 sept. 2015 8:35 AM, "Roland Knall" <rknall@gmail.com> 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@gmail.com> wrote:
> >>
> >>
> >> Le 10 sept. 2015 8:00 AM, "Roland Knall" <rknall@gmail.com> 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.
> >>
>
> ___________________________________________________________________________
> Sent via: Wireshark-dev mailing list <wireshark-dev@wireshark.org>
> Archives: https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
> mailto:wireshark-dev-request@wireshark.org
> ?subject=unsubscribe
>
[Attachment #5 (text/html)]
<div dir="ltr"><div><div><div><div><div>Hi Roland,<br><br></div>Like Pascal say, we \
talk on last Sharkfest about Automated build/Gerrit and Infrastructure \
Stuff..<br></div>And it is complicated, need to make some change on actually \
infrastructure but missing time...<br><br></div>I think, we will see after Wireshark \
2.0 to make some change on infrastructure.<br></div>(the idea is to have a regular \
build (actually trunk buildbot) before merging but can be limited the number of \
commit by day...<br><br></div>Regards,<br><div><div><div><div><div><div><div><div><div \
class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 10, 2015 at 8:42 AM, \
Pascal Quantin <span dir="ltr"><<a href="mailto:pascal.quantin@gmail.com" \
target="_blank">pascal.quantin@gmail.com</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><span class=""><p dir="ltr"><br> Le 10 sept. 2015 8:35 AM, \
"Roland Knall" <<a href="mailto:rknall@gmail.com" \
target="_blank">rknall@gmail.com</a>> a écrit :<br> ><br>
> Hi Pascal<br>
><br>
> 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.<br> ><br>
> 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.<br> ><br>
> 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.</p> </span><p \
dir="ltr">Don't worry, I did not take it personally and I can understand your \
frustration. <br> 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). </p><span class="HOEnZb"><font color="#888888"> \
</font></span><p dir="ltr"><span class="HOEnZb"><font color="#888888">Pascal. \
<br></font></span><span class=""> ><br>
> regards,<br>
> Roland<br>
><br>
> On Thu, Sep 10, 2015 at 8:30 AM, Pascal Quantin <<a \
href="mailto:pascal.quantin@gmail.com" \
target="_blank">pascal.quantin@gmail.com</a>> wrote:<br> >><br>
>><br>
>> Le 10 sept. 2015 8:00 AM, "Roland Knall" <<a \
href="mailto:rknall@gmail.com" target="_blank">rknall@gmail.com</a>> a écrit \
:<br> >> ><br>
>> > Hi<br>
>> ><br>
>> > 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.<br> >> ><br>
>> > 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. <br> >> ><br>
>> > 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.<br> >> ><br>
>> > 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.<br> \
>> ><br> >> > regards,<br>
>> > Roland<br>
>> ><br>
>> ><br>
>><br>
>> Hi Roland,<br>
>><br>
>> 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. <br> >> 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.<br> >> 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.<br> >><br>
>> Pascal. <br>
>><br>
</span></p>
<br>___________________________________________________________________________<br>
Sent via: Wireshark-dev mailing list <<a \
href="mailto:wireshark-dev@wireshark.org">wireshark-dev@wireshark.org</a>><br>
Archives: <a href="https://www.wireshark.org/lists/wireshark-dev" \
rel="noreferrer" target="_blank">https://www.wireshark.org/lists/wireshark-dev</a><br>
Unsubscribe: <a href="https://wireshark.org/mailman/options/wireshark-dev" \
rel="noreferrer" target="_blank">https://wireshark.org/mailman/options/wireshark-dev</a><br>
mailto:<a href="mailto:wireshark-dev-request@wireshark.org">wireshark-dev-request@wi \
reshark.org</a>?subject=unsubscribe<br></blockquote></div><br></div></div></div></div></div></div></div></div></div></div>
___________________________________________________________________________
Sent via: Wireshark-dev mailing list <wireshark-dev@wireshark.org>
Archives: https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
mailto:wireshark-dev-request@wireshark.org?subject=unsubscribe
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic