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.

to illustrate the source of my confusion.  if someone passed -m or -g to
pkgzonecreate, we'd produce an error in the getopts parsing block before
we ever reach the block of code with the error messages you've modified.

am i missing something?

>
> I think I've addressed all of your concerns in the webrevs below.
>
> webrev (only differences from 1st webrev):
> http://cr.opensolaris.org/~swalker/pkg-origins-v1-v2/
>
> webrev (complete):
> http://cr.opensolaris.org/~swalker/pkg-origins-v2/
>
> Finally, is there any way you can try this changeset out locally to
> see if I missed anything?  As I mentioned before, I'm new to zones
> stuff.
>

pkgcreatezone:

- left over debugging code?
        339 exit $(fail_incomplete "$f_img")

- 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.

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.

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

Reply via email to