Hi Ethan,
Thanks for the review.
On 8/23/2012 9:10 PM, Ethan Quach wrote:
Nirmal,
The updated changes look fine.
(You probably already did this, but don't forget rerun unit tests as
well.)
Yes, I re-ran the unit tests and they all pass.
Thanks,
Nirmal
thanks,
-ethan
On 08/23/12 04: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
