[opensource-dev] Review Request: open-2 (improved progress messages during dependency checks) and open-31 (more standardized short form option switches)

[Nicky Perian]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/150/#review357
-----------------------------------------------------------

Ship it!


Can this be placed as an hg changeset? I'll say Ship It in hopes a test method is forthcoming.

- Nicky


On Feb. 12, 2011, 1:13 p.m., Oz Linden wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/150/
> -----------------------------------------------------------
> 
> (Updated Feb. 12, 2011, 1:13 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> -------
> 
> 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)
> 
> 
> This addresses bugs open-2 and open-31.
> 
> 
> Diffs
> -----
> 
>   autobuild/autobuild_main.py UNKNOWN 
>   autobuild/autobuild_tool_install.py UNKNOWN 
>   autobuild/common.py UNKNOWN 
>   autobuild/configfile.py UNKNOWN 
> 
> Diff: http://codereview.secondlife.com/r/150/diff
> 
> 
> Testing
> -------
> 
> Manually verified using configuring and building in viewer-autobuild
> 
> 
> Thanks,
> 
> Oz
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20110213/a40c85ca/attachment.htm