Hey Sanjay, At this late date in development, if it is OK with you, I would like to handle this by filing a bug and addressing it after feature freeze.
Joe sanjay nadkarni wrote: > if I run shcomp -n on the ksh93 files I see the following: >> nadkarni at skinnymoose:/tmp$ shcomp -n create_vm /dev/null >> create_vm: warning: line 89: `...` obsolete, use $(...) >> create_vm: warning: line 89: " quote may be missing >> create_vm: warning: line 90: `...` obsolete, use $(...) >> create_vm: warning: line 176: -ir invalid typeset option order >> create_vm: warning: line 223: `...` obsolete, use $(...) >> create_vm: warning: line 284: `...` obsolete, use $(...) >> nadkarni at skinnymoose:/tmp$ shcomp -n export_vm /dev/null >> export_vm: warning: line 158: `...` obsolete, use $(...) >> export_vm: warning: line 158: `...` obsolete, use $(...) >> export_vm: warning: line 213: `...` obsolete, use $(...) >> export_vm: warning: line 229: `...` obsolete, use $(...) >> export_vm: warning: line 237: `...` obsolete, use $(...) > Can you please address these. > > -Sanjay > > > Glenn Lagasse wrote: >> Hi Jack! >> >> * Jack Schwartz (Jack.A.Schwartz at Sun.COM) wrote: >> >>> 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. >>> >> >> Ok. I'll 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) >>> >> >> I looked into this, since the transfer module does indeed specifically >> open the file itself I'm pretty comfortable with this suggestion and so >> I'll make this change since I agree with what you're saying. >> >> >>> - - - >>> >>> 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: >>> >> >> I'd like to change this, but it will change quite a few places in the >> code and while seemingly innocuous I already have a couple of >> regressions in this code that I'm tracking down and I'm leary to make >> that situation worse. Since this is existing code and I'm not >> regressing anything I'm going to pass on addressing this other than I >> can offer to file a low priority bug to clean it up. >> >> Thanks Jack! >> >> > > _______________________________________________ > caiman-discuss mailing list > caiman-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
