MR does not merge

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

MR does not merge

GHC - devs mailing list

Ben

Six days ago I submitted this MR

https://gitlab.haskell.org/ghc/ghc/merge_requests/109

Just tiny refactorings.  I said “merge when validated”

But six days later, it still appears not to have merged.  What’s up?  I was expecting it to merge in a matter of an hour or two.

Thanks

Simon


_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

Re: MR does not merge

Matthew Pickering
There is problem with the interaction between "merge when validated"
and "fast forward merge only" option.

If anyone commits to master between clicking the button and validation
finishing then the merge will fail as the patch needs to be rebased
before it can be merged.

I'm not sure what the plan to deal with this is.

On Wed, Jan 16, 2019 at 2:49 PM Simon Peyton Jones via ghc-devs
<[hidden email]> wrote:

>
> Ben
>
> Six days ago I submitted this MR
>
> https://gitlab.haskell.org/ghc/ghc/merge_requests/109
>
> Just tiny refactorings.  I said “merge when validated”
>
> But six days later, it still appears not to have merged.  What’s up?  I was expecting it to merge in a matter of an hour or two.
>
> Thanks
>
> Simon
>
> _______________________________________________
> ghc-devs mailing list
> [hidden email]
> http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

RE: MR does not merge

GHC - devs mailing list
But a key aspect of our workflow is:

* Make a patch, discuss review etc.
* Validate it locally
* Fire and forget (expect email if it fails to merge)

If not that, then how *can* I get a patch committed?

In parallel, might someone commit the MR below?

Simon

|  -----Original Message-----
|  From: Matthew Pickering <[hidden email]>
|  Sent: 16 January 2019 14:56
|  To: Simon Peyton Jones <[hidden email]>
|  Cc: Ben Gamari <[hidden email]>; [hidden email]
|  Subject: Re: MR does not merge
|  
|  There is problem with the interaction between "merge when validated"
|  and "fast forward merge only" option.
|  
|  If anyone commits to master between clicking the button and validation
|  finishing then the merge will fail as the patch needs to be rebased
|  before it can be merged.
|  
|  I'm not sure what the plan to deal with this is.
|  
|  On Wed, Jan 16, 2019 at 2:49 PM Simon Peyton Jones via ghc-devs <ghc-
|  [hidden email]> wrote:
|  >
|  > Ben
|  >
|  > Six days ago I submitted this MR
|  >
|  > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
|  > ab.haskell.org%2Fghc%2Fghc%2Fmerge_requests%2F109&amp;data=02%7C01%7Cs
|  > imonpj%40microsoft.com%7C63dabcb38771412f95b308d67bc2b2cc%7C72f988bf86
|  > f141af91ab2d7cd011db47%7C1%7C0%7C636832473503704904&amp;sdata=sJ2YvYGP
|  > jRanq7nhP1IKhztYPjZA5z8629Lu2DoaFFU%3D&amp;reserved=0
|  >
|  > Just tiny refactorings.  I said “merge when validated”
|  >
|  > But six days later, it still appears not to have merged.  What’s up?  I
|  was expecting it to merge in a matter of an hour or two.
|  >
|  > Thanks
|  >
|  > Simon
|  >
|  > _______________________________________________
|  > ghc-devs mailing list
|  > [hidden email]
|  > https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmail.
|  > haskell.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fghc-devs&amp;data=02%7C01
|  > %7Csimonpj%40microsoft.com%7C63dabcb38771412f95b308d67bc2b2cc%7C72f988
|  > bf86f141af91ab2d7cd011db47%7C1%7C0%7C636832473503704904&amp;sdata=c1Yy
|  > oHS2yBRMtJOOK8xqLcmbYiGj%2BBPVOn5xygSWkr8%3D&amp;reserved=0
_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

Re: MR does not merge

Moritz Angermann-2
In reply to this post by Matthew Pickering
I wonder if gitlab could have a feature like what bors offers. Validate and merge or [rebase and validate and merge]+. Thus eventually merging it or rejecting it due to conflict or validation failure.

Sent from my iPhone

> On 16 Jan 2019, at 10:55 PM, Matthew Pickering <[hidden email]> wrote:
>
> There is problem with the interaction between "merge when validated"
> and "fast forward merge only" option.
>
> If anyone commits to master between clicking the button and validation
> finishing then the merge will fail as the patch needs to be rebased
> before it can be merged.
>
> I'm not sure what the plan to deal with this is.
>
> On Wed, Jan 16, 2019 at 2:49 PM Simon Peyton Jones via ghc-devs
> <[hidden email]> wrote:
>>
>> Ben
>>
>> Six days ago I submitted this MR
>>
>> https://gitlab.haskell.org/ghc/ghc/merge_requests/109
>>
>> Just tiny refactorings.  I said “merge when validated”
>>
>> But six days later, it still appears not to have merged.  What’s up?  I was expecting it to merge in a matter of an hour or two.
>>
>> Thanks
>>
>> Simon
>>
>> _______________________________________________
>> ghc-devs mailing list
>> [hidden email]
>> http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
> _______________________________________________
> ghc-devs mailing list
> [hidden email]
> http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

