Closed Bug 1207653 Opened 9 years ago Closed 9 years ago

Introduce a task to build fxos simulator based on mulet

Categories

(Firefox Build System :: Task Configuration, task)

Product:
Firefox Build System
Component:
Task Configuration
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
mozilla44

People

(Reporter: ochameau, Assigned: ochameau)

References

Details

(Whiteboard: [b2g-build-support])

Attachments

(2 files, 3 obsolete files)

Pass build_url/img_url to post-build tasks
2.43 KB, patch
garndt
: review+
Details | Diff | Splinter Review
patch v4
7.90 KB, patch
ochameau
: review+
Details | Diff | Splinter Review 
In order to shutdown b2g builds/tests, one of the last thing left to be done 
is to build fxos simulator addon nightly builds.
Today, buildbot nor taskcluster does that.
I started crafting a build script and a new yml file to do that.
I would need some feedback on it.
I basically need to run every now and then (at least once per day)
and requires a mulet build to operate.
So in my patch, I introduced a test task against mulet target.
Note that there is no tests to run on fxos simulator addon, we just need to build it.
Attached patch patch v1 (obsolete) — — Splinter Review 
Here is some questions I had while working on this patch:
- Test versus Build:
Ideally, it would be better to execute that as a Build,
given that it isn't really a test. But it doesn't sound that important
as we do not need to run any test suite against the simulator addon.

- Location of build-simulator.sh:
I'm wondering if it wouldn't be better to move build-simulator.sh
from taskcluster folder to mozilla-central/b2g/simulator/ folder.
It is full of stuff really specific to the simulator
and I would like to make it reviewed and maintained by simulator experts
who are reviewers of b2g/simulator folder.
Again, not a big deal to me.

- YML inheritance:
I modified a lot of yml files to specify mulet_simulator.yml
and "simulator" test name (Just did what we do for other tests).
I haven't tried to verify, but may be everything is inherited and I just need to modify
taskcluster/tasks/job_flags.yml ??

- Fine control of when it runs:
We do not need to build it on every branch, nor every commit.
If we can save ressources, that would be great if we can run it just on mozilla-central.
And also, may be not for each commit, but that sounds more complex?
Comment on attachment 8664939 [details] [diff] [review]
patch v1

Greg, I'm flagging you for review based on mercurial history...
Feel free to redirect to whoever makes sense.
Attachment #8664939 - Flags: review?(garndt)
Comment on attachment 8664939 [details] [diff] [review]
patch v1

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

Some comments in the review.  I think the major piece is figuring out the caching for gaia and gecko as they are not typically done in test tasks.  Perhaps the post-build step is the right place for this? Not too sure.  This will get clearer in the next iteration of the in-tree configuration where we could have an arbitrary set of tasks.

Let's talk about the caching and some of other comments and you can flag me again.  Would love to take another look! Thanks for working on this.

::: testing/taskcluster/scripts/builder/build-simulator.sh
@@ +1,4 @@
> +#!/bin/bash
> +set -ex
> +
> +if [ -z $1 ]; then

Could we just check to make sure 3 arguments are passed in and if not spit out a help text? Also, do we want to test that these are actually directories rather than just an argument passed in?

@@ +13,5 @@
> +fi
> +GAIA_DIR=$2
> +
> +if [ -z $3 ]; then
> +  echo "First arg should be URL to mulet tarball"

If we stick to checking each arg separately, this is the third argument.

::: testing/taskcluster/tasks/job_flags.yml
@@ +256,5 @@
>      allowed_build_tasks:
>        tasks/builds/b2g_desktop_opt.yml:
>          task: tasks/tests/b2g_reftests_sanity_oop.yml
>          chunks: 1
> +  simulator:

See my other comment about post-build.  Perhaps this could be changed to be a post-build task 

https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/tasks/branches/try/job_flags.yml?offset=0#199

::: testing/taskcluster/tasks/tests/mulet_simulator.yml
@@ +1,3 @@
> +---
> +$inherits:
> +  from: 'tasks/test.yml'

Hrm, there is something that was added for post-build tasks to run...would this be better suited as a post-build rather than a test? Really seems this should stand apart.

Here is a task someone added to be "post-build" that maybe could serve as some kind of template
https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/tasks/post-builds/upload_symbols.yml

