1037160 - Experiments source code should live in central repository

Status: RESOLVED WONTFIX

(Firefox :: General, defect)

Description

[Gregory Szorc [:gps]]

Currently, the source code for Telemetry Experiments lives in user repositories. e.g. https://hg.mozilla.org/users/felipc_gmail.com/search-experiment and https://github.com/bwinton/tile-switcher. The built XPIs are checked into https://hg.mozilla.org/webtools/telemetry-experiment-server.

This isn't optimal for productivity and auditing. It's not conducive for discovery. I had to ask someone where to find the source code because I couldn't find any references in bugs and search results were revealing nothing. Maybe I wasn't looking hard enough. But I shouldn't need to.

The source for all the Telemetry Experiments should live in a single repository and should have a consistent build mechanism.

IMO that repository is https://hg.mozilla.org/webtools/telemetry-experiment-server or even mozilla-central. Since telemetry-experiment-server contains a build script, my vote is to put source in that repo, have the build script generate .xpi files, and just not check in .xpi files into that repo.

Comment 1

[Gregory Szorc [:gps]]

Attachment: Store search test experiment source instead of XPI

Now that we have build scripts for individual experiments, we can check
in the source code of experiments instead of XPIs. This patch does that
for the searchtest-en-beta experiment.

The contents of the existing xpi were simply uncompressed and added to
source control. A build.py script was introduced. It uses the helpers
from bug 1035333 to produce an XPI.

Here is the output of `zipinfo <xpi>` from before:

Archive:  experiments/searchtest-en-beta/experiment.xpi   13559   16
-rw-r--r--  2.0 unx     1249 b- defN  1-Jul-14 13:39 install.rdf
-rw-r--r--  2.0 unx      101 b- defN  1-Jul-14 13:39 options.xul
-rw-r--r--  2.0 unx        1 b- defN  1-Jul-14 13:39 defaults/preferences/prefs.js
drwxr-xr-x  2.0 unx        0 b- stor  1-Jan-80 00:00 locale/
?rw-r--r--  2.0 unx       16 b- stor  1-Jan-80 00:00 locales.json
-rwxr-xr-x  2.0 unx    13230 b- defN 16-Aug-14 13:14 bootstrap.js
drwxr-xr-x  2.0 unx        0 b- stor  1-Jan-80 00:00 resources/
drwxr-xr-x  2.0 unx        0 b- stor  1-Jan-80 00:00 resources/addon-sdk/
drwxr-xr-x  2.0 unx        0 b- stor  1-Jan-80 00:00 resources/addon-sdk/lib/
drwxr-xr-x  2.0 unx        0 b- stor  1-Jan-80 00:00 resources/fx-searchtest/
drwxr-xr-x  2.0 unx        0 b- stor  1-Jan-80 00:00 resources/fx-searchtest/lib/
-rw-r--r--  2.0 unx     7299 b- defN 30-Jun-14 22:17 resources/fx-searchtest/lib/dropdownnote.js
-rw-r--r--  2.0 unx     6069 b- defN  9-Aug-14 13:00 resources/fx-searchtest/lib/main.js
-rw-r--r--  2.0 unx     1087 b- defN 26-Jul-14 20:42 resources/fx-searchtest/lib/notification-counter.js
drwxr-xr-x  2.0 unx        0 b- stor  1-Jan-80 00:00 resources/fx-searchtest/tests/
-rw-r--r--  2.0 unx     2354 b- defN  1-Jul-14 13:39 harness-options.json
16 files, 31406 bytes uncompressed, 11587 bytes compressed:  63.1%

And with the new Python code:

Archive:  out/[e-mail]/experiment.xpi   13397 15
drwxr-xr-x  2.0 unx        0 b- stor  1-Jan-80 00:00 defaults/
drwxr-xr-x  2.0 unx        0 b- stor  1-Jan-80 00:00 defaults/preferences/
drwxr-xr-x  2.0 unx        0 b- stor  1-Jan-80 00:00 locale/
drwxr-xr-x  2.0 unx        0 b- stor  1-Jan-80 00:00 resources/
drwxr-xr-x  2.0 unx        0 b- stor  1-Jan-80 00:00 resources/fx-searchtest/
drwxr-xr-x  2.0 unx        0 b- stor  1-Jan-80 00:00 resources/fx-searchtest/lib/
-rw-r--r--  2.0 unx    13230 b- defN  1-Jan-80 00:00 bootstrap.js
-rw-r--r--  2.0 unx        1 b- defN  1-Jan-80 00:00 defaults/preferences/prefs.js
-rw-r--r--  2.0 unx     2354 b- defN  1-Jan-80 00:00 harness-options.json
-rw-r--r--  2.0 unx     1249 b- defN  1-Jan-80 00:00 install.rdf
-rw-r--r--  2.0 unx       16 b- defN  1-Jan-80 00:00 locales.json
-rw-r--r--  2.0 unx      101 b- defN  1-Jan-80 00:00 options.xul
-rw-r--r--  2.0 unx     7299 b- defN  1-Jan-80 00:00 resources/fx-searchtest/lib/dropdownnote.js
-rw-r--r--  2.0 unx     6069 b- defN  1-Jan-80 00:00 resources/fx-searchtest/lib/main.js
-rw-r--r--  2.0 unx     1087 b- defN  1-Jan-80 00:00 resources/fx-searchtest/lib/notification-counter.js
15 files, 31406 bytes uncompressed, 11589 bytes compressed:  63.1%

