1144463 - Add dolphin-512 config

Status: RESOLVED FIXED

(Taskcluster :: General, defect)

Comment 1

[James Lal [:lightsofapollo]]

Attachment: MozReview Request: bz://1144463/lightsofapollo

/r/5577 - Bug 1144463 - Testing adding dolphin r=kli

Pull down this commit:

hg pull review -r 8a5beabcf69ff4fae6e8605f34e7b7e95f19eaeb

Comment 2

[Kai-Zhen Li [:seinlin][:kli]]

Attachment: Add dolphin-512 and update docker

James, I can successfully build on my local machine. Could you review this patch? 

I also push to try [1], but it is pending for a long time and the build is not started. Should I push 'quay.io/mozilla/builder:0.5.3' before I test on try?

[1] https://treeherder.allizom.org/#/jobs?repo=try&revision=68f3b8bf5710

Comment 3

[James Lal [:lightsofapollo]]

The problem here is simple at this point we need to setup a worker type on http://aws-provisioner.taskcluster.net/ called "dolphin-512" those types are not automatically created (you can copy the setup for emulator-kk for example since we don't need anything special for dolphin builds like we do for the flames). I will take a look my Monday if you still need help.

Comment 4

[Kai-Zhen Li [:seinlin][:kli]]

I created a work type called dolphin-512 and re-trigger the task, but it is still pending.
http://docs.taskcluster.net/tools/task-inspector/#9BRf4tkAQ8m1-4s4LrZq9w/0

In task definition, builder 0.5.3 is required. I build it on my local machine, but I didn't push to quay.io. Do I need to push it? Or Will it be built from the patch? All changes of builder is included in patch which is pushed to try.

Comment 5

[Kai-Zhen Li [:seinlin][:kli]]

Now the task is failed, I can see quay.io/mozilla/builder:0.5.3 is downloaded. A new problem is encountered.

Comment 6

[James Lal [:lightsofapollo]]

You will indeed need to push it to quay.io

Comment 7

[James Lal [:lightsofapollo]]

Comment on attachment 8581207 [details] [diff] [review]
Add dolphin-512 and update docker

r- only because I think ./build-dolphin.sh is missing (that is why your task failed) ?

Comment 8

[Kai-Zhen Li [:seinlin][:kli]]

Attachment: Add dolphin-512 and update docker

James, Now the builds are green [1]. Could you review the patch?
In new patch,  instead of checkout l10n repos, devices' language.json is passed to b2g_build.py as parameter.

https://treeherder.allizom.org/#/jobs?repo=try&revision=6cdf462c8c2f

Comment 9

[James Lal [:lightsofapollo]]

Comment on attachment 8585124 [details] [diff] [review]
Add dolphin-512 and update docker

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

::: testing/docker/builder/VERSION
@@ -1,1 @@
> -0.5.2

(reminder) prior to landing can you please verify that we are not clobbering a version that already exists?

::: testing/taskcluster/scripts/phone-builder/build-dolphin.sh
@@ +33,5 @@
> +
> +# L10N_LIST=`grep ':' $WORKSPACE/B2G/device/sprd/scx15/languages.json | cut -d '"' -f 2`
> +# for l10n in $L10N_LIST; do
> +# # Checkout l10n repos
> +# tc-vcs checkout $WORKSPACE/B2G/gaia-l10n/$l10n https://hg.mozilla.org/releases/gaia-l10n/v2_1/$l10n

looks like we can remove this right ?

::: testing/taskcluster/tasks/builds/b2g_dolphin_512_eng.yml
@@ +3,5 @@
> +task:
> +  scopes:
> +    - 'docker-worker:cache:build-dolphin-512-eng'
> +  metadata:
> +    name: B2G Dolphin 512 Eng

Can you prefix name with [TC] ( makes it clear to sheriffs / etc... what built this thing )

::: testing/taskcluster/tasks/builds/b2g_dolphin_512_opt.yml
@@ +3,5 @@
> +task:
> +  scopes:
> +    - 'docker-worker:cache:build-dolphin-512-opt'
> +  metadata:
> +    name: B2G Dolphin 512 Opt

Can you prefix name with [TC] ( makes it clear to sheriffs / etc... what built this thing )

::: testing/taskcluster/tasks/builds/b2g_dolphin_base.yml
@@ +10,5 @@
> +      REPO_TRACE: 1
> +      VARIANT: user
> +      DEBUG: 0
> +
> +    # Emulators can take a very long time to build!

