1277407 - Adjust cannot be built without keyfile, making local development difficult

Status: RESOLVED FIXED

(Firefox Build System :: Android Studio and Gradle Integration, defect)

Description

[Michael Comella (:mcomella) [needinfo or I won't see it]]

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

Comment 1

[Michael Comella (:mcomella) [needinfo or I won't see it]]

Attachment: Bug 1277407 - Add Adjust sandbox keyfile to tree.

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/

Comment 2

[Michael Comella (:mcomella) [needinfo or I won't see it]]

Attachment: Bug 1277407 - Add docs about using adjust sandbox & updating outdated info.

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

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8759894 [details]
Bug 1277407 - Add Adjust sandbox keyfile to tree.

https://reviewboard.mozilla.org/r/57724/#review54700

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8759895 [details]
Bug 1277407 - Add docs about using adjust sandbox & updating outdated info.

https://reviewboard.mozilla.org/r/57726/#review54702

Comment 5

[Michael Comella (:mcomella) [needinfo or I won't see it]]

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

[Wes Kocher (:KWierso)]

https://hg.mozilla.org/mozilla-central/rev/6dde3c79db7d
https://hg.mozilla.org/mozilla-central/rev/309157e8ac25

Comment 7

[Michael Comella (:mcomella) [needinfo or I won't see it]]

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

Comment 8

[Liz Henry (:lizzard)]

Comment on attachment 8759894 [details]
Bug 1277407 - Add Adjust sandbox keyfile to tree.

Sounds necessary for developers/tests, please uplift to aurora and beta

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3e9799b06fe4
https://hg.mozilla.org/releases/mozilla-aurora/rev/2ce49ca61992

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3ee14b37d5d4
https://hg.mozilla.org/releases/mozilla-beta/rev/599d69a0d339

Comment 11

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/releases/mozilla-beta/rev/3c1c9ff0e307d7cce9461509d06fce6742651423 - see bug 1277553 comment 17