Re: MR does not merge

Evan Laforge
In reply to this post by Matthew Pickering
At work we use "marge bot", https://github.com/smarkets/marge-bot
which is an automatic bot that will do the rebase and resubmit thing.
I think it's pretty essential, because otherwise any intervening merge
means you have to babysit the merge button.  It's also more efficient
if you have expensive CI, because it serializes the runs.

BTW, I've extended marge with a "try rebase, then try merge" strategy,
which is useful if people merge from head, and a "merge CI run" which
is useful if you have an expensive CI you want to run only on merge,
not on every single branch push.

On Wed, Jan 16, 2019 at 10:56 PM Matthew Pickering
<[hidden email]> wrote:

>
> There is problem with the interaction between "merge when validated"
> and "fast forward merge only" option.
>
> If anyone commits to master between clicking the button and validation
> finishing then the merge will fail as the patch needs to be rebased
> before it can be merged.
>
> I'm not sure what the plan to deal with this is.
>
> On Wed, Jan 16, 2019 at 2:49 PM Simon Peyton Jones via ghc-devs
> <[hidden email]> wrote:
> >
> > Ben
> >
> > Six days ago I submitted this MR
> >
> > https://gitlab.haskell.org/ghc/ghc/merge_requests/109
> >
> > Just tiny refactorings.  I said “merge when validated”
> >
> > But six days later, it still appears not to have merged.  What’s up?  I was expecting it to merge in a matter of an hour or two.
> >
> > Thanks
> >
> > Simon
> >
> > _______________________________________________
> > ghc-devs mailing list
> > [hidden email]
> > http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
> _______________________________________________
> ghc-devs mailing list
> [hidden email]
> http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

Re: MR does not merge

Phyx
The problem is with the different platforms that need to be tested. I could see this getting stuck in an infinite retest and rebase loop because you have to wait on different builds to finish not just one. 

So I think this is a harder problem to solve than we're making it out to be. The system needs some concept of fairness and prevention of this retry cycle. And you can't not retest the patch after a rebase as the functionality of the already merged patch may be conflicting. 

I imagine the bit cannot serialize all testing on all platforms otherwise the slowest platform becomes the problem. 

On Wed, Jan 16, 2019, 15:20 Evan Laforge <[hidden email]> wrote:
At work we use "marge bot", https://github.com/smarkets/marge-bot
which is an automatic bot that will do the rebase and resubmit thing.
I think it's pretty essential, because otherwise any intervening merge
means you have to babysit the merge button.  It's also more efficient
if you have expensive CI, because it serializes the runs.

BTW, I've extended marge with a "try rebase, then try merge" strategy,
which is useful if people merge from head, and a "merge CI run" which
is useful if you have an expensive CI you want to run only on merge,
not on every single branch push.

On Wed, Jan 16, 2019 at 10:56 PM Matthew Pickering
<[hidden email]> wrote:
>
> There is problem with the interaction between "merge when validated"
> and "fast forward merge only" option.
>
> If anyone commits to master between clicking the button and validation
> finishing then the merge will fail as the patch needs to be rebased
> before it can be merged.
>
> I'm not sure what the plan to deal with this is.
>
> On Wed, Jan 16, 2019 at 2:49 PM Simon Peyton Jones via ghc-devs
> <[hidden email]> wrote:
> >
> > Ben
> >
> > Six days ago I submitted this MR
> >
> > https://gitlab.haskell.org/ghc/ghc/merge_requests/109
> >
> > Just tiny refactorings.  I said “merge when validated”
> >
> > But six days later, it still appears not to have merged.  What’s up?  I was expecting it to merge in a matter of an hour or two.
> >
> > Thanks
> >
> > Simon
> >
> > _______________________________________________
> > ghc-devs mailing list
> > [hidden email]
> > http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
> _______________________________________________
> ghc-devs mailing list
> [hidden email]
> http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

RE: MR does not merge

GHC - devs mailing list
In reply to this post by Evan Laforge
To me it seems clear that GUARANTEEING that a patch won't break the master branch is difficult; it serialises all patches, and it may require many iterations.

Here's an obvious alternative for the "merge to master" pipeline;

1. Rebase the MR on master.  If that fails, send email to the author.

2. Full validate.  If that fails, send email to the author.

3. Rebase on master again (if master has changed). If that fails,
   send email to author

4. Commit to master

Note that

* There is a small possibility that changes between starting (2) and
  finishing (3) will meant that, even though (3) succeeds cleanly, there
  is a semantic bug that means that the result fails.

  But it is extremely unlikely.  And if it does happen, all subsequent
  builds will fail, so we'll soon know.  Belt-and-braces: you could validate
  the result of (4) so that blame is always correctly attributed.