Also this might be better suited to run on build machines as they already have cached copied of gecko.

@@ +10,5 @@
> +    command:
> +      - /bin/bash
> +      - -c
> +      - >
> +        tc-vcs checkout gecko {{base_repository}} {{head_repository}} {{head_rev}} {{head_ref}} &&

So cloning gecko and gaia here doesn't make use of any caches so each time the simulator is build it will be pulling down a cache and then doing a hg/git pull.  The test tasks by default don't usually pull these down so there was no reason to include caching.

nit: it's easier to look at this if absolute paths are used in most places.
Attachment #8664939 - Flags: review?(garndt) → review-
Wander, you did a lot with builds, do you want to just spot check this and see what you think?
Flags: needinfo?(wcosta)
(In reply to Alexandre Poirot [:ochameau] from comment #1)
> Created attachment 8664939 [details] [diff] [review]
> patch v1
> 
> Here is some questions I had while working on this patch:
> - Test versus Build:
> Ideally, it would be better to execute that as a Build,
> given that it isn't really a test. But it doesn't sound that important
> as we do not need to run any test suite against the simulator addon.
> 

I agree with Greg, maybe this is better handled as a post-build.

> - Location of build-simulator.sh:
> I'm wondering if it wouldn't be better to move build-simulator.sh
> from taskcluster folder to mozilla-central/b2g/simulator/ folder.
> It is full of stuff really specific to the simulator
> and I would like to make it reviewed and maintained by simulator experts
> who are reviewers of b2g/simulator folder.
> Again, not a big deal to me.
> 

I don't have any objection on that.

> - YML inheritance:
> I modified a lot of yml files to specify mulet_simulator.yml
> and "simulator" test name (Just did what we do for other tests).
> I haven't tried to verify, but may be everything is inherited and I just
> need to modify
> taskcluster/tasks/job_flags.yml ??
> 

Nope, this file was removed recently (Bug 1207677). All branch configuration is under testing/taskcluster/tasks/branches.

> - Fine control of when it runs:
> We do not need to build it on every branch, nor every commit.
> If we can save ressources, that would be great if we can run it just on
> mozilla-central.

Just add it to testing/taskcluster/tasks/branches/mozilla-central

> And also, may be not for each commit, but that sounds more complex?

Taskcluster only supports per-commit job triggering for now.
Flags: needinfo?(wcosta)
Comment on attachment 8664939 [details] [diff] [review]
patch v1

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

::: testing/taskcluster/tasks/branches/base_jobs.yml
@@ +224,5 @@
>          task: tasks/tests/b2g_reftests_sanity_oop.yml
> +  simulator:
> +    allowed_build_tasks:
> +      tasks/builds/mulet_linux.yml:
> +        task: tasks/tests/mulet_simulator.yml

If you want this only on mozilla-central, you need to move it to mozilla-central/job_flags.yml

::: testing/taskcluster/tasks/job_flags.yml
@@ +259,5 @@
>          chunks: 1
> +  simulator:
> +    allowed_build_tasks:
> +      tasks/builds/mulet_linux.yml:
> +        task: tasks/tests/mulet_simulator.yml

This job_flags.yml is not used afaik.
Whiteboard: [tc-build-support]
Whiteboard: [tc-build-support] → [b2g-build-support]
Attached patch patch v3 (obsolete) — — Splinter Review 
Fixed settings file path, added some prefs required to start the DebuggerServer on Mulet.

Greg, Here is a new version with all previous comments addressed.
It now uses a post-build step. This is not easy to find the link to download
the addon file, but I think that's ok. Hopefully we can have a simple link
that would refer to the last build once it lands and runs against m-c.

Here is the page for one post-build try: https://tools.taskcluster.net/task-graph-inspector/#MbkS06CTQpCMGj8PJvaImA/VRTlHXZhRbaK3eRpveD4lg/0
And the link from this page to the addon: https://queue.taskcluster.net/v1/task/VRTlHXZhRbaK3eRpveD4lg/runs/0/artifacts/public/build/fx-os-simulator-2.5.20150928132704-linux64.xpi
Any idea what would be such stable link for "latest build"?

I'm attaching a second patch in order to set "build_url" also in post-build tasks.

Ryan, Could you review the bash script to build the addon?
I would like us to be in charge of maintaining this script.
Attachment #8665581 - Attachment is obsolete: true
Attachment #8666745 - Flags: review?(jryans)
Attachment #8666745 - Flags: review?(garndt)
Comment on attachment 8666745 [details] [diff] [review]
patch v3

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

Will we be allowed to review future changes to `build-simulator.sh`?

::: testing/taskcluster/scripts/builder/build-simulator.sh
@@ +81,5 @@
> +
> +APP_BUILDID=$(sed -n "s/BuildID=\([0-9]\{8\}\)/\1/p" $WORKDIR/firefox/application.ini)
> +echo "BUILDID $APP_BUILDID -- VERSION $VERSION"
> +
> +XPI_NAME=fx-os-simulator-$VERSION.$APP_BUILDID-$PLATFORM.xpi

Existing XPIs use `fxos` (not `fx-os`), probably better to keep it the same, I see no reason to be different.

@@ +98,5 @@
> +
> +# Replace all template string in install.rdf
> +sed -e "s/@ADDON_VERSION@/$ADDON_VERSION/" \
> +    -e "s/@ADDON_ID@/$ADDON_ID/" \
> +    -e "s#@ADDON_UPDATE_URL@#$UPDATE_URL#" \

Should this line use slashes?
Attachment #8666745 - Flags: review?(jryans) → review+
I suppose we should also update the add-on `minVersion` to require 44 for a compatible WebIDE.
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #16)
> I suppose we should also update the add-on `minVersion` to require 44 for a
> compatible WebIDE.

Done in bug 1191868, attachment 8666917 [details] [diff] [review].

> Will we be allowed to review future changes to `build-simulator.sh`?

I hope it's not an issue, otherwise I would move this script to b2g/simulator/.
The task can easily call it from both folders.
I don't know if it makes any difference regarding taskcluster/sherifs/automation/... ?

> ::: testing/taskcluster/scripts/builder/build-simulator.sh
> @@ +81,5 @@
> > +XPI_NAME=fx-os-simulator-$VERSION.$APP_BUILDID-$PLATFORM.xpi
> 
> Existing XPIs use `fxos` (not `fx-os`), probably better to keep it the same,
> I see no reason to be different.

Modified.

> @@ +98,5 @@
> > +# Replace all template string in install.rdf
> > +sed -e "s/@ADDON_VERSION@/$ADDON_VERSION/" \
> > +    -e "s/@ADDON_ID@/$ADDON_ID/" \
> > +    -e "s#@ADDON_UPDATE_URL@#$UPDATE_URL#" \
> 
> Should this line use slashes?

No. Bash...
I have to use another separator as there is already slashes in $UPDATE_URL.
Comment on attachment 8666746 [details] [diff] [review]
Pass build_url/img_url to post-build tasks

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

LGTM
Attachment #8666746 - Flags: review?(garndt) → review+
Attachment #8666745 - Flags: review?(garndt) → review+
Greg, What about URLs I mentioned in comment 13?
Is there equivalent URLs that would always refer to the latest build?

Also, this patch only targets try builds (testing/taskcluster/tasks/branches/try/job_flags.yml).
I'll followup with one for mozilla-central. I need to modify testing/taskcluster/tasks/branches/mozilla-central/job_flags.yml, right?
I was wondering about the try yml... I imagine simulator is going to be automatically built whenever someone ask for mulet to be built, right? Can we make that really optional somehow and only execute the post-build when explicitely requested in try commit message??
Otherwise, I may just remove it from try and only set it on m-c...
Flags: needinfo?(garndt)
Attached patch patch v4 — — Splinter Review 
Landed patch, with jryans comment addressed.
Attachment #8666745 - Attachment is obsolete: true
Attachment #8667196 - Flags: review+
If you want it to be only on m-c, that's the file you would edit.  As it is now, there is not much flexibility with the post-build stuff.  They aren't associated to try flags so they will get run for each build that's triggered.

as far as the artifact URL...if you know of the artifact name and task id, it would be in the form of:

https://queue.taskcluster.net/task/<taskId>/artifacts/<name>

That will return do you the artifact for the latest successful fun for that task ID>
Flags: needinfo?(garndt)
Blocks: 1211453
Blocks: 1213011
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: