www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - What's up with pull request buildbots?

reply "Dylan Knutson" <tcdknutson gmail.com> writes:
Hello,
I'm a bit confused as to how the DMD buildbot is supposed to 
work: it seems like 50% of the time (ballparked from the first 15 
or so pull requests), the buildbot just doesn't report failures 
or successes. This bugs (heh) me a little bit because there are 
tons of months old pull requests just waiting in the pipeline, 
stuck at "Determining merge status". I don't know if this is part 
of the review process or caused by the yellow status, but for 
instance I've opened up a bug report a week or so prior:
http://d.puremagic.com/issues/show_bug.cgi?id=10113
and a few days afterwards, a pull request was submitted:
https://github.com/D-Programming-Language/dmd/pull/2080
which, turns out, was more or less a dup of another pull request, 
submitted 6 months ago:
https://github.com/D-Programming-Language/dmd/pull/1358

However, neither of these pull requests have been merged. And 
neither of the bugs have been fixed (my report was a dup of 
http://d.puremagic.com/issues/show_bug.cgi?id=2950, submitted 4 
years ago!).

So we're stuck with a buildbot system that leaves valuable pull 
requests from ever getting merged, and duplicate fixes being 
written because previous fixes were submitted and never accepted 
so long ago.

On a somewhat related note: Why is the Puremagic bugtracker still 
used, when Github has bug tracking with (IMO) better repo 
integration, discussion, and searching (which I think we can all 
agree on), on a repository *hosted at Github*? I can understand 
the reluctance to switch over, but what's keeping us from 
accepting new issues on Github, closing Puremagic down from 
accepting new requests, and working through the open requests on 
Puremagic until all can be taken care of have been?

Thanks for your time,
Dylan Knutson
Jun 02 2013
parent reply "Adam Wilson" <flyboynw gmail.com> writes:
On Sun, 02 Jun 2013 22:27:12 -0700, Dylan Knutson <tcdknutson gmail.com>  
wrote:


 Hello,
 I'm a bit confused as to how the DMD buildbot is supposed to work: it  
 seems like 50% of the time (ballparked from the first 15 or so pull  
 requests), the buildbot just doesn't report failures or successes. This  
 bugs (heh) me a little bit because there are tons of months old pull  
 requests just waiting in the pipeline, stuck at "Determining merge  
 status". I don't know if this is part of the review process or caused by  
 the yellow status, but for instance I've opened up a bug report a week  
 or so prior:
 http://d.puremagic.com/issues/show_bug.cgi?id=10113
 and a few days afterwards, a pull request was submitted:
 https://github.com/D-Programming-Language/dmd/pull/2080
 which, turns out, was more or less a dup of another pull request,  
 submitted 6 months ago:
 https://github.com/D-Programming-Language/dmd/pull/1358


I'll see if I can't give you a stab at an explanation. Note that I did not  
write the Auto-Tester, that is the work of Brad Anderson. Here are some  
facts about the AT as I understand them:
- The AT works on a most current pull first basis, so a pull that is older  
than 6 months will naturally be scheduled much later.
- The AT schedules work based on how many testing machines are available,  
so each pull is tested only once per listed platform, and when multiple  
build servers for a given platform are available, different pulls can be  
tested simultaneously on that platform.
- Pull requests are tested by cloning [DMD/DRuntime/Phobos] master and  
merging the pull into it. This ensures a clean pull against master.
- The AT must restart pull request testing every time code is merged into  
DMD/DRuntime/Phobos because of the above testing strategy.
- If many pulls are merged into [DMD/DRuntime/Phobos] master in a given  
period of, for example a day, the AT will restart prior to completing a  
full test pass.
- With the current number of open pulls the AT can complete a full testing  
pass in roughly 8 hours.
- Each pull takes a rough average of 15 minutes to test.

The lack of success/failure reports on older pulls is directly due to the  
above testing strategy, essentially the AT never made it that far before  
someone merged a pull into [DMD/DRuntime/Phobos] master and forced a  
restart.


 However, neither of these pull requests have been merged. And neither of  
 the bugs have been fixed (my report was a dup of  
 http://d.puremagic.com/issues/show_bug.cgi?id=2950, submitted 4 years  
 ago!).

 So we're stuck with a buildbot system that leaves valuable pull requests  
 from ever getting merged, and duplicate fixes being written because  
 previous fixes were submitted and never accepted so long ago.


The AT system is intentionally incapable of merging to master, so it  
itself is not leaving pulls behind. However if a pull is out-of-date it is  
likely that it will fail the AT anyways due the codebase delta in the  
target project. This is why pulls are tested in a newest first order.


 On a somewhat related note: Why is the Puremagic bugtracker still used,  
 when Github has bug tracking with (IMO) better repo integration,  
 discussion, and searching (which I think we can all agree on), on a  
 repository *hosted at Github*? I can understand the reluctance to switch  
 over, but what's keeping us from accepting new issues on Github, closing  
 Puremagic down from accepting new requests, and working through the open  
 requests on Puremagic until all can be taken care of have been?