* The process is highly parallel.  You can be validating multiple patches
  at the same time, in stark contrast to the completely serial nature of
  our current story.

Can Gitlab do this?

Simon

|  -----Original Message-----
|  From: Evan Laforge <[hidden email]>
|  Sent: 16 January 2019 15:20
|  To: Matthew Pickering <[hidden email]>
|  Cc: Simon Peyton Jones <[hidden email]>; [hidden email]
|  Subject: Re: MR does not merge
|  
|  At work we use "marge bot",
|  https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.
|  com%2Fsmarkets%2Fmarge-
|  bot&amp;data=02%7C01%7Csimonpj%40microsoft.com%7Cb0a1fe00d27e4879ae1e08d6
|  7bc614f2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636832488027160974&
|  amp;sdata=f99fNqu5kgFuFK%2BUEU8d3Z4JDlQek1yYuY2OM9AMFf8%3D&amp;reserved=0
|  which is an automatic bot that will do the rebase and resubmit thing.
|  I think it's pretty essential, because otherwise any intervening merge
|  means you have to babysit the merge button.  It's also more efficient if
|  you have expensive CI, because it serializes the runs.
|  
|  BTW, I've extended marge with a "try rebase, then try merge" strategy,
|  which is useful if people merge from head, and a "merge CI run" which is
|  useful if you have an expensive CI you want to run only on merge, not on
|  every single branch push.
|  
|  On Wed, Jan 16, 2019 at 10:56 PM Matthew Pickering
|  <[hidden email]> wrote:
|  >
|  > There is problem with the interaction between "merge when validated"
|  > and "fast forward merge only" option.
|  >
|  > If anyone commits to master between clicking the button and validation
|  > finishing then the merge will fail as the patch needs to be rebased
|  > before it can be merged.
|  >
|  > I'm not sure what the plan to deal with this is.
|  >
|  > On Wed, Jan 16, 2019 at 2:49 PM Simon Peyton Jones via ghc-devs
|  > <[hidden email]> wrote:
|  > >
|  > > Ben
|  > >
|  > > Six days ago I submitted this MR
|  > >
|  > > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
|  > > tlab.haskell.org%2Fghc%2Fghc%2Fmerge_requests%2F109&amp;data=02%7C01
|  > > %7Csimonpj%40microsoft.com%7Cb0a1fe00d27e4879ae1e08d67bc614f2%7C72f9
|  > > 88bf86f141af91ab2d7cd011db47%7C1%7C0%7C636832488027160974&amp;sdata=
|  > > OjHtPXsIpjeW7tPJvheT%2F4hh4aOW7h82bdcifdQ6cfA%3D&amp;reserved=0
|  > >
|  > > Just tiny refactorings.  I said “merge when validated”
|  > >
|  > > But six days later, it still appears not to have merged.  What’s up?
|  I was expecting it to merge in a matter of an hour or two.
|  > >
|  > > Thanks
|  > >
|  > > Simon
|  > >
|  > > _______________________________________________
|  > > ghc-devs mailing list
|  > > [hidden email]
|  > > https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmai
|  > > l.haskell.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fghc-devs&amp;data=02%
|  > > 7C01%7Csimonpj%40microsoft.com%7Cb0a1fe00d27e4879ae1e08d67bc614f2%7C
|  > > 72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636832488027160974&amp;sd
|  > > ata=AxazRR%2FEUVEB93FYHAaLig%2FYPBR%2BLH9eAciYs5NTaNU%3D&amp;reserve
|  > > d=0
|  > _______________________________________________
|  > ghc-devs mailing list
|  > [hidden email]
|  > https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmail.
|  > haskell.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fghc-devs&amp;data=02%7C01
|  > %7Csimonpj%40microsoft.com%7Cb0a1fe00d27e4879ae1e08d67bc614f2%7C72f988
|  > bf86f141af91ab2d7cd011db47%7C1%7C0%7C636832488027160974&amp;sdata=Axaz
|  > RR%2FEUVEB93FYHAaLig%2FYPBR%2BLH9eAciYs5NTaNU%3D&amp;reserved=0
_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

Re: MR does not merge

Richard Eisenberg-4
+1 on Simon's plan. I explicitly agree with the move not to validate before the final commit (if a rebase is necessary); note that the successful rebase is guaranteed not to have any conflicts (or else step 3 would have failed). We should continue to do CI on the commits that have actually landed, so the final result is validated.

And here's another idea to grease the wheels: I imagine the majority of commits have essentially no chance of showing breakage in the variety of platforms/build methods that we have. Right now, we have [skip ci] as a way of telling the infrastructure to skip. What if we had [full ci] requesting that all the different configurations were run before merging? Without [full ci], only the Linux validation would take place; if that's green, merge. Regardless of the setting of [full ci], the full CI matrix would be run after merging. (IIUC, this is different than [skip ci], which also stops CI from happening after the merge.) Note that this new plan requires opt-in when the author is worried about breakage, but would still catch breakage ere long when the full CI matrix is run after merging. In the meantime, even if full CI fails and the patch author is working on fixing it, the rest of us still know that the build succeeds at least on Linux, so we can continue to plow forward. (Except maybe for Simon. Sorry, Simon.)

