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

(Updated Feb. 13, 2011, 5:21 a.m.)


Review request for Viewer.


Changes
-------

updated diff to include correct parent for better display

fixed the use of the -V short form for --version in the source_environment 
subcommand (which defined it separately for some reason)


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 (updated)
-----

  autobuild/autobuild_main.py 9ee2db08d677 
  autobuild/autobuild_tool_install.py 9ee2db08d677 
  autobuild/autobuild_tool_source_environment.py 9ee2db08d677 
  autobuild/common.py 9ee2db08d677 
  autobuild/configfile.py 9ee2db08d677 

Diff: http://codereview.secondlife.com/r/150/diff


Testing
-------

Manually verified using configuring and building in viewer-autobuild


Thanks,

Oz

_______________________________________________
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Reply via email to