
Submitter:	Oz Linden	Reviewers
Branch:		Groups:	viewer
Bugs:	open-2, open-31	People:
Change Number:	None	Repository:	autobuild

Description:

I combined open-31 into this because that change is so small and simple that it didn't seem worth a separate commit (besides, while testing the rest of this, I really wanted the short form of --verbose).

open-31 changes the short form of --version to -V, so that the short form of the more commonly used --verbose can be the lower case -v, and adds -n as a short form for --dry-run (as it is used in many versions of make, and in other commands like scp and rsync).

open-2 corrects the problem that (at least in the viewer build) there is a long delay during which there are no progress messages.  The delay is the configure step while packages are checked, including possibly downloading, extracting, and installing them.  Increasing the logging level with the --verbose or --debug switches did not increase the amount of logging during this phase.

The fundamental problem proved to be that the package checks were done in recursively invoked instances of autobuild (I am guessing that the process hierarchy is autobuild->cmake->autobuild, but it could have just been autobuild calling itself directly; this fix should work regardless of the depth of the process tree).  The verbosity control options were not passed through to the child processes, and since there could be some other process in between (as cmake above), it seemed more robust (and relies less on correct configuration) to pass the verbosity control to the children through the environment.

The default verbosity is now read from the AUTOBUILD_LOGGING environment variable, whose contents can be --debug, --verbose, --quiet, or the one letter forms of any of those options (if the environment variable is not set, then the default value is to log warnings and worse, as before).  Any of the options used on the command line override any value from the environment.  Regardless of how the verbosity level is established, the environment variable AUTOBUILD_LOGGING is set for all child processes, so that if any of those (directly or indirectly) are another invocation of autobuild, then that recursive child will use the same verbosity level as the parent (unless, of course the recursive invocation uses an explicit option).  It is true that this also allows the user to set a default verbosity in the shell environment, and that works, but it wasn't really the motivation for the change and I did not add anything to the documentation about it.

With that change made, the options correctly controlled whether or not logging is visible during the package checking phase.  However, the resulting messages had an inconsistent level of detail - some operations (such as uninstall) were very verbose, while others (some of which might take significant time) were logged only at high verbosity levels.  This led to the addition of a few short log messages at the default 'warning' level (which really has the dual meaning "something that might be a problem" and "something that should be seen at the default logging level") in order to make sure that some message is printed before any potentially long operation (downloads, extracts, installs, and uninstalls).  Some other very detailed messages were changed from info to debug levels, as the information they display is really only useful when debugging either a new autobuild configuration or autobuild itself.

I think that the combination of these changes makes the default verbosity reasonably informative (no long silences) without being overwhelming.

(there is a failure displaying the diff for autobuild.configfile.py because it is a one word change of the logging level for a log message added by one of my other changes that has not yet been applied to the trunk)

Testing Done:

Manually verified using configuring and building in viewer-autobuild

Diff revision 3 (Latest)

Jump to revision:	1 2 3
Changes between r3 and:	1 2 3

Files Changed:

--- Revision 9ee2db08d677
+++ New Change
     # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
     # THE SOFTWARE.
     # $/LicenseInfo$
     import sys
     import os
     import common
     import argparse
     import logging
     ## Environment variable name used for default log level verbosity
     AUTOBUILD_LOGLEVEL = 'AUTOBUILD_LOGLEVEL'
     class run_help(argparse.Action):
         def __call__(self, parser, namespace, values, option_string=None):
             parser.parent.search_for_and_import_tools(parser.parent.tools_list)
             parser.parent.register_tools(parser.parent.tools_list)
             print parser.format_help()
             parser.exit(0)
     class Autobuild(object):
         def __init__(self):
             self.parser = argparse.ArgumentParser(
                 description='Autobuild', prog='autobuild', add_help=False)
