928452 - Create a mozversion package for getting the application version

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Initially this would be useful for getting the gecko/gaia/build revisions for B2G. We have needed this in b2gperf, eideticker, gaia endurance tests, and likely other places (gaiatest will need it for the html reports soon too). Rather than duplicating this, let's create a package to centralise it.

This is the code b2gperf uses, which takes into account if gaia has been flashed to the device, and requires the sources.xml file that comes alongside the builds: https://github.com/mozilla/b2gperf/blob/0093495d30d4aaba67cffb897527f763dd50ee58/b2gperf/b2gperf.py#L91

Comment 1

[Dave Hunt [:davehunt] [he/him] ⌚BST]

This could work similar to mozinfo, however it would need several bits of information. We should also establish what information we want to obtain.

For desktop Firefox or Thunderbird we would need to specify the path to the binary, and I would expect to at least return the version (25.0.1), the build ID (20131112160018), and perhaps the revision (http://hg.mozilla.org/releases/mozilla-release/rev/d20d499b219f).

For B2G devices (and emulators?) we would need to provide the means for mozdevice to pull files, and the path to a sources.xml file that matches the target (if we could work out another way to get this, such as from FTP, that would be even better). For desktop builds we would need the path to the binary. We should return the gaia, gecko, and build revisions. If possible we should return both the git and hg revisions for these where applicable. We should consider also returning the deviceinfo.os setting (1.3.0.0-prerelease) and perhaps the device name ro.product.device (unagi, inari, etc).

I would see the command line tool simply taking an app name argument to determine and the various other necessary arguments, and simply printing out the results. I could see the API with various child classes for each app type taking the necessary arguments and returning a dict.

How does that sound? Requesting feedback (via needinfo) from several people so we can get started with this.

Comment 2

[Andrew Halberstadt [:ahal]]

I don't know if I'm convinced this needs to live in mozbase (though not saying I couldn't be convinced). If mozbase is the best place, would it be appropriate to just add a function to mozinfo instead of creating a whole new module?

Also for b2g we should probably get the entire manifest xml, not just gaia/gonk/gecko.

Comment 3

[Jonathan Griffin (:jgriffin)]

I'd rather not depend on an external sources.xml for B2G, since that may or may not be available, depending on the circumstances.  Instead, I think we should pull sources.xml off the device, and use that for everything except gecko and gaia; we can get the gaia version from parsing a file in the settings app, and the gecko version from application.ini.

Bonus points for providing both the hg and git revisions of gecko, but I'm not sure we have a good way to do that (no mapping service), so we could punt that to a later release.

I'm not sure we want it in mozinfo, since I don't know that we want every consumer of mozinfo to be dependent on mozdevice and its dependencies.

Comment 4

[Jeff Hammel]

AIUI, this is targeted for b2g+desktop(+android?); is that correct?
If so, mozversion sees an appropriate name.

I'd want to see an API (func/class definitions, CLI usage) before I
could much give specific feedback, but I like the idea.

(In reply to Andrew Halberstadt [:ahal] from comment #2)
> I don't know if I'm convinced this needs to live in mozbase (though not
> saying I couldn't be convinced).

Any reason not to put it in mozbase?  Our original strategy for mozbase is

"Mozbase is a set of easy-to-use Python packages forming a
supplemental standard library for Mozilla. It provides consistency and
reduces redundancy in automation and other system-level software. All
of Mozilla's test harnesses use mozbase to some degree, including
Talos, mochitest, reftest, Autophone, and Eideticker."
- https://wiki.mozilla.org/Auto-tools/Projects/Mozbase

Seems like a fit to me, though I don't have a stake in it.

> If mozbase is the best place, would it be
> appropriate to just add a function to mozinfo instead of creating a whole
> new module?

I see little overhead in
making new modules and mostly prefer smaller atomic modules. Also,
what :jgriffin said re deps in comment 3; for this reason, I'd prefer
it not be its own module.

mozrunner currently has the get_repositoryInfo function:

https://github.com/mozilla/mozbase/blob/master/mozrunner/mozrunner/local.py#L14\
3

'Twould be nice to deprecate.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

So this is a request which I already had filed as bug 769606 a while back.

Comment 6

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Bug 946254 details some further B2G related information that would be valuable to return.

Comment 7

[William Lachance (:wlach)]

This sounds great to me. Looking forward to the initial implementation!

Comment 8

[Dave Hunt [:davehunt] [he/him] ⌚BST]

I've started to look into this, and will provide an initial patch for feedback once I have something I'm happy with.

Comment 9

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Initial proposal

Here's an initial working version. I'm interested in getting feedback. This does not currently support Fennec (I understand I can get the necessary information from the APK, so it shouldn't be too hard to add).

From the command line:

$ mozversion --binary=/path/to/binary

The binary path is optional. If it's omitted then we assume we want the details from a remote application (B2G only at the moment).

The output will be something like:

application_buildid: 20131213040203
application_changeset: 8b5875dc7e31
application_name: B2G
application_repository: http://hg.mozilla.org/mozilla-central
application_version: 29.0a1
device_firmware_date: 1380051975
device_firmware_version_incremental: 139
device_firmware_version_release: 4.0.4
device_id: msm7627a
gaia_date: 1386934135
gaia_revision: 390b313a254a947d12e3cdbcde19d7d1619ff63c
platform_buildid: 20131213040203
platform_changeset: 8b5875dc7e31
platform_repository: http://hg.mozilla.org/mozilla-central

From code:
>>> import mozversion
>>> mozversion.B2GVersion().info.get('gaia_revision')
'390b313a254a947d12e3cdbcde19d7d1619ff63c'
>>> firefox = mozversion.FirefoxVersion('/Applications/FirefoxRelease.app/Contents/MacOS/firefox-bin')
>>> firefox.info.get('application_changeset')
'd20d499b219f'

For B2G I'm unable to get the platform revision, which is currently obtained from the sources.xml in b2gperf. This might not be a useful value though, so we may not need it.

Comment 10

[William Lachance (:wlach)]

Comment on attachment 8350558 [details] [diff] [review]
Initial proposal

Review of attachment 8350558 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. I think we do indeed want the platform revision, FWIW. I believe someone specifically requested it for Eideticker. There's some code to grab it from the sources.xml here:

https://github.com/mozilla/eideticker/blob/master/bin/update-dashboard.py#L23

::: mozversion/mozversion/mozversion.py
@@ +19,5 @@
> +class Version():
> +
> +    def __init__(self):
> +        self.info = {}
> +        self.logger = mozlog.getLogger(self.__class__.__name__)

You could use the LoggingMixin. See the example at the bottom of the page here:

http://mozbase.readthedocs.org/en/latest/mozlog.html

Comment 11

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to William Lachance (:wlach) from comment #10)
> Comment on attachment 8350558 [details] [diff] [review]
> Initial proposal
> 
> Review of attachment 8350558 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good. I think we do indeed want the platform revision, FWIW. I believe
> someone specifically requested it for Eideticker. There's some code to grab
> it from the sources.xml here:
> 
> https://github.com/mozilla/eideticker/blob/master/bin/update-dashboard.py#L23

Yeah, I currently do the same with b2gperf: https://github.com/mozilla/b2gperf/blob/a778a2c299f37008f10361d686559f319953687a/b2gperf/b2gperf.py#L118 do you know if there's any other way to get this? I could add an optional --sources command line argument. If it's present we can get extra information like this.

> ::: mozversion/mozversion/mozversion.py
> @@ +19,5 @@
> > +class Version():
> > +
> > +    def __init__(self):
> > +        self.info = {}
> > +        self.logger = mozlog.getLogger(self.__class__.__name__)
> 
> You could use the LoggingMixin. See the example at the bottom of the page
> here:
> 
> http://mozbase.readthedocs.org/en/latest/mozlog.html

Nice, thanks! :)

Comment 12

[Andrew Halberstadt [:ahal]]

Comment on attachment 8350558 [details] [diff] [review]
Initial proposal

Review of attachment 8350558 [details] [diff] [review]:
-----------------------------------------------------------------

Looks like a good start. I'm f-'ing because I don't think we should have separate entry points (ThunderbirdVersion, FirefoxVersion, B2GVersion, etc). We should just have a single method that does the right thing for us. There's a few other optional suggestions too.

::: mozversion/mozversion/mozversion.py
@@ +22,5 @@
> +        self.info = {}
> +        self.logger = mozlog.getLogger(self.__class__.__name__)
> +
> +    def get_gaia_info(self, app_zip):
> +        with app_zip.open('resources/gaia_commit.txt') as f:

Could we put this method on a B2G specific subclass or something?

@@ +44,5 @@
> +
> +
> +class LocalVersion(Version):
> +
> +    def __init__(self, binary, **kwargs):

What's the point of these being classes, if they only have logic in __init__? Might as well just make everything a module level function.

@@ +124,5 @@
> +    return LocalVersion(binary)
> +
> +
> +def ThunderbirdVersion(binary):
> +    return LocalVersion(binary)

I think it would be more useful if the public facing API had a single call point and we were then able to deduce which platform the config belongs to and automatically return the correct thing. I'm also not really a fan of the "looks like a class" style API here. IMO mozversion.get_version() looks better than mozversion.FirefoxVersion(), but I guess that's personal preference.

Comment 13

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Revised proposal

:ahal have I understood your feedback correctly in this patch? I've also added the option for a sources.xml file.

The output is now something like:

application_buildid: 20131220040201
application_changeset: 599100c4ebfe
application_name: B2G
application_repository: http://hg.mozilla.org/mozilla-central
application_version: 29.0a1
build_changeset: 59605a7c026ff06cc1613af3938579b1dddc6cfe
device_firmware_date: 1380051975
device_firmware_version_incremental: 139
device_firmware_version_release: 4.0.4
device_id: msm7627a
gaia_changeset: 574f79512a7b8a9ab99211b16a857ab812d7994e
gaia_date: 1387542271
gecko_changeset: 3a2d8af198510726b063a217438fcf2591f4dfcf
platform_buildid: 20131220040201
platform_changeset: 599100c4ebfe
platform_repository: http://hg.mozilla.org/mozilla-central

Note addition of build_changeset and gecko_changeset (the git changeset from sources.xml).

From code has also changed:

>>> mozversion.get_version()._info.get('gaia_changeset')
mozversion.mozversion.RemoteB2GVersion INFO | Unable to find system/sources.xml
'574f79512a7b8a9ab99211b16a857ab812d7994e'

>>> mozversion.get_version(binary='/Applications/FirefoxRelease.app/Contents/MacOS/firefox-bin')._info.get('application_changeset')
'd20d499b219f'

Comment 14

[Andrew Halberstadt [:ahal]]

Comment on attachment 8356183 [details] [diff] [review]
Revised proposal

Review of attachment 8356183 [details] [diff] [review]:
-----------------------------------------------------------------

Yep thanks, this looks good! Just a couple minor comments.

::: mozversion/mozversion/__init__.py
@@ +1,5 @@
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this file,
> +# You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +from mozversion import *

Will there ever be a need to import the version classes directly? We should just expose "get_version" if that is all anyone will ever use, otherwise all those other classes might confuse people.

::: mozversion/mozversion/mozversion.py
@@ +153,5 @@
> +        if version._info.get('application_name') == 'B2G':
> +            version = LocalB2GVersion(binary, sources=sources)
> +    else:
> +        version = RemoteB2GVersion(sources=sources)
> +    return version

Instead of returning the class here, I'd just return version._info as that is all anyone will ever care about. That way when run from the command line, we can just print the return value to stdout, or we can manipulate the dict if imported programatically.

Comment 15

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Revised proposal

Thanks for the feedback guys. I've updated and added documentation for the new package. I'm still only requesting feedback at this stage as I'd also like to add a few basic tests before requesting a review.

Comment 16

[Andrew Halberstadt [:ahal]]

Comment on attachment 8356560 [details] [diff] [review]
Revised proposal

Review of attachment 8356560 [details] [diff] [review]:
-----------------------------------------------------------------

Looking good! I have one optional suggestion, but it might be bikeshedding, so feel free to ignore if you disagree.

::: mozversion/mozversion/mozversion.py
@@ +104,5 @@
> +            dm = mozdevice.DeviceManagerADB()
> +        elif dm_type == 'sut':
> +            host = os.environ.get('TEST_DEVICE')
> +            if not host:
> +                raise Exception('Must specify host with SUT!')

Says we must specify host, but doesn't say how. Is there a sane default we could look for?

@@ +159,5 @@
> +        version = LocalVersion(binary)
> +        if version._info.get('application_name') == 'B2G':
> +            version = LocalB2GVersion(binary, sources=sources)
> +    else:
> +        version = RemoteB2GVersion(sources=sources)

So if no binary or sources is passed in, the default is for a dmADB instance to be created, which assumes adb is on the path and that one and only one device is connected. Is this the desired default behaviour? Maybe looking for application.ini in the cwd would be a better first default?

My vote would be (though this is getting into bikeshed territory):
1) Merge binary and sources into one variable and autodetect what gets passed in (checking for the executable bit should be a good enough differentiator)
2) If nothing is passed in search cwd for either application.ini or sources.xml
3) If nothing is found look for adb/sut connected devices
4) If nothing is found raise exception

