SDK-built XPI bundles should identify version of SDK used to build bundle

RESOLVED FIXED in 1.0b2

Status

Add-on SDK
General
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: myk, Assigned: warner)

Tracking

unspecified
1.0b2

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

8 years ago
In order to make it easy to figure out the version of the SDK that was used to build a given XPI (f.e. to determine if the XPI contains a flaw from an older version of the SDK), XPI bundles built by the SDK should explicitly identify the version of the SDK that was used to build them via a property in a manifest or somesuch.

Brian: is this something you could take on?
(Assignee)

Comment 1

8 years ago
sure thing
Assignee: nobody → warner-bugzilla
Status: NEW → ASSIGNED
(Reporter)

Comment 2

8 years ago
The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".
Component: Jetpack SDK → General
Product: Mozilla Labs → Add-on SDK
QA Contact: jetpack-sdk → general
(Assignee)

Comment 3

8 years ago
Created attachment 494489 [details] [diff] [review]
quick patch to add .version and copy it into harness-options.json

Here's a quick patch to add a ".version" file (which will need to be kept up-to-date) and copy it into the XPI's "harness-options.json".

I can't think of an easy way to unit test this, though, because cuddlefish/__init__.py doesn't make it easy to execute the .version-reading code without also invoking a whole bunch of other commands. Atul: any ideas? I know we should probably just rewrite __init__.py one of the days..
Attachment #494489 - Flags: feedback?(avarma)

Comment 4

8 years ago
Comment on attachment 494489 [details] [diff] [review]
quick patch to add .version and copy it into harness-options.json

Hey, rather than using 'mydir' to get to sdk_top, could you use the already-defined local 'env_root' variable instead?

Ah, the ol' in-dire-need-of-refactoring __init__.py. I guess one easy way to unit test it might be to simply create a cuddlefish.version submodule as cuddlefish/version.py, which exports a single function of the following form:

  def get(env_root=os.environ.get('CUDDLEFISH_ROOT')):
    try:
      f = open(os.path.join(env_root, ".version"), "r")
      return f.read().strip()
    except EnvironmentError:
      # unable to open .version file
      return None

... and just unit test that?

Also, is there a particular reason we are catching the exception there? Shouldn't .version always exist, and if it doesn't, something has gone horribly wrong and we "crash early, crash often"?
Attachment #494489 - Flags: feedback?(avarma) → feedback+
(Assignee)

Comment 5

8 years ago
> Hey, rather than using 'mydir' to get to sdk_top, could you use the
> already-defined local 'env_root' variable instead?

Oh, yeah, sure. I had assumed that env_root pointed to the top of the active
package, but I see that it's really coming from $CUDDLEFISH_ROOT.

That 'mydir' actually came from the 'app_extension_dir' code later on. We
could fix them both to use env_root, or we could leave app_extension_dir
alone and fix it later.

> Ah, the ol' in-dire-need-of-refactoring __init__.py. I guess one easy way to
> unit test it might be to simply create a cuddlefish.version submodule as
> cuddlefish/version.py, which exports a single function of the following form:

Yeah, that works.. I'll do that.

> Also, is there a particular reason we are catching the exception there?
> Shouldn't .version always exist, and if it doesn't, something has gone
> horribly wrong and we "crash early, crash often"?

Hm, yeah, I guess that's a good idea. One lingering idea is to use ".version"
to handle released copies of the SDK (i.e. from a tarball), but to notice
.git/ and use 'git describe' to compute a version number when you've got a
live version-controlled tree (which would give you much more accurate data
than just "1.0b2pre"). I was considering not checking .version into the tree
(since updating it is a hassle) and only creating it during a "make
release-tarball" process of some sort, so tolerate it being missing might be
useful. But instead, I think we can require that either 1) you can get the
version from .git, or 2) you can get the version from .version, and explode
if both attempts fail.

I'll attach a new patch shortly. Thanks!
(Assignee)

Comment 6

8 years ago
Created attachment 496979 [details]
Pointer to pull request
(Assignee)

Updated

8 years ago
Attachment #496979 - Flags: review?(avarma)
(Assignee)

Updated

8 years ago
Attachment #496979 - Flags: review?(avarma) → review?(myk)
(Reporter)

Comment 7

8 years ago
Comment on attachment 496979 [details]
Pointer to pull request

Looks good, r=myk.
Attachment #496979 - Flags: review?(myk) → review+
(Assignee)

Comment 8

8 years ago
landed in https://github.com/mozilla/addon-sdk/commit/4bf5557fb03a82ecec166db1996d38db536d7ec7
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: 0.9 → 1.0b2
You need to log in before you can comment on or make changes to this bug.