According to the Bug Stats page (http://dlang.org/bugstats.php) there are  
nearly 3000 open issues in the BugZilla and over 7000 closed issues! Now I  
tend to agree that better systems than BugZilla exist. But consider the  
effort required to move all of those bugs to a new system, automated  
tooling or no, it wouldn't be easy or quick. And consider that GitHub  
issues lacks much of meta information capability that BugZilla has, we  
would loose a fantastic amount of information. Yes of course GitHub has  
the best integration with it's own issue tracker, but we've achieved a  
fairly high level of integration between GitHub and BugZilla, so that has  
become almost a non-issue these days.


 Thanks for your time,
 Dylan Knutson


-- 
Adam Wilson
IRC: LightBender
Project Coordinator
The Horizon Project
http://www.thehorizonproject.org/
Jun 02 2013
next sibling parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Sunday, June 02, 2013 22:49:08 Adam Wilson wrote:

 I'll see if I can't give you a stab at an explanation. Note that I did not
 write the Auto-Tester, that is the work of Brad Anderson.


Brad Roberts actually, with some help from Daniel Murphy (who created the pull 
tester), and I don't know who else.

- Jonathan M Davis
Jun 02 2013
parent "Adam Wilson" <flyboynw gmail.com> writes:
On Sun, 02 Jun 2013 22:54:51 -0700, Jonathan M Davis <jmdavisProg gmx.com>  
wrote:


 On Sunday, June 02, 2013 22:49:08 Adam Wilson wrote:

 I'll see if I can't give you a stab at an explanation. Note that I did  
 not
 write the Auto-Tester, that is the work of Brad Anderson.


 Brad Roberts actually, with some help from Daniel Murphy (who created  
 the pull
 tester), and I don't know who else.

 - Jonathan M Davis


I keep missing this up. It doesn't help that I was talking to Brad  
Anderson (eco) on IRC. Sorry Brad Roberts!

-- 
Adam Wilson
IRC: LightBender
Project Coordinator
The Horizon Project
http://www.thehorizonproject.org/
Jun 04 2013
prev sibling next sibling parent "Brad Anderson" <eco gnuk.net> writes:
On Monday, 3 June 2013 at 05:49:22 UTC, Adam Wilson wrote:

 On Sun, 02 Jun 2013 22:27:12 -0700, Dylan Knutson 
 <tcdknutson gmail.com> wrote:


 Hello,
 I'm a bit confused as to how the DMD buildbot is supposed to 
 work: it seems like 50% of the time (ballparked from the first 
 15 or so pull requests), the buildbot just doesn't report 
 failures or successes. This bugs (heh) me a little bit because 
 there are tons of months old pull requests just waiting in the 
 pipeline, stuck at "Determining merge status". I don't know if 
 this is part of the review process or caused by the yellow 
 status, but for instance I've opened up a bug report a week or 
 so prior:
 http://d.puremagic.com/issues/show_bug.cgi?id=10113
 and a few days afterwards, a pull request was submitted:
 https://github.com/D-Programming-Language/dmd/pull/2080
 which, turns out, was more or less a dup of another pull 
 request, submitted 6 months ago:
 https://github.com/D-Programming-Language/dmd/pull/1358


 I'll see if I can't give you a stab at an explanation. Note 
 that I did not write the Auto-Tester, that is the work of Brad 
 Anderson. Here are some facts about the AT as I understand them:


I wish I could take credit for that.  It's the work of the 
awesome Brad Roberts though.
Jun 02 2013
prev sibling next sibling parent reply "Dylan Knutson" <tcdknutson gmail.com> writes:
 - If many pulls are merged into [DMD/DRuntime/Phobos] master in 
 a given period of, for example a day, the AT will restart prior 
 to completing a full test pass.
 - With the current number of open pulls the AT can complete a 
 full testing pass in roughly 8 hours.
 - Each pull takes a rough average of 15 minutes to test.


Thanks for the explanation. I suppose the fix for my issue would 
be for reviewers to merge the small bugfixes a bit faster. I 
don't want to sound like I'm rushing the reviewers; as it is the 
project has amazing community contribution, and I'm constantly 
blown away at how much a group of volunteers can accomplish.

On a side note, there are a few (lots) of pull requests that are 
several months old, and master has changed too much for them to 
be compatible and/or relevant. Perhaps those should just be 
closed?


 But consider the  effort required to move all of those bugs to 
 a new system, automated  tooling or no, it wouldn't be easy or 
 quick.


Sorry; let me clarify: I'd suggest that BZ stop accepting new 
issues, and use the GHI tracker for new bugs. Then, just use BZ 
as needed until all resolvable issues there have been resolved. 
As for loss of meta information on Github, eh, I suppose so, but 
GHI offers issue referencing and tagging, so that's something I 
guess? I don't expect to convince anyone to switch, just see why 
it isn't used in the first place.

Thank you,
Dylan Knutson
Jun 03 2013
next sibling parent reply Robert <jfanatiker gmx.at> writes:
On Mon, 2013-06-03 at 16:39 +0200, Dylan Knutson wrote:

 On a side note, there are a few (lots) of pull requests that are 
 several months old, and master has changed too much for them to 
 be compatible and/or relevant. Perhaps those should just be 
 closed?


I have an open pull request which is already months old and no longer
compatible to master. This is because I have not received any feedback
since submission. It does not mean that it is irrelevant or that I am
not willing to make it compatible again as soon as any reviewer has time
for a review.

So it has to be there, if not for anything else than just for for the
sake of being there and being seen:
 - So that no-one else implements the same thing again for no reason
 - Maintainers know that there is something they need to take care of at
some point.

Pull requests should be closed by maintainers if the pull request won't
get accepted even if improved quality wise or if the submitter is no
longer available for making it ready, but not just because they are old.

Best regards,

Robert
Jun 04 2013
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/4/13 3:52 AM, Robert wrote:

 On Mon, 2013-06-03 at 16:39 +0200, Dylan Knutson wrote:

 On a side note, there are a few (lots) of pull requests that are
 several months old, and master has changed too much for them to
 be compatible and/or relevant. Perhaps those should just be
 closed?


 I have an open pull request which is already months old and no longer
 compatible to master. This is because I have not received any feedback
 since submission.


Apologies. What's the number? I'll look into it as soon as I can.

Andrei
Jun 04 2013
next sibling parent Robert <jfanatiker gmx.at> writes:
 
 Apologies. What's the number? I'll look into it as soon as I can.
 
 Andrei
 


I will fix it according to changes in master ASAP.

Best regards,

Robert
Jun 04 2013
prev sibling parent Robert <jfanatiker gmx.at> writes:
 Apologies. What's the number? I'll look into it as soon as I can.
 
 Andrei


Wow, I have not anticipated this kind of reaction. Thanks a lot!

It can be found here:

https://github.com/D-Programming-Language/dmd/pull/1839

Best regards,

Robert
Jun 04 2013
prev sibling next sibling parent Brad Roberts <braddr puremagic.com> writes:
On 6/4/13 12:52 AM, Robert wrote:

 On Mon, 2013-06-03 at 16:39 +0200, Dylan Knutson wrote:

 On a side note, there are a few (lots) of pull requests that are
 several months old, and master has changed too much for them to
 be compatible and/or relevant. Perhaps those should just be
 closed?


 I have an open pull request which is already months old and no longer
 compatible to master. This is because I have not received any feedback
 since submission. It does not mean that it is irrelevant or that I am
 not willing to make it compatible again as soon as any reviewer has time
 for a review.

 So it has to be there, if not for anything else than just for for the
 sake of being there and being seen:
   - So that no-one else implements the same thing again for no reason
   - Maintainers know that there is something they need to take care of at
 some point.

 Pull requests should be closed by maintainers if the pull request won't
 get accepted even if improved quality wise or if the submitter is no
 longer available for making it ready, but not just because they are old.

 Best regards,

 Robert


Essentially, the submitter of the pull request needs to drive the change.  The
submission rate 
exceeds the attention span of the set of people with review and pull rights. 
So old stuff remains 
old unless brought up periodically.  This tends to be the case with every large
project.
Jun 04 2013
prev sibling parent reply "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Tuesday, June 04, 2013 09:52:57 Robert wrote:

 Pull requests should be closed by maintainers if the pull request won't
 get accepted even if improved quality wise or if the submitter is no
 longer available for making it ready, but not just because they are old.


I think that the problem is more that the Phobos maintainers are busy and 
don't necessarily do a good job as a group of reviewing pull requests in 
general, let alone making sure that each and every pull request gets reviewed 
and processed in a reasonable time frame. It's something that we need to work 
on. I think that it's rarely the case that a particular pull request has no 
comments on it because it's undesirable. It just hasn't been looked at yet 
like it should have been yet. If it were truly undesirable, it would at 
minimum have comments saying so if not have been closed.

- Jonathan M Davis
Jun 04 2013
parent "monarch_dodra" <monarchdodra gmail.com> writes:
On Tuesday, 4 June 2013 at 17:47:36 UTC, Jonathan M Davis wrote:

 On Tuesday, June 04, 2013 09:52:57 Robert wrote:

 Pull requests should be closed by maintainers if the pull 
 request won't
 get accepted even if improved quality wise or if the submitter 
 is no
 longer available for making it ready, but not just because 
 they are old.


 I think that the problem is more that the Phobos maintainers 
 are busy and
 don't necessarily do a good job as a group of reviewing pull 
 requests in
 general, let alone making sure that each and every pull request 
 gets reviewed
 and processed in a reasonable time frame. It's something that 
 we need to work
 on. I think that it's rarely the case that a particular pull 
 request has no
 comments on it because it's undesirable. It just hasn't been 
 looked at yet
 like it should have been yet. If it were truly undesirable, it 
 would at
 minimum have comments saying so if not have been closed.

 - Jonathan M Davis


Truth. I've had a pull closed in less than an hour once. On the 
other hand, I have one which has been open for about 7 months now.

It's all a balance of importance vs resource/cost ratio, and 
reviewer interest. I've closed a few of my own ("valid") pulls 
myself: they weren't bad, just that I judge reviewer times would 
be better spent on more important things.

It's a bit frustrating at times, but I find it more than balances 
when you realize that you actually *get* to participate at all, 
or even partake in specs discussion...
Jun 04 2013
prev sibling next sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 6/2/2013 10:49 PM, Adam Wilson wrote:

 Note that I did not write
 the Auto-Tester, that is the work of Brad Anderson.


Brad Roberts
Jun 04 2013
parent "Adam Wilson" <flyboynw gmail.com> writes:
On Tue, 04 Jun 2013 16:02:01 -0700, Walter Bright  
<newshound2 digitalmars.com> wrote:


 On 6/2/2013 10:49 PM, Adam Wilson wrote:

 Note that I did not write
 the Auto-Tester, that is the work of Brad Anderson.


 Brad Roberts


*sigh* I've lost count of how many times I've made that goof. Sorry Mr.  
Roberts!

-- 
Adam Wilson
IRC: LightBender
Project Coordinator
The Horizon Project
http://www.thehorizonproject.org/
Jun 04 2013
prev sibling next sibling parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 06/03/2013 07:49 AM, Adam Wilson wrote:

 - Pull requests are tested by cloning [DMD/DRuntime/Phobos] master and merging
 the pull into it. This ensures a clean pull against master.
 - The AT must restart pull request testing every time code is merged into
 DMD/DRuntime/Phobos because of the above testing strategy.
 - If many pulls are merged into [DMD/DRuntime/Phobos] master in a given period
 of, for example a day, the AT will restart prior to completing a full test
pass.
 - With the current number of open pulls the AT can complete a full testing pass
 in roughly 8 hours.
 - Each pull takes a rough average of 15 minutes to test.


What I don't understand is how the number of "pending" tests fluctuates.  Not
too long ago a pull request of mine to Phobos had passed 9, with 1 pending.  Now
it's pending 10.  I have not pushed any changes in the interim ... !
Jun 05 2013
parent "Brad Anderson" <eco gnuk.net> writes:
On Wednesday, 5 June 2013 at 16:22:19 UTC, Joseph Rushton 
Wakeling wrote:

 On 06/03/2013 07:49 AM, Adam Wilson wrote:

 - Pull requests are tested by cloning [DMD/DRuntime/Phobos] 
 master and merging
 the pull into it. This ensures a clean pull against master.
 - The AT must restart pull request testing every time code is 
 merged into
 DMD/DRuntime/Phobos because of the above testing strategy.
 - If many pulls are merged into [DMD/DRuntime/Phobos] master 
 in a given period
 of, for example a day, the AT will restart prior to completing 
 a full test pass.
 - With the current number of open pulls the AT can complete a 
 full testing pass
 in roughly 8 hours.
 - Each pull takes a rough average of 15 minutes to test.


 What I don't understand is how the number of "pending" tests 
 fluctuates.  Not
 too long ago a pull request of mine to Phobos had passed 9, 
 with 1 pending.  Now
 it's pending 10.  I have not pushed any changes in the interim 
 ... !


Every commit to master causes retesting of all pull requests.
Jun 05 2013
prev sibling next sibling parent "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Wednesday, June 05, 2013 18:22:09 Joseph Rushton Wakeling wrote:

 What I don't understand is how the number of "pending" tests fluctuates. 
 Not too long ago a pull request of mine to Phobos had passed 9, with 1
 pending. Now it's pending 10. I have not pushed any changes in the
 interim ... !


Probably because another commit was merged into dmd, druntime, or Phobos, so 
it had to start again.

- Jonathna M Davis
Jun 05 2013
prev sibling parent Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 06/05/2013 07:06 PM, Jonathan M Davis wrote:

 Probably because another commit was merged into dmd, druntime, or Phobos, so 
 it had to start again.


Ahh, OK.  Thanks to you and Brad for the clarification! :-)
Jun 05 2013