I don't know if this new idea is necessary, but it might clear logjams if we notice that they happen often.

Richard

> On Jan 17, 2019, at 7:41 AM, Simon Peyton Jones via ghc-devs <[hidden email]> wrote:
>
> To me it seems clear that GUARANTEEING that a patch won't break the master branch is difficult; it serialises all patches, and it may require many iterations.
>
> Here's an obvious alternative for the "merge to master" pipeline;
>
> 1. Rebase the MR on master.  If that fails, send email to the author.
>
> 2. Full validate.  If that fails, send email to the author.
>
> 3. Rebase on master again (if master has changed). If that fails,
>   send email to author
>
> 4. Commit to master
>
> Note that
>
> * There is a small possibility that changes between starting (2) and
>  finishing (3) will meant that, even though (3) succeeds cleanly, there
>  is a semantic bug that means that the result fails.
>
>  But it is extremely unlikely.  And if it does happen, all subsequent
>  builds will fail, so we'll soon know.  Belt-and-braces: you could validate
>  the result of (4) so that blame is always correctly attributed.
>
> * The process is highly parallel.  You can be validating multiple patches
>  at the same time, in stark contrast to the completely serial nature of
>  our current story.
>
> Can Gitlab do this?
>
> Simon
>
> |  -----Original Message-----
> |  From: Evan Laforge <[hidden email]>
> |  Sent: 16 January 2019 15:20
> |  To: Matthew Pickering <[hidden email]>
> |  Cc: Simon Peyton Jones <[hidden email]>; [hidden email]
> |  Subject: Re: MR does not merge
> |  
> |  At work we use "marge bot",
> |  https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.
> |  com%2Fsmarkets%2Fmarge-
> |  bot&amp;data=02%7C01%7Csimonpj%40microsoft.com%7Cb0a1fe00d27e4879ae1e08d6
> |  7bc614f2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636832488027160974&
> |  amp;sdata=f99fNqu5kgFuFK%2BUEU8d3Z4JDlQek1yYuY2OM9AMFf8%3D&amp;reserved=0
> |  which is an automatic bot that will do the rebase and resubmit thing.
> |  I think it's pretty essential, because otherwise any intervening merge
> |  means you have to babysit the merge button.  It's also more efficient if
> |  you have expensive CI, because it serializes the runs.
> |  
> |  BTW, I've extended marge with a "try rebase, then try merge" strategy,
> |  which is useful if people merge from head, and a "merge CI run" which is
> |  useful if you have an expensive CI you want to run only on merge, not on
> |  every single branch push.
> |  
> |  On Wed, Jan 16, 2019 at 10:56 PM Matthew Pickering
> |  <[hidden email]> wrote:
> |  >
> |  > There is problem with the interaction between "merge when validated"
> |  > and "fast forward merge only" option.
> |  >
> |  > If anyone commits to master between clicking the button and validation
> |  > finishing then the merge will fail as the patch needs to be rebased
> |  > before it can be merged.
> |  >
> |  > I'm not sure what the plan to deal with this is.
> |  >
> |  > On Wed, Jan 16, 2019 at 2:49 PM Simon Peyton Jones via ghc-devs
> |  > <[hidden email]> wrote:
> |  > >
> |  > > Ben
> |  > >
> |  > > Six days ago I submitted this MR
> |  > >
> |  > > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> |  > > tlab.haskell.org%2Fghc%2Fghc%2Fmerge_requests%2F109&amp;data=02%7C01
> |  > > %7Csimonpj%40microsoft.com%7Cb0a1fe00d27e4879ae1e08d67bc614f2%7C72f9
> |  > > 88bf86f141af91ab2d7cd011db47%7C1%7C0%7C636832488027160974&amp;sdata=
> |  > > OjHtPXsIpjeW7tPJvheT%2F4hh4aOW7h82bdcifdQ6cfA%3D&amp;reserved=0
> |  > >
> |  > > Just tiny refactorings.  I said “merge when validated”
> |  > >
> |  > > But six days later, it still appears not to have merged.  What’s up?
> |  I was expecting it to merge in a matter of an hour or two.
> |  > >
> |  > > Thanks
> |  > >
> |  > > Simon
> |  > >
> |  > > _______________________________________________
> |  > > ghc-devs mailing list
> |  > > [hidden email]
> |  > > https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmai
> |  > > l.haskell.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fghc-devs&amp;data=02%
> |  > > 7C01%7Csimonpj%40microsoft.com%7Cb0a1fe00d27e4879ae1e08d67bc614f2%7C
> |  > > 72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636832488027160974&amp;sd
> |  > > ata=AxazRR%2FEUVEB93FYHAaLig%2FYPBR%2BLH9eAciYs5NTaNU%3D&amp;reserve
> |  > > d=0
> |  > _______________________________________________
> |  > ghc-devs mailing list
> |  > [hidden email]
> |  > https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmail.
> |  > haskell.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fghc-devs&amp;data=02%7C01
> |  > %7Csimonpj%40microsoft.com%7Cb0a1fe00d27e4879ae1e08d67bc614f2%7C72f988
> |  > bf86f141af91ab2d7cd011db47%7C1%7C0%7C636832488027160974&amp;sdata=Axaz
> |  > RR%2FEUVEB93FYHAaLig%2FYPBR%2BLH9eAciYs5NTaNU%3D&amp;reserved=0
> _______________________________________________
> ghc-devs mailing list
> [hidden email]
> http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