Comment 17

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Create a mozversion package for getting the application version. v1.0

Not sure who's best for a review here so feel free to redirect it. It now has documentation and a bunch of tests. :)

(In reply to Andrew Halberstadt [:ahal] from comment #16)
> ::: mozversion/mozversion/mozversion.py
> @@ +104,5 @@
> > +            dm = mozdevice.DeviceManagerADB()
> > +        elif dm_type == 'sut':
> > +            host = os.environ.get('TEST_DEVICE')
> > +            if not host:
> > +                raise Exception('Must specify host with SUT!')
> 
> Says we must specify host, but doesn't say how. Is there a sane default we
> could look for?

This was taken from mozdevice. I'm not too familiar with SUT but happy to provide a default. What would be a suitable value?

> @@ +159,5 @@
> > +        version = LocalVersion(binary)
> > +        if version._info.get('application_name') == 'B2G':
> > +            version = LocalB2GVersion(binary, sources=sources)
> > +    else:
> > +        version = RemoteB2GVersion(sources=sources)
> 
> So if no binary or sources is passed in, the default is for a dmADB instance
> to be created, which assumes adb is on the path and that one and only one
> device is connected. Is this the desired default behaviour? Maybe looking
> for application.ini in the cwd would be a better first default?
> 
> My vote would be (though this is getting into bikeshed territory):
> 1) Merge binary and sources into one variable and autodetect what gets
> passed in (checking for the executable bit should be a good enough
> differentiator)
> 2) If nothing is passed in search cwd for either application.ini or
> sources.xml
> 3) If nothing is found look for adb/sut connected devices
> 4) If nothing is found raise exception

