Hi Glenn and Joe. Regarding: > <http://cr.opensolaris.org/~glagasse/webrev.diffs>
For round 2, all files look good to me, except I still have some comments on im_pop.py: 573: If (QUIT_ON_PKG_FAILURE == True) and ips_set_auth fails, do calls to ips_cleanup_authorities() get made to clean up the UNSET_AUTH_LIST? Likewise for ips_cleanup_mirrors() and UNSET_MIRROR_LIST? Are such calls necessary? The main just raises an exception and bails. Since this is lifted from existing code (DC_tm.py) and you don't feel comfortable changing it, I'm happy with your offer to file a bug. - - - The use of PKGFILE between its flush on 523 and its close on 529 still don't make sense to me. Close it in one process before using it by another. The close will do the flush. What you have will work, but it would be cleaner to do this: for pkg in PKGS: PKGFILE.write(...) PKGFILE.close() STATUS = ips_pkg_op(PKG_FILE_NAME, ...) Likewise for 479-505: for pkg in PKGS: PKGFILE.write(pkg + '\n') PKGFILE.close() STATUS = ips_contents_verify(PKG_FILE_NAME, ...) if STATUS and QUIT_ON_PKG_FAILURE == 'true': os.unlink(PKG_FILE_NAME) raise (...) ... STATUS = ips_pkg_op(PKG_FILE_NAME, ...) if STATUS and QUIT_ON_PKG_FAILURE == 'true': os.unlink(PKG_FILE_NAME) raise (...) os.unlink(PKG_FILE_NAME) - - - Nit: Change QUIT_ON_PKG_FAILURE to be a boolean instead of a string; then you can just say if STATUS and QUIT_ON_PKG_FAILURE: Thanks, Jack
