On 05/09/12 15:56, Drew Fisher wrote:
On 5/9/12 6:39 AM, Jan Damborsky wrote:
Hi Drew,
please see my comments below. They are all
related to 7094123, other bug fixes look
good to me.
Thank you,
Jan
general comment
---------------
I am not quite sure that suggested fix will correctly work
in all cases without changing related code handling
unconfiguration.
In particular, milestone-unconfig (responsible for removing
old SC profiles) seems to remove only profiles residing directly
in /etc/svc/profile/site/ directory, but doesn't remove
subdirectories, as those may contain user site profiles - those
should not be touched by sysconfig:
...
#
# Preserve profiles in the /etc/svc/profile/site directory
# which customize configuration process. All other profiles in that
# directory are removed. However profiles located in sub-directories
# of /etc/svc/profile/site are not altered. The command
# "find $SMF_PROFILE_SITE_DIR/* -prune" effectively equates to max
# depth 1, so only occurs in the $SMF_PROFILE_SITE_DIR.
#
find $SMF_PROFILE_SITE_DIR/* -prune -a ! -type d \
-a ! -name $CONFIG_XML ! -name $UNCONFIG_XML | xargs rm -f
...
I assume that problem will disappear with SC groupings project,
when it introduces different layout of SC profiles.
system-config/__init__.py
-------------------------
801-804 - this code cleans up temporary profile directory.
I am wondering if this needs some adjustment, as it currently
deletes only regular files. Perhaps we could just remove the
whole dir by os.rmtree(custom_profile_dir), then followed
by os.mkdir(custom_profile_dir).
838 - Looking at doc, os.path.split(options.profile)[1])))
would evaluate to empty string if options.profile contained
trailing slash. Do we need to somehow account for that case ?
Ugh. I had forgotten about that -prune flag in milestone-unconfig.
You're right that this fix won't deal with that problem. I think I'm
going to revert the change for 7094123 entirely but leave the other
three fixes in as changing how milestone-unconfig works (that rm -f
call is just terrible) is not something that we're going to fix until
the SC groups project goes back.
I've respun the webrev with the fix backed out:
https://cr.opensolaris.org/action/browse/caiman/drewfish/sysconfig_fixes_3/webrev/
Can you take a quick look?
Looks good to me, Drew.
Jan
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
