----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/150/#review358 -----------------------------------------------------------
Ship it! This looks good to me. - Wolfpup On Feb. 13, 2011, 5:21 a.m., Oz Linden wrote: > > ----------------------------------------------------------- > 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. > > > 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 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
