maven.mozilla.org: Only spin the beetmover task on RELBRANCHes (on mozilla-release)

RESOLVED FIXED

Status

defect
RESOLVED FIXED
10 months ago
8 months ago

People

(Reporter: jlorenzo, Assigned: jlorenzo)

Tracking

unspecified

Firefox Tracking Flags

(geckoview62 wontfix, geckoview63 wontfix, firefox-esr60 wontfix, firefox63 wontfix, firefox64 fixed, firefox65 fixed)

Details

Attachments

(3 attachments, 2 obsolete attachments)

The Geckoview team has dedicated RELBRANCHes (like [1]) to backport some of the geckoview fixes on release. They are the sole users of this branch. :cpeterson told me they want a release for each push they make on these RELBRANCHes. Therefore, taskgraph must schedule a beetmover_geckoview task[2] only on these branches when on mozilla-release.

Sadly, taskgraph doesn't have access to the branch data. To me, the data should be passed this way:


1. mozilla-taskcluster exposes the branch of the last changeset of the push in the JSON-e context at [3] (because it already has the full data of push)
2. in-tree .taskcluster.yml passes the data to the definition of the decision task. More precisely, it passes it to the `mach taskgraph decision` arguments[4].
3. `mach taskgraph decision` converts the arguments to a taskgraph params.
4. beetmover_geckoview takes the branch from  taskgraph params.


[1] https://hg.mozilla.org/releases/mozilla-release/shortlog/GECKOVIEW_62_RELBRANCH
[2] https://searchfox.org/mozilla-central/rev/0ec9f5972ef3e4a479720630ad7285a03776cdfc/taskcluster/ci/beetmover-geckoview/kind.yml
[3] https://github.com/taskcluster/mozilla-taskcluster/blob/8c5eebcc838064f0fc43142fe6b77c2ef187370f/src/jobs/taskcluster_graph.js#L232
[4] https://searchfox.org/mozilla-central/rev/0ec9f5972ef3e4a479720630ad7285a03776cdfc/.taskcluster.yml#187
[5] https://searchfox.org/mozilla-central/rev/0ec9f5972ef3e4a479720630ad7285a03776cdfc/taskcluster/taskgraph/decision.py#148
Does it look like a sane plan to you, Dustin?
Flags: needinfo?(dustin)
I had to read this twice to figure out that the word "branch" means two different things.  Happily, within Taskgraph we're careful to call one of those "project", so I think it will be OK.  Perhaps the parameter can be named something super-specific like `hg_branch`?

Also, could the decision task figure this out for itself based on the revision that it already has?  I don't know if that would be better or worse than changing mozilla-taskcluster, to be honest.

Mozilla-taskcluster is old and kind of cranky, but this additional functionality shouldn't be difficult to add.

So, yes, sounds like a good plan.
Flags: needinfo?(dustin)
geckoview: only spin beetmover tasks on GECKOVIEW_XX_RELBRANCH
Thank you for the suggestion, Dustin! My initial though was that we wanted the decision task to as offline as possible, that's why I suggested the changes in mozilla-taskcluster. But if I'm allowed to make a call to hg.m.o, then the problem is easier to solve! 

I took your `hg_branch` suggestion.

On another note, the current implementation makes these tasks part of the ship graph. I had to cancel today's tasks in Fennec's 63.0 graph[1]. :cpeterson didn't want to confuse consumers of maven.mozilla.org. 

