Fennec system addons are not being bundled

RESOLVED DUPLICATE of bug 1459004

Status

defect
P1
normal
RESOLVED DUPLICATE of bug 1459004
Last year
Last year

People

(Reporter: wisniewskit, Assigned: wisniewskit)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [geckoview:klar:p2])

Attachments

(1 attachment)

In bug 1453691 I enabled the relevent features flag that should cause built_in_addons.json to generate the list of system addons, yet they are still not appearing in Fennec nightly builds (not present in about:support).

As noted in bug 1453691 comment 38, this seems likely to be caused by some kind of build quirk; the built_in_addons.json is not appearing for me on a local build when I do a "mach build", despite the work done in bug 1457321. I have to specifically do a "mach build faster" for it to be generated.

kmag, do you have any ideas as to why this might be happening?
Flags: needinfo?(kmaglione+bmo)
Actually, it seems like the problem might have been simpler than I expected. With this patch, it's building for me properly with just "mach build", and all I had to do was add the word "tools" (to match the non-Android Makefile's version).
Comment on attachment 8983637 [details]
Bug 1466539 - Change the Android '.PHONY: features' target to 'tools features::' instead of just 'features::', so that built_in_addons.json is actually generated.

Looks fine to me, but Makefile changes need review from a build peer.
Flags: needinfo?(kmaglione+bmo)
Attachment #8983637 - Flags: review?(kmaglione+bmo) → review?(core-build-config-reviews)
Comment on attachment 8983637 [details]
Bug 1466539 - Change the Android '.PHONY: features' target to 'tools features::' instead of just 'features::', so that built_in_addons.json is actually generated.

https://reviewboard.mozilla.org/r/249478/#review255976

Yes, this should address this problem.  Sucks to add a tools:: tier back into `m/a/base/Makefile.in`, but all of this stuff is poorly supported.
Attachment #8983637 - Flags: review+
Attachment #8983637 - Flags: review?(core-build-config-reviews)
Thanks to both of you! Requesting check-in.
Keywords: checkin-needed
Assignee: nobody → wisniewskit
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/b08454f26dc5
Change the Android '.PHONY: features' target to 'tools features::' instead of just 'features::', so that built_in_addons.json is actually generated. r=nalexander
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b08454f26dc5
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Backed out changeset b08454f26dc5 (bug 1466539) for breaking the Android nightlies

Backout: https://hg.mozilla.org/mozilla-central/rev/199a085199815cc99daa658956a7c9436e1d436b

Failure push: 
https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=e83a0d04ce6a2ebe09b7bb6f801a44b8dbe2c56b
Status: RESOLVED → REOPENED
Flags: needinfo?(wisniewskit)
Resolution: FIXED → ---
kmag, any idea why we'd get this trace for treeherder builds, when it's building fine for me locally with mach build (that is, the directory actually exists for me as objdir-droid/dist/bin/dictionaries and has two dictionary symlinks in it?)

>Traceback (most recent call last):
>File "/usr/lib/python2.7/runpy.py", line 174, in _run_module_as_main
>  "__main__", fname, loader, pkg_name)
>File "/usr/lib/python2.7/runpy.py", line 72, in _run_code
>  exec code in run_globals
>File "/builds/worker/workspace/build/src/python/mozbuild/mozbuild/action/generate_builtin_addons.py", line 46, in <module>
>  sys.exit(main(sys.argv[1:]))
>File "/builds/worker/workspace/build/src/python/mozbuild/mozbuild/action/generate_builtin_addons.py", line 35, in main
>  "dictionaries": find_dictionaries("dictionaries"),
>  File "/builds/worker/workspace/build/src/python/mozbuild/mozbuild/action/generate_builtin_addons.py", line 28, in find_dictionaries
>    for filename in os.listdir(os.path.join(bindir, path)):
>OSError: [Errno 2] No such file or directory: '/builds/worker/workspace/build/src/obj-firefox/dist/bin/dictionaries'
Flags: needinfo?(wisniewskit) → needinfo?(kmaglione+bmo)
Nick, any idea about Comment #9?
Flags: needinfo?(nalexander)
My best guess is that the task winds up running too early on automation, for some reason.

It might be better to just wait for bug 1459004 at this point.
Flags: needinfo?(kmaglione+bmo)
Depends on: 1459004
Flags: needinfo?(nalexander)
Priority: -- → P1
Whiteboard: [geckoview:klar:p2]
When this landed (comment 6), we noticed these perf regressions on mobile:

== Change summary for alert #13674 (as of Wed, 06 Jun 2018 19:07:42 GMT) ==

Regressions:

  7%  remote-nytimes android-4-2-armv7-api16 opt      2,898.75 -> 3,105.92
  7%  remote-blank android-6-0-armv8-api16 opt        372.71 -> 397.80
  5%  remote-nytimes android-6-0-armv8-api16 opt      980.43 -> 1,028.16

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=13674
The backout that followed (comment 8) canceled the regressions above:

== Change summary for alert #13713 (as of Thu, 07 Jun 2018 18:07:33 GMT) ==

Improvements:

  7%  remote-blank android-6-0-armv8-api16 opt      395.61 -> 368.02
  5%  remote-nytimes android-4-2-armv7-api16 opt    3,098.06 -> 2,939.77

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=13713
snorp: the perf regression above suggests that actually building features is slowing down Fennec substantially.  Problem is, I _think_ that the only "feature" is a dictionary, which should already be there.

So this is either quantifying the cost of "processing a manifest at startup time" or there's something actually being loaded that wasn't before, which I don't think should have happened.

Is this something we should be investigating for GV startup time?  It would be relevant if GV was going to load "features", which I don't know it will.
Flags: needinfo?(snorp)
Since bug 1453691, we're bundling a system add-on to override the UA for Facebook.

Loading dictionaries is also non-trivial, but not substantial enough to cause that kind of regression.
(In reply to Nick Alexander :nalexander from comment #14)
> snorp: the perf regression above suggests that actually building features is
> slowing down Fennec substantially.  Problem is, I _think_ that the only
> "feature" is a dictionary, which should already be there.
> 
> So this is either quantifying the cost of "processing a manifest at startup
> time" or there's something actually being loaded that wasn't before, which I
> don't think should have happened.
> 
> Is this something we should be investigating for GV startup time?  It would
> be relevant if GV was going to load "features", which I don't know it will.

I don't really know what "features" does, so I couldn't say without more information. Is the dictionary here for spelling correction? Hyphenation? I think we would likely want those.

Looking at the subtests for the regression, it seems only the first run was affected. So maybe it's not so bad? We really just need to get someone to profile this.
Flags: needinfo?(snorp)
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #16)
> I don't really know what "features" does, so I couldn't say without more
> information.

System add-ons and dictionaries.

> Is the dictionary here for spelling correction?

Spelling correction.

> Looking at the subtests for the regression, it seems only the first run was
> affected. So maybe it's not so bad? We really just need to get someone to
> profile this.

That wouldn't be entirely surprising. A lot of the stuff that's
required for loading a system add-on will wind up in the startup
cache after the first run.
I think we can close this as a dupe of Bug 1459004.
Status: REOPENED → RESOLVED
Closed: Last yearLast year
Resolution: --- → DUPLICATE
Duplicate of bug: 1459004
You need to log in before you can comment on or make changes to this bug.