Closed Bug 1175934 Opened 9 years ago Closed 9 years ago

[B2G] Add support to build blobfree images

Categories

(Release Engineering :: Applications: MozharnessCore, defect, P2)

ARM
Gonk (Firefox OS)
defect

Tracking

(firefox42 fixed)

RESOLVED FIXED
Tracking Status
firefox42 --- fixed

People

(Reporter: wcosta, Assigned: wcosta)

References

Details

Attachments

(7 files, 7 obsolete files)

40 bytes, text/x-review-board-request
garndt
: review+
Details
40 bytes, text/x-review-board-request
jlund
: review+
Details
501 bytes, patch
garndt
: review+
Details | Diff | Splinter Review
5.83 MB, text/x-log
Details
3.08 KB, patch
jlund
: review+
Details | Diff | Splinter Review
7.85 KB, patch
jlund
: review+
garndt
: review+
Details | Diff | Splinter Review
1.86 KB, patch
garndt
: review+
Details | Diff | Splinter Review
B2G can create blob free images, through the command "./build.sh blobfree"
OS: Unspecified → Gonk (Firefox OS)
Priority: -- → P2
Hardware: Unspecified → ARM
Flags: needinfo?(nhirata.bugzilla)
Assignee: nobody → wcosta
Status: NEW → ASSIGNED
Depends on: 1178388
Bug 1175934: Add blob free build for flame and spark devices. r=garndt
Attachment #8627521 - Flags: review?(garndt)
I cannot run task-graph to completely test this patch due to Bug 1177183, however.
Comment on attachment 8627521 [details]
MozReview Request: Bug 1175934 part 1: Copy blobfree zip to public upload dir. r?garndt

https://reviewboard.mozilla.org/r/12259/#review10755

::: testing/taskcluster/tasks/builds/b2g_aries_spark_debug_blobfree.yml:11
(Diff revision 1)
> +    name: '[TC] B2G Aries Debug (blobfree)'

I wasn't sure anymore, but do we need [TC] in these names still? lightsofapollo might know and I'm curious to understand what we should be doing here.

::: testing/taskcluster/tasks/builds/b2g_aries_spark_debug_blobfree.yml:33
(Diff revision 1)
> +      symbol: B

We should clarify with the teams what the group symbol and symbol should be for all of these.  As it stands now, these share the same symbols as blob builds.  So on a push to m-c, this would appear on the device image debug line in treeherder:

Aries( B B ) where one B is for the blob free build and the other with blobs.  Could get confusing when trying to find specific jobs.  Gets even worse if any of them are retriggered.

::: testing/taskcluster/tasks/builds/b2g_aries_spark_eng_blobfree.yml:39
(Diff revision 1)
> +      img: 'public/build/aries.zip'

Would the validate script break when processing this?

https://dxr.mozilla.org/mozilla-central/source/testing/docker/phone-builder/bin/validate_task.py#42

::: testing/taskcluster/tasks/builds/b2g_aries_spark_debug_blobfree.yml:34
(Diff revision 1)
> +      groupSymbol: Aries

I'm not sure wwhat the cleaner, straight forward route is for this, but either the symbols could indicate if they are blob free or not, or the group symbol could be difference.  Maybe Aries() and Aries-Blob-Free() or something. I'm not sure, I would defer to others for their opinion on what they want to see on TH.

::: testing/taskcluster/tasks/builds/b2g_aries_spark_debug_blobfree.yml:30
(Diff revision 1)
> +      - production

Since these are newer builds and we're not sure how well it will go, do we want to enable these on production right from the start?
Attachment #8627521 - Flags: review?(garndt)
Bug 1175934: Add blob free build configs. r=garndt

The buildfree target builds phone images without blobs, allowing them to
be freely available.
Attachment #8628506 - Flags: review?(garndt)
Comment on attachment 8627521 [details]
MozReview Request: Bug 1175934 part 1: Copy blobfree zip to public upload dir. r?garndt

