Shawn Walker wrote:

> On 09/22/11 14:32, Danek Duvall wrote:
> >This webrev:
> >
> >     https://cr.opensolaris.org/action/browse/pkg/dduvall/alias-move/
> >
> >fixes
> >
> >     18947 driver alias moving to a brand new driver action causes an error 
> > message about sharing
> >
> >I'd like to get this in for FCS, as we have one internal customer who is
> >running into this, and will need to have this functionality fixed before
> >they can ship their changes.
> 
> src/modules/actions/driver.py:
>   line 230: I'd assign alias to alia_conflict here and then use that
> variable on line 239 instead of relying on the last assignment in the
> for loop above.  That's not technically guaranteed behaviour for the
> python interpreter from what I recall.

Fixed.

>   line 245:  Is there a reason this raises RuntimeError instead of
> ActionExecutionError?
> 
>   lines 255-267: Instead of printing, could this import the logger
> object and call logger.error() to display these messages?  That would
> allow the packagemanager to also get them.

Both good suggestions, but I'm uncomfortable making the first change so
late in the game (I see one spot where this RuntimeError might be caught,
and I don't know what effect the change would have), and while
logger.warning() is the right call in the second case, there are a bunch of
other places where this needs to be done, and I don't want to go making
that change all the places it needs to happen.

Diff inline:

    --- a/src/modules/actions/driver.py
    +++ b/src/modules/actions/driver.py
    @@ -174,7 +174,7 @@ class DriverAction(generic.Action):
     
                     file_db = {}
                     alias_lines = {}
    -                alias_conflict = False
    +                alias_conflict = None
                     lines = []
                     try:
                             for fields in DriverAction.__gen_read_binding_file(
    @@ -208,9 +208,9 @@ class DriverAction(generic.Action):
                             a2d = {}
                             for alias, name in (
                                 (a, n)
    +                            for n, act_list in driver_actions.iteritems()
    +                            for act in act_list
                                 for a in act.attrlist("alias")
    -                            for act in act_list
    -                            for n, act_list in driver_actions.iteritems()
                             ):
                                     a2d.setdefault(alias, set()).add(name)
     
    @@ -227,16 +227,16 @@ class DriverAction(generic.Action):
                             names = a2d[alias]
                             assert self.attrs["name"] in names
                             if len(names) > 1:
    -                                alias_conflict = True
    +                                alias_conflict = alias
                                     break
     
                     if alias_conflict:
                             be_name = getattr(image.bootenv, "be_name_clone", 
None)
    -                        name, line = alias_lines[alias][0]
    +                        name, line = alias_lines[alias_conflict][0]
                             errdict = {
                                 "new": self.attrs["name"],
                                 "old": name,
    -                            "alias": alias,
    +                            "alias": alias_conflict,
                                 "line": line,
                                 "be": be_name,
                                 "imgroot": image.get_root()

Thanks,
Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to