Closed
Bug 1277407
Opened 6 years ago
Closed 6 years ago
Adjust cannot be built without keyfile, making local development difficult
Categories
(Firefox Build System :: Android Studio and Gradle Integration, defect)
Firefox Build System
Android Studio and Gradle Integration
All
Android
Tracking
(firefox49 fixed, firefox50 fixed)
RESOLVED
FIXED
mozilla50
People
(Reporter: mcomella, Assigned: mcomella)
References
Details
Attachments
(2 files)
58 bytes,
text/x-review-board-request
|
sebastian
:
review+
lizzard
:
approval-mozilla-aurora+
lizzard
:
approval-mozilla-beta+
|
Details |
58 bytes,
text/x-review-board-request
|
sebastian
:
review+
|
Details |
Jonathan and I discovered that bug 1274956 landed changes to our adjust config [1] such that `--with-adjust-sdk-keyfile` must be specified in order to build with MOZ_INSTALL_TRACKING. This means that an official adjust SDK key file must be included, which most developers probably don't have. Looking at the extended commit summary: This does a few things. First, it makes non-official builds use the Adjust sandbox. Second, I observe that the fake sandbox key no longer sends anything, so it's no longer valuable; this patch instead requires an Adjust token if install tracking is enabled, since we can't provide a default any more. Third, it removes a spurious default in configure.in; without this default, builders can easily enable Adjust locally using the following in their mozconfig: ac_add_options --with-adjust-sdk-keyfile=/path/to/adjust-sdk.keyfile export MOZ_INSTALL_TRACKING=1 With the default, the "export" had no impact, because it was overwritten immediately. --- Reading into this, I think the mistake is that the comment assumes Adjust can be built locally without MOZ_INSTALL_TRACKING, but that's not true. Notably, I never had the same concerns about the usefulness of the Adjust sandbox (my changes were reflected in the Sandbox, albeit they are slightly delayed – perhaps that's what caused the confusion?). I think the solution here is to largely revert this changeset. [1]: https://hg.mozilla.org/mozilla-central/rev/ac75f8f4c01d
Assignee | ||
Comment 1•6 years ago
|
||
Additionally, we add this file to the tree so it can be used by bug 1277553. The following commit will add the docs explaining how to use the file I added. The added file contents are slurped directly into AdjustConstants.MOZ_INSTALL_TRACKING_ADJUST_SDK_APP_TOKEN. We get the sandbox token from the previously working value in [1]. An alternative solution would be to remove the assertion that `--with-adjust-sdk-keyfile` is specified and provide a default value if the key is not specified. However, after investigating bug 1277553, I dislike this approach because we lose the compile-time checks that a key file is specified, which is dangerous for release/beta builds where the key file may not be valid, we push a release, and we don't get the data we expect. Ideally, we specify `--with-adjust-sdk-keyfile` for non-MOZILLA_OFFICIAL builds, but I don't know how to do that. [1]: https://hg.mozilla.org/mozilla-central/rev/ac75f8f4c01d#l1.26 Review commit: https://reviewboard.mozilla.org/r/57724/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/57724/
Attachment #8759894 -
Flags: review?(s.kaspari)
Attachment #8759895 -
Flags: review?(s.kaspari)
Assignee | ||
Comment 2•6 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/57726/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/57726/
Comment 3•6 years ago
|
||
Comment on attachment 8759894 [details] Bug 1277407 - Add Adjust sandbox keyfile to tree. https://reviewboard.mozilla.org/r/57724/#review54700
Attachment #8759894 -
Flags: review?(s.kaspari) → review+
Comment 4•6 years ago
|
||
Comment on attachment 8759895 [details] Bug 1277407 - Add docs about using adjust sandbox & updating outdated info. https://reviewboard.mozilla.org/r/57726/#review54702
Attachment #8759895 -
Flags: review?(s.kaspari) → review+
Assignee | ||
Comment 5•6 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/6dde3c79db7dc15038b66c6cffdcbf7c1678712e Bug 1277407 - Add Adjust sandbox keyfile to tree. r=sebastian https://hg.mozilla.org/integration/fx-team/rev/309157e8ac25ba4d59c19b03c9476f08e14ad5e3 Bug 1277407 - Add docs about using adjust sandbox & updating outdated info. r=sebastian
Comment 6•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6dde3c79db7d https://hg.mozilla.org/mozilla-central/rev/309157e8ac25
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Assignee | ||
Comment 7•6 years ago
|
||
Comment on attachment 8759894 [details] Bug 1277407 - Add Adjust sandbox keyfile to tree. This bug should be uplifted with bug 1277553. Approval Request Comment [Feature/regressing bug #]: bug 1274956 [User impact if declined]: RyanVM's 48 beta -> 47 release simulation will permafail, as well as his 49 aurora -> 48 beta [Describe test coverage new/current, TreeHerder]: Tested locally. [Risks and why]: None – this bug only adds an unused file (but usable by local developers) that will be used in bug 1277553 and updates docs. [String/UUID change made/needed]: None
Attachment #8759894 -
Flags: approval-mozilla-beta?
Attachment #8759894 -
Flags: approval-mozilla-aurora?
Comment on attachment 8759894 [details] Bug 1277407 - Add Adjust sandbox keyfile to tree. Sounds necessary for developers/tests, please uplift to aurora and beta
Attachment #8759894 -
Flags: approval-mozilla-beta?
Attachment #8759894 -
Flags: approval-mozilla-beta+
Attachment #8759894 -
Flags: approval-mozilla-aurora?
Attachment #8759894 -
Flags: approval-mozilla-aurora+
Comment 9•6 years ago
|
||
bugherderuplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/3e9799b06fe4 https://hg.mozilla.org/releases/mozilla-aurora/rev/2ce49ca61992
status-firefox49:
--- → fixed
Comment 10•6 years ago
|
||
bugherderuplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/3ee14b37d5d4 https://hg.mozilla.org/releases/mozilla-beta/rev/599d69a0d339
status-firefox48:
--- → fixed
Comment 11•6 years ago
|
||
Backed out in https://hg.mozilla.org/releases/mozilla-beta/rev/3c1c9ff0e307d7cce9461509d06fce6742651423 - see bug 1277553 comment 17
status-firefox48:
fixed → ---
Updated•3 years ago
|
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 50 → mozilla50
You need to log in
before you can comment on or make changes to this bug.
Description
•