Closed
Bug 1037160
Opened 10 years ago
Closed 10 years ago
Experiments source code should live in central repository
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: gps, Assigned: gps)
References
Details
Attachments
(1 file)
(deleted),
patch
|
benjamin
:
review-
|
Details | Diff | Splinter Review |
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.
Flags: firefox-backlog?
Assignee | ||
Comment 1•10 years ago
|
||
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/fx-searchtest-en-beta31@mozilla.org/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.
Attachment #8454174 -
Flags: review?(benjamin)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → gps
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•10 years ago
|
||
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).
Depends on: 1035333
Flags: firefox-backlog?
Assignee | ||
Comment 3•10 years ago
|
||
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•10 years ago
|
||
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.
Attachment #8454174 -
Flags: review?(benjamin) → review-
Comment 5•10 years ago
|
||
And importing full history is not a requirement, but if it's easy to do that's fine.
Comment 6•10 years ago
|
||
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.
Assignee | ||
Comment 7•10 years ago
|
||
(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•10 years ago
|
||
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.
Assignee | ||
Comment 9•10 years ago
|
||
(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•10 years ago
|
||
That's fine.
Assignee | ||
Comment 11•10 years ago
|
||
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.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•