nit: copy/paste you can probably lower this value too ?

Comment 10

[James Lal [:lightsofapollo]]

r+ with few small comments ( feel free to land on BI once those are addressed )

Comment 11

[Kai-Zhen Li [:seinlin][:kli]]

https://hg.mozilla.org/integration/b2g-inbound/rev/ee5a8ba64485

Comment 12

[Kai-Zhen Li [:seinlin][:kli]]

try result is green: https://treeherder.allizom.org/#/jobs?repo=try&revision=dda63cd17730

Comment 13

[Kai-Zhen Li [:seinlin][:kli]]

Attachment: Update builder version in phone-builder

James, Now version of quay.io/mozilla/builder is 0.5.4, so I think this file also need to be updated accordingly. Would you mind review this patch? Thanks!

Comment 14

[Wander Lairson Costa]

Comment on attachment 8585331 [details] [diff] [review]
Update builder version in phone-builder

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

::: testing/docker/phone-builder/Dockerfile
@@ -1,1 @@
> -FROM          quay.io/mozilla/builder:0.5.3

Usually, even when I only upgrade the base image version, I also upgrade the inherited image version. This makes easier to trace problems caused by side effects of the new image.

Comment 15

[Kai-Zhen Li [:seinlin][:kli]]

Do you mean I should also update phone-builder version from 0.0.11 to 0.0.12 in 'testing/docker/phone-builder/VERSION'?

Comment 16

[Wander Lairson Costa]

(In reply to Kai-Zhen Li [:kli][:seinlin] from comment #15)
> Do you mean I should also update phone-builder version from 0.0.11 to 0.0.12
> in 'testing/docker/phone-builder/VERSION'?

Yep :)

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ee5a8ba64485

Comment 18

[James Lal [:lightsofapollo]]

reverted https://hg.mozilla.org/integration/b2g-inbound/rev/6bc7b1b0ad81

Comment 19

[Kai-Zhen Li [:seinlin][:kli]]

Comment on attachment 8585331 [details] [diff] [review]
Update builder version in phone-builder

Now patch in b2g-inbound was backed out [1]. There will be a new full patch which also includes this version update.

https://hg.mozilla.org/integration/b2g-inbound/rev/6bc7b1b0ad81

Comment 20

[Kai-Zhen Li [:seinlin][:kli]]

Attachment: bug-1144463-add-dolphin_512-n-update-dolphin-2.patch

Hi Wander, other than the part James've reviewed, I also update phone-builder's Dockerfile and VERSION in new patch. Would you mind review this? Thanks!

Comment 21

[Wander Lairson Costa]

Comment on attachment 8586159 [details] [diff] [review]
bug-1144463-add-dolphin_512-n-update-dolphin-2.patch

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

::: testing/taskcluster/scripts/phone-builder/build-dolphin.sh
@@ +38,5 @@
> +debug_flag=""
> +if [ 0$B2G_DEBUG -ne 0 ]; then
> +  debug_flag='--debug'
> +fi
> +

Instead of all the above stuff, you should use pre-build.sh script.

@@ +62,5 @@
> +mv $WORKSPACE/B2G/upload/b2g-*.android-arm.tar.gz $HOME/artifacts/b2g-android-arm.tar.gz
> +mv $WORKSPACE/B2G/upload/${TARGET}.zip $HOME/artifacts/${TARGET}.zip
> +mv $WORKSPACE/B2G/upload/gaia.zip $HOME/artifacts/gaia.zip
> +
> +ccache -s

If I see correctly, the only difference to build-phone.sh is the language file, right? If that's the case I would more inclined to patch tc to allow customize the language file path than duplicating the script. But this is up to you, we already have a bunch of duplicate code in the scripts and sooner or later we will have to fix that. If you think it is better having a separate script, I am not against it.

::: testing/taskcluster/tasks/builds/b2g_dolphin_512_eng.yml
@@ +15,5 @@
> +        platform: b2g-device-image
> +
> +  payload:
> +    cache:
> +      build-dolphin-512-eng: /home/worker/object-folder

s/object-folder/workspace

@@ +19,5 @@
> +      build-dolphin-512-eng: /home/worker/object-folder
> +    env:
> +      TARGET: 'dolphin-512'
> +      VARIANT: eng
> +

I see no reference to dolphin.zip, is it ok?

