1386906 - Add Pocket keys to build system

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, enhancement, P1)

Comment 1

[Chenxia Liu [:liuche]]

This is the Pocket equivalent of bug 1365089 (which handled Leanplum SDK), for adding Pocket Stories to Android Activity Stream.

Comment 2

[Chenxia Liu [:liuche]]

Hi Nevin, saw that you added the Leanplum keys, so I'm flagging you for review here.

I followed the Leanplum bug 1365089 as a guideline - to test this,

Comment 3

[Chenxia Liu [:liuche]]

Attachment: Bug 1386906 - Add Pocket keys to build system.

Review commit: https://reviewboard.mozilla.org/r/164976/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/164976/

Comment 4

[Chenxia Liu [:liuche]]

Comment on attachment 8893910 [details]
Bug 1386906 - Add Pocket keys to build system.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/164976/diff/1-2/

Comment 5

[Chenxia Liu [:liuche]]

I've also filed bug 1387553 for the releng component of adding Pocket keys, and I'll make sure that lands before this one.

Comment 6

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8893910 [details]
Bug 1386906 - Add Pocket keys to build system.

https://reviewboard.mozilla.org/r/164976/#review170432

::: mobile/android/base/generate_build_config.py:54
(Diff revision 2)
>                  'MOZ_ANDROID_MOZILLA_ONLINE',
>                  'MOZ_ANDROID_PWA',
>                  'MOZ_LEANPLUM_SDK_KEY',
>                  'MOZ_LEANPLUM_SDK_CLIENTID',
> +                'MOZ_ANDROID_POCKET',
> +                'MOZ_POCKET_API_KEY',

I don't think we want 'MOZ_POCKET_API_KEY' in this list (it's set by the bit added below).

::: mobile/android/moz.configure:182
(Diff revision 2)
> +    if android_pocket:
> +        if not pocket_api_keyfile:

These could be combined and put on the same line as well.

Comment 7

[Chenxia Liu [:liuche]]

Comment on attachment 8893910 [details]
Bug 1386906 - Add Pocket keys to build system.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/164976/diff/2-3/

Comment 8

[Chenxia Liu [:liuche]]

Thanks, pushed a new version.

I'll push to try once I get the releng bug fixed and the keys on the builders.

Comment 9

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8893910 [details]
Bug 1386906 - Add Pocket keys to build system.

https://reviewboard.mozilla.org/r/164976/#review170444

Comment 10

[Chenxia Liu [:liuche]]

Comment on attachment 8893910 [details]
Bug 1386906 - Add Pocket keys to build system.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/164976/diff/3-4/

Comment 11

[Chenxia Liu [:liuche]]

Comment on attachment 8893910 [details]
Bug 1386906 - Add Pocket keys to build system.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/164976/diff/4-5/

Comment 12

[Chenxia Liu [:liuche]]

Built this on try and was able to get keys.

I made a few tiny changes: fixed a 'f' typo to 'fi', clarified the comment in mozconfigs/common, and changed the default mozconfig build var for Pocket to True.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/59bc932c44ea
Add Pocket keys to build system. r=chmanchester

Comment 14

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

https://hg.mozilla.org/mozilla-central/rev/59bc932c44ea

Comment 15

[Axel Hecht [:Pike]]

Wes, I'm pretty sure this broke single-locale fennec builds in one of the infamous "secret is not in repack slave" ways, https://treeherder.mozilla.org/logviewer.html#?job_id=122539768&repo=mozilla-central&lineNumber=1305.

Can you back this out again?

Comment 16

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

Backed out in https://hg.mozilla.org/mozilla-central/rev/d0afe5d6350426e338c1acff28a4a0c38ba1965f

Comment 17

[Chenxia Liu [:liuche]]

Okay, I see what went wrong here - when I set MOZ_ANDROID_POCKET config to True that meant MOZ_ANDROID_POCKET was present even in the l10n-nightly builds, even though I included the --without-pocket-api-keyfile flag. I've fixed this in the patch for bug 1389257 by setting the env variable only in the relevant mozconfigs (and excluding the l10n mozconfig files) rather than True by default, so I'll re-land those together. Should have re-requested review after all...!

Comment 18

[Chenxia Liu [:liuche]]

Comment on attachment 8893910 [details]
Bug 1386906 - Add Pocket keys to build system.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/164976/diff/5-6/

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/73ad9f441386
Add Pocket keys to build system. r=chmanchester

Comment 20

[Intermittent Failures Robot]

16 failures in 166 pushes (0.096 failures/push) were associated with this bug yesterday.   

Repository breakdown:
* mozilla-central: 16

Platform breakdown:
* android-4-0-armv7-api15: 16

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1386906&startday=2017-08-11&endday=2017-08-11&tree=all

Comment 21

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/73ad9f441386

Comment 22

[Intermittent Failures Robot]

16 failures in 901 pushes (0.018 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* mozilla-central: 16

Platform breakdown:
* android-4-0-armv7-api15: 16

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1386906&startday=2017-08-07&endday=2017-08-13&tree=all