Re: MR does not merge

Matthew Pickering
I am against any proposed change which makes it easier to break the
build on any platform. Someone, usually Ben, has to waste a lot of
time fixing these breakages and if master does not build it affects
every developer.

If a commit breaks the build then it should prevent the patch being
merged. Now that CI is robust, this is a realistic goal.

My preferred solution is to start using "marge-bot" which is battle
tested solution for this precise problem. We don't need to reinvent
our own solution. Hopefully the native support in gitlab will improve
over time but for now it is the highest impact change we can make
whilst maintaining the core principles of CI.

Cheers,

Matt

On Thu, Jan 17, 2019 at 1:12 PM Richard Eisenberg <[hidden email]> wrote:

>
> +1 on Simon's plan. I explicitly agree with the move not to validate before the final commit (if a rebase is necessary); note that the successful rebase is guaranteed not to have any conflicts (or else step 3 would have failed). We should continue to do CI on the commits that have actually landed, so the final result is validated.
>
> And here's another idea to grease the wheels: I imagine the majority of commits have essentially no chance of showing breakage in the variety of platforms/build methods that we have. Right now, we have [skip ci] as a way of telling the infrastructure to skip. What if we had [full ci] requesting that all the different configurations were run before merging? Without [full ci], only the Linux validation would take place; if that's green, merge. Regardless of the setting of [full ci], the full CI matrix would be run after merging. (IIUC, this is different than [skip ci], which also stops CI from happening after the merge.) Note that this new plan requires opt-in when the author is worried about breakage, but would still catch breakage ere long when the full CI matrix is run after merging. In the meantime, even if full CI fails and the patch author is working on fixing it, the rest of us still know that the build succeeds at least on Linux, so we can continue to plow forward. (Except maybe for Simon. Sorry, Simon.)
>
> I don't know if this new idea is necessary, but it might clear logjams if we notice that they happen often.
>
> Richard
>
> > On Jan 17, 2019, at 7:41 AM, Simon Peyton Jones via ghc-devs <[hidden email]> wrote:
> >
> > To me it seems clear that GUARANTEEING that a patch won't break the master branch is difficult; it serialises all patches, and it may require many iterations.
> >
> > Here's an obvious alternative for the "merge to master" pipeline;
> >
> > 1. Rebase the MR on master.  If that fails, send email to the author.
> >
> > 2. Full validate.  If that fails, send email to the author.
> >
> > 3. Rebase on master again (if master has changed). If that fails,
> >   send email to author
> >
> > 4. Commit to master
> >
> > Note that
> >
> > * There is a small possibility that changes between starting (2) and
> >  finishing (3) will meant that, even though (3) succeeds cleanly, there
> >  is a semantic bug that means that the result fails.
> >
> >  But it is extremely unlikely.  And if it does happen, all subsequent
> >  builds will fail, so we'll soon know.  Belt-and-braces: you could validate
> >  the result of (4) so that blame is always correctly attributed.
> >
> > * The process is highly parallel.  You can be validating multiple patches
> >  at the same time, in stark contrast to the completely serial nature of
> >  our current story.
> >
> > Can Gitlab do this?
> >
> > Simon
> >
> > |  -----Original Message-----
> > |  From: Evan Laforge <[hidden email]>
> > |  Sent: 16 January 2019 15:20
> > |  To: Matthew Pickering <[hidden email]>
> > |  Cc: Simon Peyton Jones <[hidden email]>; [hidden email]
> > |  Subject: Re: MR does not merge
> > |
> > |  At work we use "marge bot",
> > |  https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.
> > |  com%2Fsmarkets%2Fmarge-
> > |  bot&amp;data=02%7C01%7Csimonpj%40microsoft.com%7Cb0a1fe00d27e4879ae1e08d6
> > |  7bc614f2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636832488027160974&
> > |  amp;sdata=f99fNqu5kgFuFK%2BUEU8d3Z4JDlQek1yYuY2OM9AMFf8%3D&amp;reserved=0
> > |  which is an automatic bot that will do the rebase and resubmit thing.
> > |  I think it's pretty essential, because otherwise any intervening merge
> > |  means you have to babysit the merge button.  It's also more efficient if
> > |  you have expensive CI, because it serializes the runs.
> > |
> > |  BTW, I've extended marge with a "try rebase, then try merge" strategy,
> > |  which is useful if people merge from head, and a "merge CI run" which is
> > |  useful if you have an expensive CI you want to run only on merge, not on
> > |  every single branch push.
> > |
> > |  On Wed, Jan 16, 2019 at 10:56 PM Matthew Pickering
> > |  <[hidden email]> wrote:
> > |  >
> > |  > There is problem with the interaction between "merge when validated"
> > |  > and "fast forward merge only" option.
> > |  >
> > |  > If anyone commits to master between clicking the button and validation
> > |  > finishing then the merge will fail as the patch needs to be rebased
> > |  > before it can be merged.
> > |  >
> > |  > I'm not sure what the plan to deal with this is.
> > |  >
> > |  > On Wed, Jan 16, 2019 at 2:49 PM Simon Peyton Jones via ghc-devs
> > |  > <[hidden email]> wrote:
> > |  > >
> > |  > > Ben
> > |  > >
> > |  > > Six days ago I submitted this MR
> > |  > >
> > |  > > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> > |  > > tlab.haskell.org%2Fghc%2Fghc%2Fmerge_requests%2F109&amp;data=02%7C01
> > |  > > %7Csimonpj%40microsoft.com%7Cb0a1fe00d27e4879ae1e08d67bc614f2%7C72f9
> > |  > > 88bf86f141af91ab2d7cd011db47%7C1%7C0%7C636832488027160974&amp;sdata=
> > |  > > OjHtPXsIpjeW7tPJvheT%2F4hh4aOW7h82bdcifdQ6cfA%3D&amp;reserved=0
> > |  > >
> > |  > > Just tiny refactorings.  I said “merge when validated”
> > |  > >
> > |  > > But six days later, it still appears not to have merged.  What’s up?
> > |  I was expecting it to merge in a matter of an hour or two.
> > |  > >
> > |  > > Thanks
> > |  > >
> > |  > > Simon
> > |  > >
> > |  > > _______________________________________________
> > |  > > ghc-devs mailing list
> > |  > > [hidden email]
> > |  > > https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmai
> > |  > > l.haskell.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fghc-devs&amp;data=02%
> > |  > > 7C01%7Csimonpj%40microsoft.com%7Cb0a1fe00d27e4879ae1e08d67bc614f2%7C
> > |  > > 72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636832488027160974&amp;sd
> > |  > > ata=AxazRR%2FEUVEB93FYHAaLig%2FYPBR%2BLH9eAciYs5NTaNU%3D&amp;reserve
> > |  > > d=0
> > |  > _______________________________________________
> > |  > ghc-devs mailing list
> > |  > [hidden email]
> > |  > https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmail.
> > |  > haskell.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fghc-devs&amp;data=02%7C01
> > |  > %7Csimonpj%40microsoft.com%7Cb0a1fe00d27e4879ae1e08d67bc614f2%7C72f988
> > |  > bf86f141af91ab2d7cd011db47%7C1%7C0%7C636832488027160974&amp;sdata=Axaz
> > |  > RR%2FEUVEB93FYHAaLig%2FYPBR%2BLH9eAciYs5NTaNU%3D&amp;reserved=0
> > _______________________________________________
> > ghc-devs mailing list
> > [hidden email]
> > http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
>
_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

