Add a Mozmill version argument option on command line

VERIFIED FIXED

Status

Testing Graveyard
Mozmill
VERIFIED FIXED
8 years ago
a year ago

People

(Reporter: aaronmt, Assigned: ahal)

Tracking

Details

(Whiteboard: [mozmill-1.4.2+])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

8 years ago
Can we please add a -v argument to mozmill in order to print the current version?

Currently, I run the program and scan for load_entry_point('mozmill==1.4.2b1') -- which seems silly.
(Reporter)

Updated

8 years ago
Summary: Add a Mozmill version argument → Add a Mozmill version argument on command line
(Reporter)

Updated

8 years ago
Summary: Add a Mozmill version argument on command line → Add a Mozmill version argument option on command line
This shouldn't be simple to do and would help us a lot. The OptionsParser already support that. Can we get this one-liner for 1.4.2?
Whiteboard: [mozmill-1.4.2?]

Updated

8 years ago
Whiteboard: [mozmill-1.4.2?] → [mozmill-1.4.2+]
(Assignee)

Updated

8 years ago
Assignee: nobody → ahalberstadt

Comment 2

8 years ago
Make sure to do this correctly and take the number from setuptools metadata.  It should/will only work if the package is installed from setuptools
(Assignee)

Comment 3

8 years ago
Yep, it's almost done.  I'm planning to use '--info' instead of '-v' and print out the other metadata as well.

I'm currently using pkg_resources.get_distribution("Mozmill").get_metadata('...')

Comment 4

8 years ago
Looks like the right approach
(Assignee)

Comment 5

8 years ago
Created attachment 460687 [details] [diff] [review]
Prints metadata of specified module with --info command

Prints any metadata it can find when --info is specified.

Note that the print_info method is implemented in mozrunner, so --info will work for any modules inheriting the CLI from there (i.e mozmill and jsbridge).
Attachment #460687 - Flags: review?(jhammel)

Comment 7

