[users at bb.net] Gitlab, MRs, and try builds, oh my

[Dan Kegel]

Har.  That change is a little too trivial, as the changeset points
to master without the change.  I have a bit more work to do.

Also, the hook probably needs to filter out events like
'merge request closed' and not cause builds when they happen.
- Dan


On Mon, May 14, 2018 at 10:46 AM, Dan Kegel <dank at kegel.com> wrote:
> On Mon, May 14, 2018 at 8:22 AM, Dan Kegel <dank at kegel.com> wrote:
>> In days of yore, my old gitlab trybuild gateway read source_branch
>> from the merge request,
>> and used that to decide which branch to run the try build against.
>
> To expand on that, the thinking is there are many ephemeral
> merge request branches, so one can't go changing
> master.cfg to know about all of them; that means merge request
> builds really should be against the target branch.
>
> If that is indeed correct, the fix seems trivial (passes tests, but
> not tested in practice):
>
> --- a/master/buildbot/test/unit/test_www_hooks_gitlab.py
> +++ b/master/buildbot/test/unit/test_www_hooks_gitlab.py
> @@ -222,7 +222,8 @@ class
> TestChangeHookConfiguredWithGitChange(unittest.TestCase):
>              calendar.timegm(change["when_timestamp"].utctimetuple()),
>              1325626589
>          )
> -        self.assertEqual(change["branch"], "ms-viewport")
> +        self.assertEqual(change["branch"], "master")
> +        self.assertEqual(change['properties']["source_branch"], 'ms-viewport')
>          self.assertEqual(change['properties']["target_branch"], 'master')
>          self.assertEqual(change["category"], "merge_request")
>
> diff --git a/master/buildbot/www/hooks/gitlab.py
> b/master/buildbot/www/hooks/gitlab.py
> index f2fc4383b..8d55bd92d 100644
> --- a/master/buildbot/www/hooks/gitlab.py
> +++ b/master/buildbot/www/hooks/gitlab.py
> @@ -115,12 +115,13 @@ class GitLabHandler(BaseHookHandler):
>              'comments': "MR#{}: {}\n\n{}".format(attrs['iid'],
> attrs['title'], attrs['description']),
>              'revision': commit['id'],
>              'when_timestamp': when_timestamp,
> -            'branch': attrs['source_branch'],
> +            'branch': attrs['target_branch'],
>              'repository': repo_url,
>              'project': project,
>              'category': event,
>              'revlink': attrs['url'],
>              'properties': {
> +                'source_branch': attrs['source_branch'],
>                  'target_branch': attrs['target_branch'],
>                  'target_repository': attrs['target']['git_http_url'],
>                  'event': event,
>
> Is anyone depending on the existing behavior?
>
>> There is one further wrinkle: my old buildbot instance used naming conventions
>> to map project + branch to trybuilders
>
> I'm hoping I can put that logic into master.cfg, but haven't checked yet.
> - Dan