[pkg-discuss] [REVIEW] Changes for bugs 9969, 11715, 11793

[Shawn Walker]

Edward Pilatowicz wrote:
> On Mon, Nov 16, 2009 at 02:37:10PM -0600, Shawn Walker wrote:
>> Edward Pilatowicz wrote:
>>> On Fri, Nov 13, 2009 at 06:17:58PM -0600, Shawn Walker wrote:
>>>> Greetings,
>>>>
>>>> The following webrev contains changes for the following bugs:
>>>>
>>>>  9969 client support for multiple origins desired
>>>>  11715 ImageConfig does not handle None for publisher correctly
>>>>  11793 image-create example partially disagrees with the usage
>>>>
>>>> This adds the long-awaited multiple origins support to the client
>>>> while retaining some level of basic compatibility with older
>>>> clients.
>>>>
>>>> webrev:
>>>> http://cr.opensolaris.org/~swalker/pkg-origins/
>> ...
>>> - pkgzonecreate: you updated the usage error messages to mention some
>>>  new flags.  -g and -m.  but i don't see these flags used anywhere.
>> They're used; see lines 405, 411, 446, and 477.
>>
>> The comment read 'line 167 # IPS options aren't allowed when
>> installing from a system image.'; so I assumed that meant that I
>> should prevent other publisher options from being used here as well.
>>
> 
> hm.  the error message you've modified is an error message for
> pkgcreatezone.  the -m and -g options are options that get passed
> internally to pkg.  they are not options that get specified on the
> pkgcreatezone command line.  (or at least it appears that way to me.)
> hence pkgcreatezone couldn't complain about them.

Ah, ok, the error check here wasn't clear to me.  I thought it was 
trying to prevent passing pkg options to pkgcreatezone.

> pkgcreatezone:
...
> - unneeded:
> 	372         [[ -z "$origin" ]] && continue
> 	...
> 	388         [[ -z "$mirror" ]] && continue
> 
> this looks great to me.  i recently touched this same code and i did all
> my testing manually.  i just installed the modified binaries and
> scripts on a test machine, changed the publisher configuration of the
> global zone, and then tried installing zones.  unfortunatly you've got
> more combos of this to test than i did.  i wouldn't spend much time
> testing the -P option to pkgcreatezone.  we plan to deprecate this and
> just haven't gotten around to ripping it out yet.

Okay, that's basically what I've been doing; just checking zone creation 
and attach.

> wrt more extensive zones testing in general, there is the zones test
> suite, but honestly it won't have coverage for the stuff your changing
> here.  hence i'd recommend skipping it for this change.

Thanks and sorry for the double debug problem :/

I'll fix the above, then merge with gate tip, and post one last complete 
webrev.

Cheers,
-- 
Shawn Walker