[1] https://tools.taskcluster.net/groups/aUFuHBj0QZeHB8jFxwSymA/tasks/dyU-MDmCRxCQH0lbe-CpzA/details
Assignee: nobody → jlorenzo
Status: NEW → ASSIGNED
(In reply to Johan Lorenzo [:jlorenzo] from comment #4)
> Thank you for the suggestion, Dustin! My initial though was that we wanted
> the decision task to as offline as possible, that's why I suggested the
> changes in mozilla-taskcluster. But if I'm allowed to make a call to hg.m.o,
> then the problem is easier to solve! 

You shouldn't need to make a call to hg.m.o; relbranch info is part of the commit metadata, so is available locally (`hg branch`) will tell you the branch.
(In reply to Johan Lorenzo [:jlorenzo] from comment #4)
> On another note, the current implementation makes these tasks part of the
> ship graph. I had to cancel today's tasks in Fennec's 63.0 graph[1].
> :cpeterson didn't want to confuse consumers of maven.mozilla.org. 

SGTM. As long as we keep creating GeckoView relbranches (GECKOVIEW_##_RELBRANCH), we should publish the relbranch GeckoView builds to maven.mozilla.org and not any GeckoView builds from mozilla-release's "default" branch. The GeckoView relbranch will also have the same or more recent code than the default branch.
(In reply to Tom Prince [:tomprince] from comment #6)
> You shouldn't need to make a call to hg.m.o [...]

Great suggestion! I changed the PR to use `hg identify` (`hg branch` only works on the current revision). It works great!
I've been thinking about this off and on off all day, since I got the review request.

The goals, as I understand it

1. Publish builds to maven as part of nightlies, and part of shipping beta builds.
2. Publish builds for every push to mozilla-release on the GECKOVIEW_*_RELBRANCHes.
3. Don't publish builds as part of the release process on mozilla-release.

Additionally,

4. It appears that builds and tests for desktop platforms are run on GECKOVIEW_*_RELBRANHes[1]


Taking 2+4 into account, it seems like we probably want a different target task that filters out desktop jobs and includes the beetmover-geckoview job.

For 3, I definitely think we should put the configuration of which branches are where beetmover-gv gets run into the yaml (maybe something like the below)

run-on-branches:
    by-project:
        mozilla-release:
            - GECKOVIEW_\d+_RELBRANCH$
        default:
            - .*

I do have some more complicated thoughts on how to handle 3, that are probably beyond the scope of this issue. Although projects and hg-branches (to use the terminology used in the attached patch) are different things, they way they are being used *here* are really being used for the same purpose, so it would be nice to treat the the same way (at least by the time we get to transforms). One random thought, is that maybe the geckoview relbranch should be represented as something like project == `mozilla-release/geckoview` or something. Also, in the world where we move to using hg-branches instead of projects[2] for trains, we'll need something like this.


[1] For example https://treeherder.mozilla.org/#/jobs?repo=mozilla-release&revision=494b2c115bc1 . The tests don't run on every push, because they are optimized away when only android-relevant files are touched.
[2] https://docs.google.com/document/d/1mddoxa2i9ZgPLL_lGOZD0yNyS_ck3v1mzvp4GTEhC-0/edit#heading=h.fiq8y72wp8qg
Note that the GeckoView team no longer plans to create a relbranch for GeckoView 63, so there is no need to land these changes in mozilla-release for 63! Publishing GeckoView builds from mozilla-release's default branch to Maven is OK for 63 (since there will be no GeckoView relbranch for 63), but we will still want the above changes for GeckoView 64+.

Tom, Johan and I met today to clarify the GeckoView/Maven requirements. We came up with a proposal similar to yours.

Johan was going to look into a "GV relbranch mode" for Beetmover with this logic:

  if on mozilla-release repo:
    if on GV relbranch:
      Turn off desktop builds.
      Do publish GeckoView builds to Maven.
    else: # not on GV relbranch, so on m-r `default` or other branch
      Don’t publish GeckoView builds to Maven.

Our meeting notes:

https://docs.google.com/document/d/1Bt87pUEbHDbGyyY23IPaCNVEQI5-Slhf4I88eUrLmbU/edit
Attachment #9019418 - Attachment is obsolete: true
Attachment #9019418 - Flags: review?(mozilla)
Attachment #9020388 - Flags: review?(mozilla)
Comment on attachment 9020388 [details] [diff] [review]
[build/braindump] taskgraph-diff: Add "hg_branch" data

Review of attachment 9020388 [details] [diff] [review]:
-----------------------------------------------------------------

This looks fine, although I don't think we need review for changes like this to braindump.
Attachment #9020388 - Flags: review?(mozilla) → review+
Pushed by jlorenzo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/deb0260b33c1
geckoview: only spin beetmover tasks on GECKOVIEW_XX_RELBRANCH r=dustin,tomprince
Comment on attachment 9020388 [details] [diff] [review]
[build/braindump] taskgraph-diff: Add "hg_branch" data

(In reply to Tom Prince [:tomprince] from comment #12)
> This looks fine, although I don't think we need review for changes like this
> to braindump.

Ack. I'd just prefer to have another pair of eyes looking, in case I do something outstanding. 

Landed at https://hg.mozilla.org/build/braindump/rev/56f54e63a30d5210d499fb914614b1ec05c1c310
Attachment #9020388 - Flags: checked-in+
Thank you for the backout! I'm sorry I broke these test. I fixed them in latest revision. Relanded at https://hg.mozilla.org/integration/autoland/rev/90ccf3e2726d525f10872e97f94c5bbc3395d065
Flags: needinfo?(jlorenzo)
https://hg.mozilla.org/mozilla-central/rev/90ccf3e2726d
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Posted patch 1500897-follow-up.patch (obsolete) — Splinter Review
As per IRC/PM. Can you please get this landed. I can't upload to phab and I don't have the inbound repo.
Attachment #9020959 - Flags: review?(dustin)
Comment on attachment 9020959 [details] [diff] [review]
1500897-follow-up.patch

OK, we'll do this in
https://phabricator.services.mozilla.com/D10135
Attachment #9020959 - Attachment is obsolete: true
Attachment #9020959 - Flags: review?(dustin)
Attachment #9020962 - Attachment description: Get hg_branch from GECKO repo → Bug 1500897 - Follow-up: Get hg_branch from GECKO repo. r=dustin,jorgk
Attachment #9020962 - Attachment description: Bug 1500897 - Follow-up: Get hg_branch from GECKO repo. r=dustin,jorgk → Bug 1500897 - Follow-up: Get hg_branch from GECKO repo. r=dustin,r=
Attachment #9020962 - Attachment description: Bug 1500897 - Follow-up: Get hg_branch from GECKO repo. r=dustin,r= → Bug 1500897 - Follow-up: Get hg_branch from GECKO repo.
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/integration/autoland/rev/c2e7a489d786
Follow-up: Get hg_branch from GECKO repo. r=jorgk
Phabricator insisted on r=jorgk although the patch was reviewed (unofficially) by Dustin who uploaded it. Sigh.
https://hg.mozilla.org/mozilla-central/rev/c2e7a489d786
Status: REOPENED → RESOLVED
Closed: 10 months ago10 months ago
Resolution: --- → FIXED
I'm sorry for the breakage. Per the patch, I guess it broke comm. However, I couldn't find the breakage on comm-central. Would you have a link to the failure it originally caused? I'd like to understand what went wrong.
Flags: needinfo?(jorgk)
Okay, thanks. For the record, I hadn't looked into these jobs because I didn't see the changeset (not the bug) being list in the pushlog.
Umm, is there some misunderstanding here? This was an M-C fix, so that doesn't show up in the C-C pushlog.

If you take the last bad and first good M-C changesets from which TB was built, you get:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=4c7772c170a1848c4e57fea0087c351fd2&tochange=c3f01d72374b19dab985700e9b839bb7fa
And c2e7a489d786 is in there.
You're right, I'm confused about how the M-C patch got included in the C-C taskgraph. Would you have a link that explais how  taskgraph is run in Comms?

In the meantime, I landed both patches on beta:
* https://hg.mozilla.org/releases/mozilla-beta/rev/a73b5f672656ab521b8cf7394eb9ee385d164d0c
* https://hg.mozilla.org/releases/mozilla-beta/rev/a402a8a002ff288d5d4ee5fc7474e8d4694a58c6
(In reply to Johan Lorenzo [:jlorenzo] from comment #29)
> Would you have a link that explais how  taskgraph is run in Comms?
Rob, Tom, can you please answer this?
Flags: needinfo?(rob)
Flags: needinfo?(mozilla)
I couldn't locate a document that explains how this works. But here's the basics. 

First, note that Mozilla Taskcluster configuration and code all lives in the "taskcluster" directory at the root of the source tree. Within "taskcluster" there's a "ci" directory where the task descriptios (kind.yml) files are, and a few other directories, the most interesting (at least to me) of which is "taskgraph". "taskgraph" contains the code that takes those kind.yml files and turns them into a series of commands to run. (Way over-simplifying here.)

Comm code has to be placed at the root of a mozilla source checkout. Within "comm", there is a "taskcluster" directory that holds a "ci" directory.  This is where the kind.yml files describing the tasks to run to build Thunderbird are. Comm doesn't have its own "taskgraph" directory, it uses the code that's in Mozilla's tree.

Before anything can be built, the source code needs to be checked out. This is done with a script called run-task. The full run-task command is found  in ".taskcluster.yml" at the root of a Mozilla source checkout; here's the important part:

/builds/worker/bin/run-task --vcs-checkout=/builds/worker/checkouts/gecko

In a nutshell, run-task clones a Mozilla Mercurial repository at /builds/worker/checkouts/gecko. Which repository and what revision are controlled by environment variables that get set by Taskcluster when it runs run-task.

In contrast, the run-task commandline for a Comm build:

/builds/worker/bin/run-task --vcs-checkout=/builds/worker/checkouts/gecko --comm-checkout=/builds/worker/checkouts/gecko/comm

Again, the Mozilla tree gets cloned into /builds/worker/checkouts/gecko. The additional --comm-checkout argument instructs run-task to clone a Comm repository into a "comm" directory found within the Mozilla clone.

Now that the source is checked out, Taskcluster calls "mach taskgraph decision" internally to create Decision tasks. A number of environment a little to 
 
./mach --log-no-times taskgraph decision --head-repository="$GECKO_HEAD_REPOSITORY" --head-rev="$GECKO_HEAD_REV"

--head-repository will point to the URL for one of Mozilla's Mercurial repositories, like "mozilla-central", and --head-rev is the revision of the code to checkout and build from. This is found in the ".taskcluster.yml" file at the root of a mozilla- source checkout.

In the "comm" directory, there is another ".taskcluster.yml" file that is modified slightly to create a Decision that will lead to building Thunderbird. That ".taskcluster.yml" file also runs mach, but with some additional arguments:

./mach --log-no-times taskgraph decision --head-repository="$GECKO_HEAD_REPOSITORY" --head-rev="$GECKO_HEAD_REV" \
       --comm-head-repository="$COMM_HEAD_REPOSITORY" --comm-head-rev="$COMM_HEAD_REV" --root=comm/taskcluster/ci

The additional --comm-head-repository and --comm-head-rev arguments are pretty self-explanatory, they refer to a Comm repository and revision.

For the --root parameter to make sense, it's important to know that "run-task" and in turn "mach" get executed with the current directory set to /builds/worker/checkouts/gecko. The --root parameter tells mach to use the Taskcluster configuration (kind.yml) files. As I said above, the Taskgraph code in Mozilla's tree gets used regardless of whether its a Mozilla or Comm build.

I hope that helps.
Flags: needinfo?(rob)
Flags: needinfo?(mozilla)
You need to log in before you can comment on or make changes to this bug.