8 years ago
(In reply to comment #6)
> Why can't we use:
> http://docs.python.org/library/optparse.html#printing-a-version-string

We can, but ultimately the version should be that from setup.py
(Assignee)

Comment 8

8 years ago
Even if we use --version we'd still need to supply the version to optparse from somewhere, so we'd still need to access the egg metadata. And if we're going there already, why not print out the rest of the metadata while were at it?
Sure, what I meant was, why can't we use print_info to return the string we can print out with --version. I'm in favor of using the default paths as much as possible. Is there something limiting us from using it?

Comment 10

8 years ago
Comment on attachment 460687 [details] [diff] [review]
Prints metadata of specified module with --info command

>diff --git a/jsbridge/jsbridge/__init__.py b/jsbridge/jsbridge/__init__.py
>index 2003b4b..1398316 100644
>--- a/jsbridge/jsbridge/__init__.py
>+++ b/jsbridge/jsbridge/__init__.py
>@@ -85,6 +85,8 @@ def wait_and_create_network(host, port, timeout=wait_to_create_timeout):
> 
> class CLI(mozrunner.CLI):
>     """Command line interface."""
>+    
>+    module = "jsbridge"

I would prefer not noting the name of the module here (and in the other two places), but I can't think of a clever way of doing this off the top of my head, so I guess this is fine.


>@@ -47,6 +47,7 @@ import zipfile
> import optparse
> import killableprocess
> import subprocess
>+import pkg_resources
> from xml.etree import ElementTree
> from distutils import dir_util
> from time import sleep
>@@ -472,6 +473,7 @@ class CLI(object):
> 
>     runner_class = FirefoxRunner
>     profile_class = FirefoxProfile
>+    module = "mozrunner"
> 
>     parser_options = {("-b", "--binary",): dict(dest="binary", help="Binary path.",
>                                                 metavar=None, default=None),
>@@ -480,6 +482,9 @@ class CLI(object):
>                       ('-a', "--addons",): dict(dest="addons", 
>                                                 help="Addons paths to install.",
>                                                 metavar=None, default=None),
>+                      ("--info",): dict(dest="info", default=False,
>+                                        action="store_true",
>+                                        help="Print module information.")
>                      }
> 
>     def __init__(self):
>@@ -489,11 +494,26 @@ class CLI(object):
>             self.parser.add_option(*names, **opts)
>         (self.options, self.args) = self.parser.parse_args()
> 
>+        if self.options.info:
>+            self.print_info()
>+            sys.exit(0)
>+            

Ideally, the metadata should be obtained first and stored on the instance object.  This way, if an item of metadata, such as the version, is needed in the program, it may be programmatically retrieved.  Also, if the metadata is not available (i.e. if you are running the package without installing it via setuptools) then you can not add the --info switch.  This is currently difficult due to the way we store options, but when I fix this for 2.0 it should be done.  Then the print info can just print out this dictionary.

>         # XXX should use action='append' instead of rolling our own
>         try:
>             self.addons = self.options.addons.split(',')
>         except:
>             self.addons = []
>+            
>+    def print_info(self):
>+        dist = pkg_resources.get_distribution(self.module)
>+        if dist.has_metadata("PKG-INFO"):
>+            metadata = ["Name", "Version", "Summary", "Home-page", "Author", 
>+                        "Author-email", "License", "Description", "Platform"]

See above comment.  Also, this should be a set()

>+            for line in dist.get_metadata_lines("PKG-INFO"):
>+                if line[0:line.find(":")] in metadata:
>+                    print line
>+        if dist.has_metadata("requires.txt"):
>+            print "\nDependencies: \n" + dist.get_metadata("requires.txt")            
> 
>     def create_runner(self):
>         """ Get the runner object """

Please fetch the metadata first and store on the CLI instance.
Attachment #460687 - Flags: review?(jhammel) → review-
(Assignee)

Comment 11

8 years ago
Created attachment 460969 [details] [diff] [review]
Attaches metadata to CLI instance and allows --version as well

Stores the metadata in a dictionary which is attached to the CLI instance.  Also allows --version and adds a convenience print_metadata method.
Attachment #460687 - Attachment is obsolete: true
Attachment #460969 - Flags: review?(jhammel)

Comment 12

8 years ago
Comment on attachment 460969 [details] [diff] [review]
Attaches metadata to CLI instance and allows --version as well

>@@ -472,6 +473,8 @@ class CLI(object):
> 
>     runner_class = FirefoxRunner
>     profile_class = FirefoxProfile
>+    module = "mozrunner"
>+    metadata = {}

Is there a reason to set this on the class-level?
 
>     parser_options = {("-b", "--binary",): dict(dest="binary", help="Binary path.",
>                                                 metavar=None, default=None),
>@@ -480,20 +483,44 @@ class CLI(object):
>                       ('-a', "--addons",): dict(dest="addons", 
>                                                 help="Addons paths to install.",
>                                                 metavar=None, default=None),
>+                      ("--info",): dict(dest="info", default=False,
>+                                        action="store_true",
>+                                        help="Print module information")
>                      }
> 
>     def __init__(self):
>         """ Setup command line parser and parse arguments """
>-        self.parser = optparse.OptionParser()
>+        self.metadata = self.get_metadata_from_egg()
>+        self.parser = optparse.OptionParser(version="%prog " + self.metadata["Version"])
>         for names, opts in self.parser_options.items():
>             self.parser.add_option(*names, **opts)
>         (self.options, self.args) = self.parser.parse_args()
> 
>+        if self.options.info:
>+            self.print_metadata()
>+            sys.exit(0)
>+            
>         # XXX should use action='append' instead of rolling our own
>         try:
>             self.addons = self.options.addons.split(',')
>         except:
>             self.addons = []
>+            
>+    def get_metadata_from_egg(self):
>+        ret = {}
>+        dist = pkg_resources.get_distribution(self.module)
>+        if dist.has_metadata("PKG-INFO"):
>+            for line in dist.get_metadata_lines("PKG-INFO"):
>+                ret[line[0:line.find(":")]] = line[line.find(":") + 1:]

This is a bit ugly.  How about `key, value = line.split(':', 1); ret[key] = value`

>+        if dist.has_metadata("requires.txt"):
>+            ret["Dependencies"] = "\n" + dist.get_metadata("requires.txt")    
>+        return ret
>+        
>+    def print_metadata(self, data=["Name", "Version", "Summary", "Home-page", 
>+                                   "Author", "Author-email", "License", "Platform", "Dependencies"]):

You should use a tuple for data as a default argument, not a list

>+        for key in data:
>+            if key in self.metadata:
>+                print key + ": " + self.metadata[key]
> 
>     def create_runner(self):
>         """ Get the runner object """

Looks good, basically, but would recommend those changes.
Attachment #460969 - Flags: review?(jhammel) → review+
(Assignee)

Comment 13

8 years ago
Comments addressed and changes checked in.

master: http://github.com/mozautomation/mozmill/commit/59808df2f140ade7628bcbf34ff3ecf7b230c4d9

1.4.2: http://github.com/mozautomation/mozmill/commit/0842f69b7564200f7f818b4052742d22d6399f67
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
That looks fantastic. Now it's much easier to keep track of Mozmill versions. Thanks!
Status: RESOLVED → VERIFIED

Updated

8 years ago
Duplicate of this bug: 562173
Depends on: 587880
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.