970658 - Additional extensions should be cached

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Build, defect)

Description

[Kevin Grandon :kgrandon]

We currently download a screen reader as an additional extension. This is useful for accessibility testing, but dramatically slows down DEBUG profiles when you are building.

I think it would be useful to cache the screen reader somewhere on disk, and simply copy it over when needed. Doing a make clean could blow the cached version away.

Comment 1

[Alexandre Poirot [:ochameau]]

The addon is only ~20kB, so it shouldn't be so long to download, but may be AMO ftps are slow to respond :/

But at the end, I'm wondering if anyone uses this addon. Do you know anyone using it?

Comment 2

[Kevin Grandon :kgrandon]

Our accessibility team heavily uses this addon. Perhaps we could just check it into the tree as well.

Comment 3

[Alexandre Poirot [:ochameau]]

Oh great! Eitan, as you originaly introduced the addon and the additional-extensions script, may be you have some thoughts?

Comment 4

[Eitan Isaacson [:eeejay]]

(In reply to Kevin Grandon :kgrandon from comment #2)
> Our accessibility team heavily uses this addon. Perhaps we could just check
> it into the tree as well.

Our accessibility team would like it if others used it as well :) perhaps a session somewhere/sometime would be good.

(In reply to Alexandre Poirot (:ochameau) from comment #3)
> Oh great! Eitan, as you originaly introduced the addon and the
> additional-extensions script, may be you have some thoughts?

It should not be re-downloading it after each make, only after a make clean, as Kevin says. It should already be caching it, if it is not, then we have a bug.

Comment 5

[Eitan Isaacson [:eeejay]]

I could try to fix this today..

Comment 6

[Eitan Isaacson [:eeejay]]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20245

Comment 7

[Kevin Grandon :kgrandon]

Comment on attachment 8437091 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20245

I didn't test it, but seems like it should be good. Yuren or Alex should probably review this one.

Comment 8

[Alexandre Poirot [:ochameau]]

I'm not able to test it because of this failure:

[additional-extensions] load downloaded extensions
Invalid JSON file : /home/alex/gaia/build_stage/additional-extensions/downloaded.json
[additional-extensions] load additional extensions
[additional-extensions] load custom extensions
Invalid JSON file : /home/alex/gaia/build/config/custom-extensions.json
[additional-extensions] Installing Screen Reader Simulator...
[additional-extensions] download from https://addons.mozilla.org/firefox/downloads/latest/440614/addon-440614-latest.xpi
[additional-extensions] error downloading http://addons.mozilla.org/firefox/downloads/latest/440614/addon-440614-latest.xpi

But I think it was already failing before this patch.

Comment 9

[Alexandre Poirot [:ochameau]]

wget this url works but I see, many, slow, redirections...

Comment 10

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8437091 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20245

cancel review flag to me since Alex is reviewing it :-)

Comment 11

[Alexandre Poirot [:ochameau]]

Comment on attachment 8437091 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20245

The patch looks good but I'm not able to test it. `DEBUG=1 make` never ends, with the following message:
[additional-extensions] load downloaded extensions
Invalid JSON file : /home/alex/gaia/build_stage/additional-extensions/downloaded.json
[additional-extensions] load additional extensions
[additional-extensions] load custom extensions
Invalid JSON file : /home/alex/gaia/build/config/custom-extensions.json
[additional-extensions] Installing Screen Reader Simulator...
[additional-extensions] download from https://addons.mozilla.org/firefox/downloads/latest/440614/addon-440614-latest.xpi
[additional-extensions] error downloading http://addons.mozilla.org/firefox/downloads/latest/440614/addon-440614-latest.xpi


Note that I'm able to download http://addons.mozilla.org/firefox/downloads/latest/440614/addon-440614-latest.xpi via wget.

Comment 12

[Eitan Isaacson [:eeejay]]

(In reply to Alexandre Poirot (:ochameau) from comment #11)
> Comment on attachment 8437091 [details] [review]
> Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20245
> 
> The patch looks good but I'm not able to test it. `DEBUG=1 make` never ends,
> with the following message:
> [additional-extensions] load downloaded extensions
> Invalid JSON file :
> /home/alex/gaia/build_stage/additional-extensions/downloaded.json
> [additional-extensions] load additional extensions
> [additional-extensions] load custom extensions
> Invalid JSON file : /home/alex/gaia/build/config/custom-extensions.json
> [additional-extensions] Installing Screen Reader Simulator...
> [additional-extensions] download from
> https://addons.mozilla.org/firefox/downloads/latest/440614/addon-440614-
> latest.xpi
> [additional-extensions] error downloading
> http://addons.mozilla.org/firefox/downloads/latest/440614/addon-440614-
> latest.xpi
> 
> 
> Note that I'm able to download
> http://addons.mozilla.org/firefox/downloads/latest/440614/addon-440614-
> latest.xpi via wget.

Is this because of the redirects?

Also, for the life of me, I can't figure out why this is failing in CI. I see that the make command exits with code 2, but no other logging.

Comment 13

[Alexandre Poirot [:ochameau]]

I must say the test runner output is awful, especially on TBPL where instead of having nice colors, you get cryptic output. The failing assertion is hidden in this mess:
14:59:43     INFO -  [0m  1) Build Integration tests make with DEBUG=1:
14:59:43     INFO -  [0m[31m     Uncaught expected false to be truthy[0m[90m

But it isn't really helpful. We have no idea which assertion is failing :'(
I was able to reproduce this failure locally and tracked it down to this line:
  https://github.com/mozilla-b2g/gaia/blob/master/build/test/integration/build.test.js#L670

Also, I see the following exception:
WARNING: A completion condition encountered an error while we were spinning the event loop. Condition: OS.File: flush pending requests, warn about unclosed files, shut down service. Phase: web-workers-shutdown State: (none)
WARNING: Error: OS.File has been shut down.

I also chased this down and it looks like some bug inside of xpcshell that has been fixed. We just need to bump xulrunner version we are using. We may fix that in bug 943892 (moving to a recent b2g desktop should fix that as well)

Otherwise I tested the patch again and it looks good now!

Comment 14

[Eitan Isaacson [:eeejay]]

Comment on attachment 8437091 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20245

Thanks for finding the issue. Pushed an update and rebased. It passes!

Comment 15

[Alexandre Poirot [:ochameau]]

Comment on attachment 8437091 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20245

Looks good, thanks!

Comment 16

[Eitan Isaacson [:eeejay]]

https://github.com/mozilla-b2g/gaia/commit/d2d822e455959207a47a4a5605296c6d3e28d3ce