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

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

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?

Comment 1

[Brian Warner [:warner :bwarner]]

sure thing

Comment 2

[Myk Melez [:myk] [@mykmelez]]

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".

Comment 3

[Brian Warner [:warner :bwarner]]

Attachment: 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..

Comment 4

[Atul Varma [:atul]]

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"?

Comment 5

[Brian Warner [:warner :bwarner]]

> 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!

Comment 6

[Brian Warner [:warner :bwarner]]

Attachment: Pointer to pull request

Comment 7

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 496979 [details]
Pointer to pull request

Looks good, r=myk.

Comment 8

[Brian Warner [:warner :bwarner]]

landed in https://github.com/mozilla/addon-sdk/commit/4bf5557fb03a82ecec166db1996d38db536d7ec7