Closed Bug 1277407 Opened 3 years ago Closed 3 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

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)
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 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+
https://hg.mozilla.org/mozilla-central/rev/6dde3c79db7d
https://hg.mozilla.org/mozilla-central/rev/309157e8ac25
Status: NEW → RESOLVED
Closed: 3 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.