I agree it might not be the expected default behaviour. B2G might not be remote (this works with the desktop builds too), so we need to be able to specify both the binary path and the sources path. I'm open for suggestions, and agree that might be a good idea check for application.ini in the current working directory and if it is, infer the binary path from that.

Comment 18

[Andrew Halberstadt [:ahal]]

Comment on attachment 8357183 [details] [diff] [review]
Create a mozversion package for getting the application version. v1.0

Review of attachment 8357183 [details] [diff] [review]:
-----------------------------------------------------------------

I'd still prefer better default behaviour, but this looks like a solid start, r+

(In reply to Dave Hunt (:davehunt) from comment #17)
> > Says we must specify host, but doesn't say how. Is there a sane default we
> > could look for?
> 
> This was taken from mozdevice. I'm not too familiar with SUT but happy to
> provide a default. What would be a suitable value?

I have no idea :).. but maybe we could at least give the name of the env variable that needs to be set.

> I agree it might not be the expected default behaviour. B2G might not be
> remote (this works with the desktop builds too), so we need to be able to
> specify both the binary path and the sources path.

That last part's not true, if it's a binary we could open application.ini and look for Name=B2G to detect b2g desktop.

Comment 19

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Andrew Halberstadt [:ahal] from comment #18)
> Comment on attachment 8357183 [details] [diff] [review]
> Create a mozversion package for getting the application version. v1.0
> 
> Review of attachment 8357183 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'd still prefer better default behaviour, but this looks like a solid
> start, r+

