On 01/31/11 02:04 PM, Karen Tung wrote:
Hi Keith,

As we discussed offline, your implementation of Popen will delay printing any stdout and stderr to the log files until the command is completed. This is not desirable for
application that wants to see any output/error from long running command.
Users of those application might think the command is hung if they don't see
the expected output from the commands in the log files.
It would be best to display any output/error from commands as soon as it is "sensible".

Thanks for pointing out that use case! I've spun up a modified webrev. The logic for the logging case is now much different - the general flow copied from the existing exec_cmds_output_to_log function.


I see that you removed the reference to install_extra in pkg/transforms/synthetics. Are you also going to also remove the reference to repo.extra in pkg/README.pkg so all
references are gone, even though it is harmless.

Per the Note at the top of README.pkg, that file was only ever a reference and was never modified from its original source, the ON gate. It has many references to things that don't apply to slim_source. As such, I don't feel that maintenance of that file is worthwhile at this time.

Thanks,
Keith


Thanks,

--Karen



On 01/31/11 12:20, Keith Mitchell wrote:
Hi all,

I'd like a code review for my fixes for:

    15957  <http://defect.opensolaris.org/bz/show_bug.cgi?id=15957>  Implement 
solaris_install.Popen
    7014402  
<http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=7014402>  SUNW* 
packages showing up in install-redistributable


webrev:
http://cr.opensolaris.org/~kemitche/webrev.15957/

Notes:
* solaris_install.Popen is a drop-in replacement for subprocess.Popen - as verification, the test code actually runs the full subprocess.Popen test suite against solaris_install.Popen. * "exec_cmd" and its family have not yet been removed. I will file bugs against the various installers to replace usage of such functions with solaris_install.Popen invocations; this allows for new consumers to use solaris_install.Popen now, rather than waiting an indefinite amount of time for me (or someone) to pull out those functions and widely re-test a large portion of the slim_source codebase. * I initially considered doing a global find/replace of subprocess.Popen with solaris_install.Popen. However, since solaris_install.Popen is in the solaris_install module, in many cases that results in a new Python import of a module not in the Python standard library. This has potential implications on packaging dependencies; as such, there are risk and time factors involved in such a find/replace (in the form of at least sanity checking most installers). As there will be bugs to remove "exec_cmd" functions anyway, it seems appropriate to have those bugs also replace subprocess.Popen with solaris_install.Popen. (The current approach also is probably easier for the various projects to merge with)

Thanks,
Keith


_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss


_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

Reply via email to