The latest changes look good to me. Thanks,
Darren. On 23/08/2012 12:26, Nirmal Agarwal wrote: > Hi Ethan, > > I have updated the webrev. > > webrev : > https://cr.opensolaris.org/action/browse/caiman/nirmal27/7192373-rev2/webrev/ > > webrev diff : > https://cr.opensolaris.org/action/browse/caiman/nirmal27/7192373-diff/webrev.diff/ > > I have re-run the following tests : > --> AI installation on physical system with only IPS packages specified > in manifest > --> AI installation on physical system with SVR4 packages specified in > manifest > --> AI installation on physical system with IPS packages and Non global > zones in the manifest > --> Non Global Zone installation on an installed system using manifest > with SVR4 packages specified. > --> Non Global Zone installation on an installed system using manifest > with IPS packages only > > Thanks, > Nirmal > > On 08/23/12 02:44, Nirmal Agarwal wrote: >> Hi Ethan, >> >> On 8/23/2012 1:58 AM, Ethan Quach wrote: >>> >>> >>> On 08/22/12 12:25, Nirmal Agarwal wrote: >>>> Hi Ethan, >>>> >>>> Thanks for the review. >>>> On 8/23/2012 12:37 AM, Ethan Quach wrote: >>>>> Nirmal, >>>>> >>>>> The code changes look ok, though I have a question. >>>>> >>>>> Are there any consequences of running 'pkgadm sync ... ' if a >>>>> pkgadd hadn't happened? I ask because there seem to be other code >>>>> paths that run into _cleanup() where the pkgadd might have not been >>>>> run, like check_cancel_event() or from inside _parse_input() where >>>>> the input parameters might have been bad. >>>>> >>>> "pkgadm sync .." will sync the pending operations on contents db >>>> file. In case where pkgadd have not been run, there will be no >>>> pending operations and it will just return. So there are no >>>> consequences in other scenarios. >>> >>> Understood, but it seems we can set when we know we've run a >>> pkgadd/rm command. For example, could we just set a dirty bit right >>> before line 444, and check against that bit in _cleanup() before >>> actually running pkgadm sync? >>> >> I will make the change and re-run the tests. >> >> Thanks, >> Nirmal >>> (This assumes, of course, that the call to pkgadd/rm here is the only >>> place in this module that could possibly launch the pkgserv, which >>> seems to be the case.) >>> >>> >>> thanks, >>> -ethan >>> >>>> >>>> Thanks, >>>> Nirmal >>>> >>>>> >>>>> thanks, >>>>> -ethan >>>>> >>>>> >>>>> On 08/22/12 07:35, Nirmal Agarwal wrote: >>>>>> Hi all, >>>>>> >>>>>> Can I please get 2 code reviews for CR 7192373. >>>>>> >>>>>> 7192373 AI installation fails when SVR4 packages are specified in >>>>>> manifest >>>>>> >>>>>> Webrev: >>>>>> https://cr.opensolaris.org/action/browse/caiman/nirmal27/7192373/webrev/ >>>>>> >>>>>> >>>>>> Pep8 is clean. >>>>>> Pylint output is unchanged. >>>>>> >>>>>> Unit tests : Pass >>>>>> >>>>>> Testing : >>>>>> --> AI installation on physical system with SVR4 packages >>>>>> specified in manifest >>>>>> --> Non Global Zone installation on an installed system using >>>>>> manifest with SVR4 packages specified. >>>>>> >>>>>> Regards, >>>>>> Nirmal >>>>>> >>>>>> _______________________________________________ >>>>>> 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 >> _______________________________________________ >> 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 > _______________________________________________ caiman-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