Notable differences:

* File times are normalized to the "zip file epoch"
* Content is sorted in the new version. Directory first, then by filename
* Permissions are more consistent (locales.json was missing its file
  type bits)
* The apparently empty resources/addon-sdk directories have been
  excluded.

Comment 2

[Gregory Szorc [:gps]]

Patches that enable this magic are in bug 1035333. This patch should mostly be a rubber stamp (but please look at the results of zipinfo in the commit comment to be sure).

Comment 3

[Gregory Szorc [:gps]]

Open question: I /could/ import the source with full history from the existing user repo. Give the word and I will do this instead of a single commit import.

Comment 4

[Benjamin Smedberg]

Comment on attachment 8454174 [details] [diff] [review]
Store search test experiment source instead of XPI

The search test uses the addon SDK, and so the source code for that is not the contents of the .xpi. We should either add support for building using the SDK, or just check in the .xpi for that one and exclude it from this system for now.

Why is there a separate build.py script in each experiment? It seems that the root build script could handle that automatically.

Comment 5

[Benjamin Smedberg]

And importing full history is not a requirement, but if it's easy to do that's fine.

Comment 6

[Benjamin Smedberg]

And actually since the SDK will not be present on the build machine, we should continue to check in the generated .xpi for the SDK-based addons.

Comment 7

[Gregory Szorc [:gps]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #4)
> Comment on attachment 8454174 [details] [diff] [review]
> Store search test experiment source instead of XPI
> 
> The search test uses the addon SDK, and so the source code for that is not
> the contents of the .xpi. We should either add support for building using
> the SDK, or just check in the .xpi for that one and exclude it from this
> system for now.
> 
> Why is there a separate build.py script in each experiment? It seems that
> the root build script could handle that automatically.

The question on separate build.py scripts really belongs in bug 1035333. Short version is different experiments may have different build procedures. The one-off translation experiments for example will use templates to stage add-on files into a temporary directory before packaging. I figured it was easier to establish separate build.py scripts instead of establishing central rules. There is a concern for DRY here, but I believe I mitigated that by make "build an XPI from local files" a one-liner.

Also, I'm not sure what the comment about the SDK means. The contents of the XPI are effectively identical, no?

Comment 8

[Benjamin Smedberg]

The source code for an SDK-based addon (with a package.json and so forth) isn't the same as the final XPI. e.g. http://hg.mozilla.org/users/felipc_gmail.com/search-experiment/file/default/

Overall, I'm fine with doing this, but I don't think it's necessary and maybe it would be good enough to link to the experiment repositories.

Comment 9

[Gregory Szorc [:gps]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #8)
> The source code for an SDK-based addon (with a package.json and so forth)
> isn't the same as the final XPI. e.g.
> http://hg.mozilla.org/users/felipc_gmail.com/search-experiment/file/default/

Ah, I didn't realize this!

I agree that this patch as-is shouldn't land!

> Overall, I'm fine with doing this, but I don't think it's necessary and
> maybe it would be good enough to link to the experiment repositories.

The patch in this bug was really a simple PoC to vet patches in bug 1035333. I figured it would land without much effort.

Given it isn't as trivial as I thought, IMO we should defer this bug until later. I believe we should, however, make an attempt to aggregate code and building of XPIs into the telemetry-experiment-server repo for future experiments. if you agree, perhaps this bug is WONTFIX since bug 1035333 establishes a mechanism to accomplish this?

Comment 10

[Benjamin Smedberg]

That's fine.

Comment 11

[Gregory Szorc [:gps]]

What's done is done. We have a mechanism for building experiments. If we really want to capture the source from the old experiments in source control, we can always reopen.