Bug 1175934 part 1: Add blob free build for flame and spark devices. r=garndt
Attachment #8627521 - Attachment description: MozReview Request: Bug 1175934: Add blob free build for flame and spark devices. r=garndt → MozReview Request: Bug 1175934 part 1: Add blob free build for flame and spark devices. r=garndt
Attachment #8627521 - Flags: review?(garndt)
Bug 1175934 part 2: Allow blob free images go on public. r=garndt

Die, blobs! Die!
Attachment #8628526 - Flags: review?(garndt)
I don't see the |build.sh blobfree| statement in the logs of those runs :(
(In reply to Alexandre LISSY :gerard-majax from comment #9)
> I don't see the |build.sh blobfree| statement in the logs of those runs :(

Indeed there was a mistake in the mozharness patch, I am testing the fix, thanks.
I wonder why I didn't get a full flash on aries while testing...
Comment on attachment 8627521 [details]
MozReview Request: Bug 1175934 part 1: Copy blobfree zip to public upload dir. r?garndt

https://reviewboard.mozilla.org/r/12259/#review10929

Ship It!
Attachment #8627521 - Flags: review?(garndt) → review+
Comment on attachment 8628526 [details]
MozReview Request: Bug 1175934 part 2: Support blobfree images. r?garndt

https://reviewboard.mozilla.org/r/12395/#review10931

Ship It!
Attachment #8628526 - Flags: review?(garndt) → review+
https://reviewboard.mozilla.org/r/12391/#review10935

I gave this a quick look over and it seems ok, but I'm no MH config expert.
Comment on attachment 8628506 [details]
MozReview Request: Bug 1175934 part 1: Allow build target configuration from config file. r=jlund

https://reviewboard.mozilla.org/r/12393/#review10937

Ship It!
Attachment #8628506 - Flags: review?(garndt) → review+
Attachment #8628506 - Attachment description: MozReview Request: Bug 1175934: Add blob free build configs. r=garndt → MozReview Request: Bug 1175934 part 1: Allow build target configuration from config file. r=jlund
Attachment #8628506 - Flags: review+ → review?(jlund)
Comment on attachment 8628506 [details]
MozReview Request: Bug 1175934 part 1: Allow build target configuration from config file. r=jlund

Bug 1175934 part 1: Allow build target configuration from config file. r=jlund

For now, we are going to provide builds with and without blobs. Instead
of going into the pain of creating new devices targets just to add
blobfree in config.json, we allow configuration of build targets in the
mozharness config file.
Bug 1175934 part 2: Add blob free build configs.

The buildfree target builds phone images without blobs, allowing them to
be freely available.
Attachment #8629078 - Flags: review?(garndt)
Comment on attachment 8629078 [details]
MozReview Request: Bug 1175934 part 2: Add blob free build configs.

https://reviewboard.mozilla.org/r/12533/#review11017

Ship It!
Attachment #8629078 - Flags: review?(garndt) → review+
Comment on attachment 8628506 [details]
MozReview Request: Bug 1175934 part 1: Allow build target configuration from config file. r=jlund

https://reviewboard.mozilla.org/r/12393/#review11087

Ship It!
Attachment #8628506 - Flags: review?(jlund) → review+
url:        https://hg.mozilla.org/integration/b2g-inbound/rev/5b4aed5949f1e9135b07f2efc82261ee9469c3a9
changeset:  5b4aed5949f1e9135b07f2efc82261ee9469c3a9
user:       Wander Lairson Costa <wcosta@mozilla.com>
date:       Mon Jul 13 07:55:22 2015 -0300
description:
Bug 1175934 part 1: Add blob free build for flame and spark devices. r=garndt

url:        https://hg.mozilla.org/integration/b2g-inbound/rev/83348c0ea47bcc4e0d91150c2cb45c295bcaca50
changeset:  83348c0ea47bcc4e0d91150c2cb45c295bcaca50
user:       Wander Lairson Costa <wcosta@mozilla.com>
date:       Mon Jul 13 07:55:22 2015 -0300
description:
Bug 1175934 part 2: Allow blob free images go on public. r=garndt

Die, blobs! Die!
Originally the part 2 contained the version update to 0.0.l7, but after rebase, it was removed because another commit already contained it. This late patch upgrade de docker image version.
Attachment #8632768 - Flags: review?(garndt)
Attachment #8632768 - Flags: review?(garndt) → review+
url:        https://hg.mozilla.org/integration/b2g-inbound/rev/b7d35cc9ea8c30cba1bb78df0c6a078715841ecc
changeset:  b7d35cc9ea8c30cba1bb78df0c6a078715841ecc
user:       Wander Lairson Costa <wcosta@mozilla.com>
date:       Mon Jul 13 10:21:02 2015 -0300
description:
Bug 1175934 part 3: Update phone-builder version. r=garndt
Flags: needinfo?(nhirata.bugzilla)
Sorry to ask dumb question, but I still don't see blobfree dist zip file on recent tasks, like https://tools.taskcluster.net/task-inspector/#PAUBPHW2QXqQjkyrmjX_aA/0
Flags: needinfo?(wcosta)
(In reply to Alexandre LISSY :gerard-majax from comment #27)
> Sorry to ask dumb question, but I still don't see blobfree dist zip file on
> recent tasks, like
> https://tools.taskcluster.net/task-inspector/#PAUBPHW2QXqQjkyrmjX_aA/0

It is running "./build.sh -j8 blobfree". Is there any additional step besides that to generate blob free?
Flags: needinfo?(wcosta) → needinfo?(lissyx+mozillians)
(In reply to Wander Lairson Costa [:wcosta] from comment #28)
> (In reply to Alexandre LISSY :gerard-majax from comment #27)
> > Sorry to ask dumb question, but I still don't see blobfree dist zip file on
> > recent tasks, like
> > https://tools.taskcluster.net/task-inspector/#PAUBPHW2QXqQjkyrmjX_aA/0
> 
> It is running "./build.sh -j8 blobfree". Is there any additional step
> besides that to generate blob free?

No but the file we want to make easily and publicly accessible is <device>.blobfree-dist.zip
Flags: needinfo?(lissyx+mozillians)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
url:        https://hg.mozilla.org/integration/b2g-inbound/rev/e810d4d7ad99942b31dbe68c38f78c98f09e4172
changeset:  e810d4d7ad99942b31dbe68c38f78c98f09e4172
user:       Wander Lairson Costa <wcosta@mozilla.com>
date:       Mon Jul 20 10:50:23 2015 -0300
description:
Bug 1175934: backout revision b7d35cc9ea8c. No need to create new builds. r=me

url:        https://hg.mozilla.org/integration/b2g-inbound/rev/47d33944a71962482a7d4c89cc419b0ac84f29a4
changeset:  47d33944a71962482a7d4c89cc419b0ac84f29a4
user:       Wander Lairson Costa <wcosta@mozilla.com>
date:       Mon Jul 20 11:01:24 2015 -0300
description:
Bug 1175934: backout revision 83348c0ea47b. No need to create new builds. r=me

url:        https://hg.mozilla.org/integration/b2g-inbound/rev/f7d9b77a7afb616b1a7cce12185256da99cba9be
changeset:  f7d9b77a7afb616b1a7cce12185256da99cba9be
user:       Wander Lairson Costa <wcosta@mozilla.com>
date:       Mon Jul 20 11:29:13 2015 -0300
description:
Bug 1175934: backout revision 5b4aed5949f1. No need to create new builds. r=me
Depends on: 1185643
Attached file blobfree.log
That is the expected output.
Attachment #8627521 - Attachment description: MozReview Request: Bug 1175934 part 1: Add blob free build for flame and spark devices. r=garndt → MozReview Request: Bug 1175934 part 1: Copy blobfree zip to public upload dir. r?garndt
Comment on attachment 8627521 [details]
MozReview Request: Bug 1175934 part 1: Copy blobfree zip to public upload dir. r?garndt

Bug 1175934 part 1: Copy blobfree zip to public upload dir. r?garndt

If we ./build.sh blobfree, move the zip file to public upload
dir.

Also, we don't need a blobfree specific config file anymore.
Comment on attachment 8628526 [details]
MozReview Request: Bug 1175934 part 2: Support blobfree images. r?garndt

Bug 1175934 part 2: Support blobfree images. r?garndt

When we ./build.sh blobfree, we have a new zip file called
<target>.blobfree-dist.zip which contains a blobfree image. We copy this
file to public artifact.

We also refactor the build scripts a little bit to deduplicate the post
build code.
Attachment #8628526 - Attachment description: MozReview Request: Bug 1175934 part 2: Allow blob free images go on public. r=garndt → MozReview Request: Bug 1175934 part 2: Support blobfree images. r?garndt
Attachment #8628526 - Attachment is obsolete: true
Attachment #8629078 - Attachment is obsolete: true
If we ./build.sh blobfree, move the zip file to public upload
dir.

Also, we don't need a blobfree specific config file anymore.
When we ./build.sh blobfree, we have a new zip file called
<target>.blobfree-dist.zip which contains a blobfree image. We copy this
file to public artifact.

We also refactor the build scripts a little bit to deduplicate the post
build code.
Attachment #8640226 - Flags: review?(garndt)
Attachment #8640227 - Flags: review?(garndt)
Attachment #8640226 - Flags: review?(garndt) → review?(jlund)
That looks good except I've noticed some discrepencies in bin/, xbin/ and lack of bootanimation. But those are files built so it looks good.
Attachment #8640226 - Attachment is obsolete: true
Attachment #8640226 - Flags: review?(jlund)
Attachment #8640227 - Attachment is obsolete: true
Attachment #8640227 - Flags: review?(garndt)
If we ./build.sh blobfree, move the zip file to public upload
dir.

Also, we don't need a blobfree specific config file anymore.
Attached patch part 2: Support blobfree images. (obsolete) — Splinter Review
When we ./build.sh blobfree, we have a new zip file called
<target>.blobfree-dist.zip which contains a blobfree image. We copy this
file to public artifact.

We also refactor the build scripts a little bit to deduplicate the post
build code.
All phone builds must have MOZILLA_OFFICIAL and
ENABLE_DEFAULT_BOOTANIMATION build flags.
Checking https://tools.taskcluster.net/task-inspector/#LEa8U2YcTMWOFrxRGbVCCw/0 ...
Flags: needinfo?(lissyx+mozillians)
(In reply to Alexandre LISSY :gerard-majax from comment #45)
> Checking
> https://tools.taskcluster.net/task-inspector/#LEa8U2YcTMWOFrxRGbVCCw/0 ...

Looks much better except it lacks updater binary :). But from a blobfree point of view, it's okay !
Attachment #8640473 - Attachment is obsolete: true
Attachment #8640474 - Attachment is obsolete: true
Attachment #8640475 - Attachment is obsolete: true
Alexandre, could you give a check on those?
Flags: needinfo?(lissyx+mozillians)
If we ./build.sh blobfree, move the zip file to public upload
dir.

Also, we don't need a blobfree specific config file anymore.
When we ./build.sh blobfree, we have a new zip file called
<target>.blobfree-dist.zip which contains a blobfree image. We copy this
file to public artifact.

We also refactor the build scripts a little bit to deduplicate the post
build code.
All phone builds must have B2G_UPDATER, MOZILLA_OFFICIAL and
ENABLE_DEFAULT_BOOTANIMATION build flags.
Attachment #8641646 - Flags: review?(jlund)
Attachment #8641647 - Flags: review?(jlund)
Attachment #8641647 - Flags: review?(garndt)
Attachment #8641648 - Flags: review?(garndt)
Comment on attachment 8641647 [details] [diff] [review]
part 2: Support blobfree images.

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

The taskcluster pieces of this look good to me.
Attachment #8641647 - Flags: review?(garndt) → review+
Comment on attachment 8641648 [details] [diff] [review]
part 3: Add more build flags to phone builds.

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

I'm not sure what those options do, but as long as they don't change the behaviors in the engineering build (in regards to flashing devices and having ADB enabled), then this looks good to me.
Attachment #8641648 - Flags: review?(garndt) → review+
Comment on attachment 8641646 [details] [diff] [review]
part 1: Copy blobfree zip to public upload dir.

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

sweet. ship it

::: testing/mozharness/scripts/b2g_build.py
@@ +710,5 @@
>                      files.append(f)
>                  if base_pattern in public_upload_patterns:
>                      public_files.append(f)
>  
> +        device_name = self.config['target'].split('-')[0]

this seems fragile. should we have another config item for this hard coded or do you think it's safe?
Attachment #8641646 - Flags: review?(jlund) → review+
Comment on attachment 8641647 [details] [diff] [review]
part 2: Support blobfree images.

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

mozharness pieces look sane

::: testing/mozharness/scripts/b2g_build.py
@@ +521,5 @@
>          self.run_command(["diff", "-u", sourcesfile_orig, sourcesfile], success_codes=[1])
>  
>      def generate_build_command(self, target=None):
>          cmd = ['./build.sh']
>          if target is not None:

sanity check: so if target == "", it will make it past this point. In which case, we will always append '-j{0}'. that's what we want right?
Attachment #8641647 - Flags: review?(jlund) → review+
(In reply to Jordan Lund (:jlund) from comment #54)
> Comment on attachment 8641646 [details] [diff] [review]
> part 1: Copy blobfree zip to public upload dir.
> 
> Review of attachment 8641646 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> sweet. ship it
> 
> ::: testing/mozharness/scripts/b2g_build.py
> @@ +710,5 @@
> >                      files.append(f)
> >                  if base_pattern in public_upload_patterns:
> >                      public_files.append(f)
> >  
> > +        device_name = self.config['target'].split('-')[0]
> 
> this seems fragile. should we have another config item for this hard coded
> or do you think it's safe?

Even if it is an empty string, the code will work (of course the build will fail, but not because this code).
(In reply to Jordan Lund (:jlund) from comment #55)
> Comment on attachment 8641647 [details] [diff] [review]
> part 2: Support blobfree images.
> 
> Review of attachment 8641647 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> mozharness pieces look sane
> 
> ::: testing/mozharness/scripts/b2g_build.py
> @@ +521,5 @@
> >          self.run_command(["diff", "-u", sourcesfile_orig, sourcesfile], success_codes=[1])
> >  
> >      def generate_build_command(self, target=None):
> >          cmd = ['./build.sh']
> >          if target is not None:
> 
> sanity check: so if target == "", it will make it past this point. In which
> case, we will always append '-j{0}'. that's what we want right?

Yes, it is.
url:        https://hg.mozilla.org/integration/b2g-inbound/rev/5f4839ec1475be192508f3df6af815acd21ddae3
changeset:  5f4839ec1475be192508f3df6af815acd21ddae3
user:       Wander Lairson Costa <wcosta@mozilla.com>
date:       Fri Jul 31 17:24:46 2015 -0300
description:
Bug 1175934 part 1: Copy blobfree zip to public upload dir.

If we ./build.sh blobfree, move the zip file to public upload
dir.

Also, we don't need a blobfree specific config file anymore.

url:        https://hg.mozilla.org/integration/b2g-inbound/rev/e6f3a48c462bf2604260800aeb03d4e4b335c303
changeset:  e6f3a48c462bf2604260800aeb03d4e4b335c303
user:       Wander Lairson Costa <wcosta@mozilla.com>
date:       Fri Jul 31 17:24:46 2015 -0300
description:
Bug 1175934 part 2: Support blobfree images.

When we ./build.sh blobfree, we have a new zip file called
<target>.blobfree-dist.zip which contains a blobfree image. We copy this
file to public artifact.

We also refactor the build scripts a little bit to deduplicate the post
build code.

url:        https://hg.mozilla.org/integration/b2g-inbound/rev/6fc853368a5aae767ba52daa232bc52ba91cc7e8
changeset:  6fc853368a5aae767ba52daa232bc52ba91cc7e8
user:       Wander Lairson Costa <wcosta@mozilla.com>
date:       Fri Jul 31 17:24:46 2015 -0300
description:
Bug 1175934 part 3: Add more build flags to phone builds.

All phone builds must have B2G_UPDATER, MOZILLA_OFFICIAL and
ENABLE_DEFAULT_BOOTANIMATION build flags.
Flags: needinfo?(lissyx+mozillians)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: