Hi Karen,
thank you very much for review.
Sure I can take care of 7093060 :-)
Drew, would you mind if I take over that CR from you ?
Being in that code, I also decided to address
7098714 system/network installed via GUI and text installer fails pkg
verify check
WRT code review itself, I addressed all your comments, with one small
exception.
save_menu_lst() considers missing 'boot/grub/menu.lst' to be a valid case,
while save_files_directories() spits warning if it's asked to save
missing file.
So I decided to keep save_menu_lst() as a separate function - it contains
necessary check plus call to save_files_directories().
I updated webrev accordingly:
https://cr.opensolaris.org/action/browse/caiman/dambi/cr-7162079-2/webrev-2/
So far I have tested Sparc text installation, build of other images
is now in progress.
Jan
On 04/25/12 23:52, Karen Tung wrote:
Hi Jan,
I really like the improvements you made to the
save_files_directories() function
in pre_pkg_img_mod.py. With those improvements, you almost fixed bug
7093060 also.
Do you mind taking that bug as well? :-)
I agree with Ginnie that the save_etc_files() function you added is not
really needed. I guess you were trying to follow the "style" in this
file.
It would greatly improve readability of the code if you could just call
save_files_directories(["/etc/shadow"], preserve=True)
Another comment I have is that you moved the saving of /etc/system to
the save_etc_files()
function. I think the function doing the modification,
modify_etc_system(), is the better place
to save the file. That way, people don't need to look all over to see
whether /etc/system is saved
before it is modified.
As for the /etc/inet/hosts file that you added to the save_etc_files()
function, and removed
the existing save_etc_inet_hosts(). The save_etc_inet_hosts()
function is no longer necessary,
the saving of /etc/inet/hosts can be done inline.
Because the improvements you did to save_files_directories(), you can
also replace lines 164-183 with
1 single call. Furthermore, you can get rid of the save_menu_lst()
function, and just save that file inline.
Thanks,
--Karen
On 04/24/12 03:42 AM, Jan Damborsky wrote:
Hi,
I would appreciate code review of changes for
7162079 CPIO installation propagates install root password to the target
webrev:
https://cr.opensolaris.org/action/browse/caiman/dambi/cr-7162079/webrev/
Thank you,
Jan
Testing done:
* built text/AI/LiveCD install images using modified Distro Const.
Workarounds for 7162039 and7162553were applied on LiveCD.
* tested text, AI, LiveCD installations.
Verified that shadow(4) was restored from 'save' area in case
when CPIO transfer method was used for installation (text, LiveCD).
* run unit tests (new unit test added for introduced
method PrePkgImgMod.save_etc_files()
_______________________________________________
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