I'll work on an updated patch with improved default behaviour.

> (In reply to Dave Hunt (:davehunt) from comment #17)
> > > Says we must specify host, but doesn't say how. Is there a sane default we
> > > could look for?
> > 
> > This was taken from mozdevice. I'm not too familiar with SUT but happy to
> > provide a default. What would be a suitable value?
> 
> I have no idea :).. but maybe we could at least give the name of the env
> variable that needs to be set.

I'll update the exception message.

> > I agree it might not be the expected default behaviour. B2G might not be
> > remote (this works with the desktop builds too), so we need to be able to
> > specify both the binary path and the sources path.
> 
> That last part's not true, if it's a binary we could open application.ini
> and look for Name=B2G to detect b2g desktop.

Yep, and I already do that, but we still need an optional sources.xml path as that's not relative to the binary path.

Comment 20

[Jonathan Griffin (:jgriffin)]

Is there a reason not to get sources.xml from the device, at /system/sources.xml?

Comment 21

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Jonathan Griffin (:jgriffin) from comment #20)
> Is there a reason not to get sources.xml from the device, at
> /system/sources.xml?

We do try, but in my experience it's rarely there:

    if sources is None:
        path = 'system/sources.xml'
        if dm.fileExists(path):
            sources = StringIO(dm.pullFile(path))
        else:
            self.info('Unable to find %s' % path)