Re: MR does not merge

Ben Gamari-3
In reply to this post by GHC - devs mailing list
Simon Peyton Jones via ghc-devs <[hidden email]> writes:

> Ben
>
> Six days ago I submitted this MR
> https://gitlab.haskell.org/ghc/ghc/merge_requests/109
> Just tiny refactorings.  I said "merge when validated"
> But six days later, it still appears not to have merged. What's up? I
> was expecting it to merge in a matter of an hour or two.

Indeed this is a known issue that I have been working [1] with upstream
to resolve. A fix, which looks similar to the plan you propse, is
milestoned for the May release. In the meantime I have been handling
merging manually as I did under Phabricator: batch up mergeable MRs into
a merge branch, push the merge branch for CI, and merge it after it
validates.

Marge bot [2], mentioned by Evan, is another workaround for this
issue.However, my understanding is that it is unnecessarily serial which
I feared may slow the rate of merge. We could consider using it until a
proper upstream solution arrives, however.

In general I agree with Matthew that we should try very hard to avoid
ever merging broken code; the cost of bad patches is extremely high. The
solution that GitLab is looking to implement maintains a stack of MRs to
be merged, optimistically assuming that all will pass CI. When a new MR
is requested to be merged it is rebased onto the end of the stack and CI
jobs are started. This allows it to avoid testing commits serially. If
an MR low in the stack fails then it is dropped from the stack, all MRs
above it are rebased, and retested.

Cheers,

- Ben


[1] https://gitlab.com/gitlab-org/gitlab-ee/issues/9186
[2] https://github.com/smarkets/marge-bot

