Closed
Bug 1277407
Opened 9 years ago
Closed 9 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•9 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•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/57726/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/57726/
Comment 3•9 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•9 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•9 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•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6dde3c79db7d
https://hg.mozilla.org/mozilla-central/rev/309157e8ac25
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Assignee | ||
Comment 7•9 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 8•9 years ago
|
||
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•9 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/3e9799b06fe4
https://hg.mozilla.org/releases/mozilla-aurora/rev/2ce49ca61992
status-firefox49:
--- → fixed
Comment 10•9 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/3ee14b37d5d4
https://hg.mozilla.org/releases/mozilla-beta/rev/599d69a0d339
status-firefox48:
--- → fixed
Comment 11•9 years ago
|
||
Backed out in https://hg.mozilla.org/releases/mozilla-beta/rev/3c1c9ff0e307d7cce9461509d06fce6742651423 - see bug 1277553 comment 17
status-firefox48:
fixed → ---
Updated•6 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
•