Comment 22

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8357183 [details] [diff] [review]
Create a mozversion package for getting the application version. v1.0

Review of attachment 8357183 [details] [diff] [review]:
-----------------------------------------------------------------

Interesting, I didn't know that sources.xml sometimes isn't present.

Comment 23

[William Lachance (:wlach)]

Comment on attachment 8357183 [details] [diff] [review]
Create a mozversion package for getting the application version. v1.0

Review of attachment 8357183 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for taking so long on this. Looks good, only have some minor comments.

::: docs/mozversion.rst
@@ +34,5 @@
> +
> +---sources
> +''''''''''
> +
> +The path to the sources.xml that accompanies the target application. (B2G only)

I would say B2G / FirefoxOS here (or just FirefoxOS).

::: mozversion/mozversion/mozversion.py
@@ +98,5 @@
> +
> +    def __init__(self, sources=None, **kwargs):
> +        B2GVersion.__init__(self, sources, **kwargs)
> +
> +        dm_type = os.environ.get('DM_TRANS', 'adb')

IMO we should be passing dm_type and host as parameters, not as environment variables.

(ok for the command line client to parse out these environment variables and pass them to this class, of course)

@@ +109,5 @@
> +            dm = mozdevice.DeviceManagerSUT(host=host)
> +        else:
> +            raise Exception('Unknown device manager type: %s' % dm_type)
> +
> +        if sources is None:

if not sources:

Comment 24

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Create a mozversion package for getting the application version. v1.1

Okay, addressed the review comments. Also, we now check the current working directory for application.ini and sources.xml. This means you can run it from the binary directory and default behaviour will show you the version information.

Comment 25

[William Lachance (:wlach)]

Comment on attachment 8365341 [details] [diff] [review]
Create a mozversion package for getting the application version. v1.1

Review of attachment 8365341 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm for a first pass

Comment 26

[Andrew Halberstadt [:ahal]]

Comment on attachment 8365341 [details] [diff] [review]
Create a mozversion package for getting the application version. v1.1

Review of attachment 8365341 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks! Ship it :)

Comment 27

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Yay! Landed in:
https://github.com/mozilla/mozbase/commit/6baa74e8b71374a2279a3b478617449adcd38423

Released as:
https://pypi.python.org/pypi/mozversion/0.1

Comment 28

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Dave Hunt (:davehunt) from comment #24)
> Okay, addressed the review comments. Also, we now check the current working
> directory for application.ini and sources.xml. This means you can run it

Keep in mind that this logic for application.ini will not always work at least for desktop builds. I think for dynamically linked builds the application is located in another folder. We have had this problem in the past with Mozmill.

Otherwise great to see that this feature has finally landed! Thanks Dave!