_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

signature.asc (497 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: MR does not merge

Ben Gamari-2
In reply to this post by Matthew Pickering
Matthew Pickering <[hidden email]> writes:

> I am against any proposed change which makes it easier to break the
> build on any platform. Someone, usually Ben, has to waste a lot of
> time fixing these breakages and if master does not build it affects
> every developer.
>
Indeed, I agree with Matthew here. Breakages are very painful both when
they happen and after when we need to bisect through them.

> If a commit breaks the build then it should prevent the patch being
> merged. Now that CI is robust, this is a realistic goal.
>
> My preferred solution is to start using "marge-bot" which is battle
> tested solution for this precise problem. We don't need to reinvent
> our own solution. Hopefully the native support in gitlab will improve
> over time but for now it is the highest impact change we can make
> whilst maintaining the core principles of CI.
>
I would be fine with trying to deploy it. My thought was that it would
be "only" a few months of manually working around the issue until an
upstream solution arrived. However, I agree that this may be just long
enough to justify deploying an interim solution.

I can try this today if others agree this is a reasonable way forward.

Cheers,

- Ben


_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

signature.asc (497 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

RE: MR does not merge

GHC - devs mailing list
In reply to this post by Ben Gamari-3
|  Indeed this is a known issue that I have been working [1] with upstream
|  to resolve.

Thanks.  I'm not equipped to express a well-informed opinion about what the best thing to do is.  But in the meantime I WOULD be grateful for explicit workflow advice.  Specifically:

* What steps should I take to get a patch committed to master,
  assuming I've done the review stuff and want to press "go"?

Meanwhile, could you commit MR 109 please?

Simon

|  -----Original Message-----
|  From: Ben Gamari <[hidden email]>
|  Sent: 17 January 2019 15:10
|  To: Simon Peyton Jones <[hidden email]>
|  Cc: [hidden email]
|  Subject: Re: MR does not merge
|  
|  Simon Peyton Jones via ghc-devs <[hidden email]> writes:
|  
|  > Ben
|  >
|  > Six days ago I submitted this MR
|  > https://gitlab.haskell.org/ghc/ghc/merge_requests/109
|  > Just tiny refactorings.  I said "merge when validated"
|  > But six days later, it still appears not to have merged. What's up? I
|  > was expecting it to merge in a matter of an hour or two.
|  
|  Indeed this is a known issue that I have been working [1] with upstream
|  to resolve. A fix, which looks similar to the plan you propse, is
|  milestoned for the May release. In the meantime I have been handling
|  merging manually as I did under Phabricator: batch up mergeable MRs into
|  a merge branch, push the merge branch for CI, and merge it after it
|  validates.
|  
|  Marge bot [2], mentioned by Evan, is another workaround for this
|  issue.However, my understanding is that it is unnecessarily serial which
|  I feared may slow the rate of merge. We could consider using it until a
|  proper upstream solution arrives, however.
|  
|  In general I agree with Matthew that we should try very hard to avoid
|  ever merging broken code; the cost of bad patches is extremely high. The
|  solution that GitLab is looking to implement maintains a stack of MRs to
|  be merged, optimistically assuming that all will pass CI. When a new MR
|  is requested to be merged it is rebased onto the end of the stack and CI
|  jobs are started. This allows it to avoid testing commits serially. If an
|  MR low in the stack fails then it is dropped from the stack, all MRs
|  above it are rebased, and retested.
|  
|  Cheers,
|  
|  - Ben
|  
|  
|  [1] https://gitlab.com/gitlab-org/gitlab-ee/issues/9186
|  [2] https://github.com/smarkets/marge-bot
_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

RE: MR does not merge

Ben Gamari-3
Simon Peyton Jones via ghc-devs <[hidden email]> writes:

> |  Indeed this is a known issue that I have been working [1] with upstream
> |  to resolve.
>
> Thanks. I'm not equipped to express a well-informed opinion about what
> the best thing to do is. But in the meantime I WOULD be grateful for
> explicit workflow advice. Specifically:
>
> * What steps should I take to get a patch committed to master,
>   assuming I've done the review stuff and want to press "go"?
>
At the moment it's largely just a matter of when a bulk merge happens; I
did a large merge on Wednesday and another yesterday.

However, as Matthew suggested I think it may make sense to try using
Marge bot to eliminate this manual process with little cost. It doesn't
take particularly long to put together a bulk merge but it does require
some form of human intervention which generally implies latency.

Cheers,

- Ben


_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

signature.asc (497 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: MR does not merge

Clinton Mead
Hi All

I'm not a GHC dev so my understanding of this process is limited to this thread but just my thoughts.

My understanding is that we want to achieve the following two goals:

1. Never allow code which breaks tests to be committed to master.
2. Ensure that master is up to date as soon as possible with recently submitted merge requests (MR).

The issue seems to be that the only way to ensure 1 is to use a serial "rebase/test/make master branch" process on every MR. Which means if you get a lot of MRs in a row you can get a queue of MRs blowing out.

So what I propose is the following:

1. Keep a queue of pending MRs.
2. When the previous test is complete, create a branch (lets call it "pending") which is all the MRs in the queue rebased firstly on master and then each other. Drop any MRs which fail this rebasing.
3. Run tests against "pending"
4. If the tests pass, "pending" becomes "master". However, if the CI for "pending" fails, "split" pending into two (half the MRs in each, perhaps interleaving their size also), rebase them separately on master call them "pending1" and "pending2". If there's only one MR pending, don't "split" it (you can't), just report the test failure to the MR owner.
5. If either "pending1" or "pending2" passes, it becomes "master". Also, whether either or both of "pending1" or "pending2" fails, go back to step 4 for these. If they both pass (which probably should never happen) maybe just merge one into master arbitrarily and put the other MRs in the pending MR queue.
6. Once we've merged all our MRs in to master (and perhaps through the binary search above found the broken MR) start this process again with the current pending MRs.

With this process we ensure master is never broken, but we can test and merge n MRs in log(n) time, so the MR queue will not grow arbitrarily long if the rate of submitted MRs exceeds the rate we run CI tests on them.

"Marge-bot" mentioned almost does what I suggest, except in the case of a failure it runs the MRs one-by-one, instead of binary split like I suggest. Perhaps my proposal could be best implemented as a patch to Marge-bot.


On Sat, Jan 19, 2019 at 2:42 AM Ben Gamari <[hidden email]> wrote:
Simon Peyton Jones via ghc-devs <[hidden email]> writes:

> |  Indeed this is a known issue that I have been working [1] with upstream
> |  to resolve.
>
> Thanks. I'm not equipped to express a well-informed opinion about what
> the best thing to do is. But in the meantime I WOULD be grateful for
> explicit workflow advice. Specifically:
>
> * What steps should I take to get a patch committed to master,
>   assuming I've done the review stuff and want to press "go"?
>
At the moment it's largely just a matter of when a bulk merge happens; I
did a large merge on Wednesday and another yesterday.

However, as Matthew suggested I think it may make sense to try using
Marge bot to eliminate this manual process with little cost. It doesn't
take particularly long to put together a bulk merge but it does require
some form of human intervention which generally implies latency.

Cheers,

- Ben

_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

Re: MR does not merge

Ben Gamari-3
Clinton Mead <[hidden email]> writes:

> Hi All
>
> I'm not a GHC dev so my understanding of this process is limited to this
> thread but just my thoughts.
>
> My understanding is that we want to achieve the following two goals:
>
> 1. Never allow code which breaks tests to be committed to master.
> 2. Ensure that master is up to date as soon as possible with recently
> submitted merge requests (MR).
>
> The issue seems to be that the only way to ensure 1 is to use a serial
> "rebase/test/make master branch" process on every MR. Which means if you
> get a lot of MRs in a row you can get a queue of MRs blowing out.
>
> So what I propose is the following:
>
> 1. Keep a queue of pending MRs.
> 2. When the previous test is complete, create a branch (lets call it
> "pending") which is all the MRs in the queue rebased firstly on master and
> then each other. Drop any MRs which fail this rebasing.
> 3. Run tests against "pending"
> 4. If the tests pass, "pending" becomes "master". However, if the CI for
> "pending" fails, "split" pending into two (half the MRs in each, perhaps
> interleaving their size also), rebase them separately on master call them
> "pending1" and "pending2". If there's only one MR pending, don't "split" it
> (you can't), just report the test failure to the MR owner.
> 5. If either "pending1" or "pending2" passes, it becomes "master". Also,
> whether either or both of "pending1" or "pending2" fails, go back to step 4
> for these. If they both pass (which probably should never happen) maybe
> just merge one into master arbitrarily and put the other MRs in the pending
> MR queue.
> 6. Once we've merged all our MRs in to master (and perhaps through the
> binary search above found the broken MR) start this process again with the
> current pending MRs.
>
> With this process we ensure master is never broken, but we can test and
> merge n MRs in log(n) time, so the MR queue will not grow arbitrarily long
> if the rate of submitted MRs exceeds the rate we run CI tests on them.
>
> "Marge-bot" mentioned almost does what I suggest, except in the case of a
> failure it runs the MRs one-by-one, instead of binary split like I suggest.
> Perhaps my proposal could be best implemented as a patch to Marge-bot.
>
What you propose is similar to what upstream is planning [1]. While it
would be nice to have a better solution until this plan comes to
fruition, I'm personally a bit reluctant to put much effort into
marge-bot given that it really is just a stop-gap solution.

Of course, if someone else would like to pick up this project we would
be quite appreciative.

Cheers,

- Ben


[1] https://gitlab.com/gitlab-org/gitlab-ee/issues/9186

_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

signature.asc (497 bytes) Download Attachment