::: testing/taskcluster/tasks/builds/b2g_dolphin_512_opt.yml
@@ +14,5 @@
> +        platform: b2g-device-image
> +
> +  payload:
> +    cache:
> +      build-dolphin-512-opt: /home/worker/object-folder

s/object-folder/workspace

::: testing/taskcluster/tasks/builds/b2g_dolphin_eng.yml
@@ -8,3 @@
>    payload:
>      cache:
> -      build-hamachi-eng: /home/worker/object-folder

s/object-folder/workspace

::: testing/taskcluster/tasks/builds/b2g_dolphin_opt.yml
@@ +14,5 @@
> +        platform: b2g-device-image
> +
> +  payload:
> +    cache:
> +      build-dolphin-opt: /home/worker/object-folder

s/object-folder/workspace

Comment 22

[Kai-Zhen Li [:seinlin][:kli]]

Attachment: bug-1144463-add-dolphin_512-n-update-dolphin-3.patch

Wcosta, I update the patch as you suggested. Dolphin build do not need dolphin.zip, as all blob can be built from source tree. I also push the new patch to try [1] to make sure it doesn't break other build.

[1] https://treeherder.allizom.org/#/jobs?repo=try&revision=6313e82a9ec8

Comment 23

[Wander Lairson Costa]

Comment on attachment 8587497 [details] [diff] [review]
bug-1144463-add-dolphin_512-n-update-dolphin-3.patch

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

lgtm

Comment 24

[Kai-Zhen Li [:seinlin][:kli]]

Wcosta, Thanks! Before push to inbound, could you tell me how should I build and push phone-builder 0.0.12 to quay?

Comment 25

[Wander Lairson Costa]

To build phone-builder, you need to pass the S3 credentials to it. Unfortunately making this easy to get is still on our task list. Basically, you need to type this:

./build.sh phone-builder -i <aws access token> -k <aws secret key>

If you need an image built for tests, I can build it for you.

Comment 26

[Kai-Zhen Li [:seinlin][:kli]]

Unfortunately, I didn't have aws token and key. Could you build one and push to quay? I'd like to make sure my patch doesn't break any build on TC. Thanks!

Comment 27

[James Lal [:lightsofapollo]]

I did want to point out that we didn't need to use the phone builder for dolphin since it does not use any magic bits from our partners (just builder would work)

Comment 28

[Wander Lairson Costa]

https://hg.mozilla.org/build/mozharness/file/5d6c7ea6334c/configs/balrog

Comment 29

[Wander Lairson Costa]

Ops, forgot my last comment... So, I pushed phone-buider 0.0.12 to quay.io. Could you please push b2g-build and builder images too?

Comment 30

[Wander Lairson Costa]

(In reply to James Lal [:lightsofapollo] from comment #27)
> I did want to point out that we didn't need to use the phone builder for
> dolphin since it does not use any magic bits from our partners (just builder
> would work)

Anyway, I think it is a good idea to use phone-builder to build dolphin, even if it is not necessary. It is intuitive for newcomers infer that phone images are built by the phone-builder docker.

Comment 31

[Kai-Zhen Li [:seinlin][:kli]]

Wcosta, Thanks! I push to try again, now all builds seem happy on TC [1]. I also try to use builder and it also does work properly. When I notice that there is a phone-builder, I think it could be intuitive to build a phone images by phone-builder.

[1] https://treeherder.allizom.org/#/jobs?repo=try&revision=c4ec5ada2818

Comment 32

[Kai-Zhen Li [:seinlin][:kli]]

https://hg.mozilla.org/integration/b2g-inbound/rev/2727e5aa21cd

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2727e5aa21cd

Comment 34

[Kai-Zhen Li [:seinlin][:kli]]

Attachment: bug-1144463-fix-2.patch

Wcosta, I fixed the execute permission of build-dolphin and update the path in dolphin config file. Would you mind have a look to this patch? Thanks!

Comment 35

[Wander Lairson Costa]

Comment on attachment 8596641 [details] [diff] [review]
bug-1144463-fix-2.patch

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

lgtm

Comment 36

[Kai-Zhen Li [:seinlin][:kli]]

Wcosta, Thanks! I'll land it when try result are green.

https://treeherder.allizom.org/#/jobs?repo=try&revision=bd81f0dcd654

Comment 37

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/31cdda5d73e0

Comment 38

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/31cdda5d73e0

Comment 39

[Pete Moore [:pmoore][:pete]]

Resetting Version and Target Milestone that accidentally got changed...