-        self.parser.add_argument('-v', '--version', action='version', version='%(prog)s 1.0')
+        self.parser.add_argument('-V', '--version', action='version', version='%(prog)s 1.0')
             self.subparsers = self.parser.add_subparsers(title='Sub Commands',
                 description='Valid Sub Commands',
                 help='Sub Command help')
         def listdir(self, dir):
             files = os.listdir(dir)
             if 'AutobuildTool_test.py' in files:
                 del files[files.index('AutobuildTool_test.py')]
             return files
             tool_module_name = 'autobuild_tool_' + tool
             tool_file_name = tool_module_name + '.py'
             full_tool_path = os.path.join(autobuild_package_dir, tool_file_name)
             if os.path.exists(full_tool_path):
                 possible_tool_module = __import__(tool_module_name, globals(), locals(), []);
                 if(getattr(possible_tool_module, 'AutobuildTool', None)):
                     tools_list.append(possible_tool_module)
                     instance = self.register_tool(possible_tool_module)
                     return instance
             return -1
         def get_default_loglevel_from_environment(self):
             """
             Returns a default log level based on the AUTOBUILD_LOGLEVEL environment variable
             This may be used directly by the user, but in combination with the
             set_recursive_loglevel method below also ensures
             that any recursive invocation of autobuild inherits the same level as the
             parent, even if intervening commands do not pass it through.
             """
             try:
                 environment_level = os.environ[AUTOBUILD_LOGLEVEL]
             except KeyError:
                 environment_level = ''
             if environment_level == '--quiet' or environment_level == '-q' :
                 return logging.ERROR
             elif environment_level == '':
                 return logging.WARNING
             elif environment_level == '--verbose' or environment_level == '-v' :
                 return logging.INFO
             elif environment_level == '--debug' or environment_level == '-d' :
                 return logging.DEBUG
             else:
                 raise AutobuildError("invalid %s value '%s'" % (AUTOBUILD_LOGLEVEL, environment_level))
         def set_recursive_loglevel(self, logger, level):
             """
             Sets the logger level, and also saves the equivalent option argument
             in the AUTOBUILD_LOGLEVEL environment variable so that any recursive
             invocation of autobuild uses the same level
             """
             logger.setLevel(level)
             if level == logging.ERROR:
                 os.environ[AUTOBUILD_LOGLEVEL] = '--quiet'
             elif level == logging.WARNING:
                 os.environ[AUTOBUILD_LOGLEVEL] = ''
             elif level == logging.INFO:
                 os.environ[AUTOBUILD_LOGLEVEL] = '--verbose'
             elif level == logging.DEBUG:
                 os.environ[AUTOBUILD_LOGLEVEL] = '--debug'
             else:
                 raise common.AutobuildError("invalid effective log level %s" % logging.getLevelName(level))
         def main(self, args_in):
             logger = logging.getLogger('autobuild')
             logger.addHandler(logging.StreamHandler())
             default_loglevel = self.get_default_loglevel_from_environment()
             self.tools_list = []
             self.parser.parent = self
             self.parser.add_argument('--help',
                 help='find all valid Autobuild Tools and show help', action=run_help,
                 nargs='?', default=argparse.SUPPRESS)
             argdefs = (
-            (('--dry-run',),
+            (('-n', '--dry-run',),
                     dict(help='run tool in dry run mode if available', action='store_true')),
              (('--quiet',),
                 ## NOTE: if the mapping of verbosity controls (--{quiet,verbose,debug})
                 ##       is changed here, it must be changed to match in set_recursive_loglevel
                 ##       and get_default_loglevel_from_environment methods above.
                  (('-q', '--quiet',),
                     dict(help='minimal output', action='store_const',
-                    const=logging.ERROR, dest='logging_level', default=logging.WARNING)),
+                     const=logging.ERROR, dest='logging_level', default=default_loglevel)),
-             (('--verbose',),
+             (('-v', '--verbose',),
                     dict(help='verbose output', action='store_const', const=logging.INFO, dest='logging_level')),
-             (('--debug',),
+             (('-d', '--debug',),
                     dict(help='debug output', action='store_const', const=logging.DEBUG, dest='logging_level')),
+            )
             for args, kwds in argdefs:
                 self.parser.add_argument(*args, **kwds)
             tool_to_run = -1;
             for arg in args_in:
                 if arg[0] != '-':
                     tool_to_run = self.try_to_import_tool(arg, self.tools_list)
                     if tool_to_run != -1:
                         # Define all the global arguments as also being legal
                         # for the subcommand, e.g. support both
                         # autobuild --dry-run upload args... and
                         # autobuild upload --dry-run args...
                         for args, kwds in argdefs:
                             self.new_tool_subparser.add_argument(*args, **kwds)
                     break
             args = self.parser.parse_args(args_in)
-        logger = logging.getLogger('autobuild')
+        self.set_recursive_loglevel(logger, args.logging_level)
         logger.setLevel(args.logging_level)
-            logger.addHandler(logging.StreamHandler())
             if tool_to_run != -1:
                 tool_to_run.run(args)
             return 0
     if __name__ == "__main__":
         sys.exit( Autobuild().main( sys.argv[1:] ) )

autobuild/autobuild_tool_install.py
Revision 9ee2db08d677	New Change

autobuild/autobuild_tool_source_environment.py
Revision 9ee2db08d677	New Change

autobuild/common.py
Revision 9ee2db08d677	New Change

autobuild/configfile.py
Revision 9ee2db08d677	New Change

You have a pending review.

Other reviews

Your comment

Review Board 1.6.11

This change has been marked as submitted.

open-2 (improved progress messages during dependency checks) and open-31 (more standardized short form option switches)

Diff revision 3 (Latest)