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)

All
Android
defect
Not set
normal

Tracking

(firefox49 fixed, firefox50 fixed)

RESOLVED FIXED
mozilla50
Tracking Status
firefox49 --- fixed
firefox50 --- fixed

People

(Reporter: mcomella, Assigned: mcomella)

References

Details

Attachments

(2 files)

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
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)
Attachment #8759894 - Flags: review?(s.kaspari) → review+
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+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
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+
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.

Attachment

General

Created:
Updated:
Size: